-
-
Notifications
You must be signed in to change notification settings - Fork 675
fix: ensure HTTP/2 sends Content-Length for empty POST requests #4613
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This change aligns HTTP/2 behavior with HTTP/1 by ensuring that empty POST requests (and other methods that expect payloads) include a Content-Length: 0 header. According to RFC 9110, a user agent SHOULD send Content-Length in a request when the method defines a meaning for enclosed content, even when the value is 0 (indicating empty content). Previously, HTTP/2 requests would omit the Content-Length header for empty POST requests, while HTTP/1 requests correctly included it. This inconsistency could cause issues with servers like IIS that reject empty POST requests without a Content-Length header (returning 411 Length Required). Changes: - Modified lib/dispatcher/client-h2.js to only omit Content-Length when both contentLength is 0 AND the method doesn't expect a payload - Updated the assertion to allow contentLength === 0 without a body - Updated test to verify Content-Length: 0 is sent for empty POST with H2 - Updated test comments to reflect the corrected behavior Fixes #4594 Ref #4612 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Matteo Collina <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4613 +/- ##
==========================================
- Coverage 92.93% 92.93% -0.01%
==========================================
Files 106 106
Lines 32973 33091 +118
==========================================
+ Hits 30645 30753 +108
- Misses 2328 2338 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
CI failures are unrelated |
Co-authored-by: Frédéric Delaporte <[email protected]>
|
@metcoder95 can I get a fresh approval? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
* Add tests for empty POST and content-length * fix: ensure HTTP/2 sends Content-Length for empty POST requests This change aligns HTTP/2 behavior with HTTP/1 by ensuring that empty POST requests (and other methods that expect payloads) include a Content-Length: 0 header. According to RFC 9110, a user agent SHOULD send Content-Length in a request when the method defines a meaning for enclosed content, even when the value is 0 (indicating empty content). Previously, HTTP/2 requests would omit the Content-Length header for empty POST requests, while HTTP/1 requests correctly included it. This inconsistency could cause issues with servers like IIS that reject empty POST requests without a Content-Length header (returning 411 Length Required). Changes: - Modified lib/dispatcher/client-h2.js to only omit Content-Length when both contentLength is 0 AND the method doesn't expect a payload - Updated the assertion to allow contentLength === 0 without a body - Updated test to verify Content-Length: 0 is sent for empty POST with H2 - Updated test comments to reflect the corrected behavior Fixes #4594 Ref #4612 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Matteo Collina <[email protected]> * Update client-h2.js Co-authored-by: Frédéric Delaporte <[email protected]> --------- Signed-off-by: Matteo Collina <[email protected]> Co-authored-by: Frédéric Delaporte <[email protected]> Co-authored-by: Claude <[email protected]>
This relates to...
Fixes #4594
Related to #4612
Rationale
This PR addresses the discrepancy in behavior between HTTP/1 and HTTP/2 when handling empty POST requests.
According to RFC 9110:
Previously:
Content-Length: 0Content-Lengthheader entirelyThis inconsistency causes compatibility issues with servers like Internet Information Server (IIS), which reject empty POST requests without a Content-Length header, returning
411 Length Required.Changes
Implementation (
lib/dispatcher/client-h2.js)Content-Lengthwhen bothcontentLength === 0AND the method doesn't expect a payload (like GET, HEAD)Content-Length: 0is now sent even when the body is emptycontentLength === 0without a bodyTests (
test/fetch/http2.js)Content-Length: 0Test Results
All HTTP/2 tests pass (51 tests total):
[Fetch] Empty POST without h2 has Content-Length[Fetch] Empty POST with h2 has Content-LengthBreaking Changes and Deprecations
None. This is a bug fix that aligns behavior with RFC 9110 recommendations and HTTP/1 implementation.
Status
🤖 Generated with Claude Code