From fbe80e6df244c1234e0ed5d4ae4c1690b4eb4626 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sun, 28 Sep 2025 14:24:19 +0200 Subject: [PATCH] Add proper error message if session provider can not be created (#35520) the middleware that creates the session provider just panics if on creation the config is wrong. this is not catched and so you just get an cryptic stacktrace with no point where to look at (as user). ## Before ``` 2025/09/16 03:56:37 ...xer/stats/indexer.go:87:populateRepoIndexer() [I] Done (re)populating the repo stats indexer with existing repositories 2025/09/16 03:56:37 modules/ssh/ssh.go:387:Listen() [I] Adding SSH host key: /var/lib/gitea/data/ssh/gitea.rsa 2025/09/16 03:56:37 modules/ssh/init.go:26:Init() [I] SSH server started on :1234. Cipher list ([chacha20-poly1305@openssh.com aes128-ctr aes192-ctr aes256-ctr aes128-gcm@openssh.com aes256-gcm@openssh.com]), key exchange algorithms ([curve25519-sha256 ecdh-sha2-nistp256 ecdh-sha2-nistp384 ecdh-sha2-nistp521 diffie-hellman-group14-sha256 diffie-hellman-group14-sha1]), MACs ([hmac-sha2-256-etm@openssh.com hmac-sha2-256 hmac-sha1]) 2025/09/16 03:56:37 ...s/graceful/server.go:50:NewServer() [I] Starting new SSH server: tcp::1234 on PID: 83337 2025/09/16 03:56:38 cmd/web.go:231:func1() [F] PANIC: dial tcp 127.0.0.1:6379: connect: connection refused gitea.com/go-chi/session@v0.0.0-20240316035857-16768d98ec96/session.go:239 (0x1cdb908) code.gitea.io/gitea/routers/common/middleware.go:108 (0x2547f5a) code.gitea.io/gitea/routers/web/web.go:270 (0x278b8e9) code.gitea.io/gitea/routers/init.go:185 (0x2850d89) code.gitea.io/gitea/cmd/web.go:211 (0x295c5ad) code.gitea.io/gitea/cmd/web.go:262 (0x295cacb) code.gitea.io/gitea/cmd/main.go:111 (0x2953422) github.com/urfave/cli/v2@v2.27.2/command.go:276 (0x1cc3dfd) github.com/urfave/cli/v2@v2.27.2/command.go:269 (0x1cc4084) github.com/urfave/cli/v2@v2.27.2/app.go:333 (0x1cc086a) github.com/urfave/cli/v2@v2.27.2/app.go:307 (0x2953f18) code.gitea.io/gitea/cmd/main.go:172 (0x2953efc) code.gitea.io/gitea/main.go:46 (0x2998498) runtime/proc.go:283 (0x4471ca) runtime/asm_amd64.s:1700 (0x484a20) ``` ## After ``` 2025/09/22 22:52:35 .../templates/htmlrenderer.go:118:initHTMLRenderer() [D] Creating static HTML Renderer 2025/09/22 22:52:35 routers/web/web.go:273:Routes() [F] common.Sessioner failed: failed to create session middleware: dial tcp 127.0.0.1:6379: connect: connection refused ``` --------- Signed-off-by: 6543 <6543@obermui.de> --- go.mod | 2 +- go.sum | 4 ++-- modules/session/db.go | 13 +++++++------ modules/session/redis.go | 26 +++++++++++++++----------- modules/session/virtual.go | 20 +++++++++++++------- routers/common/middleware.go | 9 +++++++-- routers/install/routes.go | 7 ++++++- routers/web/web.go | 6 +++++- 8 files changed, 56 insertions(+), 31 deletions(-) diff --git a/go.mod b/go.mod index 73cbae7637..a82f0cd05b 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 f973e4bca5..4f329a89ba 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 7b5be1bc70..577e20a45e 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()) + return 0, 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 d89d8bc6e2..083869f4e1 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,13 +176,18 @@ func (p *RedisProvider) Regenerate(oldsid, sid string) (_ session.RawStore, err poldsid := p.prefix + oldsid psid := p.prefix + sid - if p.Exist(sid) { + if exist, err := p.Exist(sid); err != nil { + return nil, err + } else if exist { return nil, fmt.Errorf("new sid '%s' already exists", sid) - } else if !p.Exist(oldsid) { + } + 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 { + 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. @@ -211,12 +218,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 2e29b5fc6f..35a995d2d0 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 2ba02de8ed..07adee18ce 100644 --- a/routers/common/middleware.go +++ b/routers/common/middleware.go @@ -107,8 +107,8 @@ 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() (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, @@ -119,4 +119,9 @@ func Sessioner() func(next http.Handler) http.Handler { SameSite: setting.SessionConfig.SameSite, Domain: setting.SessionConfig.Domain, }) + if err != nil { + return nil, fmt.Errorf("failed to create session middleware: %w", err) + } + + return middleware, nil } diff --git a/routers/install/routes.go b/routers/install/routes.go index bc7a0eb48c..e4f833e751 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 98cdf8b964..5ee211b576 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()))