From 61457cdf6b49225ae831fd9fb084deadd8bdb0fb Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 19 Apr 2024 12:03:53 +0800 Subject: [PATCH] Avoid importing `modules/web/middleware` in `modules/session` (#30584) Related to #30375. It doesn't make sense to import `modules/web/middleware` and `modules/setting` in `modules/web/session` since the last one is more low-level. And it looks like a workaround to call `DeleteLegacySiteCookie` in `RegenerateSession`, so maybe we could reverse the importing by registering hook functions. --- modules/session/store.go | 13 ++++++------- modules/web/middleware/cookie.go | 15 ++++++++++++--- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/modules/session/store.go b/modules/session/store.go index 2f7ab7760b..70988fcdc5 100644 --- a/modules/session/store.go +++ b/modules/session/store.go @@ -6,9 +6,6 @@ package session import ( "net/http" - "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/web/middleware" - "gitea.com/go-chi/session" ) @@ -21,10 +18,12 @@ type Store interface { // RegenerateSession regenerates the underlying session and returns the new store func RegenerateSession(resp http.ResponseWriter, req *http.Request) (Store, error) { - // Ensure that a cookie with a trailing slash does not take precedence over - // the cookie written by the middleware. - middleware.DeleteLegacySiteCookie(resp, setting.SessionConfig.CookieName) - + for _, f := range BeforeRegenerateSession { + f(resp, req) + } s, err := session.RegenerateSession(resp, req) return s, err } + +// BeforeRegenerateSession is a list of functions that are called before a session is regenerated. +var BeforeRegenerateSession []func(http.ResponseWriter, *http.Request) diff --git a/modules/web/middleware/cookie.go b/modules/web/middleware/cookie.go index 0bed726793..ec6b06f993 100644 --- a/modules/web/middleware/cookie.go +++ b/modules/web/middleware/cookie.go @@ -9,6 +9,7 @@ import ( "net/url" "strings" + "code.gitea.io/gitea/modules/session" "code.gitea.io/gitea/modules/setting" ) @@ -48,12 +49,12 @@ func SetSiteCookie(resp http.ResponseWriter, name, value string, maxAge int) { // Previous versions would use a cookie path with a trailing /. // These are more specific than cookies without a trailing /, so // we need to delete these if they exist. - DeleteLegacySiteCookie(resp, name) + deleteLegacySiteCookie(resp, name) } -// DeleteLegacySiteCookie deletes the cookie with the given name at the cookie +// deleteLegacySiteCookie deletes the cookie with the given name at the cookie // path with a trailing /, which would unintentionally override the cookie. -func DeleteLegacySiteCookie(resp http.ResponseWriter, name string) { +func deleteLegacySiteCookie(resp http.ResponseWriter, name string) { if setting.SessionConfig.CookiePath == "" || strings.HasSuffix(setting.SessionConfig.CookiePath, "/") { // If the cookie path ends with /, no legacy cookies will take // precedence, so do nothing. The exception is that cookies with no @@ -74,3 +75,11 @@ func DeleteLegacySiteCookie(resp http.ResponseWriter, name string) { } resp.Header().Add("Set-Cookie", cookie.String()) } + +func init() { + session.BeforeRegenerateSession = append(session.BeforeRegenerateSession, func(resp http.ResponseWriter, _ *http.Request) { + // Ensure that a cookie with a trailing slash does not take precedence over + // the cookie written by the middleware. + deleteLegacySiteCookie(resp, setting.SessionConfig.CookieName) + }) +}