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]