wwbmmm opened a new pull request, #3629: URL: https://github.com/apache/brpc/pull/3629
### What problem does this PR solve? Problem Summary: `http_parser_execute()` keeps going after `on_message_complete`, and `HttpMessage::ParseFromIOBuf()` checks `Completed()` only between IOBuf blocks (`ParseFromArray()` never does). When pipelined HTTP/1 messages arrive in the same read, the url, header fields and body of the next message are appended to the same `HttpMessage`: two requests become one request whose path is both paths joined, with both header sets merged and both bodies concatenated, and the client gets a single response. Responses read by a client are merged the same way. The merge goes on across IOBuf blocks and reads as long as no block ends exactly at a message end, so one `HttpMessage` also grows past the per-message header limit, and every merged request parses the whole joined url again (quadratic parsing cost). ### What is changed and the side effects? Changed: - `HttpMessage::on_message_complete_cb()` pauses the parser once the message is complete, so `ParseFromArray()` and `ParseFromIOBuf()` return the bytes of exactly one message. `HPE_PAUSED` is treated as success instead of an error. - `ParseHttpMessage()` pops the bytes of that message and returns it; `InputMessenger` parses the remaining bytes with a new `HttpContext`, exactly as it already does when the messages arrive in separate reads. - Tests: added `HttpMessageTest.parse_stops_at_end_of_pipelined_request`, `parse_stops_at_end_of_pipelined_chunked_request` and `parse_stops_at_end_of_pipelined_response` in `test/brpc_http_message_unittest.cpp`, and `HttpTest.pipelined_requests_get_one_response_each` in `test/brpc_http_rpc_protocol_unittest.cpp`. `request_sanity`/`response_sanity` now expect the CRLF after the body to be left unparsed, since it is not part of the message. Side effects: - Pipelined requests that arrive in one read are each served instead of being merged. As with separate reads, they are processed concurrently, so their responses may come back in any order. - A 1xx interim response followed by the final response in the same read becomes two responses, as when they arrive in separate reads. This is consistent with the existing behavior of separate reads. - Bytes after a message are left for the next message instead of being consumed by the current one. Performance effects: no regression on the normal path; merged messages no longer cause repeated re-parsing of a growing url. Breaking backward compatibility: no public interface change. `ParseFromArray()`/`ParseFromIOBuf()` now stop at the end of the message, which is documented in `http_message.h`. --- ### Check List: - Please make sure your changes are compilable. - When providing us with a new feature, it is best to add related tests. - Please follow [Contributor Covenant Code of Conduct](https://github.com/apache/brpc/blob/master/CODE_OF_CONDUCT.md). Tests run (CMake Debug build): `brpc_http_message_unittest`, `brpc_http_rpc_protocol_unittest`, `brpc_http_parser_unittest`, `brpc_server_unittest`, `brpc_builtin_service_unittest`, `brpc_socket_unittest`, `brpc_alpn_protocol_unittest` (run from `test/` so the test certs are found) and `brpc_input_messenger_unittest`. The three new `HttpMessageTest.parse_stops_at_end_of_pipelined_*` tests and the new `HttpTest.pipelined_requests_get_one_response_each` test fail without the source change and pass with it. `brpc_channel_unittest`/`brpc_http_rpc_protocol_unittest` pass except the DNS-dependent `ChannelTest.parse_hostname` and `HttpTest.parse_http_address` cases, which cannot resolve external names in the offline environment. --- 🤖 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]
