diff options
Diffstat (limited to 'modules/httplib')
-rw-r--r-- | modules/httplib/request.go | 6 | ||||
-rw-r--r-- | modules/httplib/serve_test.go | 10 | ||||
-rw-r--r-- | modules/httplib/url.go | 28 | ||||
-rw-r--r-- | modules/httplib/url_test.go | 37 |
4 files changed, 62 insertions, 19 deletions
diff --git a/modules/httplib/request.go b/modules/httplib/request.go index 5e40922896..49ea6f4b73 100644 --- a/modules/httplib/request.go +++ b/modules/httplib/request.go @@ -108,7 +108,7 @@ func (r *Request) Body(data any) *Request { switch t := data.(type) { case nil: // do nothing case string: - bf := bytes.NewBufferString(t) + bf := strings.NewReader(t) r.req.Body = io.NopCloser(bf) r.req.ContentLength = int64(len(t)) case []byte: @@ -143,13 +143,13 @@ func (r *Request) getResponse() (*http.Response, error) { paramBody = paramBody[0 : len(paramBody)-1] } - if r.req.Method == "GET" && len(paramBody) > 0 { + if r.req.Method == http.MethodGet && len(paramBody) > 0 { if strings.Contains(r.url, "?") { r.url += "&" + paramBody } else { r.url = r.url + "?" + paramBody } - } else if r.req.Method == "POST" && r.req.Body == nil && len(paramBody) > 0 { + } else if r.req.Method == http.MethodPost && r.req.Body == nil && len(paramBody) > 0 { r.Header("Content-Type", "application/x-www-form-urlencoded") r.Body(paramBody) // string } diff --git a/modules/httplib/serve_test.go b/modules/httplib/serve_test.go index 06c95bc594..78b88c9b5f 100644 --- a/modules/httplib/serve_test.go +++ b/modules/httplib/serve_test.go @@ -4,11 +4,11 @@ package httplib import ( - "fmt" "net/http" "net/http/httptest" "net/url" "os" + "strconv" "strings" "testing" @@ -23,14 +23,14 @@ func TestServeContentByReader(t *testing.T) { _, rangeStr, _ := strings.Cut(t.Name(), "_range_") r := &http.Request{Header: http.Header{}, Form: url.Values{}} if rangeStr != "" { - r.Header.Set("Range", fmt.Sprintf("bytes=%s", rangeStr)) + r.Header.Set("Range", "bytes="+rangeStr) } reader := strings.NewReader(data) w := httptest.NewRecorder() ServeContentByReader(r, w, int64(len(data)), reader, &ServeHeaderOptions{}) assert.Equal(t, expectedStatusCode, w.Code) if expectedStatusCode == http.StatusPartialContent || expectedStatusCode == http.StatusOK { - assert.Equal(t, fmt.Sprint(len(expectedContent)), w.Header().Get("Content-Length")) + assert.Equal(t, strconv.Itoa(len(expectedContent)), w.Header().Get("Content-Length")) assert.Equal(t, expectedContent, w.Body.String()) } } @@ -68,7 +68,7 @@ func TestServeContentByReadSeeker(t *testing.T) { _, rangeStr, _ := strings.Cut(t.Name(), "_range_") r := &http.Request{Header: http.Header{}, Form: url.Values{}} if rangeStr != "" { - r.Header.Set("Range", fmt.Sprintf("bytes=%s", rangeStr)) + r.Header.Set("Range", "bytes="+rangeStr) } seekReader, err := os.OpenFile(tmpFile, os.O_RDONLY, 0o644) @@ -79,7 +79,7 @@ func TestServeContentByReadSeeker(t *testing.T) { ServeContentByReadSeeker(r, w, nil, seekReader, &ServeHeaderOptions{}) assert.Equal(t, expectedStatusCode, w.Code) if expectedStatusCode == http.StatusPartialContent || expectedStatusCode == http.StatusOK { - assert.Equal(t, fmt.Sprint(len(expectedContent)), w.Header().Get("Content-Length")) + assert.Equal(t, strconv.Itoa(len(expectedContent)), w.Header().Get("Content-Length")) assert.Equal(t, expectedContent, w.Body.String()) } } diff --git a/modules/httplib/url.go b/modules/httplib/url.go index 5d5b64dc0c..f51506ac3b 100644 --- a/modules/httplib/url.go +++ b/modules/httplib/url.go @@ -53,28 +53,34 @@ 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 an extra config option, 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/", which is not accessible by end users. - // So in the future maybe it should introduce a new config option, to let site admin decide how to guess the AppURL. + // 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_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+"/") + } reqScheme := getRequestScheme(req) if reqScheme == "" { + // if no reverse proxy header, try to use "Host" header for absolute URL + if setting.PublicURLDetection == setting.PublicURLAuto && req.Host != "" { + return util.Iif(req.TLS == nil, "http://", "https://") + req.Host + } + // fall back to default AppURL return strings.TrimSuffix(setting.AppURL, setting.AppSubURL+"/") } // X-Forwarded-Host has many problems: non-standard, not well-defined (X-Forwarded-Port or not), conflicts with Host header. @@ -88,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 d57653646b..0ffb0cac05 100644 --- a/modules/httplib/url_test.go +++ b/modules/httplib/url_test.go @@ -5,6 +5,7 @@ package httplib import ( "context" + "crypto/tls" "net/http" "testing" @@ -39,6 +40,42 @@ 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")() + headersWithProto := http.Header{"X-Forwarded-Proto": {"https"}} + + t.Run("Legacy", func(t *testing.T) { + defer test.MockVariableValue(&setting.PublicURLDetection, setting.PublicURLLegacy)() + + assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(t.Context())) + + // 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)) + + // 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.PublicURLDetection, setting.PublicURLAuto)() + + assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(t.Context())) + + // 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(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) { defer test.MockVariableValue(&setting.Protocol, "http")() defer test.MockVariableValue(&setting.AppURL, "http://cfg-host/sub/")() |