Skip to content

Commit

Permalink
Only use Host header from reverse proxy (#32060)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
wxiaoguang authored Sep 20, 2024
1 parent 55d5a74 commit 3b10fd9
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 13 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/pull-db-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 3 additions & 10 deletions modules/httplib/url.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 + "/"
Expand All @@ -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:
Expand Down
5 changes: 3 additions & 2 deletions modules/httplib/url_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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"))
}

0 comments on commit 3b10fd9

Please sign in to comment.