wwbmmm opened a new pull request, #3474:
URL: https://github.com/apache/brpc/pull/3474
### What problem does this PR solve?
Issue Number: N/A
Problem Summary:
The HTTP/2 protocol is enabled by default on every brpc server port and
runs before any authentication. Several receive-side limits of the
HTTP/2/HPACK path were advertised to the peer but never enforced, which
lets a single unauthenticated connection grow server memory without a
bound ("HPACK bomb", pinned streams, control-frame ack floods) or, when
combined with the dynamic-table accounting issue fixed in #3462, drive
the HPACK decoder table to desync and crash.
### What is changed and the side effects?
Changed:
- `H2Settings::max_concurrent_streams`: bounded default (1024,
`DEFAULT_MAX_CONCURRENT_STREAMS`) and server-side enforcement: streams
opened beyond the advertised limit are rejected with
`RST_STREAM(REFUSED_STREAM)` (RFC 7540 5.1.2), so a peer can no longer
pin unlimited pending streams per connection. Set it to
`std::numeric_limits<uint32_t>::max()` to restore the previous
unlimited behavior.
- `H2Settings::max_header_list_size`: bounded default (1MB,
`DEFAULT_MAX_HEADER_LIST_SIZE`) and receive-side enforcement in
`H2StreamContext::ConsumeHeaders` (cumulative decoded size,
RFC 7540 10.5.1) plus a cap on accumulated HEADERS/CONTINUATION
fragments. Oversized header blocks are rejected (connection error).
New `-h2_client_max_header_list_size` (default 1MB) controls the
client-side limit; 0/negative restores unlimited.
- HPACK dynamic table size updates are now validated against the table
size this endpoint advertised (what `HPacker::Init` was called with)
rather than the RFC default of 4096, closing a queue/accounting
desync that was reachable when an operator configured a smaller
`header_table_size`.
- h2 control-frame replies no longer bypass
`-socket_max_unwritten_bytes`: `WriteAck` now respects the write cap
by default (new `-h2_ack_ignore_eovercrowded`, default false). A
failed WINDOW_UPDATE send is re-deferred for a later retry instead of
losing window credits, and an unrecoverable connection error now
closes the connection after GOAWAY (RFC 7540 5.4.1) instead of
continuing to parse the corrupt stream.
Side effects:
- Performance effects: negligible; the added checks are a couple of
additions/comparisons per decoded header and per stream insertion,
and the defaults only apply to peers that did not negotiate a higher
limit.
- Breaking backward compatibility: deployments relying on unlimited
concurrent streams (default `UINT32_MAX`) or unlimited header lists
must raise the corresponding `ServerOptions.h2_settings` fields or
gflags. Requests with more than 1024 concurrent in-flight streams per
connection or a decoded header list larger than 1MB are now rejected;
compliant clients retry REFUSED_STREAM streams later.
### Check List:
- [x] Please make sure your changes are compilable.
- [x] When providing us with a new feature, it is best to add related tests.
- [x] Please follow [Contributor Covenant Code of
Conduct](https://github.com/apache/brpc/blob/master/CODE_OF_CONDUCT.md).
--
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]
