Parse HTTP/1.1 responses, not just requests - #56
Merged
Conversation
Glyph11 hardens the request direction only, which leaves the client half of
HTTP/1.1 unserved: a client writes requests and reads responses, so none of
the existing entry points apply to it.
Adds BinaryResponse and UltraHardenedParser.TryExtractFullResponseHeader{ROM,
Validated}, mirroring the request parser. The header block reuses its rules
unchanged - bare LF, obs-fold, whitespace before the colon, token and
field-value validation, the Content-Length format and duplicate checks, and
the Transfer-Encoding + Content-Length rejection, which is a desync vector in
this direction too.
What does not carry over:
- No Host rule. That is a request requirement; applying it would reject
every response.
- The first line is HTTP-version SP status-code SP [reason-phrase]. The
status code must be exactly three digits and at least 100; the reason
phrase is optional, may be empty, and is charset-checked but never
interpreted.
- "HTTP/1.1 200\r\n" with no trailing space is accepted. The grammar asks
for the SP even when the phrase is empty, but origins send this and it
creates no ambiguity, so rejecting it would only cost interop.
BodyFramingDetector.DetectResponseBodyFraming takes the REQUEST METHOD, and
has to: a HEAD response carries the Content-Length its body would have had
and no body, so framing a response from its own headers alone reads the next
response as this one's content. 1xx, 204 and 304 are bodyless whatever the
headers say, a 2xx to CONNECT is a tunnel rather than a body, and a response
with no framing header at all runs until the connection closes - the new
BodyFraming.UntilClose, which has no request equivalent.
ParserLimits gains MaxReasonPhraseLength (default 512).
37 new tests, 403 green overall.
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.
Why
Glyph11 hardens one direction. A client writes requests and reads responses, so today none of the entry points apply to it -
BinaryRequestcarriesMethod,PathandQueryParameters,DetectBodyFramingis typed to it, and evenHttpParseExceptioncarries "the status code to return", which is server framing.This adds the other half. The header block is where the value is and it carries over untouched: bare LF, obs-fold, whitespace before the colon, token and field-value validation, Content-Length format and duplicate checks, and the
Transfer-Encoding+Content-Lengthrejection - a desync vector in this direction too, where the remainder of a mis-framed response becomes the head of the next one.API
Three decisions worth reviewing
DetectResponseBodyFramingtakes the request method, and cannot not take it. AHEADresponse carries theContent-Lengthits body would have had, and no body. Framing it from its own headers reads the next response as this one's content. Same for a 2xx toCONNECT, which is a tunnel. This is the one place the response API cannot mirror the request API, so it is the part most worth a second opinion.HTTP/1.1 200\r\nis accepted. RFC 9112 section 4 asks for the SP even when the reason phrase is empty. Origins send it without. There is no parsing ambiguity, so rejecting it buys nothing and costs interop.BodyFraming.UntilCloseis new. A response with no framing header runs to end of connection (section 6.3) - the only framing HTTP/1.0 origins ever had. A request can never be framed this way, so the enum gained a member rather than reinterpretingNone.Also:
ParserLimitsgainsMaxReasonPhraseLength(default 512).Note on the multi-segment path
TryExtractFullResponseHeaderValidatedlinearizes viaToArray(), mirroring the request entry point. That is a fair default for a server, where the request header usually arrives in one segment - but a client reading off a socket spans segments as the common case, so the XML doc points those callers at the ROM overload with their own contiguous buffer. Changing the library's linearization strategy felt like a separate PR.Tests
37 new tests covering the status line, the shared header rules, the Host rule not applying, and every framing branch including the HEAD trap. Full suite: 403 passing, 0 failing.