Repository navigation
[v26.x] http2: do not reset stream before END_STREAM is sent - #66672
barathraj048 wants to merge 1 commit into
Conversation
|
Review requested:
|
When a compat API response finishes and the request had no body, kMaybeDestroy() takes the graceful-close path and sends RST_STREAM(NO_ERROR) right away. If the connection flow-control window is exhausted at that point, the empty END_STREAM DATA frame is still queued, so the reset goes out first and the END_STREAM frame is dropped, truncating an otherwise complete response. Skip the graceful close when the request already ended with its HEADERS frame, since there is no request body to abort. Fixes: nodejs#66525 Refs: nodejs#64265 Refs: nodejs#64663 Signed-off-by: Barath <barathraj048@gmail.com>
4279500 to
ec8c98c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v26.x-staging #66672 +/- ##
=================================================
+ Coverage 90.24% 90.74% +0.49%
=================================================
Files 729 774 +45
Lines 242760 271784 +29024
Branches 46044 52069 +6025
=================================================
+ Hits 219073 246619 +27546
- Misses 15133 16065 +932
- Partials 8554 9100 +546
🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
692f720 to
7eb230a
Compare
Failed to start CIFull Auto Start CI output |
|
The v26.x-staging branch was force-pushed after I opened this, and the |
|
Thanks for the reviews and for starting CI. The only failure in 78480 is |
When a compat API response finishes and the request had no body,
kMaybeDestroy() takes the graceful-close path and sends RST_STREAM(NO_ERROR)
right away. If the connection flow-control window is exhausted at that
point, the empty END_STREAM DATA frame is still queued, so the reset goes
out first and END_STREAM is dropped. The client gets the full body but no
END_STREAM, so it treats the response as incomplete.
This skips the graceful close when the request already ended with its
HEADERS frame, since there is no request body to stop.
This targets v26.x-staging because main is not affected: the trailer
changes in #64265 avoid this path, but the v26 backport (#64663) left
them out. I could also reproduce it on v22.16.0, and the issue reports
v24.x too.
Testing:
RST_STREAM(code=0)before the fix,DATA(0B, END_STREAM)after.checks that /small gets END_STREAM before any reset, and that the server
stream still closes on its own.
python tools/test.py "parallel/test-http2-*": 283 passed (Windows x64).Fixes: #66525
Refs: #64265
Refs: #64663