Refactor "route" related code, fix Safari cookie bug (#24330)

Fix #24176

Clean some misuses of route package, clean some legacy FIXMEs

---------

Co-authored-by: Giteabot <teabot@gitea.io>

3 files changed, 61 insertions, 0 deletions

diff --git a/modules/context/api.go b/modules/context/api.go
index d405c9972b..ae245ec1cb 100644
--- a/modules/context/api.go
+++ b/modules/context/api.go
@@ -343,6 +343,7 @@ func RepoRefForAPI(next http.Handler) http.Handler {
 			}
 			ctx.Repo.Commit = commit
 			ctx.Repo.TreePath = ctx.Params("*")
+			next.ServeHTTP(w, req)
 			return
 		}
 
diff --git a/modules/context/context.go b/modules/context/context.go
index f262c7cce7..702da8a965 100644
--- a/modules/context/context.go
+++ b/modules/context/context.go
@@ -446,6 +446,17 @@ func (ctx *Context) JSON(status int, content interface{}) {
 	}
 }
 
+func removeSessionCookieHeader(w http.ResponseWriter) {
+	cookies := w.Header()["Set-Cookie"]
+	w.Header().Del("Set-Cookie")
+	for _, cookie := range cookies {
+		if strings.HasPrefix(cookie, setting.SessionConfig.CookieName+"=") {
+			continue
+		}
+		w.Header().Add("Set-Cookie", cookie)
+	}
+}
+
 // Redirect redirects the request
 func (ctx *Context) Redirect(location string, status ...int) {
 	code := http.StatusSeeOther
@@ -453,6 +464,15 @@ func (ctx *Context) Redirect(location string, status ...int) {
 		code = status[0]
 	}
 
+	if strings.Contains(location, "://") || strings.HasPrefix(location, "//") {
+		// Some browsers (Safari) have buggy behavior for Cookie + Cache + External Redirection, eg: /my-path => https://other/path
+		// 1. the first request to "/my-path" contains cookie
+		// 2. some time later, the request to "/my-path" doesn't contain cookie (caused by Prevent web tracking)
+		// 3. Gitea's Sessioner doesn't see the session cookie, so it generates a new session id, and returns it to browser
+		// 4. then the browser accepts the empty session, then the user is logged out
+		// So in this case, we should remove the session cookie from the response header
+		removeSessionCookieHeader(ctx.Resp)
+	}
 	http.Redirect(ctx.Resp, ctx.Req, location, code)
 }
 
diff --git a/modules/context/context_test.go b/modules/context/context_test.go
new file mode 100644
index 0000000000..e1460c1fd7
--- /dev/null
+++ b/modules/context/context_test.go
@@ -0,0 +1,40 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package context
+
+import (
+	"net/http"
+	"testing"
+
+	"code.gitea.io/gitea/modules/setting"
+
+	"github.com/stretchr/testify/assert"
+)
+
+type mockResponseWriter struct {
+	header http.Header
+}
+
+func (m *mockResponseWriter) Header() http.Header {
+	return m.header
+}
+
+func (m *mockResponseWriter) Write(bytes []byte) (int, error) {
+	panic("implement me")
+}
+
+func (m *mockResponseWriter) WriteHeader(statusCode int) {
+	panic("implement me")
+}
+
+func TestRemoveSessionCookieHeader(t *testing.T) {
+	w := &mockResponseWriter{}
+	w.header = http.Header{}
+	w.header.Add("Set-Cookie", (&http.Cookie{Name: setting.SessionConfig.CookieName, Value: "foo"}).String())
+	w.header.Add("Set-Cookie", (&http.Cookie{Name: "other", Value: "bar"}).String())
+	assert.Len(t, w.Header().Values("Set-Cookie"), 2)
+	removeSessionCookieHeader(w)
+	assert.Len(t, w.Header().Values("Set-Cookie"), 1)
+	assert.Contains(t, "other=bar", w.Header().Get("Set-Cookie"))
+}