Skip to content

Commit 944f1ec

Browse files
GiteaBotwolfogre
andauthored
Avoid importing modules/web/middleware in modules/session (#30584) (#30588)
Backport #30584 by @wolfogre 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. Co-authored-by: Jason Song <[email protected]>
1 parent a05d098 commit 944f1ec

File tree

2 files changed

+18
-10
lines changed

2 files changed

+18
-10
lines changed

modules/session/store.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,6 @@ package session
66
import (
77
"net/http"
88

9-
"code.gitea.io/gitea/modules/setting"
10-
"code.gitea.io/gitea/modules/web/middleware"
11-
129
"gitea.com/go-chi/session"
1310
)
1411

@@ -21,10 +18,12 @@ type Store interface {
2118

2219
// RegenerateSession regenerates the underlying session and returns the new store
2320
func RegenerateSession(resp http.ResponseWriter, req *http.Request) (Store, error) {
24-
// Ensure that a cookie with a trailing slash does not take precedence over
25-
// the cookie written by the middleware.
26-
middleware.DeleteLegacySiteCookie(resp, setting.SessionConfig.CookieName)
27-
21+
for _, f := range BeforeRegenerateSession {
22+
f(resp, req)
23+
}
2824
s, err := session.RegenerateSession(resp, req)
2925
return s, err
3026
}
27+
28+
// BeforeRegenerateSession is a list of functions that are called before a session is regenerated.
29+
var BeforeRegenerateSession []func(http.ResponseWriter, *http.Request)

modules/web/middleware/cookie.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"net/url"
1010
"strings"
1111

12+
"code.gitea.io/gitea/modules/session"
1213
"code.gitea.io/gitea/modules/setting"
1314
)
1415

@@ -48,12 +49,12 @@ func SetSiteCookie(resp http.ResponseWriter, name, value string, maxAge int) {
4849
// Previous versions would use a cookie path with a trailing /.
4950
// These are more specific than cookies without a trailing /, so
5051
// we need to delete these if they exist.
51-
DeleteLegacySiteCookie(resp, name)
52+
deleteLegacySiteCookie(resp, name)
5253
}
5354

54-
// DeleteLegacySiteCookie deletes the cookie with the given name at the cookie
55+
// deleteLegacySiteCookie deletes the cookie with the given name at the cookie
5556
// path with a trailing /, which would unintentionally override the cookie.
56-
func DeleteLegacySiteCookie(resp http.ResponseWriter, name string) {
57+
func deleteLegacySiteCookie(resp http.ResponseWriter, name string) {
5758
if setting.SessionConfig.CookiePath == "" || strings.HasSuffix(setting.SessionConfig.CookiePath, "/") {
5859
// If the cookie path ends with /, no legacy cookies will take
5960
// precedence, so do nothing. The exception is that cookies with no
@@ -74,3 +75,11 @@ func DeleteLegacySiteCookie(resp http.ResponseWriter, name string) {
7475
}
7576
resp.Header().Add("Set-Cookie", cookie.String())
7677
}
78+
79+
func init() {
80+
session.BeforeRegenerateSession = append(session.BeforeRegenerateSession, func(resp http.ResponseWriter, _ *http.Request) {
81+
// Ensure that a cookie with a trailing slash does not take precedence over
82+
// the cookie written by the middleware.
83+
deleteLegacySiteCookie(resp, setting.SessionConfig.CookieName)
84+
})
85+
}

0 commit comments

Comments
 (0)