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

cap Content-Length before narrowing to int in Http2Client#3431
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
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