Skip to content

feat: Add SetMaxResponseSize to limit response body size (#406) - #512

Open
ManuelReschke wants to merge 2 commits into
imroc:masterfrom
ManuelReschke:feat_406
Open

feat: Add SetMaxResponseSize to limit response body size (#406)#512
ManuelReschke wants to merge 2 commits into
imroc:masterfrom
ManuelReschke:feat_406

Conversation

@ManuelReschke

Copy link
Copy Markdown
Contributor

Add SetMaxResponseSize to limit response body size

Fixes #406

Summary

This change adds a built-in maximum response body size so callers no longer need to combine DisableAutoReadResponse() with a manual io.LimitReader / io.CopyN workaround.

  • Add Client.SetMaxResponseSize and Request.SetMaxResponseSize (request overrides client; 0 means unlimited).
  • When Content-Length is known and exceeds the limit, close the body without reading it and return ResponseBodyTooLargeError (saves bandwidth and memory).
  • When length is unknown (e.g. chunked), wrap the body with a limiting reader that stops at the configured size and returns a sticky *ResponseBodyTooLargeError.
  • Skip the Content-Length early reject for HEAD (advertised length, empty body) so ParallelDownload size probes keep working.
  • Support errors.Is(err, ErrResponseBodyTooLarge) and errors.As for structured inspection of Limit / ContentLength.

Motivation

Users making many requests reported unexpected bandwidth use and wanted a first-class size cap on response bodies. The previous workaround was:

client.DisableAutoReadResponse()
resp, err := client.R().Get(url)
// manually limit reads from resp.Body

A library-level option is more ergonomic and applies consistently to auto-read, manual body reads, downloads, and result unmarshalling.

Behavior notes

• The limit applies to bytes delivered to the application after transport-level Content-Encoding handling (e.g. gzip). For auto-decompressed responses, ContentLength is often -1, so only the streaming limit applies.
• Early rejection closes the body immediately and may prevent keep-alive reuse for that connection (intentional for oversized / untrusted payloads).
• Request.SetMaxResponseSize(0) disables the client-level limit for that request.
• Negative values are treated as unlimited (0).

Example

client := req.C().SetMaxResponseSize(1 << 20) // 1 MiB

resp, err := client.R().Get("https://example.com/large")
if errors.Is(err, req.ErrResponseBodyTooLarge) {
    var e *req.ResponseBodyTooLargeError
    if errors.As(err, &e) {
        // e.Limit, e.ContentLength (-1 if exceeded while reading)
    }
}

// Per-request override
resp, err = client.R().SetMaxResponseSize(64 << 10).Get(url)

Tests

Added coverage for:

• bodies within and exactly at the limit
• Content-Length early reject without buffering
• chunked bodies that exceed the limit while reading
• request-level override and disable of the client limit
• DisableAutoReadResponse + manual / ToBytes reads
• SetOutput / SetOutputFile download paths
• HEAD not failing on advertised Content-Length
• download still running after non-size errors (e.g. unmarshal failure)
• clone preservation and sticky reader errors

Allow clients and individual requests to cap how much response body data
is accepted. When Content-Length exceeds the limit the body is not read,
saving bandwidth; otherwise a limiting reader stops at the configured
size and returns ResponseBodyTooLargeError.
HEAD responses advertise Content-Length without a body; rejecting them
broke ParallelDownload size probes. Narrow the handleDownload skip to
Content-Length early rejects only, and document compression/keep-alive
behavior. Expand tests for HEAD and download-after-unmarshal.

@imroc imroc left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review (PR Review Loop):

Scope: Adds Client.SetMaxResponseSize / Request.SetMaxResponseSize to cap response body size (fixes #406). Files: client.go, client_wrapper.go, middleware.go, request.go, response.go, req_test.go, new response_maxsize_test.go (396 lines). No modified stdlib or quic-go code touched.

Verification:

  • Checked out the PR branch locally: go build ./... clean, go test ./... -count=1 -short all 22 packages pass.
  • CI passes on both Go 1.25.x and 1.26.x.

Design review:

  • Request overrides client; 0 disablesgetMaxResponseSize() correctly resolves request → client → 0. Request.SetMaxResponseSize(0) stores a non-nil *int64 pointing to 0, so it correctly distinguishes "not set" (nil) from "explicitly unlimited" (0). Good.
  • HEAD exceptionapplyMaxResponseSize skips the Content-Length early reject for HEAD so ParallelDownload size probes still work. Correct: HEAD advertises Content-Length but has no body.
  • Content-Length early reject — when cl > max, body is closed immediately and replaced with http.NoBody. Bandwidth and memory are not wasted. The connection may not be reusable (early close), which is documented and intentional for untrusted endpoints.
  • Streaming limit (chunked)maxResponseBodyReader reads at most n+1 bytes to detect crossing the limit, then returns *ResponseBodyTooLargeError with ContentLength: -1. Sticky error on subsequent reads. Correct.
  • CloneClient.Clone() uses value copy (cc := *c), so maxResponseSize (int64) is copied correctly. TestMaxResponseSizeClone verifies independence after mutating the clone.
  • middleware.gohandleDownload correctly skips when ErrResponseBodyTooLarge was already returned and r.body == nil, preventing creation of empty output files on early reject.
  • Error APIErrResponseBodyTooLarge sentinel + *ResponseBodyTooLargeError with Is() supports both errors.Is and errors.As. Clean.

Minor observations (non-blocking):

  • response.go:maxResponseBodyReader.Read — when len(p) == 0, returns (0, nil). This is fine for io.Copy which never sends zero-length reads, but a defensive io.ErrNoProgress could be considered if callers misuse it. Not a blocker since io.Copy and io.ReadAll are the expected callers.
  • response_maxsize_test.go:TestMaxResponseSizeDownloadStillRunsAfterUnmarshalError — the test has some defensive conditionals around unmarshal behavior, which is reasonable given it tests interaction between two subsystems (size limit + download-after-unmarshal-failure). It does not weaken existing tests.

Test coverage: Excellent — 19 new test cases covering within/exact/over limit, Content-Length vs chunked, request override, disable, DisableAutoRead, ToBytes, download (file + writer), HEAD, clone, error messages, sticky reader, success result unmarshalling. No existing tests deleted or weakened.

No blockers. Approving.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support size limit of response body?

2 participants