From d0ad670628845665909c2e6f404cd8009da6fb52 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 03:10:18 +0000 Subject: [PATCH 1/2] Test a body of exactly the maximum size, chunked or not A chunked body of exactly MaxResponseSize bytes is refused with ErrResponseTooLarge; this test fails until the fix. The upstream sends the end of the chunked body only after the test has read that many bytes, so the read that finds the limit used up comes first. A body sent with a Content-Length, and a body one byte over the limit under either framing, already behave as intended. Model: opus-5-5 --- internal/httpfetcher/fetch_internal_test.go | 134 ++++++++++++++++++++ 1 file changed, 134 insertions(+) diff --git a/internal/httpfetcher/fetch_internal_test.go b/internal/httpfetcher/fetch_internal_test.go index 83c78c0..c443987 100644 --- a/internal/httpfetcher/fetch_internal_test.go +++ b/internal/httpfetcher/fetch_internal_test.go @@ -9,6 +9,7 @@ import ( "net/http" "net/http/httptest" "slices" + "strconv" "strings" "sync" "testing" @@ -297,6 +298,139 @@ func TestFetchMaxResponseSizeEnforced(t *testing.T) { } } +// fetchWithMaxResponseSize serves handler as the fake upstream and fetches +// its /image route with MaxResponseSize set to maxResponseSize. +func fetchWithMaxResponseSize( + t *testing.T, + maxResponseSize int64, + handler http.HandlerFunc, +) *FetchResult { + t.Helper() + + srv := httptest.NewServer(handler) + t.Cleanup(srv.Close) + + cfg := DefaultConfig() + cfg.MaxResponseSize = maxResponseSize + + f, _ := newServerFetcher(t, srv, cfg) + + res := fetchImage(t, f, "/image") + t.Cleanup(func() { _ = res.Content.Close() }) + + return res +} + +// TestFetchContentLengthBodyAtMaxResponseSize checks that a body sent with a +// Content-Length is read whole at exactly MaxResponseSize bytes and refused +// at one byte more. +func TestFetchContentLengthBodyAtMaxResponseSize(t *testing.T) { + t.Parallel() + + const maxResponseSize = 8 + + for _, tc := range []struct { + size int + wantErr error + }{ + {maxResponseSize, nil}, + {maxResponseSize + 1, ErrResponseTooLarge}, + } { + t.Run(strconv.Itoa(tc.size)+" bytes", func(t *testing.T) { + t.Parallel() + + body := strings.Repeat("x", tc.size) + + res := fetchWithMaxResponseSize(t, maxResponseSize, + func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", contentTypeJPEG) + w.Header().Set("Content-Length", strconv.Itoa(tc.size)) + _, _ = io.WriteString(w, body) + }) + + if res.ContentLength != int64(tc.size) { + t.Fatalf("ContentLength = %d, want %d", res.ContentLength, tc.size) + } + + data, err := io.ReadAll(res.Content) + t.Logf("body of %d bytes: read %d, error %v", tc.size, len(data), err) + + if !errors.Is(err, tc.wantErr) { + t.Fatalf("read error = %v, want %v", err, tc.wantErr) + } + + if err == nil && string(data) != body { + t.Errorf("read %q, want %q", data, body) + } + }) + } +} + +// TestFetchChunkedBodyAtMaxResponseSize checks that a chunked body is read +// whole at exactly MaxResponseSize bytes and refused at one byte more. The +// upstream sends the end of the body only once the test has read +// MaxResponseSize bytes, so the read after them comes before the end of the +// body has arrived, as it can from a real upstream. +func TestFetchChunkedBodyAtMaxResponseSize(t *testing.T) { + t.Parallel() + + const maxResponseSize = 8 + + for _, tc := range []struct { + size int + wantErr error + }{ + {maxResponseSize, nil}, + {maxResponseSize + 1, ErrResponseTooLarge}, + } { + t.Run(strconv.Itoa(tc.size)+" bytes", func(t *testing.T) { + t.Parallel() + + body := strings.Repeat("x", tc.size) + endBody := make(chan struct{}) + + res := fetchWithMaxResponseSize(t, maxResponseSize, + func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", contentTypeJPEG) + _, _ = io.WriteString(w, body) + // With no Content-Length, flushing sends the body + // chunked; its end is sent when the handler returns. + _ = http.NewResponseController(w).Flush() + + select { + case <-endBody: + case <-r.Context().Done(): + } + }) + + if res.ContentLength != -1 { + t.Fatalf("ContentLength = %d, want -1 (chunked)", res.ContentLength) + } + + head := make([]byte, maxResponseSize) + + _, err := io.ReadFull(res.Content, head) + if err != nil { + t.Fatalf("read the first %d bytes: %v", maxResponseSize, err) + } + + close(endBody) + + rest, err := io.ReadAll(res.Content) + data := string(head) + string(rest) + t.Logf("body of %d bytes: read %d, error %v", tc.size, len(data), err) + + if !errors.Is(err, tc.wantErr) { + t.Fatalf("read error = %v, want %v", err, tc.wantErr) + } + + if err == nil && data != body { + t.Errorf("read %q, want %q", data, body) + } + }) + } +} + func TestFetchSemaphoreReleasedOnError(t *testing.T) { t.Parallel() -- 2.54.0 From 3afc70999b41b3e4c0d66aa71431fe524807255a Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 03:32:31 +0000 Subject: [PATCH 2/2] Accept a chunked upstream body of exactly the maximum size (closes #248) Once MaxResponseSize bytes were read, limitedReader.Read refused the next read without looking for the end of the body. A body sent with a Content-Length gets io.EOF with its last bytes, but the end of a chunked body can come in a later read, so a chunked body of exactly the limit was refused. Once the limit is used up, Read now reads one more byte: at the end of the body it returns io.EOF, and with more data it returns ErrResponseTooLarge, as does every later Read. Model: opus-5-5 --- TODO.md | 9 +++++++++ internal/httpfetcher/httpfetcher.go | 23 +++++++++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/TODO.md b/TODO.md index b85bf37..c046779 100644 --- a/TODO.md +++ b/TODO.md @@ -38,6 +38,15 @@ P2: security: per-origin rate limiting commented entry under `example.com`. Getting Started and "Running under upaas" in `README.md` say that every image on an allowlisted host is served without a signature, so an allowlisted host must be one the operator controls. +- 2026-10-15 a chunked upstream body of exactly `upstream_max_response_size` + bytes is accepted (closes #248): once the limit was used up, the fetcher's + next read refused the body without looking for its end. Go's HTTP client + returns the end of a body sent with a `Content-Length` together with its last + bytes, but the end of a chunked body can come in a later read, so such a body + was refused after it had been downloaded whole, and the image routes + answered 500. Once the limit is used up, the fetcher now reads one more byte: + if the body has ended it is accepted, and if there is more it is refused with + `ErrResponseTooLarge`, as before. - 2026-10-13 `fit=outside` no longer makes an output above 8192 on a side (closes #257): it scales the image to cover the requested size, and the side scaled past that size was held to no maximum, so `1x8192?fit=outside` of a diff --git a/internal/httpfetcher/httpfetcher.go b/internal/httpfetcher/httpfetcher.go index 9d09295..1428040 100644 --- a/internal/httpfetcher/httpfetcher.go +++ b/internal/httpfetcher/httpfetcher.go @@ -660,14 +660,33 @@ func dialSSRFSafe( return nil, fmt.Errorf("%w to %s", errConnectFailed, host) } -// limitedReader wraps a reader and limits the number of bytes read. +// limitedReader wraps a reader and refuses a body longer than remaining +// bytes with ErrResponseTooLarge. remaining goes below 0 once the body is +// found to be longer. type limitedReader struct { reader io.Reader remaining int64 } +// Read reads at most the remaining bytes. Once they are read, it reads one +// more byte to tell the end of the body from more data: at the end it +// returns the wrapped reader's error, io.EOF; with more data it returns +// ErrResponseTooLarge, as does every later Read. func (r *limitedReader) Read(p []byte) (int, error) { - if r.remaining <= 0 { + if r.remaining < 0 { + return 0, ErrResponseTooLarge + } + + if r.remaining == 0 { + var extra [1]byte + + n, err := r.reader.Read(extra[:]) + r.remaining -= int64(n) + + if n == 0 { + return 0, err + } + return 0, ErrResponseTooLarge } -- 2.54.0