From 234d88f2931bdca78fc10e46f5f2d7b2d226cbf0 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 7 Mar 2024 10:03:41 +0800 Subject: [PATCH 1/5] Use strict protocol check when redirect (#29642) --- modules/context/base.go | 2 +- services/context/base_test.go | 47 +++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 services/context/base_test.go diff --git a/modules/context/base.go b/modules/context/base.go index 8df1dde866d5b..87da9ff3605ad 100644 --- a/modules/context/base.go +++ b/modules/context/base.go @@ -255,7 +255,7 @@ func (b *Base) Redirect(location string, status ...int) { code = status[0] } - if strings.Contains(location, "://") || strings.HasPrefix(location, "//") { + if strings.HasPrefix(location, "http://") || strings.HasPrefix(location, "https://") || 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) diff --git a/services/context/base_test.go b/services/context/base_test.go new file mode 100644 index 0000000000000..823f20e00bc0d --- /dev/null +++ b/services/context/base_test.go @@ -0,0 +1,47 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package context + +import ( + "net/http" + "net/http/httptest" + "testing" + + "code.gitea.io/gitea/modules/setting" + + "github.com/stretchr/testify/assert" +) + +func TestRedirect(t *testing.T) { + req, _ := http.NewRequest("GET", "/", nil) + + cases := []struct { + url string + keep bool + }{ + {"http://test", false}, + {"https://test", false}, + {"//test", false}, + {"/://test", true}, + {"/test", true}, + } + for _, c := range cases { + resp := httptest.NewRecorder() + b, cleanup := NewBaseContext(resp, req) + resp.Header().Add("Set-Cookie", (&http.Cookie{Name: setting.SessionConfig.CookieName, Value: "dummy"}).String()) + b.Redirect(c.url) + cleanup() + has := resp.Header().Get("Set-Cookie") == "i_like_gitea=dummy" + assert.Equal(t, c.keep, has, "url = %q", c.url) + } + + req, _ = http.NewRequest("GET", "/", nil) + resp := httptest.NewRecorder() + req.Header.Add("HX-Request", "true") + b, cleanup := NewBaseContext(resp, req) + b.Redirect("/other") + cleanup() + assert.Equal(t, "/other", resp.Header().Get("HX-Redirect")) + assert.Equal(t, http.StatusNoContent, resp.Code) +} From e3391598ea1296248a2c0f0516f09e26bd055c38 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 7 Mar 2024 11:18:11 +0800 Subject: [PATCH 2/5] Update base_test.go --- services/context/base_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/services/context/base_test.go b/services/context/base_test.go index 823f20e00bc0d..4802687dd2e54 100644 --- a/services/context/base_test.go +++ b/services/context/base_test.go @@ -8,8 +8,9 @@ import ( "net/http/httptest" "testing" + "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/setting" - + "github.com/stretchr/testify/assert" ) @@ -28,7 +29,7 @@ func TestRedirect(t *testing.T) { } for _, c := range cases { resp := httptest.NewRecorder() - b, cleanup := NewBaseContext(resp, req) + b, cleanup := context.NewBaseContext(resp, req) resp.Header().Add("Set-Cookie", (&http.Cookie{Name: setting.SessionConfig.CookieName, Value: "dummy"}).String()) b.Redirect(c.url) cleanup() @@ -39,7 +40,7 @@ func TestRedirect(t *testing.T) { req, _ = http.NewRequest("GET", "/", nil) resp := httptest.NewRecorder() req.Header.Add("HX-Request", "true") - b, cleanup := NewBaseContext(resp, req) + b, cleanup := context.NewBaseContext(resp, req) b.Redirect("/other") cleanup() assert.Equal(t, "/other", resp.Header().Get("HX-Redirect")) From 5099b954a67afe77c049f7092de1fe123667c943 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 7 Mar 2024 11:18:58 +0800 Subject: [PATCH 3/5] Update services/context/base_test.go --- services/context/base_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/services/context/base_test.go b/services/context/base_test.go index 4802687dd2e54..3aa21bd72f6b0 100644 --- a/services/context/base_test.go +++ b/services/context/base_test.go @@ -10,7 +10,6 @@ import ( "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/setting" - "github.com/stretchr/testify/assert" ) From 5a39602d12516f36f971fa7e494b90671c536edc Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 7 Mar 2024 11:19:12 +0800 Subject: [PATCH 4/5] Update base_test.go --- services/context/base_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/services/context/base_test.go b/services/context/base_test.go index 3aa21bd72f6b0..ac62fcef237a4 100644 --- a/services/context/base_test.go +++ b/services/context/base_test.go @@ -10,6 +10,7 @@ import ( "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/setting" + "github.com/stretchr/testify/assert" ) From e021bd74f6f5b3d0631e6f30e97e8b4dfb2a9657 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 7 Mar 2024 11:32:17 +0800 Subject: [PATCH 5/5] Update base_test.go --- services/context/base_test.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/services/context/base_test.go b/services/context/base_test.go index ac62fcef237a4..96ccfa73e3901 100644 --- a/services/context/base_test.go +++ b/services/context/base_test.go @@ -36,13 +36,4 @@ func TestRedirect(t *testing.T) { has := resp.Header().Get("Set-Cookie") == "i_like_gitea=dummy" assert.Equal(t, c.keep, has, "url = %q", c.url) } - - req, _ = http.NewRequest("GET", "/", nil) - resp := httptest.NewRecorder() - req.Header.Add("HX-Request", "true") - b, cleanup := context.NewBaseContext(resp, req) - b.Redirect("/other") - cleanup() - assert.Equal(t, "/other", resp.Header().Get("HX-Redirect")) - assert.Equal(t, http.StatusNoContent, resp.Code) }