sahvx655-wq commented on code in PR #3644:
URL: https://github.com/apache/brpc/pull/3644#discussion_r4366179340
##########
src/brpc/details/http_parser.cpp:
##########
@@ -1605,6 +1605,10 @@ size_t http_parser_execute (http_parser *parser,
break;
case 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.
+ */
+ parser->flags &= ~F_CHUNKED;
Review Comment:
Good catch, confirmed it the same way: with the clear in
`h_transfer_encoding`, `Transfer-Encoding: chunked` followed by an empty
`Transfer-Encoding:` line goes straight through `s_header_value_discard_lws`,
keeps `F_CHUNKED` and parses as `HPE_OK`. Fixed in 25b0716 and added that case
to `repeated_transfer_encoding_lines`, which now gets
`HPE_INVALID_TRANSFER_ENCODING`.
I put the clear on the `:` in `s_header_field`, guarded by `header_state ==
h_transfer_encoding`, rather than right next to `uses_transfer_encoding = 1`.
That branch fires once the 17-character prefix matches, before the name has
ended, so `Transfer-Encoding-X: a` after a `chunked` line would also drop the
flag, and a valid chunked request would then be rejected (I tried it, it
returns errno 31). At the colon the name has fully matched, and an empty value
still resets the flag. There's a test for the `Transfer-Encoding-X` case too.
This also means the clear runs exactly once per header line, which covers the
Copilot note above.
--
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]
