From 167fcd179dabfa0d0993a54f8342e806d7469020 Mon Sep 17 00:00:00 2001 From: "m.huber" Date: Mon, 22 Sep 2025 22:44:45 +0200 Subject: [PATCH 1/7] Add proper error message if session provider can not be created --- routers/common/middleware.go | 14 ++++++++++++-- routers/install/routes.go | 7 ++++++- routers/web/web.go | 6 +++++- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/routers/common/middleware.go b/routers/common/middleware.go index 2ba02de8edc48..48da635f1baef 100644 --- a/routers/common/middleware.go +++ b/routers/common/middleware.go @@ -107,8 +107,16 @@ func ForwardedHeadersHandler(limit int, trustedProxies []string) func(h http.Han return proxy.ForwardedHeaders(opt) } -func Sessioner() func(next http.Handler) http.Handler { - return session.Sessioner(session.Options{ +func Sessioner() (middleware func(next http.Handler) http.Handler, err error) { + // Recover from panic if session.Sessioner fails due to invalid config + // https://gitea.com/go-chi/session/src/commit/16768d98ec9667722b876d4bed11017ce16d4572/session.go#L237-L240 + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("failed to create session middleware: %w", r) + } + }() + + middleware = session.Sessioner(session.Options{ Provider: setting.SessionConfig.Provider, ProviderConfig: setting.SessionConfig.ProviderConfig, CookieName: setting.SessionConfig.CookieName, @@ -119,4 +127,6 @@ func Sessioner() func(next http.Handler) http.Handler { SameSite: setting.SessionConfig.SameSite, Domain: setting.SessionConfig.Domain, }) + + return middleware, err } diff --git a/routers/install/routes.go b/routers/install/routes.go index bc7a0eb48c661..e4f833e751573 100644 --- a/routers/install/routes.go +++ b/routers/install/routes.go @@ -8,6 +8,7 @@ import ( "html" "net/http" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/public" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web" @@ -23,7 +24,11 @@ func Routes() *web.Router { base.Methods("GET, HEAD", "/assets/*", public.FileHandlerFunc()) r := web.NewRouter() - r.Use(common.Sessioner(), Contexter()) + if sessionMid, err := common.Sessioner(); err == nil && sessionMid != nil { + r.Use(sessionMid, Contexter()) + } else { + log.Fatal("common.Sessioner failed: %v", err) + } r.Get("/", Install) // it must be on the root, because the "install.js" use the window.location to replace the "localhost" AppURL r.Post("/", web.Bind(forms.InstallForm{}), SubmitInstall) r.Get("/post-install", InstallDone) diff --git a/routers/web/web.go b/routers/web/web.go index 98cdf8b964128..5ee211b576a0c 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -267,7 +267,11 @@ func Routes() *web.Router { routes.Get("/ssh_info", misc.SSHInfo) routes.Get("/api/healthz", healthcheck.Check) - mid = append(mid, common.Sessioner(), context.Contexter()) + if sessionMid, err := common.Sessioner(); err == nil && sessionMid != nil { + mid = append(mid, sessionMid, context.Contexter()) + } else { + log.Fatal("common.Sessioner failed: %v", err) + } // Get user from session if logged in. mid = append(mid, webAuth(buildAuthGroup())) From d81e4005d7f358fbe40cefcd3103e53327284eee Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 22 Sep 2025 23:36:38 +0200 Subject: [PATCH 2/7] fix lint --- routers/common/middleware.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/routers/common/middleware.go b/routers/common/middleware.go index 48da635f1baef..510f7b7d24c7a 100644 --- a/routers/common/middleware.go +++ b/routers/common/middleware.go @@ -112,7 +112,8 @@ func Sessioner() (middleware func(next http.Handler) http.Handler, err error) { // https://gitea.com/go-chi/session/src/commit/16768d98ec9667722b876d4bed11017ce16d4572/session.go#L237-L240 defer func() { if r := recover(); r != nil { - err = fmt.Errorf("failed to create session middleware: %w", r) + rErr := r.(error) + err = fmt.Errorf("failed to create session middleware: %w", rErr) } }() From b739a977b14538f349b793ea89b60814288a423e Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 26 Sep 2025 18:43:40 +0200 Subject: [PATCH 3/7] Addopt new interface of gitea.com/go-chi/session --- go.mod | 2 +- go.sum | 4 ++-- modules/session/db.go | 13 +++++++------ modules/session/redis.go | 31 +++++++++++++++++++------------ modules/session/virtual.go | 20 +++++++++++++------- routers/common/middleware.go | 18 ++++++------------ 6 files changed, 48 insertions(+), 40 deletions(-) diff --git a/go.mod b/go.mod index f32c3e08ef436..b6d58f781a9b9 100644 --- a/go.mod +++ b/go.mod @@ -16,7 +16,7 @@ require ( gitea.com/go-chi/binding v0.0.0-20240430071103-39a851e106ed gitea.com/go-chi/cache v0.2.1 gitea.com/go-chi/captcha v0.0.0-20240315150714-fb487f629098 - gitea.com/go-chi/session v0.0.0-20240316035857-16768d98ec96 + gitea.com/go-chi/session v0.0.0-20250926004215-636cadd82e15 gitea.com/lunny/dingtalk_webhook v0.0.0-20171025031554-e3534c89ef96 gitea.com/lunny/levelqueue v0.4.2-0.20230414023320-3c0159fe0fe4 github.com/42wim/httpsig v1.2.3 diff --git a/go.sum b/go.sum index 1853693e90dcf..62f42739462d7 100644 --- a/go.sum +++ b/go.sum @@ -43,8 +43,8 @@ gitea.com/go-chi/cache v0.2.1 h1:bfAPkvXlbcZxPCpcmDVCWoHgiBSBmZN/QosnZvEC0+g= gitea.com/go-chi/cache v0.2.1/go.mod h1:Qic0HZ8hOHW62ETGbonpwz8WYypj9NieU9659wFUJ8Q= gitea.com/go-chi/captcha v0.0.0-20240315150714-fb487f629098 h1:p2ki+WK0cIeNQuqjR98IP2KZQKRzJJiV7aTeMAFwaWo= gitea.com/go-chi/captcha v0.0.0-20240315150714-fb487f629098/go.mod h1:LjzIOHlRemuUyO7WR12fmm18VZIlCAaOt9L3yKw40pk= -gitea.com/go-chi/session v0.0.0-20240316035857-16768d98ec96 h1:IFDiMBObsP6CZIRaDLd54SR6zPYAffPXiXck5Xslu0Q= -gitea.com/go-chi/session v0.0.0-20240316035857-16768d98ec96/go.mod h1:0iEpFKnwO5dG0aF98O4eq6FMsAiXkNBaDIlUOlq4BtM= +gitea.com/go-chi/session v0.0.0-20250926004215-636cadd82e15 h1:qFYmz05u/s9664o7+XEgrlHXSPQ4uHO8/ccZGUb1uxA= +gitea.com/go-chi/session v0.0.0-20250926004215-636cadd82e15/go.mod h1:0iEpFKnwO5dG0aF98O4eq6FMsAiXkNBaDIlUOlq4BtM= gitea.com/lunny/dingtalk_webhook v0.0.0-20171025031554-e3534c89ef96 h1:+wWBi6Qfruqu7xJgjOIrKVQGiLUZdpKYCZewJ4clqhw= gitea.com/lunny/dingtalk_webhook v0.0.0-20171025031554-e3534c89ef96/go.mod h1:VyMQP6ue6MKHM8UsOXfNfuMKD0oSAWZdXVcpHIN2yaY= gitea.com/lunny/levelqueue v0.4.2-0.20230414023320-3c0159fe0fe4 h1:IFT+hup2xejHqdhS7keYWioqfmxdnfblFDTGoOwcZ+o= diff --git a/modules/session/db.go b/modules/session/db.go index 7b5be1bc705ea..5c21fd37f1a40 100644 --- a/modules/session/db.go +++ b/modules/session/db.go @@ -5,6 +5,7 @@ package session import ( "context" + "fmt" "log" "sync" @@ -121,12 +122,12 @@ func (p *DBProvider) Read(sid string) (session.RawStore, error) { } // Exist returns true if session with given ID exists. -func (p *DBProvider) Exist(sid string) bool { +func (p *DBProvider) Exist(sid string) (bool, error) { has, err := auth.ExistSession(dbContext(), sid) if err != nil { - panic("session/DB: error checking existence: " + err.Error()) + return false, fmt.Errorf("session/DB: error checking existence: %w", err) } - return has + return has, nil } // Destroy deletes a session by session ID. @@ -155,12 +156,12 @@ func (p *DBProvider) Regenerate(oldsid, sid string) (_ session.RawStore, err err } // Count counts and returns number of sessions. -func (p *DBProvider) Count() int { +func (p *DBProvider) Count() (int, error) { total, err := auth.CountSessions(dbContext()) if err != nil { - panic("session/DB: error counting records: " + err.Error()) + fmt.Errorf("session/DB: error counting records: %w", err) } - return int(total) + return int(total), nil } // GC calls GC to clean expired sessions. diff --git a/modules/session/redis.go b/modules/session/redis.go index d89d8bc6e2100..bfcb1b8670498 100644 --- a/modules/session/redis.go +++ b/modules/session/redis.go @@ -135,10 +135,12 @@ func (p *RedisProvider) Init(maxlifetime int64, configs string) (err error) { // Read returns raw session store by session ID. func (p *RedisProvider) Read(sid string) (session.RawStore, error) { psid := p.prefix + sid - if !p.Exist(sid) { + if exist, err := p.Exist(sid); err == nil && !exist { if err := p.c.Set(graceful.GetManager().HammerContext(), psid, "", p.duration).Err(); err != nil { return nil, err } + } else if err != nil { + return nil, err } var kv map[any]any @@ -159,9 +161,9 @@ func (p *RedisProvider) Read(sid string) (session.RawStore, error) { } // Exist returns true if session with given ID exists. -func (p *RedisProvider) Exist(sid string) bool { +func (p *RedisProvider) Exist(sid string) (bool, error) { v, err := p.c.Exists(graceful.GetManager().HammerContext(), p.prefix+sid).Result() - return err == nil && v == 1 + return err == nil && v == 1, err } // Destroy deletes a session by session ID. @@ -174,11 +176,19 @@ func (p *RedisProvider) Regenerate(oldsid, sid string) (_ session.RawStore, err poldsid := p.prefix + oldsid psid := p.prefix + sid - if p.Exist(sid) { + exist, err := p.Exist(sid) + if err != nil { + return nil, err + } + if exist { return nil, fmt.Errorf("new sid '%s' already exists", sid) - } else if !p.Exist(oldsid) { - // Make a fake old session. - if err = p.c.Set(graceful.GetManager().HammerContext(), poldsid, "", p.duration).Err(); err != nil { + } else { + if exist, err := p.Exist(oldsid); err == nil && !exist { + // Make a fake old session. + if err = p.c.Set(graceful.GetManager().HammerContext(), poldsid, "", p.duration).Err(); err != nil { + return nil, err + } + } else if err != nil { return nil, err } } @@ -211,12 +221,9 @@ func (p *RedisProvider) Regenerate(oldsid, sid string) (_ session.RawStore, err } // Count counts and returns number of sessions. -func (p *RedisProvider) Count() int { +func (p *RedisProvider) Count() (int, error) { size, err := p.c.DBSize(graceful.GetManager().HammerContext()).Result() - if err != nil { - return 0 - } - return int(size) + return int(size), err } // GC calls GC to clean expired sessions. diff --git a/modules/session/virtual.go b/modules/session/virtual.go index 2e29b5fc6f68a..35a995d2d0e20 100644 --- a/modules/session/virtual.go +++ b/modules/session/virtual.go @@ -59,8 +59,10 @@ func (o *VirtualSessionProvider) Init(gcLifetime int64, config string) error { func (o *VirtualSessionProvider) Read(sid string) (session.RawStore, error) { o.lock.RLock() defer o.lock.RUnlock() - if o.provider.Exist(sid) { + if exist, err := o.provider.Exist(sid); err == nil && exist { return o.provider.Read(sid) + } else if err != nil { + return nil, fmt.Errorf("check if '%s' exist failed: %w", sid, err) } kv := make(map[any]any) kv["_old_uid"] = "0" @@ -68,8 +70,8 @@ func (o *VirtualSessionProvider) Read(sid string) (session.RawStore, error) { } // Exist returns true if session with given ID exists. -func (o *VirtualSessionProvider) Exist(sid string) bool { - return true +func (o *VirtualSessionProvider) Exist(sid string) (bool, error) { + return true, nil } // Destroy deletes a session by session ID. @@ -87,7 +89,7 @@ func (o *VirtualSessionProvider) Regenerate(oldsid, sid string) (session.RawStor } // Count counts and returns number of sessions. -func (o *VirtualSessionProvider) Count() int { +func (o *VirtualSessionProvider) Count() (int, error) { o.lock.RLock() defer o.lock.RUnlock() return o.provider.Count() @@ -162,9 +164,13 @@ func (s *VirtualStore) Release() error { // Now ensure that we don't exist! realProvider := s.p.provider - if !s.released && realProvider.Exist(s.sid) { - // This is an error! - return fmt.Errorf("new sid '%s' already exists", s.sid) + if !s.released { + if exist, err := realProvider.Exist(s.sid); err == nil && exist { + // This is an error! + return fmt.Errorf("new sid '%s' already exists", s.sid) + } else if err != nil { + return fmt.Errorf("check if '%s' exist failed: %w", s.sid, err) + } } realStore, err := realProvider.Read(s.sid) if err != nil { diff --git a/routers/common/middleware.go b/routers/common/middleware.go index 510f7b7d24c7a..07adee18cec64 100644 --- a/routers/common/middleware.go +++ b/routers/common/middleware.go @@ -107,17 +107,8 @@ func ForwardedHeadersHandler(limit int, trustedProxies []string) func(h http.Han return proxy.ForwardedHeaders(opt) } -func Sessioner() (middleware func(next http.Handler) http.Handler, err error) { - // Recover from panic if session.Sessioner fails due to invalid config - // https://gitea.com/go-chi/session/src/commit/16768d98ec9667722b876d4bed11017ce16d4572/session.go#L237-L240 - defer func() { - if r := recover(); r != nil { - rErr := r.(error) - err = fmt.Errorf("failed to create session middleware: %w", rErr) - } - }() - - middleware = session.Sessioner(session.Options{ +func Sessioner() (func(next http.Handler) http.Handler, error) { + middleware, err := session.Sessioner(session.Options{ Provider: setting.SessionConfig.Provider, ProviderConfig: setting.SessionConfig.ProviderConfig, CookieName: setting.SessionConfig.CookieName, @@ -128,6 +119,9 @@ func Sessioner() (middleware func(next http.Handler) http.Handler, err error) { SameSite: setting.SessionConfig.SameSite, Domain: setting.SessionConfig.Domain, }) + if err != nil { + return nil, fmt.Errorf("failed to create session middleware: %w", err) + } - return middleware, err + return middleware, nil } From 575ca9f2fe93b72eb35cd3fef9a4e4be4287b694 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 27 Sep 2025 10:14:45 +0200 Subject: [PATCH 4/7] Apply suggestions from code review Signed-off-by: 6543 <6543@obermui.de> --- modules/session/db.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/session/db.go b/modules/session/db.go index 5c21fd37f1a40..577e20a45ed4f 100644 --- a/modules/session/db.go +++ b/modules/session/db.go @@ -159,7 +159,7 @@ func (p *DBProvider) Regenerate(oldsid, sid string) (_ session.RawStore, err err func (p *DBProvider) Count() (int, error) { total, err := auth.CountSessions(dbContext()) if err != nil { - fmt.Errorf("session/DB: error counting records: %w", err) + return 0, fmt.Errorf("session/DB: error counting records: %w", err) } return int(total), nil } From c68dbd47393739844081a86ed5e49eddef530c7b Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 27 Sep 2025 10:27:52 +0200 Subject: [PATCH 5/7] Update redis.go --- modules/session/redis.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/modules/session/redis.go b/modules/session/redis.go index bfcb1b8670498..b842b886a3aa7 100644 --- a/modules/session/redis.go +++ b/modules/session/redis.go @@ -182,12 +182,11 @@ func (p *RedisProvider) Regenerate(oldsid, sid string) (_ session.RawStore, err } if exist { return nil, fmt.Errorf("new sid '%s' already exists", sid) - } else { - if exist, err := p.Exist(oldsid); err == nil && !exist { - // Make a fake old session. - if err = p.c.Set(graceful.GetManager().HammerContext(), poldsid, "", p.duration).Err(); err != nil { - return nil, err - } + } + if exist, err := p.Exist(oldsid); err == nil && !exist { + // Make a fake old session. + if err = p.c.Set(graceful.GetManager().HammerContext(), poldsid, "", p.duration).Err(); err != nil { + return nil, err } else if err != nil { return nil, err } From 385281605e9db2b30cc2340bbe82685913f9c529 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 27 Sep 2025 19:28:25 +0200 Subject: [PATCH 6/7] Update redis.go making edits by mobile is hard ... --- modules/session/redis.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/modules/session/redis.go b/modules/session/redis.go index b842b886a3aa7..9bfc492dd1fab 100644 --- a/modules/session/redis.go +++ b/modules/session/redis.go @@ -176,20 +176,18 @@ func (p *RedisProvider) Regenerate(oldsid, sid string) (_ session.RawStore, err poldsid := p.prefix + oldsid psid := p.prefix + sid - exist, err := p.Exist(sid) - if err != nil { + if exist, err := p.Exist(sid); err != nil { return nil, err - } - if exist { + } else if exist { return nil, fmt.Errorf("new sid '%s' already exists", sid) } if exist, err := p.Exist(oldsid); err == nil && !exist { // Make a fake old session. - if err = p.c.Set(graceful.GetManager().HammerContext(), poldsid, "", p.duration).Err(); err != nil { - return nil, err - } else if err != nil { + if err := p.c.Set(graceful.GetManager().HammerContext(), poldsid, "", p.duration).Err(); err != nil { return nil, err } + } else if err != nil { + return nil, err } // do not use Rename here, because the old sid and new sid may be in different redis cluster slot. From b7d0ebb88d206a219c5f3918bc53a66954f08dd1 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 27 Sep 2025 22:17:49 +0200 Subject: [PATCH 7/7] comm on... --- modules/session/redis.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/session/redis.go b/modules/session/redis.go index 9bfc492dd1fab..083869f4e1ef1 100644 --- a/modules/session/redis.go +++ b/modules/session/redis.go @@ -187,7 +187,7 @@ func (p *RedisProvider) Regenerate(oldsid, sid string) (_ session.RawStore, err return nil, err } } else if err != nil { - return nil, err + return nil, err } // do not use Rename here, because the old sid and new sid may be in different redis cluster slot.