From 28e6b3b92450b0ca0e9c1342d400f8810e4d5e5a Mon Sep 17 00:00:00 2001 From: Phillip Stagnet Date: Mon, 3 Aug 2020 13:31:49 +0200 Subject: [PATCH 1/6] Add error to SessionExist interface Implement changed interface for all default providers as well and change tests accordingly --- session/couchbase/sess_couchbase.go | 6 +- session/ledis/ledis_session.go | 4 +- session/memcache/sess_memcache.go | 8 +-- session/mysql/sess_mysql.go | 10 ++- session/postgres/sess_postgresql.go | 10 ++- session/redis/sess_redis.go | 6 +- session/redis_cluster/redis_cluster.go | 6 +- session/redis_sentinel/sess_redis_sentinel.go | 6 +- session/sess_cookie.go | 4 +- session/sess_file.go | 9 ++- session/sess_file_test.go | 62 +++++++++++++++---- session/sess_mem.go | 6 +- session/session.go | 12 +++- session/ssdb/sess_ssdb.go | 8 +-- 14 files changed, 109 insertions(+), 48 deletions(-) diff --git a/session/couchbase/sess_couchbase.go b/session/couchbase/sess_couchbase.go index 707d042c..46ab07ab 100644 --- a/session/couchbase/sess_couchbase.go +++ b/session/couchbase/sess_couchbase.go @@ -179,16 +179,16 @@ func (cp *Provider) SessionRead(sid string) (session.Store, error) { // SessionExist Check couchbase session exist. // it checkes sid exist or not. -func (cp *Provider) SessionExist(sid string) bool { +func (cp *Provider) SessionExist(sid string) (bool, error) { cp.b = cp.getBucket() defer cp.b.Close() var doc []byte if err := cp.b.Get(sid, &doc); err != nil || doc == nil { - return false + return false, err } - return true + return true, nil } // SessionRegenerate remove oldsid and use sid to generate new session diff --git a/session/ledis/ledis_session.go b/session/ledis/ledis_session.go index ee81df67..4f578eac 100644 --- a/session/ledis/ledis_session.go +++ b/session/ledis/ledis_session.go @@ -132,9 +132,9 @@ func (lp *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist check ledis session exist by sid -func (lp *Provider) SessionExist(sid string) bool { +func (lp *Provider) SessionExist(sid string) (bool, error) { count, _ := c.Exists([]byte(sid)) - return count != 0 + return count != 0, nil } // SessionRegenerate generate new sid for ledis session diff --git a/session/memcache/sess_memcache.go b/session/memcache/sess_memcache.go index 85a2d815..e76eb8a5 100644 --- a/session/memcache/sess_memcache.go +++ b/session/memcache/sess_memcache.go @@ -149,16 +149,16 @@ func (rp *MemProvider) SessionRead(sid string) (session.Store, error) { } // SessionExist check memcache session exist by sid -func (rp *MemProvider) SessionExist(sid string) bool { +func (rp *MemProvider) SessionExist(sid string) (bool, error) { if client == nil { if err := rp.connectInit(); err != nil { - return false + return false, err } } if item, err := client.Get(sid); err != nil || len(item.Value) == 0 { - return false + return false, err } - return true + return true, nil } // SessionRegenerate generate new sid for memcache session diff --git a/session/mysql/sess_mysql.go b/session/mysql/sess_mysql.go index 301353ab..9f9547a7 100644 --- a/session/mysql/sess_mysql.go +++ b/session/mysql/sess_mysql.go @@ -164,13 +164,19 @@ func (mp *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist check mysql session exist -func (mp *Provider) SessionExist(sid string) bool { +func (mp *Provider) SessionExist(sid string) (bool, error) { c := mp.connectInit() defer c.Close() row := c.QueryRow("select session_data from "+TableName+" where session_key=?", sid) var sessiondata []byte err := row.Scan(&sessiondata) - return err != sql.ErrNoRows + if err != nil { + if err == sql.ErrNoRows { + return false, nil + } + return false, err + } + return true, nil } // SessionRegenerate generate new sid for mysql session diff --git a/session/postgres/sess_postgresql.go b/session/postgres/sess_postgresql.go index 0b8b9645..d8a1e6de 100644 --- a/session/postgres/sess_postgresql.go +++ b/session/postgres/sess_postgresql.go @@ -178,13 +178,19 @@ func (mp *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist check postgresql session exist -func (mp *Provider) SessionExist(sid string) bool { +func (mp *Provider) SessionExist(sid string) (bool, error) { c := mp.connectInit() defer c.Close() row := c.QueryRow("select session_data from session where session_key=$1", sid) var sessiondata []byte err := row.Scan(&sessiondata) - return err != sql.ErrNoRows + if err != nil { + if err == sql.ErrNoRows { + return false, nil + } + return false, err + } + return true, nil } // SessionRegenerate generate new sid for postgresql session diff --git a/session/redis/sess_redis.go b/session/redis/sess_redis.go index 5c382d61..439b14cb 100644 --- a/session/redis/sess_redis.go +++ b/session/redis/sess_redis.go @@ -211,14 +211,14 @@ func (rp *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist check redis session exist by sid -func (rp *Provider) SessionExist(sid string) bool { +func (rp *Provider) SessionExist(sid string) (bool, error) { c := rp.poollist.Get() defer c.Close() if existed, err := redis.Int(c.Do("EXISTS", sid)); err != nil || existed == 0 { - return false + return false, err } - return true + return true, nil } // SessionRegenerate generate new sid for redis session diff --git a/session/redis_cluster/redis_cluster.go b/session/redis_cluster/redis_cluster.go index 262fa2e3..d4e28327 100644 --- a/session/redis_cluster/redis_cluster.go +++ b/session/redis_cluster/redis_cluster.go @@ -176,12 +176,12 @@ func (rp *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist check redis_cluster session exist by sid -func (rp *Provider) SessionExist(sid string) bool { +func (rp *Provider) SessionExist(sid string) (bool, error) { c := rp.poollist if existed, err := c.Exists(sid).Result(); err != nil || existed == 0 { - return false + return false, err } - return true + return true, nil } // SessionRegenerate generate new sid for redis_cluster session diff --git a/session/redis_sentinel/sess_redis_sentinel.go b/session/redis_sentinel/sess_redis_sentinel.go index 6ecb2977..eead7a74 100644 --- a/session/redis_sentinel/sess_redis_sentinel.go +++ b/session/redis_sentinel/sess_redis_sentinel.go @@ -189,12 +189,12 @@ func (rp *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist check redis_sentinel session exist by sid -func (rp *Provider) SessionExist(sid string) bool { +func (rp *Provider) SessionExist(sid string) (bool, error) { c := rp.poollist if existed, err := c.Exists(sid).Result(); err != nil || existed == 0 { - return false + return false, err } - return true + return true, nil } // SessionRegenerate generate new sid for redis_sentinel session diff --git a/session/sess_cookie.go b/session/sess_cookie.go index 6ad5debc..30a7032e 100644 --- a/session/sess_cookie.go +++ b/session/sess_cookie.go @@ -147,8 +147,8 @@ func (pder *CookieProvider) SessionRead(sid string) (Store, error) { } // SessionExist Cookie session is always existed -func (pder *CookieProvider) SessionExist(sid string) bool { - return true +func (pder *CookieProvider) SessionExist(sid string) (bool, error) { + return true, nil } // SessionRegenerate Implement method, no used. diff --git a/session/sess_file.go b/session/sess_file.go index 47ad54a7..3345d5d0 100644 --- a/session/sess_file.go +++ b/session/sess_file.go @@ -176,17 +176,20 @@ func (fp *FileProvider) SessionRead(sid string) (Store, error) { // SessionExist Check file session exist. // it checks the file named from sid exist or not. -func (fp *FileProvider) SessionExist(sid string) bool { +func (fp *FileProvider) SessionExist(sid string) (bool, error) { filepder.lock.Lock() defer filepder.lock.Unlock() if len(sid) < 2 { SLogger.Println("min length of session id is 2", sid) - return false + return false, nil } _, err := os.Stat(path.Join(fp.savePath, string(sid[0]), string(sid[1]), sid)) - return err == nil + if err != nil { + return false, nil + } + return true, nil } // SessionDestroy Remove all files in this save path diff --git a/session/sess_file_test.go b/session/sess_file_test.go index 021c43fc..1e155f91 100644 --- a/session/sess_file_test.go +++ b/session/sess_file_test.go @@ -56,16 +56,24 @@ func TestFileProvider_SessionExist(t *testing.T) { _ = fp.SessionInit(180, sessionPath) - if fp.SessionExist(sid) { + exists, err := fp.SessionExist(sid) + if err != nil{ + t.Error(err) + } + if exists { t.Error() } - _, err := fp.SessionRead(sid) + _, err = fp.SessionRead(sid) if err != nil { t.Error(err) } - if !fp.SessionExist(sid) { + exists, err = fp.SessionExist(sid) + if err != nil { + t.Error(err) + } + if !exists { t.Error() } } @@ -79,15 +87,27 @@ func TestFileProvider_SessionExist2(t *testing.T) { _ = fp.SessionInit(180, sessionPath) - if fp.SessionExist(sid) { + exists, err := fp.SessionExist(sid) + if err != nil { + t.Error(err) + } + if exists { t.Error() } - if fp.SessionExist("") { + exists, err = fp.SessionExist("") + if err != nil { + t.Error(err) + } + if exists { t.Error() } - if fp.SessionExist("1") { + exists, err = fp.SessionExist("1") + if err != nil { + t.Error(err) + } + if exists { t.Error() } } @@ -171,7 +191,11 @@ func TestFileProvider_SessionRegenerate(t *testing.T) { t.Error(err) } - if !fp.SessionExist(sid) { + exists, err := fp.SessionExist(sid) + if err != nil { + t.Error(err) + } + if !exists { t.Error() } @@ -180,11 +204,19 @@ func TestFileProvider_SessionRegenerate(t *testing.T) { t.Error(err) } - if fp.SessionExist(sid) { + exists, err = fp.SessionExist(sid) + if err != nil { + t.Error(err) + } + if exists { t.Error() } - if !fp.SessionExist(sidNew) { + exists, err = fp.SessionExist(sidNew) + if err != nil { + t.Error(err) + } + if !exists { t.Error() } } @@ -203,7 +235,11 @@ func TestFileProvider_SessionDestroy(t *testing.T) { t.Error(err) } - if !fp.SessionExist(sid) { + exists, err := fp.SessionExist(sid) + if err != nil { + t.Error(err) + } + if !exists { t.Error() } @@ -212,7 +248,11 @@ func TestFileProvider_SessionDestroy(t *testing.T) { t.Error(err) } - if fp.SessionExist(sid) { + exists, err = fp.SessionExist(sid) + if err != nil { + t.Error(err) + } + if exists { t.Error() } } diff --git a/session/sess_mem.go b/session/sess_mem.go index 64d8b056..bd69ff80 100644 --- a/session/sess_mem.go +++ b/session/sess_mem.go @@ -109,13 +109,13 @@ func (pder *MemProvider) SessionRead(sid string) (Store, error) { } // SessionExist check session store exist in memory session by sid -func (pder *MemProvider) SessionExist(sid string) bool { +func (pder *MemProvider) SessionExist(sid string) (bool, error) { pder.lock.RLock() defer pder.lock.RUnlock() if _, ok := pder.sessions[sid]; ok { - return true + return true, nil } - return false + return false, nil } // SessionRegenerate generate new sid for session store in memory session diff --git a/session/session.go b/session/session.go index eb85360a..92e35de4 100644 --- a/session/session.go +++ b/session/session.go @@ -56,7 +56,7 @@ type Store interface { type Provider interface { SessionInit(gclifetime int64, config string) error SessionRead(sid string) (Store, error) - SessionExist(sid string) bool + SessionExist(sid string) (bool, error) SessionRegenerate(oldsid, sid string) (Store, error) SessionDestroy(sid string) error SessionAll() int //get all active session @@ -211,8 +211,14 @@ func (manager *Manager) SessionStart(w http.ResponseWriter, r *http.Request) (se return nil, errs } - if sid != "" && manager.provider.SessionExist(sid) { - return manager.provider.SessionRead(sid) + if sid != "" { + exists, err := manager.provider.SessionExist(sid) + if err != nil { + return nil, err + } + if exists { + return manager.provider.SessionRead(sid) + } } // Generate a new session diff --git a/session/ssdb/sess_ssdb.go b/session/ssdb/sess_ssdb.go index de0c6360..9b9eee94 100644 --- a/session/ssdb/sess_ssdb.go +++ b/session/ssdb/sess_ssdb.go @@ -68,7 +68,7 @@ func (p *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist judged whether sid is exist in session -func (p *Provider) SessionExist(sid string) bool { +func (p *Provider) SessionExist(sid string) (bool, error) { if p.client == nil { if err := p.connectInit(); err != nil { panic(err) @@ -76,12 +76,12 @@ func (p *Provider) SessionExist(sid string) bool { } value, err := p.client.Get(sid) if err != nil { - panic(err) + return false, err } if value == nil || len(value.(string)) == 0 { - return false + return false, nil } - return true + return true, nil } // SessionRegenerate regenerate session with new sid and delete oldsid From 6f5c5bd3a65561db56aca26eae4a50abef8fa5b4 Mon Sep 17 00:00:00 2001 From: Phillip Stagnet Date: Mon, 3 Aug 2020 13:33:30 +0200 Subject: [PATCH 2/6] Change interface in session README --- session/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/session/README.md b/session/README.md index 6d0a297e..a5c3bd6d 100644 --- a/session/README.md +++ b/session/README.md @@ -101,7 +101,7 @@ Maybe you will find the **memory** provider is a good example. type Provider interface { SessionInit(gclifetime int64, config string) error SessionRead(sid string) (SessionStore, error) - SessionExist(sid string) bool + SessionExist(sid string) (bool, error) SessionRegenerate(oldsid, sid string) (SessionStore, error) SessionDestroy(sid string) error SessionAll() int //get all active session From 5f2f6e4f86c36cf91ab11b227ca1d13a799c88b1 Mon Sep 17 00:00:00 2001 From: Phillip Stagnet Date: Wed, 5 Aug 2020 18:29:22 +0200 Subject: [PATCH 3/6] Add interface change in pkg folder --- pkg/session/couchbase/sess_couchbase.go | 6 +- pkg/session/ledis/ledis_session.go | 4 +- pkg/session/memcache/sess_memcache.go | 8 +-- pkg/session/mysql/sess_mysql.go | 10 ++- pkg/session/postgres/sess_postgresql.go | 10 ++- pkg/session/redis/sess_redis.go | 6 +- pkg/session/redis_cluster/redis_cluster.go | 6 +- .../redis_sentinel/sess_redis_sentinel.go | 6 +- pkg/session/sess_cookie.go | 4 +- pkg/session/sess_file.go | 8 +-- pkg/session/sess_file_test.go | 62 +++++++++++++++---- pkg/session/sess_mem.go | 6 +- pkg/session/session.go | 12 +++- pkg/session/ssdb/sess_ssdb.go | 8 +-- 14 files changed, 107 insertions(+), 49 deletions(-) diff --git a/pkg/session/couchbase/sess_couchbase.go b/pkg/session/couchbase/sess_couchbase.go index 227c0bc6..b824a938 100644 --- a/pkg/session/couchbase/sess_couchbase.go +++ b/pkg/session/couchbase/sess_couchbase.go @@ -179,16 +179,16 @@ func (cp *Provider) SessionRead(sid string) (session.Store, error) { // SessionExist Check couchbase session exist. // it checkes sid exist or not. -func (cp *Provider) SessionExist(sid string) bool { +func (cp *Provider) SessionExist(sid string) (bool, error) { cp.b = cp.getBucket() defer cp.b.Close() var doc []byte if err := cp.b.Get(sid, &doc); err != nil || doc == nil { - return false + return false, err } - return true + return true, nil } // SessionRegenerate remove oldsid and use sid to generate new session diff --git a/pkg/session/ledis/ledis_session.go b/pkg/session/ledis/ledis_session.go index a0988327..e43d70a0 100644 --- a/pkg/session/ledis/ledis_session.go +++ b/pkg/session/ledis/ledis_session.go @@ -132,9 +132,9 @@ func (lp *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist check ledis session exist by sid -func (lp *Provider) SessionExist(sid string) bool { +func (lp *Provider) SessionExist(sid string) (bool, error) { count, _ := c.Exists([]byte(sid)) - return count != 0 + return count != 0, nil } // SessionRegenerate generate new sid for ledis session diff --git a/pkg/session/memcache/sess_memcache.go b/pkg/session/memcache/sess_memcache.go index 6cd8acab..7fab842a 100644 --- a/pkg/session/memcache/sess_memcache.go +++ b/pkg/session/memcache/sess_memcache.go @@ -149,16 +149,16 @@ func (rp *MemProvider) SessionRead(sid string) (session.Store, error) { } // SessionExist check memcache session exist by sid -func (rp *MemProvider) SessionExist(sid string) bool { +func (rp *MemProvider) SessionExist(sid string) (bool, error) { if client == nil { if err := rp.connectInit(); err != nil { - return false + return false, err } } if item, err := client.Get(sid); err != nil || len(item.Value) == 0 { - return false + return false, err } - return true + return true, nil } // SessionRegenerate generate new sid for memcache session diff --git a/pkg/session/mysql/sess_mysql.go b/pkg/session/mysql/sess_mysql.go index 73738496..c641a4bf 100644 --- a/pkg/session/mysql/sess_mysql.go +++ b/pkg/session/mysql/sess_mysql.go @@ -164,13 +164,19 @@ func (mp *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist check mysql session exist -func (mp *Provider) SessionExist(sid string) bool { +func (mp *Provider) SessionExist(sid string) (bool, error) { c := mp.connectInit() defer c.Close() row := c.QueryRow("select session_data from "+TableName+" where session_key=?", sid) var sessiondata []byte err := row.Scan(&sessiondata) - return err != sql.ErrNoRows + if err != nil { + if err == sql.ErrNoRows { + return false, nil + } + return false, err + } + return true, nil } // SessionRegenerate generate new sid for mysql session diff --git a/pkg/session/postgres/sess_postgresql.go b/pkg/session/postgres/sess_postgresql.go index e6c9ed89..688c0e36 100644 --- a/pkg/session/postgres/sess_postgresql.go +++ b/pkg/session/postgres/sess_postgresql.go @@ -178,13 +178,19 @@ func (mp *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist check postgresql session exist -func (mp *Provider) SessionExist(sid string) bool { +func (mp *Provider) SessionExist(sid string) (bool, error) { c := mp.connectInit() defer c.Close() row := c.QueryRow("select session_data from session where session_key=$1", sid) var sessiondata []byte err := row.Scan(&sessiondata) - return err != sql.ErrNoRows + if err != nil { + if err == sql.ErrNoRows { + return false, nil + } + return false, err + } + return true, nil } // SessionRegenerate generate new sid for postgresql session diff --git a/pkg/session/redis/sess_redis.go b/pkg/session/redis/sess_redis.go index f569f9dd..cf2dddf4 100644 --- a/pkg/session/redis/sess_redis.go +++ b/pkg/session/redis/sess_redis.go @@ -211,14 +211,14 @@ func (rp *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist check redis session exist by sid -func (rp *Provider) SessionExist(sid string) bool { +func (rp *Provider) SessionExist(sid string) (bool, error) { c := rp.poollist.Get() defer c.Close() if existed, err := redis.Int(c.Do("EXISTS", sid)); err != nil || existed == 0 { - return false + return false, err } - return true + return true, nil } // SessionRegenerate generate new sid for redis session diff --git a/pkg/session/redis_cluster/redis_cluster.go b/pkg/session/redis_cluster/redis_cluster.go index f7fc7845..8b20ab19 100644 --- a/pkg/session/redis_cluster/redis_cluster.go +++ b/pkg/session/redis_cluster/redis_cluster.go @@ -176,12 +176,12 @@ func (rp *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist check redis_cluster session exist by sid -func (rp *Provider) SessionExist(sid string) bool { +func (rp *Provider) SessionExist(sid string) (bool, error) { c := rp.poollist if existed, err := c.Exists(sid).Result(); err != nil || existed == 0 { - return false + return false, err } - return true + return true, nil } // SessionRegenerate generate new sid for redis_cluster session diff --git a/pkg/session/redis_sentinel/sess_redis_sentinel.go b/pkg/session/redis_sentinel/sess_redis_sentinel.go index 23bebf2a..f78486be 100644 --- a/pkg/session/redis_sentinel/sess_redis_sentinel.go +++ b/pkg/session/redis_sentinel/sess_redis_sentinel.go @@ -189,12 +189,12 @@ func (rp *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist check redis_sentinel session exist by sid -func (rp *Provider) SessionExist(sid string) bool { +func (rp *Provider) SessionExist(sid string) (bool, error) { c := rp.poollist if existed, err := c.Exists(sid).Result(); err != nil || existed == 0 { - return false + return false, err } - return true + return true, nil } // SessionRegenerate generate new sid for redis_sentinel session diff --git a/pkg/session/sess_cookie.go b/pkg/session/sess_cookie.go index 6ad5debc..30a7032e 100644 --- a/pkg/session/sess_cookie.go +++ b/pkg/session/sess_cookie.go @@ -147,8 +147,8 @@ func (pder *CookieProvider) SessionRead(sid string) (Store, error) { } // SessionExist Cookie session is always existed -func (pder *CookieProvider) SessionExist(sid string) bool { - return true +func (pder *CookieProvider) SessionExist(sid string) (bool, error) { + return true, nil } // SessionRegenerate Implement method, no used. diff --git a/pkg/session/sess_file.go b/pkg/session/sess_file.go index 47ad54a7..37d5bd68 100644 --- a/pkg/session/sess_file.go +++ b/pkg/session/sess_file.go @@ -176,17 +176,17 @@ func (fp *FileProvider) SessionRead(sid string) (Store, error) { // SessionExist Check file session exist. // it checks the file named from sid exist or not. -func (fp *FileProvider) SessionExist(sid string) bool { +func (fp *FileProvider) SessionExist(sid string) (bool, error) { filepder.lock.Lock() defer filepder.lock.Unlock() if len(sid) < 2 { - SLogger.Println("min length of session id is 2", sid) - return false + SLogger.Println("min length of session id is 2 but got length: ", sid) + return false, errors.New("min length of session id is 2") } _, err := os.Stat(path.Join(fp.savePath, string(sid[0]), string(sid[1]), sid)) - return err == nil + return err == nil, nil } // SessionDestroy Remove all files in this save path diff --git a/pkg/session/sess_file_test.go b/pkg/session/sess_file_test.go index 021c43fc..64b8d94a 100644 --- a/pkg/session/sess_file_test.go +++ b/pkg/session/sess_file_test.go @@ -56,16 +56,24 @@ func TestFileProvider_SessionExist(t *testing.T) { _ = fp.SessionInit(180, sessionPath) - if fp.SessionExist(sid) { + exists, err := fp.SessionExist(sid) + if err != nil{ + t.Error(err) + } + if exists { t.Error() } - _, err := fp.SessionRead(sid) + _, err = fp.SessionRead(sid) if err != nil { t.Error(err) } - if !fp.SessionExist(sid) { + exists, err = fp.SessionExist(sid) + if err != nil { + t.Error(err) + } + if !exists { t.Error() } } @@ -79,15 +87,27 @@ func TestFileProvider_SessionExist2(t *testing.T) { _ = fp.SessionInit(180, sessionPath) - if fp.SessionExist(sid) { + exists, err := fp.SessionExist(sid) + if err != nil { + t.Error(err) + } + if exists { t.Error() } - if fp.SessionExist("") { + exists, err = fp.SessionExist("") + if err == nil { + t.Error() + } + if exists { t.Error() } - if fp.SessionExist("1") { + exists, err = fp.SessionExist("1") + if err == nil { + t.Error() + } + if exists { t.Error() } } @@ -171,7 +191,11 @@ func TestFileProvider_SessionRegenerate(t *testing.T) { t.Error(err) } - if !fp.SessionExist(sid) { + exists, err := fp.SessionExist(sid) + if err != nil { + t.Error(err) + } + if !exists { t.Error() } @@ -180,11 +204,19 @@ func TestFileProvider_SessionRegenerate(t *testing.T) { t.Error(err) } - if fp.SessionExist(sid) { + exists, err = fp.SessionExist(sid) + if err != nil { + t.Error(err) + } + if exists { t.Error() } - if !fp.SessionExist(sidNew) { + exists, err = fp.SessionExist(sidNew) + if err != nil { + t.Error(err) + } + if !exists { t.Error() } } @@ -203,7 +235,11 @@ func TestFileProvider_SessionDestroy(t *testing.T) { t.Error(err) } - if !fp.SessionExist(sid) { + exists, err := fp.SessionExist(sid) + if err != nil { + t.Error(err) + } + if !exists { t.Error() } @@ -212,7 +248,11 @@ func TestFileProvider_SessionDestroy(t *testing.T) { t.Error(err) } - if fp.SessionExist(sid) { + exists, err = fp.SessionExist(sid) + if err != nil { + t.Error(err) + } + if exists { t.Error() } } diff --git a/pkg/session/sess_mem.go b/pkg/session/sess_mem.go index 64d8b056..bd69ff80 100644 --- a/pkg/session/sess_mem.go +++ b/pkg/session/sess_mem.go @@ -109,13 +109,13 @@ func (pder *MemProvider) SessionRead(sid string) (Store, error) { } // SessionExist check session store exist in memory session by sid -func (pder *MemProvider) SessionExist(sid string) bool { +func (pder *MemProvider) SessionExist(sid string) (bool, error) { pder.lock.RLock() defer pder.lock.RUnlock() if _, ok := pder.sessions[sid]; ok { - return true + return true, nil } - return false + return false, nil } // SessionRegenerate generate new sid for session store in memory session diff --git a/pkg/session/session.go b/pkg/session/session.go index eb85360a..92e35de4 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -56,7 +56,7 @@ type Store interface { type Provider interface { SessionInit(gclifetime int64, config string) error SessionRead(sid string) (Store, error) - SessionExist(sid string) bool + SessionExist(sid string) (bool, error) SessionRegenerate(oldsid, sid string) (Store, error) SessionDestroy(sid string) error SessionAll() int //get all active session @@ -211,8 +211,14 @@ func (manager *Manager) SessionStart(w http.ResponseWriter, r *http.Request) (se return nil, errs } - if sid != "" && manager.provider.SessionExist(sid) { - return manager.provider.SessionRead(sid) + if sid != "" { + exists, err := manager.provider.SessionExist(sid) + if err != nil { + return nil, err + } + if exists { + return manager.provider.SessionRead(sid) + } } // Generate a new session diff --git a/pkg/session/ssdb/sess_ssdb.go b/pkg/session/ssdb/sess_ssdb.go index 1b382954..f950b835 100644 --- a/pkg/session/ssdb/sess_ssdb.go +++ b/pkg/session/ssdb/sess_ssdb.go @@ -68,10 +68,10 @@ func (p *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist judged whether sid is exist in session -func (p *Provider) SessionExist(sid string) bool { +func (p *Provider) SessionExist(sid string) (bool, error) { if p.client == nil { if err := p.connectInit(); err != nil { - panic(err) + return false, err } } value, err := p.client.Get(sid) @@ -79,9 +79,9 @@ func (p *Provider) SessionExist(sid string) bool { panic(err) } if value == nil || len(value.(string)) == 0 { - return false + return false, nil } - return true + return true, nil } // SessionRegenerate regenerate session with new sid and delete oldsid From 3052c64b6c47488c4c4c949629c9fabe59616447 Mon Sep 17 00:00:00 2001 From: Phillip Stagnet Date: Wed, 5 Aug 2020 18:29:47 +0200 Subject: [PATCH 4/6] Revert "Add error to SessionExist interface" This reverts commit 28e6b3b92450b0ca0e9c1342d400f8810e4d5e5a. --- session/couchbase/sess_couchbase.go | 6 +- session/ledis/ledis_session.go | 4 +- session/memcache/sess_memcache.go | 8 +-- session/mysql/sess_mysql.go | 10 +-- session/postgres/sess_postgresql.go | 10 +-- session/redis/sess_redis.go | 6 +- session/redis_cluster/redis_cluster.go | 6 +- session/redis_sentinel/sess_redis_sentinel.go | 6 +- session/sess_cookie.go | 4 +- session/sess_file.go | 9 +-- session/sess_file_test.go | 62 ++++--------------- session/sess_mem.go | 6 +- session/session.go | 12 +--- session/ssdb/sess_ssdb.go | 8 +-- 14 files changed, 48 insertions(+), 109 deletions(-) diff --git a/session/couchbase/sess_couchbase.go b/session/couchbase/sess_couchbase.go index 46ab07ab..707d042c 100644 --- a/session/couchbase/sess_couchbase.go +++ b/session/couchbase/sess_couchbase.go @@ -179,16 +179,16 @@ func (cp *Provider) SessionRead(sid string) (session.Store, error) { // SessionExist Check couchbase session exist. // it checkes sid exist or not. -func (cp *Provider) SessionExist(sid string) (bool, error) { +func (cp *Provider) SessionExist(sid string) bool { cp.b = cp.getBucket() defer cp.b.Close() var doc []byte if err := cp.b.Get(sid, &doc); err != nil || doc == nil { - return false, err + return false } - return true, nil + return true } // SessionRegenerate remove oldsid and use sid to generate new session diff --git a/session/ledis/ledis_session.go b/session/ledis/ledis_session.go index 4f578eac..ee81df67 100644 --- a/session/ledis/ledis_session.go +++ b/session/ledis/ledis_session.go @@ -132,9 +132,9 @@ func (lp *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist check ledis session exist by sid -func (lp *Provider) SessionExist(sid string) (bool, error) { +func (lp *Provider) SessionExist(sid string) bool { count, _ := c.Exists([]byte(sid)) - return count != 0, nil + return count != 0 } // SessionRegenerate generate new sid for ledis session diff --git a/session/memcache/sess_memcache.go b/session/memcache/sess_memcache.go index e76eb8a5..85a2d815 100644 --- a/session/memcache/sess_memcache.go +++ b/session/memcache/sess_memcache.go @@ -149,16 +149,16 @@ func (rp *MemProvider) SessionRead(sid string) (session.Store, error) { } // SessionExist check memcache session exist by sid -func (rp *MemProvider) SessionExist(sid string) (bool, error) { +func (rp *MemProvider) SessionExist(sid string) bool { if client == nil { if err := rp.connectInit(); err != nil { - return false, err + return false } } if item, err := client.Get(sid); err != nil || len(item.Value) == 0 { - return false, err + return false } - return true, nil + return true } // SessionRegenerate generate new sid for memcache session diff --git a/session/mysql/sess_mysql.go b/session/mysql/sess_mysql.go index 9f9547a7..301353ab 100644 --- a/session/mysql/sess_mysql.go +++ b/session/mysql/sess_mysql.go @@ -164,19 +164,13 @@ func (mp *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist check mysql session exist -func (mp *Provider) SessionExist(sid string) (bool, error) { +func (mp *Provider) SessionExist(sid string) bool { c := mp.connectInit() defer c.Close() row := c.QueryRow("select session_data from "+TableName+" where session_key=?", sid) var sessiondata []byte err := row.Scan(&sessiondata) - if err != nil { - if err == sql.ErrNoRows { - return false, nil - } - return false, err - } - return true, nil + return err != sql.ErrNoRows } // SessionRegenerate generate new sid for mysql session diff --git a/session/postgres/sess_postgresql.go b/session/postgres/sess_postgresql.go index d8a1e6de..0b8b9645 100644 --- a/session/postgres/sess_postgresql.go +++ b/session/postgres/sess_postgresql.go @@ -178,19 +178,13 @@ func (mp *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist check postgresql session exist -func (mp *Provider) SessionExist(sid string) (bool, error) { +func (mp *Provider) SessionExist(sid string) bool { c := mp.connectInit() defer c.Close() row := c.QueryRow("select session_data from session where session_key=$1", sid) var sessiondata []byte err := row.Scan(&sessiondata) - if err != nil { - if err == sql.ErrNoRows { - return false, nil - } - return false, err - } - return true, nil + return err != sql.ErrNoRows } // SessionRegenerate generate new sid for postgresql session diff --git a/session/redis/sess_redis.go b/session/redis/sess_redis.go index 439b14cb..5c382d61 100644 --- a/session/redis/sess_redis.go +++ b/session/redis/sess_redis.go @@ -211,14 +211,14 @@ func (rp *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist check redis session exist by sid -func (rp *Provider) SessionExist(sid string) (bool, error) { +func (rp *Provider) SessionExist(sid string) bool { c := rp.poollist.Get() defer c.Close() if existed, err := redis.Int(c.Do("EXISTS", sid)); err != nil || existed == 0 { - return false, err + return false } - return true, nil + return true } // SessionRegenerate generate new sid for redis session diff --git a/session/redis_cluster/redis_cluster.go b/session/redis_cluster/redis_cluster.go index d4e28327..262fa2e3 100644 --- a/session/redis_cluster/redis_cluster.go +++ b/session/redis_cluster/redis_cluster.go @@ -176,12 +176,12 @@ func (rp *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist check redis_cluster session exist by sid -func (rp *Provider) SessionExist(sid string) (bool, error) { +func (rp *Provider) SessionExist(sid string) bool { c := rp.poollist if existed, err := c.Exists(sid).Result(); err != nil || existed == 0 { - return false, err + return false } - return true, nil + return true } // SessionRegenerate generate new sid for redis_cluster session diff --git a/session/redis_sentinel/sess_redis_sentinel.go b/session/redis_sentinel/sess_redis_sentinel.go index eead7a74..6ecb2977 100644 --- a/session/redis_sentinel/sess_redis_sentinel.go +++ b/session/redis_sentinel/sess_redis_sentinel.go @@ -189,12 +189,12 @@ func (rp *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist check redis_sentinel session exist by sid -func (rp *Provider) SessionExist(sid string) (bool, error) { +func (rp *Provider) SessionExist(sid string) bool { c := rp.poollist if existed, err := c.Exists(sid).Result(); err != nil || existed == 0 { - return false, err + return false } - return true, nil + return true } // SessionRegenerate generate new sid for redis_sentinel session diff --git a/session/sess_cookie.go b/session/sess_cookie.go index 30a7032e..6ad5debc 100644 --- a/session/sess_cookie.go +++ b/session/sess_cookie.go @@ -147,8 +147,8 @@ func (pder *CookieProvider) SessionRead(sid string) (Store, error) { } // SessionExist Cookie session is always existed -func (pder *CookieProvider) SessionExist(sid string) (bool, error) { - return true, nil +func (pder *CookieProvider) SessionExist(sid string) bool { + return true } // SessionRegenerate Implement method, no used. diff --git a/session/sess_file.go b/session/sess_file.go index 3345d5d0..47ad54a7 100644 --- a/session/sess_file.go +++ b/session/sess_file.go @@ -176,20 +176,17 @@ func (fp *FileProvider) SessionRead(sid string) (Store, error) { // SessionExist Check file session exist. // it checks the file named from sid exist or not. -func (fp *FileProvider) SessionExist(sid string) (bool, error) { +func (fp *FileProvider) SessionExist(sid string) bool { filepder.lock.Lock() defer filepder.lock.Unlock() if len(sid) < 2 { SLogger.Println("min length of session id is 2", sid) - return false, nil + return false } _, err := os.Stat(path.Join(fp.savePath, string(sid[0]), string(sid[1]), sid)) - if err != nil { - return false, nil - } - return true, nil + return err == nil } // SessionDestroy Remove all files in this save path diff --git a/session/sess_file_test.go b/session/sess_file_test.go index 1e155f91..021c43fc 100644 --- a/session/sess_file_test.go +++ b/session/sess_file_test.go @@ -56,24 +56,16 @@ func TestFileProvider_SessionExist(t *testing.T) { _ = fp.SessionInit(180, sessionPath) - exists, err := fp.SessionExist(sid) - if err != nil{ - t.Error(err) - } - if exists { + if fp.SessionExist(sid) { t.Error() } - _, err = fp.SessionRead(sid) + _, err := fp.SessionRead(sid) if err != nil { t.Error(err) } - exists, err = fp.SessionExist(sid) - if err != nil { - t.Error(err) - } - if !exists { + if !fp.SessionExist(sid) { t.Error() } } @@ -87,27 +79,15 @@ func TestFileProvider_SessionExist2(t *testing.T) { _ = fp.SessionInit(180, sessionPath) - exists, err := fp.SessionExist(sid) - if err != nil { - t.Error(err) - } - if exists { + if fp.SessionExist(sid) { t.Error() } - exists, err = fp.SessionExist("") - if err != nil { - t.Error(err) - } - if exists { + if fp.SessionExist("") { t.Error() } - exists, err = fp.SessionExist("1") - if err != nil { - t.Error(err) - } - if exists { + if fp.SessionExist("1") { t.Error() } } @@ -191,11 +171,7 @@ func TestFileProvider_SessionRegenerate(t *testing.T) { t.Error(err) } - exists, err := fp.SessionExist(sid) - if err != nil { - t.Error(err) - } - if !exists { + if !fp.SessionExist(sid) { t.Error() } @@ -204,19 +180,11 @@ func TestFileProvider_SessionRegenerate(t *testing.T) { t.Error(err) } - exists, err = fp.SessionExist(sid) - if err != nil { - t.Error(err) - } - if exists { + if fp.SessionExist(sid) { t.Error() } - exists, err = fp.SessionExist(sidNew) - if err != nil { - t.Error(err) - } - if !exists { + if !fp.SessionExist(sidNew) { t.Error() } } @@ -235,11 +203,7 @@ func TestFileProvider_SessionDestroy(t *testing.T) { t.Error(err) } - exists, err := fp.SessionExist(sid) - if err != nil { - t.Error(err) - } - if !exists { + if !fp.SessionExist(sid) { t.Error() } @@ -248,11 +212,7 @@ func TestFileProvider_SessionDestroy(t *testing.T) { t.Error(err) } - exists, err = fp.SessionExist(sid) - if err != nil { - t.Error(err) - } - if exists { + if fp.SessionExist(sid) { t.Error() } } diff --git a/session/sess_mem.go b/session/sess_mem.go index bd69ff80..64d8b056 100644 --- a/session/sess_mem.go +++ b/session/sess_mem.go @@ -109,13 +109,13 @@ func (pder *MemProvider) SessionRead(sid string) (Store, error) { } // SessionExist check session store exist in memory session by sid -func (pder *MemProvider) SessionExist(sid string) (bool, error) { +func (pder *MemProvider) SessionExist(sid string) bool { pder.lock.RLock() defer pder.lock.RUnlock() if _, ok := pder.sessions[sid]; ok { - return true, nil + return true } - return false, nil + return false } // SessionRegenerate generate new sid for session store in memory session diff --git a/session/session.go b/session/session.go index 92e35de4..eb85360a 100644 --- a/session/session.go +++ b/session/session.go @@ -56,7 +56,7 @@ type Store interface { type Provider interface { SessionInit(gclifetime int64, config string) error SessionRead(sid string) (Store, error) - SessionExist(sid string) (bool, error) + SessionExist(sid string) bool SessionRegenerate(oldsid, sid string) (Store, error) SessionDestroy(sid string) error SessionAll() int //get all active session @@ -211,14 +211,8 @@ func (manager *Manager) SessionStart(w http.ResponseWriter, r *http.Request) (se return nil, errs } - if sid != "" { - exists, err := manager.provider.SessionExist(sid) - if err != nil { - return nil, err - } - if exists { - return manager.provider.SessionRead(sid) - } + if sid != "" && manager.provider.SessionExist(sid) { + return manager.provider.SessionRead(sid) } // Generate a new session diff --git a/session/ssdb/sess_ssdb.go b/session/ssdb/sess_ssdb.go index 9b9eee94..de0c6360 100644 --- a/session/ssdb/sess_ssdb.go +++ b/session/ssdb/sess_ssdb.go @@ -68,7 +68,7 @@ func (p *Provider) SessionRead(sid string) (session.Store, error) { } // SessionExist judged whether sid is exist in session -func (p *Provider) SessionExist(sid string) (bool, error) { +func (p *Provider) SessionExist(sid string) bool { if p.client == nil { if err := p.connectInit(); err != nil { panic(err) @@ -76,12 +76,12 @@ func (p *Provider) SessionExist(sid string) (bool, error) { } value, err := p.client.Get(sid) if err != nil { - return false, err + panic(err) } if value == nil || len(value.(string)) == 0 { - return false, nil + return false } - return true, nil + return true } // SessionRegenerate regenerate session with new sid and delete oldsid From 009074725eb25fefb59df82711ab4a7edb1eaa1b Mon Sep 17 00:00:00 2001 From: Phillip Stagnet Date: Wed, 5 Aug 2020 18:32:33 +0200 Subject: [PATCH 5/6] Move interface change to pkg/session/README.md --- pkg/session/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/session/README.md b/pkg/session/README.md index 6d0a297e..a5c3bd6d 100644 --- a/pkg/session/README.md +++ b/pkg/session/README.md @@ -101,7 +101,7 @@ Maybe you will find the **memory** provider is a good example. type Provider interface { SessionInit(gclifetime int64, config string) error SessionRead(sid string) (SessionStore, error) - SessionExist(sid string) bool + SessionExist(sid string) (bool, error) SessionRegenerate(oldsid, sid string) (SessionStore, error) SessionDestroy(sid string) error SessionAll() int //get all active session From 5c8c088684f7aa2070bbcf234f4fe4e103c16157 Mon Sep 17 00:00:00 2001 From: Phillip Stagnet Date: Wed, 5 Aug 2020 18:33:17 +0200 Subject: [PATCH 6/6] Revert "Change interface in session README" This reverts commit 6f5c5bd3a65561db56aca26eae4a50abef8fa5b4. --- session/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/session/README.md b/session/README.md index a5c3bd6d..6d0a297e 100644 --- a/session/README.md +++ b/session/README.md @@ -101,7 +101,7 @@ Maybe you will find the **memory** provider is a good example. type Provider interface { SessionInit(gclifetime int64, config string) error SessionRead(sid string) (SessionStore, error) - SessionExist(sid string) (bool, error) + SessionExist(sid string) bool SessionRegenerate(oldsid, sid string) (SessionStore, error) SessionDestroy(sid string) error SessionAll() int //get all active session