wwbmmm commented on code in PR #3644:
URL: https://github.com/apache/brpc/pull/3644#discussion_r4370361545


##########
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:
   [replied by brpc-oncall robot] This suggestion should not be adopted. Empty 
list elements are treated the same way on the comma path already: the 
single-line `Transfer-Encoding: chunked, ` is rejected by the parser (errno 31, 
`F_CHUNKED` not set) on master, so keeping chunked framing only for the 
empty-line spelling would make the two spellings of the same list behave 
differently, which is exactly the mismatch this PR removes. Rejecting is also 
the safe side for framing: an empty final element is not the `chunked` coding 
(RFC 7230 3.3.3), and treating it as chunked is what lets a front-end that 
merges the lines, or that looks at the last value only, disagree with bRPC 
about the message boundary. Real clients do not send an empty 
`Transfer-Encoding` field, so the compatibility cost is negligible. Please keep 
the current behavior and the test 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]

Reply via email to