Skip to content

avoid duplicate Content-Length header in DefaultClient - #3451

Open
alhudz wants to merge 1 commit into
OpenFeign:masterfrom
alhudz:content-length-duplicate
Open

avoid duplicate Content-Length header in DefaultClient#3451
alhudz wants to merge 1 commit into
OpenFeign:masterfrom
alhudz:content-length-duplicate

Conversation

@alhudz

@alhudz alhudz commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Repro: send a body-less POST/PUT whose template already carries a Content-Length header (e.g. @Headers("Content-Length: 0")), with sun.net.http.allowRestrictedHeaders=true so the header reaches the wire.
Cause: DefaultClient.convertAndSend writes the request Content-Length in the header loop and then unconditionally adds a second Content-Length: 0 for every body-less method that allows a body, so the field goes out twice. RFC 7230 §3.3.2 forbids generating multiple Content-Length fields; the duplicate is ambiguous framing and servers reject it (Tomcat returns 400, #2862).
Fix: only add the fallback Content-Length: 0 when the request does not already declare a Content-Length.

The regression test is gated on sun.net.http.allowRestrictedHeaders like the existing Content-Length tests, since HttpURLConnection otherwise drops the restricted header, and asserts the field is sent once.

@alhudz

alhudz commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

any update?

@velo

velo commented Jul 28, 2026

Copy link
Copy Markdown
Member

Thanks for the fix and the wire-level regression test! We ended up merging #3448, which removes the manual Content-Length write from the header loop entirely and lets the JDK own the header — that also covers the bodyless-request duplicate this PR guards against (the "0" fallback now fires exactly once). Holding off on merging this one to avoid stacking two untested-in-combination changes to the same code path; if you think the containsKey guard is still needed on top of #3448, happy to revisit.

@alhudz

alhudz commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Checked this against current master with #3448 in, by running my gated regression test on top of master without the containsKey guard:

repro: body-less POST with @Headers("Content-Length: 0") and sun.net.http.allowRestrictedHeaders=true
before #3448: recorded Content-Length = ["0", "0"]
on master now: recorded Content-Length = ["0"], test passes with no DefaultClient change

So agreed, the guard isn't needed any more. The header loop no longer writes Content-Length, which leaves the body-less 0 fallback as the only writer. Fine to close this one. If wire-level coverage for the #2862 case is worth keeping (the test in #3448 is mock-level and covers the body path), I can strip this down to just the gated test.

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