Skip to content

cap Content-Length before narrowing to int in Http2Client - #3431

Merged
velo merged 1 commit into
OpenFeign:masterfrom
alhudz:http2-content-length-cap
Jun 20, 2026
Merged

velo merged 1 commit into
OpenFeign:masterfrom
alhudz:http2-content-length-cap

Conversation

@alhudz

@alhudz alhudz commented Jun 20, 2026

Copy link
Copy Markdown
Contributor
  1. Http2Client.toFeignResponse narrows the response Content-Length with (int) length.getAsLong() and no range check.
  2. A server sending Content-Length: 2147483648 (2^31) wraps to -2147483648, and Content-Length: -1 is forwarded as-is.

Response.Body.length() is documented to be null when the length is unknown or greater than Integer.MAX_VALUE, and okhttp, httpclient, hc5 and googlehttpclient already cap before narrowing. The wrapped negative length also slips past the response.body().length() <= MAX_RESPONSE_BUFFER_SIZE check in InvocationContext that decides whether to buffer the whole response into memory. Capped so the int is returned only for 0 <= length <= Integer.MAX_VALUE, otherwise null.

repro: Http2ClientContentLengthTest builds an HttpResponse with a given Content-Length and reads body().length().

  • Content-Length: 2147483648 expected null, before -2147483648.
  • Content-Length: -1 expected null, before -1.
  • Content-Length: 1024 stays 1024.

@velo velo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This fixes a real edge case in the HTTP/2 adapter without changing the API surface. The range check in java11/src/main/java/feign/http2client/Http2Client.java matches the Response.Body contract, and Http2ClientContentLengthTest covers the oversized, negative, and normal-length paths. Checks are green, so this looks good to me.

@velo
velo merged commit 425cc81 into OpenFeign:master Jun 20, 2026
3 checks passed
@velo velo mentioned this pull request Jun 23, 2026
renechoi added a commit to renechoi/feign that referenced this pull request Jul 28, 2026
`convertResponse` mapped only `-1` to `null`, so any other negative
`Content-Length` reached `Response.Body#length()` unchanged.

`HttpURLConnection#getContentLength()` returns the header verbatim once
it fits in an `int`, and already returns `-1` when the value is unknown
or above `Integer.MAX_VALUE`. Measured against a raw socket reply on
JDK 17, 21 and 26:

    Content-Length: -5           -> getContentLength() = -5
    Content-Length: -1           -> getContentLength() = -1
    Content-Length: 3000000000   -> getContentLength() = -1

Widening the guard to `< 0` therefore reports a malformed value as
unknown while leaving both the `-1` and the over-2GB cases untouched.

This is the same defect fixed for `Http2Client` in OpenFeign#3431 and for
`GoogleHttpClient` in the previous commit. `Client.Default` and
`Client.Proxied` extend `DefaultClient`, so they inherit the fix.

Covered by tests mirroring `Http2ClientContentLengthTest`.

Co-authored-by: kdelay <[email protected]>
velo pushed a commit that referenced this pull request Jul 28, 2026
`convertResponse` narrowed the response `Content-Length` to `Integer`
while only guarding the upper bound, so a negative header value was
passed straight through to `Response.Body#length()`.

That field is documented as "length in bytes, if known. Null if unknown
or greater than Integer.MAX_VALUE", and every other client already
rejects negative values before narrowing:

- `OkHttpClient` (`contentLength() >= 0 && <= Integer.MAX_VALUE`)
- `ApacheHttp5Client` and `ApacheHttpClient` (same guard on the entity)
- `Http2Client` (same guard, added in #3431)

`GoogleHttpClient` was the only remaining client without it, so a
malformed `Content-Length: -1` surfaced as `length() == -1` instead of
`null`. `InvocationContext` compares that value against
`MAX_RESPONSE_BUFFER_SIZE` to decide whether a response is bufferable,
and a negative length silently passes the check.

Adds the missing lower-bound guard and reads the header once instead of
twice. Covered by tests mirroring `Http2ClientContentLengthTest`.
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.

2 participants