From 3b10fd9b3452d548ef116b27161b17f57a8e180c Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 20 Sep 2024 22:57:55 +0800 Subject: [PATCH] Only use Host header from reverse proxy (#32060) X-Forwarded-Host has many problems: non-standard, not well-defined (X-Forwarded-Port or not), conflicts with Host header, it already caused problems like #31907. So do not use X-Forwarded-Host, just use Host header directly. Official document also only uses `Host` header and never mentioned others. --- .github/workflows/pull-db-tests.yml | 3 ++- modules/httplib/url.go | 13 +++---------- modules/httplib/url_test.go | 5 +++-- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/.github/workflows/pull-db-tests.yml b/.github/workflows/pull-db-tests.yml index 246884f24b1..90804c0f0a3 100644 --- a/.github/workflows/pull-db-tests.yml +++ b/.github/workflows/pull-db-tests.yml @@ -201,7 +201,8 @@ jobs: runs-on: ubuntu-latest services: mssql: - image: mcr.microsoft.com/mssql/server:2017-latest + # some images before 2024-04 can't run on new kernels + image: mcr.microsoft.com/mssql/server:2019-latest env: ACCEPT_EULA: Y MSSQL_PID: Standard diff --git a/modules/httplib/url.go b/modules/httplib/url.go index 219dfe695c4..e3bad1e5fba 100644 --- a/modules/httplib/url.go +++ b/modules/httplib/url.go @@ -52,11 +52,6 @@ func getRequestScheme(req *http.Request) string { return "" } -func getForwardedHost(req *http.Request) string { - // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host - return req.Header.Get("X-Forwarded-Host") -} - // GuessCurrentAppURL tries to guess the current full app URL (with sub-path) by http headers. It always has a '/' suffix, exactly the same as setting.AppURL func GuessCurrentAppURL(ctx context.Context) string { return GuessCurrentHostURL(ctx) + setting.AppSubURL + "/" @@ -81,11 +76,9 @@ func GuessCurrentHostURL(ctx context.Context) string { if reqScheme == "" { return strings.TrimSuffix(setting.AppURL, setting.AppSubURL+"/") } - reqHost := getForwardedHost(req) - if reqHost == "" { - reqHost = req.Host - } - return reqScheme + "://" + reqHost + // X-Forwarded-Host has many problems: non-standard, not well-defined (X-Forwarded-Port or not), conflicts with Host header. + // So do not use X-Forwarded-Host, just use Host header directly. + return reqScheme + "://" + req.Host } // MakeAbsoluteURL tries to make a link to an absolute URL: diff --git a/modules/httplib/url_test.go b/modules/httplib/url_test.go index 28aaee6e129..fc6c91cd3a3 100644 --- a/modules/httplib/url_test.go +++ b/modules/httplib/url_test.go @@ -70,7 +70,7 @@ func TestMakeAbsoluteURL(t *testing.T) { "X-Forwarded-Proto": {"https"}, }, }) - assert.Equal(t, "https://forwarded-host/foo", MakeAbsoluteURL(ctx, "/foo")) + assert.Equal(t, "https://user-host/foo", MakeAbsoluteURL(ctx, "/foo")) } func TestIsCurrentGiteaSiteURL(t *testing.T) { @@ -119,5 +119,6 @@ func TestIsCurrentGiteaSiteURL(t *testing.T) { }, }) assert.True(t, IsCurrentGiteaSiteURL(ctx, "http://localhost:3000")) - assert.True(t, IsCurrentGiteaSiteURL(ctx, "https://forwarded-host")) + assert.True(t, IsCurrentGiteaSiteURL(ctx, "https://user-host")) + assert.False(t, IsCurrentGiteaSiteURL(ctx, "https://forwarded-host")) }