Skip to content

Surface HTTP/2 response resets after buffered data - #1343

Merged
quinnj merged 4 commits into
masterfrom
audit/h2-partial-response-reset
Aug 6, 2026
Merged

Surface HTTP/2 response resets after buffered data#1343
quinnj merged 4 commits into
masterfrom
audit/h2-partial-response-reset

Conversation

@quinnj

@quinnj quinnj commented Aug 6, 2026

Copy link
Copy Markdown
Member

Summary

  • preserve buffered HTTP/2 response DATA when a stream reset arrives
  • surface the terminal stream reset after the buffered bytes are drained
  • keep a response that already completed with END_STREAM when a later reset races in
  • clean up the body callback and stream registration without poisoning the shared HTTP/2 connection

Root cause

body_read! treated stream_done as clean EOF without checking the terminal stream error recorded by the read loop. A response without Content-Length could therefore return a truncated prefix as a successful complete body after RST_STREAM.

The reset handler also needed to distinguish normal response completion from error-driven stream_done. A late reset must not discard a response that already completed normally.

Validation

  • julia --startup-file=no --project=. test/http2_client_tests.jl
    • partial-reset matrix: 27/27
    • completed-response late-reset case: 7/7
    • completed multiplexed response survives a sibling connection failure: 7/7
    • terminal GOAWAY error details: 9/9
  • julia --startup-file=no --project=. test/http_client_transport_tests.jl — 106/106
  • julia --startup-file=no --project=. test/http_retry_tests.jl — 93/93
  • raw HTTP/2 peer probes cover partial DATA with and without Content-Length, complete responses followed by NO_ERROR or CANCEL, and a completed stream sharing a failed connection
  • independent Claude Desktop review with Fable 5 and Extra effort: CLEAN; restored-branch mutation fails only the new multiplexed regression

Co-authored by Codex

quinnj added 2 commits August 5, 2026 19:22
Preserve buffered response DATA after RST_STREAM, then raise the terminal stream error instead of reporting clean EOF. Close and unregister the body when the error surfaces.

Cover partial bodies across small and compaction-boundary payload sizes.
Ignore a late stream reset after END_STREAM completed the response normally. Add a synchronized wire-level regression that proves a non-NO_ERROR reset cannot discard the buffered complete body.
@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.38%. Comparing base (90c2359) to head (2c824e0).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1343      +/-   ##
==========================================
+ Coverage   88.36%   88.38%   +0.01%     
==========================================
  Files          30       30              
  Lines       11915    11923       +8     
==========================================
+ Hits        10529    10538       +9     
+ Misses       1386     1385       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

quinnj added 2 commits August 5, 2026 19:56
A connection failure caused by another multiplexed stream must not invalidate a response that already received END_STREAM. Remove the impossible incomplete-response branch and cover the completed-stream ownership case with a raw two-stream peer.
Assert the terminal GOAWAY error retains the peer last-stream identifier and renders its user-facing message. This pins the typed error contract used by response lifecycle failures.
@quinnj
quinnj marked this pull request as ready for review August 6, 2026 02:50
@quinnj
quinnj merged commit 8e9b80a into master Aug 6, 2026
8 checks passed
@quinnj
quinnj deleted the audit/h2-partial-response-reset branch August 6, 2026 02:50
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.

1 participant