Handle HTTP/2 1xx interim responses from the origin#13351
Open
bryancall wants to merge 7 commits into
Open
Conversation
ATS did not handle 1xx interim responses (for example 103 Early Hints) received on an outbound HTTP/2 connection to an origin. The interim response headers were decoded into the same buffer as the following final response, producing a duplicate :status pseudo-header that failed validation. ATS then reset the stream and reported it to the client as "Server closed connection while reading response header", returning a 502 even though the origin had not closed the connection. After a header block is decoded on an outbound stream, detect a 1xx status, discard the interim headers, and wait for the final response. This is applied to both the HEADERS and CONTINUATION decode paths and handles multiple consecutive interim responses. The interim response is not currently forwarded to the client; the final response is delivered normally. Also allow CONTINUATION frames on an outbound stream in the half-closed (local) state, which is the state in which an origin response is received. Previously a response whose header block spanned a CONTINUATION frame was rejected with "continuation bad state". The per-minute CONTINUATION flood limit still applies. Add a gold test that drives an HTTP/2 origin emitting 103, multiple interim responses, 100-continue, and a CONTINUATION-split interim before the final 200, verifying the client receives the 200 in each case. Fixes apache#13334
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes ATS origin-side HTTP/2 handling of informational (1xx) interim responses (e.g., 103 Early Hints) so they don’t get merged into the final response header block and trigger invalid duplicate :status parsing/validation failures. It also relaxes CONTINUATION-frame state handling for outbound (origin) streams so response headers split across CONTINUATION frames are accepted.
Changes:
- Detect and discard decoded outbound interim (1xx) response headers on both HEADERS and CONTINUATION decode paths, then wait for the final response.
- Allow CONTINUATION frames on outbound streams in
HALF_CLOSED_LOCALstate (typical for receiving an origin response). - Add a gold test with a hand-framed HTTP/2 TLS origin to exercise single/multiple interim responses,
100 Continue, and CONTINUATION-split interim responses.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/proxy/http2/Http2ConnectionState.cc |
Adds interim-response detection/discard and permits CONTINUATION in HALF_CLOSED_LOCAL for outbound/origin streams. |
tests/gold_tests/h2/http2_origin_interim_response.test.py |
New gold test driver that spins up per-mode origin processes and verifies ATS returns the final 200. |
tests/gold_tests/h2/h2_interim_origin.py |
New minimal HTTP/2 TLS origin helper that hand-frames interim and final responses for test coverage. |
Free and reset the encoded header-block buffer when discarding an interim 1xx response, so it is not leaked when the next HEADERS frame allocates a new buffer. Both the HEADERS and CONTINUATION decode paths now use a shared discard helper. In the gold test, keep the origin helper processes alive across all runs (StillRunningAfter), and correct the test origin's HTTP/2 SETTINGS handshake: do not send an unsolicited SETTINGS ACK, and ACK the client's SETTINGS frame when it is received.
The per-connection thread captured the accept-loop socket via a closure, so concurrent connections could race and handle the wrong socket. Pass the socket as a thread argument instead.
The helper origin wrote its temporary cert/key to the system temp dir with delete=False, leaving files behind across runs. Create them in the process run directory so AuTest sandbox cleanup removes them.
moonchen
requested changes
Jul 11, 2026
moonchen
left a comment
moonchen
left a comment
Contributor
There was a problem hiding this comment.
Change looks good. Two inline suggestions below on 1xx + END_STREAM handling.
Per RFC 9113 section 8.1, an informational (1xx) response carrying END_STREAM is malformed. The stream state has already advanced toward closed by the time the interim response is handled, so silently discarding it left the final response with nowhere to go and the client hanging. Return a stream PROTOCOL_ERROR instead, on both the HEADERS and CONTINUATION paths. Add a gold test case for an origin that sends a 1xx response with END_STREAM.
Use += when marking each origin StillRunningAfter; plain assignment overwrote the check, leaving only the last origin verified across runs. Close the accepted socket when the TLS handshake fails so the test origin does not leak file descriptors under repeated failures.
The test origin generates its certificate with the openssl CLI, so skip the test when openssl is not on PATH instead of failing during origin startup.
Contributor
Author
|
Thanks for the review. I addressed both points: ATS now returns a stream PROTOCOL_ERROR when a 1xx interim response sets END_STREAM (RFC 9113 section 8.1), on both the HEADERS and CONTINUATION decode paths, and I added a gold test case that exercises it. All review threads are resolved and CI is green. Could you take another look when you have a chance? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ATS did not handle 1xx interim responses (for example
103 Early Hints) received on an outbound HTTP/2 connection to an origin. The interim response headers were decoded into the same buffer as the following final response, producing a duplicate:statuspseudo-header that failed validation. ATS then reset the stream and reported it to the client as "Server closed connection while reading response header", returning a 502 even though the origin had not closed the connection.This reproduces against any origin that sends
103 Early Hintsbefore the final response over HTTP/2 (for example a Vercel-hosted origin on a browser navigation request).Fix
1xxstatus, discard the interim headers, and wait for the final response. This is applied to both the HEADERS and CONTINUATION decode paths and handles multiple consecutive interim responses. The interim response is not currently forwarded to the client; the final response is delivered normally.Tests
Adds
tests/gold_tests/h2/http2_origin_interim_response.test.pywith a small hand-framed HTTP/2 origin (Proxy Verifier cannot emit interim responses) covering a single103, multiple interim responses,100-continue, and a CONTINUATION-split interim before the final200. The test was confirmed failing before this change and passing after; the existingearly_hintsandh2origingold tests still pass.Fixes #13334