From 2576451dc2b96a05dcffce99b9a4041d0146060f Mon Sep 17 00:00:00 2001
From: Sebastien Tardif <sebtardif@ncf.ca>
Date: Sat, 29 Aug 2026 17:20:17 -0700
Subject: [PATCH] fix(uploadstore): never return a bare HTTP client for R2

When http.DefaultTransport is not *http.Transport, defaultR2HTTPClient
returned &http.Client{} with no ResponseHeaderTimeout. A stalled R2
origin then hung Save, Delete, and Serve until the caller disconnected.

Always attach a Transport with ResponseHeaderTimeout. Leave Client.Timeout
at 0 so streaming stays intact.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
---
 apps/api/internal/uploadstore/r2.go      | 15 +++---
 apps/api/internal/uploadstore/r2_test.go | 66 ++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 6 deletions(-)

diff --git a/apps/api/internal/uploadstore/r2.go b/apps/api/internal/uploadstore/r2.go
index fe6629a01..30fa7ef05 100644
--- a/apps/api/internal/uploadstore/r2.go
+++ b/apps/api/internal/uploadstore/r2.go
@@ -84,13 +84,16 @@ func NewR2(cfg R2Config) (*R2, error) {
 }
 
 func defaultR2HTTPClient() *http.Client {
-	transport, ok := http.DefaultTransport.(*http.Transport)
-	if !ok {
-		return &http.Client{}
+	var transport *http.Transport
+	if base, ok := http.DefaultTransport.(*http.Transport); ok {
+		transport = base.Clone()
+	} else {
+		transport = &http.Transport{
+			Proxy: http.ProxyFromEnvironment,
+		}
 	}
-	cloned := transport.Clone()
-	cloned.ResponseHeaderTimeout = defaultR2ResponseHeaderTimeout
-	return &http.Client{Transport: cloned}
+	transport.ResponseHeaderTimeout = defaultR2ResponseHeaderTimeout
+	return &http.Client{Transport: transport}
 }
 
 func (s *R2) Save(ctx context.Context, body io.Reader, options SaveOptions) (SavedObject, error) {
diff --git a/apps/api/internal/uploadstore/r2_test.go b/apps/api/internal/uploadstore/r2_test.go
index 1ce852d47..4bee61cb1 100644
--- a/apps/api/internal/uploadstore/r2_test.go
+++ b/apps/api/internal/uploadstore/r2_test.go
@@ -4,10 +4,12 @@ import (
 	"context"
 	"errors"
 	"io"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
+	"time"
 )
 
 func TestR2SaveServeAndDelete(t *testing.T) {
@@ -141,6 +143,64 @@ func TestR2ConfigValidation(t *testing.T) {
 	}
 }
 
+func TestDefaultR2HTTPClientFallbackUsesHeaderTimeout(t *testing.T) {
+	saved := http.DefaultTransport
+	t.Cleanup(func() { http.DefaultTransport = saved })
+	http.DefaultTransport = stubRoundTripper{}
+
+	store, err := NewR2(R2Config{
+		AccountID:       "account",
+		AccessKeyID:     "access",
+		SecretAccessKey: "secret",
+		Bucket:          "bucket",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	client := store.httpClient
+	if client.Timeout != 0 {
+		t.Fatalf("expected streaming-safe client timeout, got %s", client.Timeout)
+	}
+	transport, ok := client.Transport.(*http.Transport)
+	if !ok {
+		t.Fatalf("expected fallback *http.Transport, got %#v", client.Transport)
+	}
+	if transport.ResponseHeaderTimeout != defaultR2ResponseHeaderTimeout {
+		t.Fatalf("expected response header timeout %s, got %s", defaultR2ResponseHeaderTimeout, transport.ResponseHeaderTimeout)
+	}
+
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = ln.Close() })
+	go func() {
+		for {
+			conn, err := ln.Accept()
+			if err != nil {
+				return
+			}
+			go func(c net.Conn) {
+				defer c.Close()
+				_, _ = io.Copy(io.Discard, c)
+			}(conn)
+		}
+	}()
+
+	transport.ResponseHeaderTimeout = 50 * time.Millisecond
+	req, err := http.NewRequest(http.MethodGet, "http://"+ln.Addr().String()+"/", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = client.Do(req)
+	if err == nil {
+		t.Fatal("expected header timeout against a silent listener")
+	}
+	if !strings.Contains(err.Error(), "timeout awaiting response headers") {
+		t.Fatalf("expected response header timeout, got %v", err)
+	}
+}
+
 func TestR2SaveEmptyUploadUsesContentLength(t *testing.T) {
 	t.Parallel()
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -204,3 +264,9 @@ func TestR2RejectsKeysOutsidePrefix(t *testing.T) {
 		t.Fatalf("unexpected key: %q", key)
 	}
 }
+
+type stubRoundTripper struct{}
+
+func (stubRoundTripper) RoundTrip(*http.Request) (*http.Response, error) {
+	return nil, errors.New("unused stub transport")
+}