sahvx655-wq commented on code in PR #3644:
URL: https://github.com/apache/brpc/pull/3644#discussion_r4371663959
##########
src/brpc/details/http_parser.cpp:
##########
@@ -1565,6 +1565,14 @@ size_t http_parser_execute (http_parser *parser,
}
if (ch == ':') {
+ if (parser->header_state == h_transfer_encoding) {
+ /* Multiple `Transfer-Encoding` headers are one list (RFC 7230
+ * 3.2.2), so `chunked` from an earlier line is no longer final.
+ * Clear it here rather than at value start so that an empty
+ * value also resets it.
+ */
+ parser->flags &= ~F_CHUNKED;
Review Comment:
Leaving this as is. I checked the comma path before moving the clear to the
colon: `Transfer-Encoding: chunked, ` on a single line already ends with an
empty final element and comes back as errno 31 on master, so preserving
`F_CHUNKED` for an empty second line would give the two spellings of the same
list different framing again. That's the exact mismatch the PR is closing.
Rejecting is also the conservative side here. An empty final element isn't
`chunked`, so per RFC 7230 3.3.3 the request has no valid chunked framing, and
treating it as chunked is what lets a front-end that merges the lines, or only
looks at the last one, disagree with bRPC on where the message ends. No real
client sends an empty `Transfer-Encoding` field, so the compatibility cost is
negligible. The test at 532-538 stays as written.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]