Repository navigation
Conversation
Validate Content-Length as 1*DIGIT inside HttpRequestImpl::addHeader, the single place byte-level request headers enter the message: * An empty value, a numeric prefix such as "10x", or a value beyond the size_t range is invalid framing (RFC 9110 section 8.6). Previously an empty value was indistinguishable from an absent header, so the bytes after the header section were parsed as the next request. * A repeated Content-Length whose value differs from the one already received is rejected instead of silently keeping the first one (RFC 9112 section 6.3), closing a CL.CL request smuggling vector. * A header line with whitespace between the field name and the colon is rejected with 400, as required by RFC 9112 section 5.1. The parsed value is cached in contentLengthHeaderValue_, so the parser no longer carries its own copy of the digit scan. Fixes drogonframework#2648
The request parser needs the same 1*DIGIT validation, so share one implementation instead of keeping separate copies that can diverge.
zyuhao7
force-pushed
the
bugfix/response-framing
branch
from
October 30, 2026 06:34
41cfd0c to
cfeb398
Compare
an-tao
requested changes
Oct 30, 2026
| { | ||
| size_t existingLength = 0; | ||
| return parseContentLength(it->second, existingLength) && | ||
| existingLength == parsedLength; |
Member
There was a problem hiding this comment.
[P2] Compare duplicate values against the cached length
contentLengthHeaderValue_ already holds the parsed value from the first Content-Length field. Re-parsing it->second for every duplicate can repeatedly scan a potentially long attacker-controlled value, especially when followed by many short duplicate fields. Could we compare parsedLength directly with the cached value here, after checking that the optional is set?
Contributor
Author
Picks up the OpenSSL 4.x build fixes (an-tao/trantor#423, drogonframework#422), which the macOS 15 CI needs because Homebrew now ships OpenSSL 4.1.
zyuhao7
force-pushed
the
bugfix/response-framing
branch
from
October 31, 2026 01:46
cfeb398 to
69ee62b
Compare
The first field already caches its parsed length in contentLengthHeaderValue_, so compare against that instead of scanning the stored string again for every repeated field.
A response parser that accepts ambiguous or malformed framing can disagree with an intermediary about where the body ends, desynchronizing keep-alive and pipelined connections. - Validate Content-Length as 1*DIGIT, rejecting signs, whitespace, trailing junk and overflow. - Reject repeated Content-Length fields and Content-Length combined with Transfer-Encoding. - Detect chunked case-insensitively as the final transfer coding. - Skip the trailer section after the last chunk instead of leaving it in the buffer as the start of the next response. - Treat 304 and 1xx responses as bodyless and skip informational responses in the client.
zyuhao7
force-pushed
the
bugfix/response-framing
branch
from
October 31, 2026 02:20
69ee62b to
7984589
Compare
This branch has not been deployed
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.
Summary
The response parser trusted framing fields a proxy may interpret differently, so a crafted or broken response could desynchronize a keep-alive or pipelined connection. This aligns the parser with RFC 9112 section 6.3:
Content-Lengthfields and any response carrying bothContent-LengthandTransfer-Encoding.Content-Lengthas1*DIGIT(RFC 9110 section 8.6) via the sharedHttpUtils::parseContentLengthhelper.chunkedas the final transfer coding case-insensitively, including transfer parameters.HttpClientImplinstead of being delivered as the final response.This branch is stacked on #2652 (
bugfix/2648-content-length-framing), which addsHttpUtils::parseContentLengthand the request-side framing fix. Only the last commit,fix: harden HTTP response body framing, belongs to this PR; the first two come from #2652 and will drop out of the diff once it lands.Test plan
HttpResponseParserTest.cc: 11 cases covering malformed/duplicate/overflowingContent-Length,Content-Length+Transfer-Encoding, chunked case and parameters, trailers, 304/204, interim responses, and HEAD.integration_test_server: 10574 assertions in 29 test cases passed.