chenBright commented on code in PR #3524:
URL: https://github.com/apache/brpc/pull/3524#discussion_r3998344991


##########
src/brpc/details/http_message.cpp:
##########
@@ -131,6 +134,14 @@ int HttpMessage::on_header_value(http_parser *parser,
             http_message->_cur_value =
                 &header.AddHeader(http_message->_cur_header);
         }
+
+        if (FLAGS_http_max_header_count > 0 &&
+            header.HeaderCount() > FLAGS_http_max_header_count) {
+            LOG(ERROR) << "Too many headers, max="
+                       << FLAGS_http_max_header_count;
+            return -1;
+        }

Review Comment:
   Good catch on the ordering, but the post-add check is intentional.  At most 
   one extra header is stored, and then the whole message/stream is rejected 
   immediately, so the stored set stays bounded at max + 1 rather than growing 
   without limit.
   
   For the HTTP/1 path specifically, folded headers go through 
`GetOrAddHeader()`, 
   which reuses an existing entry without increasing the count. Checking 
`HeaderCount()` 
   after the add is therefore what correctly counts distinct headers. A 
pre-check like 
   `HeaderCount() >= max` before insertion could wrongly reject a valid folded 
continuation.
   The H2 path (AppendHeader always adds a new header) has the same max + 1 
bound 
   before rejection. So I'd keep the check after adding.



-- 
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