diff --git a/modules/api/client.go b/modules/api/client.go index 7cd02044..2892dd7c 100644 --- a/modules/api/client.go +++ b/modules/api/client.go @@ -31,9 +31,7 @@ func NewClient(login *config.Login) *Client { } httpClient := &http.Client{ - Transport: httputil.WrapTransport(&http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: login.Insecure}, - }), + Transport: httputil.WrapTransport(&tls.Config{InsecureSkipVerify: login.Insecure}), } return &Client{ diff --git a/modules/auth/oauth.go b/modules/auth/oauth.go index 52417127..c98a22fa 100644 --- a/modules/auth/oauth.go +++ b/modules/auth/oauth.go @@ -201,9 +201,7 @@ func performBrowserOAuthFlow(ctx context.Context, opts OAuthOptions) (serverURL // createHTTPClient creates an HTTP client with optional insecure setting func createHTTPClient(insecure bool) *http.Client { return &http.Client{ - Transport: httputil.WrapTransport(&http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: insecure}, - }), + Transport: httputil.WrapTransport(&tls.Config{InsecureSkipVerify: insecure}), } } diff --git a/modules/config/login.go b/modules/config/login.go index 82245184..3cd1adb7 100644 --- a/modules/config/login.go +++ b/modules/config/login.go @@ -418,9 +418,7 @@ func doOAuthRefresh(ctx context.Context, l *Login) (*oauth2.Token, error) { } httpClient := &http.Client{ - Transport: httputil.WrapTransport(&http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: l.Insecure}, - }), + Transport: httputil.WrapTransport(&tls.Config{InsecureSkipVerify: l.Insecure}), } ctx = context.WithValue(ctx, oauth2.HTTPClient, httpClient) @@ -448,15 +446,19 @@ func (l *Login) Client(options ...gitea.ClientOption) *gitea.Client { os.Exit(1) } - httpClient := &http.Client{} + // Configure transport-level timeouts so a stalled or unresponsive server + // fails fast instead of hanging forever. These bound connection setup and + // time-to-first-response-byte only, so slow-but-progressing transfers (e.g. + // large attachment uploads) are unaffected. + httpClient := &http.Client{ + Transport: httputil.WrapTransport(nil), + } if l.Insecure { cookieJar, _ := cookiejar.New(nil) // New with nil options never returns an error httpClient = &http.Client{ - Jar: cookieJar, - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, - }, + Jar: cookieJar, + Transport: httputil.WrapTransport(&tls.Config{InsecureSkipVerify: true}), } } @@ -465,6 +467,10 @@ func (l *Login) Client(options ...gitea.ClientOption) *gitea.Client { options = append([]gitea.ClientOption{gitea.SetGiteaVersion("")}, options...) } + // SetUserAgent is intentionally redundant with the User-Agent the WrapTransport + // transport already sets: this is the SDK's own guarantee, so the UA survives + // even if the client is ever given a transport that didn't come from WrapTransport. + // Both resolve to httputil.UserAgent(), so the duplicate Header.Set is a no-op. options = append(options, gitea.SetToken(l.GetAccessToken()), gitea.SetHTTPClient(httpClient), gitea.SetUserAgent(httputil.UserAgent())) if debug.IsDebug() { options = append(options, gitea.SetDebugMode()) diff --git a/modules/httputil/httputil.go b/modules/httputil/httputil.go index 92338e33..57078063 100644 --- a/modules/httputil/httputil.go +++ b/modules/httputil/httputil.go @@ -4,6 +4,7 @@ package httputil import ( + "crypto/tls" "fmt" "net/http" "runtime" @@ -20,12 +21,14 @@ func UserAgent() string { return ua } -// WrapTransport wraps an http.RoundTripper to add the User-Agent header. -func WrapTransport(base http.RoundTripper) http.RoundTripper { - if base == nil { - base = http.DefaultTransport - } - return &userAgentTransport{base: base} +// WrapTransport returns tea's standard HTTP transport: an *http.Transport +// preset with tea's connection / response-header timeouts (see timeoutTransport) +// and decorated to add the User-Agent header on every request. The supplied +// tlsConfig is attached as-is (nil is fine); callers use it for insecure / +// skip-verify logins. This is the single entry point for building a tea HTTP +// client transport, so the timeouts can't be accidentally omitted. +func WrapTransport(tlsConfig *tls.Config) http.RoundTripper { + return &userAgentTransport{base: timeoutTransport(tlsConfig)} } type userAgentTransport struct { @@ -33,6 +36,11 @@ type userAgentTransport struct { } func (t *userAgentTransport) RoundTrip(req *http.Request) (*http.Response, error) { + // Set the UA at the transport so every client built from WrapTransport + // identifies itself, including the non-SDK clients (oauth2 flow, token + // refresh) that never pass through the SDK's own SetUserAgent. For SDK + // clients this overlaps gitea.SetUserAgent; both use httputil.UserAgent(), + // so the duplicate Header.Set is a no-op. req.Header.Set("User-Agent", UserAgent()) return t.base.RoundTrip(req) } diff --git a/modules/httputil/transport.go b/modules/httputil/transport.go new file mode 100644 index 00000000..9a6a96b6 --- /dev/null +++ b/modules/httputil/transport.go @@ -0,0 +1,57 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package httputil + +import ( + "crypto/tls" + "net" + "net/http" + "time" +) + +// Timeout values applied to every Gitea API request. These are deliberately +// connection-establishment and time-to-first-response-byte timeouts, NOT an +// overall request deadline: a large release-attachment upload can legitimately +// run for minutes, and as long as bytes keep flowing none of these fire. They +// only trip when a server accepts the connection but never (or far too slowly) +// starts responding — the "hangs forever" case from a stalled or unresponsive +// server (issue #1018). +const ( + // DialTimeout bounds establishing the TCP connection. + DialTimeout = 10 * time.Second + // TLSHandshakeTimeout bounds completing the TLS handshake. + TLSHandshakeTimeout = 10 * time.Second + // ResponseHeaderTimeout bounds the wait, after the request is written, for + // the server to begin sending response headers. This is the only timeout + // that protects against a server which accepts the connection but then goes + // silent — the originally reported #1018 symptom; DialTimeout/ + // TLSHandshakeTimeout do not, because the connection already succeeded. + // + // The value must clear Gitea's legitimate synchronous pre-response work. + // Profiling a self-hosted Gitea 1.24.6 (on hardware slower than gitea.com) + // showed creating a pull request that triggers conflict detection across + // ~1500 changed files takes ~10s before the first byte (3 runs: 10.06 / + // 10.08 / 10.24s); clean-diff PR creation was ~1s and large attachment + // uploads ~9ms. 120s is ~12x that measured worst case, leaving generous + // headroom for larger repos and busier servers while still failing in two + // minutes instead of hanging forever. + ResponseHeaderTimeout = 120 * time.Second +) + +// timeoutTransport returns an *http.Transport configured with tea's standard +// timeouts. The supplied tlsConfig is attached as-is (callers use it for +// insecure / skip-verify logins). It is a clone of http.DefaultTransport so +// connection pooling, proxy support and HTTP/2 keep working. Callers obtain it +// through WrapTransport, which also adds the User-Agent header. +func timeoutTransport(tlsConfig *tls.Config) *http.Transport { + t := http.DefaultTransport.(*http.Transport).Clone() + t.DialContext = (&net.Dialer{ + Timeout: DialTimeout, + KeepAlive: 30 * time.Second, + }).DialContext + t.TLSHandshakeTimeout = TLSHandshakeTimeout + t.ResponseHeaderTimeout = ResponseHeaderTimeout + t.TLSClientConfig = tlsConfig + return t +} diff --git a/modules/httputil/transport_stall_test.go b/modules/httputil/transport_stall_test.go new file mode 100644 index 00000000..b2ace216 --- /dev/null +++ b/modules/httputil/transport_stall_test.go @@ -0,0 +1,91 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package httputil + +import ( + "net" + "net/http" + "testing" + "time" +) + +// TestWrapTransportTimeouts verifies the transport returned by WrapTransport +// carries tea's standard timeout values, so a stalled server can't make tea +// hang forever (issue #1018). +func TestWrapTransportTimeouts(t *testing.T) { + rt := WrapTransport(nil) + uat, ok := rt.(*userAgentTransport) + if !ok { + t.Fatalf("WrapTransport returned %T, want *userAgentTransport", rt) + } + tr, ok := uat.base.(*http.Transport) + if !ok { + t.Fatalf("underlying base is %T, want *http.Transport", uat.base) + } + if tr.TLSHandshakeTimeout != TLSHandshakeTimeout { + t.Errorf("TLSHandshakeTimeout = %v, want %v", tr.TLSHandshakeTimeout, TLSHandshakeTimeout) + } + if tr.ResponseHeaderTimeout != ResponseHeaderTimeout { + t.Errorf("ResponseHeaderTimeout = %v, want %v", tr.ResponseHeaderTimeout, ResponseHeaderTimeout) + } + if tr.DialContext == nil { + t.Error("DialContext is nil, want a dialer with DialTimeout") + } +} + +// newStallListener returns a listener that accepts connections, reads the +// request, then goes silent without ever sending response headers — the +// "server accepts the connection but never responds" case ResponseHeaderTimeout +// guards against. The returned closer stops the listener. +func newStallListener(t *testing.T) (addr string, closer func()) { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + done := make(chan struct{}) + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + go func(c net.Conn) { + buf := make([]byte, 4096) + _, _ = c.Read(buf) // drain the request, then never respond + <-done // hold the connection open until the test ends + c.Close() + }(conn) + } + }() + return ln.Addr().String(), func() { + close(done) + ln.Close() + } +} + +// TestResponseHeaderTimeoutFires proves a request to a server that accepts the +// connection and request but never sends response headers aborts via +// ResponseHeaderTimeout rather than hanging. It builds the transport the same +// way WrapTransport does, with a short ResponseHeaderTimeout so the test is fast. +func TestResponseHeaderTimeoutFires(t *testing.T) { + addr, closer := newStallListener(t) + defer closer() + + tr := timeoutTransport(nil) + tr.ResponseHeaderTimeout = 2 * time.Second + client := &http.Client{Transport: &userAgentTransport{base: tr}} + + start := time.Now() + _, err := client.Get("http://" + addr + "/") + elapsed := time.Since(start) + + if err == nil { + t.Fatal("expected a timeout error from stalled server, got nil") + } + if elapsed > 10*time.Second { + t.Errorf("request took %v; ResponseHeaderTimeout did not fire", elapsed) + } + t.Logf("request failed as expected after %v: %v", elapsed, err) +}