iframe-proxy

wwbmmm · 2026-10-20T07:47:40Z

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.

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

http_parser_execute() goes on 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 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. Pause the parser in on_message_complete_cb, so that ParseFromArray() and ParseFromIOBuf() return the bytes of one message; HPE_PAUSED is not an error. ParseHttpMessage() pops them and returns the message, and InputMessenger parses the rest with a new HttpContext, as it already does when the messages arrive in separate reads. request_sanity and response_sanity end their input with a CRLF after the body, which is not part of the message and is now left unparsed.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

4 open issues

When the parser is paused at on_message_complete, http_parser_execute will stop mid-block, but… · New
This loop only retries on EAGAIN. A syscall can also fail with EINTR, which would currently… · New
Fix spelling in the comment: 'Emtpy' -> 'Empty' and 'sliently' -> 'silently'. · New
This helper manually manages heap memory and assumes allocation succeeds. To make the test helper… · New

What changed in this PR

Fixes HTTP/1 message parsing so parsing stops exactly at on_message_complete, preventing pipelined messages received in a single read from being merged into one logical request/response.

Changes:

Pause http_parser on message completion and treat HPE_PAUSED as a successful parse outcome.
Update/extend unit tests to verify pipelined requests/responses are parsed as distinct messages.
Document the updated parsing contract in http_message.h.

_{Review effort: Lite}

wwbmmm · 2026-10-21T02:30:36Z

            &_parser, &g_parser_settings, blk.data(), blk.size());
        nprocessed += n;
        _parsed_block_size += n;
-        if (_parser.http_errno != 0) {
+        if (!ParserOk(_parser)) {
            // May try HTTP on other formats, failure is norm.
            RPC_VLOG << "Fail to parse http message, parser=" << _parser
                     << ", buf=" << butil::ToPrintable(buf);


[replied by brpc-oncall robot] The loop already checks Completed() right after each http_parser_execute() and breaks. on_message_complete_cb() sets the stage to HTTP_ON_MESSAGE_COMPLETE before pausing, so once the parser is paused the message is completed and no subsequent blocks are parsed.

wwbmmm · 2026-10-21T02:30:40Z

+        const ssize_t nr = read_buf.append_from_file_descriptor(sock->fd(), 4096);
+        if (nr < 0 && errno == EAGAIN) {
+            ASSERT_LT(butil::cpuwide_time_us(), start_time + 1000000L)
+                << "Too long! bodies.size()=" << bodies.size();
+            bthread_usleep(1000);
+            continue;
+        }
+        ASSERT_GT(nr, 0);


[replied by brpc-oncall robot] Fixed, the loop now retries on EINTR as well.

wwbmmm · 2026-10-21T02:30:44Z

    // Parse from butil::IOBuf.
    // Emtpy `buf' is sliently ignored, which is different from ParseFromArray.
+    // Parsing stops at the end of the message as in ParseFromArray.
    // Returns bytes parsed, -1 on failure.


[replied by brpc-oncall robot] Fixed the typos.

wwbmmm · 2026-10-21T02:30:48Z

+void AppendInOneBlock(butil::IOBuf* buf, const std::string& data) {
+    char* copy = (char*)malloc(data.size());
+    memcpy(copy, data.data(), data.size());
+    ASSERT_EQ(0, buf->append_user_data(copy, data.size(), free));
+}


[replied by brpc-oncall robot] Keeping the helper minimal on purpose: the input is a few dozen bytes, and an allocation failure would fail the test anyway, so an explicit null check would only add noise. This matches the existing test helpers in the repo.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

8 open issues

AppendInOneBlock is defined with external linkage in a test TU. This can cause ODR/linker errors… · New
This loop only retries on EAGAIN. A syscall can also fail with EINTR, which would currently…
When the parser is paused at on_message_complete, http_parser_execute will stop mid-block, but…
If append_user_data fails, ASSERT_EQ aborts the test without freeing copy, leaking memory in… · New
The timeout value 1000000L (and related sleep interval) is duplicated. Consider extracting a… · New
The timeout value 1000000L (and related sleep interval) is duplicated. Consider extracting a… · New
This helper manually manages heap memory and assumes allocation succeeds. To make the test helper…
Fix spelling in the comment: 'Emtpy' -> 'Empty' and 'sliently' -> 'silently'.

_{Review effort: Lite}

+void AppendInOneBlock(butil::IOBuf* buf, const std::string& data) {
+    char* copy = (char*)malloc(data.size());
+    memcpy(copy, data.data(), data.size());
+    ASSERT_EQ(0, buf->append_user_data(copy, data.size(), free));
+}


+    char* copy = (char*)malloc(data.size());
+    memcpy(copy, data.data(), data.size());
+    ASSERT_EQ(0, buf->append_user_data(copy, data.size(), free));


+    int64_t start_time = butil::cpuwide_time_us();
+    while (sock->fd() < 0) {
+        bthread_usleep(1000);
+        ASSERT_LT(butil::cpuwide_time_us(), start_time + 1000000L) << "Too long!";
+    }


+    start_time = butil::cpuwide_time_us();
+    while (bodies.size() < 2) {
+        const ssize_t nr = read_buf.append_from_file_descriptor(sock->fd(), 4096);
+        if (nr < 0 && (errno == EAGAIN || errno == EINTR)) {
+            ASSERT_LT(butil::cpuwide_time_us(), start_time + 1000000L)
+                << "Too long! bodies.size()=" << bodies.size();


wwbmmm requested a balanced review from Copilot October 20, 2026 07:47

Copilot AI reviewed Oct 20, 2026

View reviewed changes

Copilot stopped reviewing on behalf of wwbmmm due to an error October 20, 2026 08:50
The job was not acquired by a runner even after multiple attempts.

Address review comments and retry test read on EINTR

3628adc

wwbmmm requested a balanced review from Copilot October 21, 2026 02:30

Copilot AI reviewed Oct 21, 2026

View reviewed changes

Copilot stopped reviewing on behalf of wwbmmm due to an error October 21, 2026 02:57
The job was not acquired by a runner even after multiple attempts.

File	Description
test/brpc_http_rpc_protocol_unittest.cpp	Adds an integration test ensuring two pipelined requests yield two distinct responses.
test/brpc_http_message_unittest.cpp	Adds parsing unit tests for pipelined request/response scenarios and updates existing sanity tests for the new “stop at message end” behavior.
src/brpc/details/http_message.h	Documents that parsing stops at the end of one message and leaves remaining bytes for the next message.
src/brpc/details/http_message.cpp	Pauses parsing at message completion and treats `HPE_PAUSED` as non-error.

Sunbelt Computer Software

PL/B Language Development and Support

Navigation Menu

Repository navigation

Stop HTTP/1 parsing at the end of each message - #3629

Stop HTTP/1 parsing at the end of each message#3629
wwbmmm wants to merge 2 commits into
apache:masterfrom
wwbmmm:oncall/req-20261020-052746

wwbmmm commented Oct 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

wwbmmm Oct 21, 2026

Uh oh!

wwbmmm Oct 21, 2026

Uh oh!

wwbmmm Oct 21, 2026

Uh oh!

wwbmmm Oct 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Sunbelt Computer Software

PL/B Language Development and Support

Conversation

wwbmmm commented Oct 20, 2026

What problem does this PR solve?

What is changed and the side effects?

Check List:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

wwbmmm Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

wwbmmm Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

wwbmmm Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

wwbmmm Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants