wwbmmm opened a new pull request, #3630: URL: https://github.com/apache/brpc/pull/3630
### What problem does this PR solve? Issue Number: N/A Problem Summary: `H2UnsentRequest::AppendAndDestroySelf()` HPACK-encoded the request headers with the connection's encoder before calling `H2Context::TryToInsertClientStream()`. Encoding adds header fields to the encoder's dynamic table, so when the insert failed (ELIMIT once the open streams reach the server's `SETTINGS_MAX_CONCURRENT_STREAMS`, EOVERCROWDED when unsent DATA exceeds `-socket_max_unwritten_bytes`, or ELOGOFF after a GOAWAY), `Socket::WriteRequest::Setup` dropped the appended HEADERS frame while the encoder's dynamic table kept the new entries. The peer's decoder then lags the encoder, so later requests on the same connection are decoded with header fields of other requests (e.g. another `:path` or another `authorization` value), or fail with a compression error. ELIMIT and EOVERCROWDED are retried by the default RetryPolicy, and h2 only uses CONNECTION_TYPE_SINGLE, so a retried request itself can be decoded with the wrong fields. ### What is changed and the side effects? Changed: - Insert the stream before encoding the headers (as before 9aa41f79), so a refused request no longer mutates HPACK state. The DATA frames allowed by the current flow-control windows are collected separately and appended after the HEADERS, so frames reach the wire in the same order as before. - Added `H2UnsentMessage.append_and_destroy_self_keeps_hpack_state_on_refused_insert`, which decodes what the client writes with a separate `HPacker` standing for the server, and made `request_does_not_fail_when_body_exceeds_window` check that HEADERS are written before DATA. Without the source change the new test fails on every run. Side effects: - Performance effects: none, the emitted frames and their order are unchanged. - Breaking backward compatibility: none. Tests run: - `brpc_h2_unsent_message_unittest` (all 6 cases pass) - `brpc_grpc_protocol_unittest` (all 8 cases pass) - `brpc_http_rpc_protocol_unittest` (61/62 pass; `HttpTest.parse_http_address` fails in the local sandbox because it resolves `www.baidu.com`, unrelated to this change) --- 🤖 This PR was automatically created by brpc-oncall -- 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]
