From ed8cedce45b39e8914f72467dcee69962611b680 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 21 Apr 2025 08:50:15 +0800 Subject: [PATCH 1/5] fix --- custom/conf/app.example.ini | 11 +++++++--- modules/httplib/url.go | 23 ++++++++++---------- modules/httplib/url_test.go | 37 ++++++++++++++++++++++++--------- modules/setting/server.go | 19 ++++++++++------- routers/web/admin/admin_test.go | 1 - 5 files changed, 59 insertions(+), 32 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 53e25a8c3bdb4..b6a7ca990bc3c 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -63,14 +63,19 @@ RUN_USER = ; git ;PROTOCOL = http ;; ;; Set the domain for the server. -;; Most users should set it to the real website domain of their Gitea instance. ;DOMAIN = localhost ;; -;; The AppURL used by Gitea to generate absolute links, defaults to "{PROTOCOL}://{DOMAIN}:{HTTP_PORT}/". +;; The AppURL is used to generate public URL links, defaults to "{PROTOCOL}://{DOMAIN}:{HTTP_PORT}/". ;; Most users should set it to the real website URL of their Gitea instance when there is a reverse proxy. -;; When it is empty, Gitea will use HTTP "Host" header to generate ROOT_URL, and fall back to the default one if no "Host" header. ;ROOT_URL = ;; +;; Controls how to generate the public URL. +;; Although it defaults to "legacy" (to avoid breaking existing users), most instances should use the "auto" behavior, +;; especially when the Gitea instance needs to be accessed in a container network. +;; * legacy: generate the public URL by "Host" header if "X-Forwarded-Proto" header exists, otherwise use "ROOT_URL". +;; * auto: always use "Host" header, and also use "X-Forwarded-Proto" header if it exists. If no "Host" header, use "ROOT_URL". +;PUBLIC_URL_GENERATION = legacy +;; ;; For development purpose only. It makes Gitea handle sub-path ("/sub-path/owner/repo/...") directly when debugging without a reverse proxy. ;; DO NOT USE IT IN PRODUCTION!!! ;USE_SUB_URL_PATH = false diff --git a/modules/httplib/url.go b/modules/httplib/url.go index dabc1f5f450c5..11e5380d46ca2 100644 --- a/modules/httplib/url.go +++ b/modules/httplib/url.go @@ -53,30 +53,31 @@ func getRequestScheme(req *http.Request) string { return "" } -// 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 +// GuessCurrentAppURL tries to guess the current full public URL (with sub-path) by http headers. It always has a '/' suffix, exactly the same as setting.AppURL +// TODO: should rename it to GuessCurrentPublicURL in the future func GuessCurrentAppURL(ctx context.Context) string { return GuessCurrentHostURL(ctx) + setting.AppSubURL + "/" } // GuessCurrentHostURL tries to guess the current full host URL (no sub-path) by http headers, there is no trailing slash. func GuessCurrentHostURL(ctx context.Context) string { - req, ok := ctx.Value(RequestContextKey).(*http.Request) - if !ok { - return strings.TrimSuffix(setting.AppURL, setting.AppSubURL+"/") - } - // If no scheme provided by reverse proxy, then do not guess the AppURL, use the configured one. + // Try the best guess to get the current host URL (will be used for public URL) by http headers. // At the moment, if site admin doesn't configure the proxy headers correctly, then Gitea would guess wrong. // There are some cases: // 1. The reverse proxy is configured correctly, it passes "X-Forwarded-Proto/Host" headers. Perfect, Gitea can handle it correctly. // 2. The reverse proxy is not configured correctly, doesn't pass "X-Forwarded-Proto/Host" headers, eg: only one "proxy_pass http://gitea:3000" in Nginx. // 3. There is no reverse proxy. // Without more information, Gitea is impossible to distinguish between case 2 and case 3, then case 2 would result in - // wrong guess like guessed AppURL becomes "http://gitea:3000/" behind a "https" reverse proxy, which is not accessible by end users. - // So we introduced "UseHostHeader" option, it could be enabled by setting "ROOT_URL" to empty + // wrong guess like guessed public URL becomes "http://gitea:3000/" behind a "https" reverse proxy, which is not accessible by end users. + // So we introduced "PUBLIC_URL_GENERATION" option, to control the guessing behavior to satisfy different use cases. + req, ok := ctx.Value(RequestContextKey).(*http.Request) + if !ok { + return strings.TrimSuffix(setting.AppURL, setting.AppSubURL+"/") + } reqScheme := getRequestScheme(req) if reqScheme == "" { // if no reverse proxy header, try to use "Host" header for absolute URL - if setting.UseHostHeader && req.Host != "" { + if setting.PublicURLGeneration == setting.PublicURLAuto && req.Host != "" { return util.Iif(req.TLS == nil, "http://", "https://") + req.Host } // fall back to default AppURL @@ -93,8 +94,8 @@ func GuessCurrentHostDomain(ctx context.Context) string { return util.IfZero(domain, host) } -// MakeAbsoluteURL tries to make a link to an absolute URL: -// * If link is empty, it returns the current app URL. +// MakeAbsoluteURL tries to make a link to an absolute public URL: +// * If link is empty, it returns the current public URL. // * If link is absolute, it returns the link. // * Otherwise, it returns the current host URL + link, the link itself should have correct sub-path (AppSubURL) if needed. func MakeAbsoluteURL(ctx context.Context, link string) string { diff --git a/modules/httplib/url_test.go b/modules/httplib/url_test.go index 0e198d7d732c8..435d86901fa09 100644 --- a/modules/httplib/url_test.go +++ b/modules/httplib/url_test.go @@ -43,20 +43,37 @@ func TestIsRelativeURL(t *testing.T) { func TestGuessCurrentHostURL(t *testing.T) { defer test.MockVariableValue(&setting.AppURL, "http://cfg-host/sub/")() defer test.MockVariableValue(&setting.AppSubURL, "/sub")() - defer test.MockVariableValue(&setting.UseHostHeader, false)() + headersWithProto := http.Header{"X-Forwarded-Proto": {"https"}} - ctx := t.Context() - assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(ctx)) + t.Run("Legacy", func(t *testing.T) { + defer test.MockVariableValue(&setting.PublicURLGeneration, setting.PublicURLLegacy)() + + assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(t.Context())) + + // legacy: the "Host" is not used when there is no "X-Forwarded-Proto" header + ctx := context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host:3000"}) + assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(ctx)) + + // if "X-Forwarded-Proto" exists, then use it and "Host" header + ctx = context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host:3000", Header: headersWithProto}) + assert.Equal(t, "https://req-host:3000", GuessCurrentHostURL(ctx)) + }) + + t.Run("Auto", func(t *testing.T) { + defer test.MockVariableValue(&setting.PublicURLGeneration, setting.PublicURLAuto)() - ctx = context.WithValue(ctx, RequestContextKey, &http.Request{Host: "localhost:3000"}) - assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(ctx)) + assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(t.Context())) - defer test.MockVariableValue(&setting.UseHostHeader, true)() - ctx = context.WithValue(ctx, RequestContextKey, &http.Request{Host: "http-host:3000"}) - assert.Equal(t, "http://http-host:3000", GuessCurrentHostURL(ctx)) + // auto: always use "Host" header, the scheme is determined by "X-Forwarded-Proto" header, or TLS config if no "X-Forwarded-Proto" header + ctx := context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host:3000"}) + assert.Equal(t, "http://req-host:3000", GuessCurrentHostURL(ctx)) - ctx = context.WithValue(ctx, RequestContextKey, &http.Request{Host: "http-host", TLS: &tls.ConnectionState{}}) - assert.Equal(t, "https://http-host", GuessCurrentHostURL(ctx)) + ctx = context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host", TLS: &tls.ConnectionState{}}) + assert.Equal(t, "https://req-host", GuessCurrentHostURL(ctx)) + + ctx = context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host:3000", Header: headersWithProto}) + assert.Equal(t, "https://req-host:3000", GuessCurrentHostURL(ctx)) + }) } func TestMakeAbsoluteURL(t *testing.T) { diff --git a/modules/setting/server.go b/modules/setting/server.go index 41b0ca89599a9..6b62b1874fa7b 100644 --- a/modules/setting/server.go +++ b/modules/setting/server.go @@ -41,12 +41,20 @@ const ( LandingPageLogin LandingPage = "/user/login" ) +const ( + PublicURLAuto = "auto" + PublicURLLegacy = "legacy" +) + // Server settings var ( // AppURL is the Application ROOT_URL. It always has a '/' suffix // It maps to ini:"ROOT_URL" AppURL string + // PublicURLGeneration controls how to use the HTTP request headers to generate public URL + PublicURLGeneration string + // AppSubURL represents the sub-url mounting point for gitea, parsed from "ROOT_URL" // It is either "" or starts with '/' and ends without '/', such as '/{sub-path}'. // This value is empty if site does not have sub-url. @@ -56,9 +64,6 @@ var ( // to make it easier to debug sub-path related problems without a reverse proxy. UseSubURLPath bool - // UseHostHeader makes Gitea prefer to use the "Host" request header for construction of absolute URLs. - UseHostHeader bool - // AppDataPath is the default path for storing data. // It maps to ini:"APP_DATA_PATH" in [server] and defaults to AppWorkPath + "/data" AppDataPath string @@ -283,10 +288,10 @@ func loadServerFrom(rootCfg ConfigProvider) { PerWritePerKbTimeout = sec.Key("PER_WRITE_PER_KB_TIMEOUT").MustDuration(PerWritePerKbTimeout) defaultAppURL := string(Protocol) + "://" + Domain + ":" + HTTPPort - AppURL = sec.Key("ROOT_URL").String() - if AppURL == "" { - UseHostHeader = true - AppURL = defaultAppURL + AppURL = sec.Key("ROOT_URL").MustString(defaultAppURL) + PublicURLGeneration = sec.Key("PUBLIC_URL_GENERATION").MustString(PublicURLLegacy) + if PublicURLGeneration != PublicURLAuto && PublicURLGeneration != PublicURLLegacy { + log.Fatal("Invalid PUBLIC_URL_GENERATION value: %s", PublicURLGeneration) } // Check validity of AppURL diff --git a/routers/web/admin/admin_test.go b/routers/web/admin/admin_test.go index 04fad4663c4d3..a568c7c5c8186 100644 --- a/routers/web/admin/admin_test.go +++ b/routers/web/admin/admin_test.go @@ -76,7 +76,6 @@ func TestShadowPassword(t *testing.T) { func TestSelfCheckPost(t *testing.T) { defer test.MockVariableValue(&setting.AppURL, "http://config/sub/")() defer test.MockVariableValue(&setting.AppSubURL, "/sub")() - defer test.MockVariableValue(&setting.UseHostHeader, false)() ctx, resp := contexttest.MockContext(t, "GET http://host/sub/admin/self_check?location_origin=http://frontend") SelfCheckPost(ctx) From e804953b4444ae793750000b548c6d3c344f3c2c Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 22 Apr 2025 01:50:55 +0800 Subject: [PATCH 2/5] PUBLIC_URL_DETECTION --- custom/conf/app.example.ini | 2 +- modules/httplib/url.go | 2 +- modules/setting/server.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index b6a7ca990bc3c..01bd302d8c466 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -74,7 +74,7 @@ RUN_USER = ; git ;; especially when the Gitea instance needs to be accessed in a container network. ;; * legacy: generate the public URL by "Host" header if "X-Forwarded-Proto" header exists, otherwise use "ROOT_URL". ;; * auto: always use "Host" header, and also use "X-Forwarded-Proto" header if it exists. If no "Host" header, use "ROOT_URL". -;PUBLIC_URL_GENERATION = legacy +;PUBLIC_URL_DETECTION = legacy ;; ;; For development purpose only. It makes Gitea handle sub-path ("/sub-path/owner/repo/...") directly when debugging without a reverse proxy. ;; DO NOT USE IT IN PRODUCTION!!! diff --git a/modules/httplib/url.go b/modules/httplib/url.go index 11e5380d46ca2..9eb0d3128caf5 100644 --- a/modules/httplib/url.go +++ b/modules/httplib/url.go @@ -69,7 +69,7 @@ func GuessCurrentHostURL(ctx context.Context) string { // 3. There is no reverse proxy. // Without more information, Gitea is impossible to distinguish between case 2 and case 3, then case 2 would result in // wrong guess like guessed public URL becomes "http://gitea:3000/" behind a "https" reverse proxy, which is not accessible by end users. - // So we introduced "PUBLIC_URL_GENERATION" option, to control the guessing behavior to satisfy different use cases. + // So we introduced "PUBLIC_URL_DETECTION" option, to control the guessing behavior to satisfy different use cases. req, ok := ctx.Value(RequestContextKey).(*http.Request) if !ok { return strings.TrimSuffix(setting.AppURL, setting.AppSubURL+"/") diff --git a/modules/setting/server.go b/modules/setting/server.go index 6b62b1874fa7b..416a4d27a22c7 100644 --- a/modules/setting/server.go +++ b/modules/setting/server.go @@ -289,9 +289,9 @@ func loadServerFrom(rootCfg ConfigProvider) { defaultAppURL := string(Protocol) + "://" + Domain + ":" + HTTPPort AppURL = sec.Key("ROOT_URL").MustString(defaultAppURL) - PublicURLGeneration = sec.Key("PUBLIC_URL_GENERATION").MustString(PublicURLLegacy) + PublicURLGeneration = sec.Key("PUBLIC_URL_DETECTION").MustString(PublicURLLegacy) if PublicURLGeneration != PublicURLAuto && PublicURLGeneration != PublicURLLegacy { - log.Fatal("Invalid PUBLIC_URL_GENERATION value: %s", PublicURLGeneration) + log.Fatal("Invalid PUBLIC_URL_DETECTION value: %s", PublicURLGeneration) } // Check validity of AppURL From 493f304cb89b51cccdf3eaf20953807293c6ca69 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 22 Apr 2025 01:57:37 +0800 Subject: [PATCH 3/5] Update custom/conf/app.example.ini Co-authored-by: Denys Konovalov --- custom/conf/app.example.ini | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 01bd302d8c466..9ead4fb99cde4 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -69,10 +69,10 @@ RUN_USER = ; git ;; Most users should set it to the real website URL of their Gitea instance when there is a reverse proxy. ;ROOT_URL = ;; -;; Controls how to generate the public URL. +;; Controls how to detect the public URL. ;; Although it defaults to "legacy" (to avoid breaking existing users), most instances should use the "auto" behavior, ;; especially when the Gitea instance needs to be accessed in a container network. -;; * legacy: generate the public URL by "Host" header if "X-Forwarded-Proto" header exists, otherwise use "ROOT_URL". +;; * legacy: detect the public URL from "Host" header if "X-Forwarded-Proto" header exists, otherwise use "ROOT_URL". ;; * auto: always use "Host" header, and also use "X-Forwarded-Proto" header if it exists. If no "Host" header, use "ROOT_URL". ;PUBLIC_URL_DETECTION = legacy ;; From 1947a48fcc0f73553ad471bbdeafb3245e4bf4b8 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 22 Apr 2025 01:58:13 +0800 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Denys Konovalov --- modules/httplib/url_test.go | 2 +- modules/setting/server.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/httplib/url_test.go b/modules/httplib/url_test.go index 435d86901fa09..8cbb839152102 100644 --- a/modules/httplib/url_test.go +++ b/modules/httplib/url_test.go @@ -50,7 +50,7 @@ func TestGuessCurrentHostURL(t *testing.T) { assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(t.Context())) - // legacy: the "Host" is not used when there is no "X-Forwarded-Proto" header + // legacy: "Host" is not used when there is no "X-Forwarded-Proto" header ctx := context.WithValue(t.Context(), RequestContextKey, &http.Request{Host: "req-host:3000"}) assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(ctx)) diff --git a/modules/setting/server.go b/modules/setting/server.go index 416a4d27a22c7..8a22f6a8448c1 100644 --- a/modules/setting/server.go +++ b/modules/setting/server.go @@ -52,8 +52,8 @@ var ( // It maps to ini:"ROOT_URL" AppURL string - // PublicURLGeneration controls how to use the HTTP request headers to generate public URL - PublicURLGeneration string + // PublicURLDetection controls how to use the HTTP request headers to detect public URL + PublicURLDetection string // AppSubURL represents the sub-url mounting point for gitea, parsed from "ROOT_URL" // It is either "" or starts with '/' and ends without '/', such as '/{sub-path}'. @@ -289,9 +289,9 @@ func loadServerFrom(rootCfg ConfigProvider) { defaultAppURL := string(Protocol) + "://" + Domain + ":" + HTTPPort AppURL = sec.Key("ROOT_URL").MustString(defaultAppURL) - PublicURLGeneration = sec.Key("PUBLIC_URL_DETECTION").MustString(PublicURLLegacy) - if PublicURLGeneration != PublicURLAuto && PublicURLGeneration != PublicURLLegacy { - log.Fatal("Invalid PUBLIC_URL_DETECTION value: %s", PublicURLGeneration) + PublicURLDetection = sec.Key("PUBLIC_URL_DETECTION").MustString(PublicURLLegacy) + if PublicURLDetection != PublicURLAuto && PublicURLDetection != PublicURLLegacy { + log.Fatal("Invalid PUBLIC_URL_DETECTION value: %s", PublicURLDetection) } // Check validity of AppURL From 561f3707525663aa0c83d9f7b89a8fbdf1fa1ba0 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 22 Apr 2025 01:59:43 +0800 Subject: [PATCH 5/5] fix var name --- modules/httplib/url.go | 2 +- modules/httplib/url_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/httplib/url.go b/modules/httplib/url.go index 9eb0d3128caf5..f51506ac3b777 100644 --- a/modules/httplib/url.go +++ b/modules/httplib/url.go @@ -77,7 +77,7 @@ func GuessCurrentHostURL(ctx context.Context) string { reqScheme := getRequestScheme(req) if reqScheme == "" { // if no reverse proxy header, try to use "Host" header for absolute URL - if setting.PublicURLGeneration == setting.PublicURLAuto && req.Host != "" { + if setting.PublicURLDetection == setting.PublicURLAuto && req.Host != "" { return util.Iif(req.TLS == nil, "http://", "https://") + req.Host } // fall back to default AppURL diff --git a/modules/httplib/url_test.go b/modules/httplib/url_test.go index 8cbb839152102..0ffb0cac05de2 100644 --- a/modules/httplib/url_test.go +++ b/modules/httplib/url_test.go @@ -46,7 +46,7 @@ func TestGuessCurrentHostURL(t *testing.T) { headersWithProto := http.Header{"X-Forwarded-Proto": {"https"}} t.Run("Legacy", func(t *testing.T) { - defer test.MockVariableValue(&setting.PublicURLGeneration, setting.PublicURLLegacy)() + defer test.MockVariableValue(&setting.PublicURLDetection, setting.PublicURLLegacy)() assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(t.Context())) @@ -60,7 +60,7 @@ func TestGuessCurrentHostURL(t *testing.T) { }) t.Run("Auto", func(t *testing.T) { - defer test.MockVariableValue(&setting.PublicURLGeneration, setting.PublicURLAuto)() + defer test.MockVariableValue(&setting.PublicURLDetection, setting.PublicURLAuto)() assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(t.Context()))