wwbmmm commented on code in PR #3629:
URL: https://github.com/apache/brpc/pull/3629#discussion_r4332420160
##########
src/brpc/details/http_message.cpp:
##########
@@ -550,7 +565,7 @@ ssize_t HttpMessage::ParseFromIOBuf(const butil::IOBuf
&buf) {
&_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);
Review Comment:
[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.
##########
test/brpc_http_rpc_protocol_unittest.cpp:
##########
@@ -3081,6 +3082,61 @@ TEST_F(HttpTest, http_expect) {
ASSERT_EQ(imsg_guard->header().status_code(), brpc::HTTP_STATUS_OK);
}
+TEST_F(HttpTest, pipelined_requests_get_one_response_each) {
+ brpc::Server server;
+ server.set_version("pipelined");
+ ASSERT_EQ(0, server.Start(0, nullptr));
+
+ const butil::EndPoint ep = server.listen_address();
+ brpc::SocketOptions options;
+ options.remote_side = ep;
+ brpc::SocketId id;
+ ASSERT_EQ(0, brpc::Socket::Create(options, &id));
+ brpc::SocketUniquePtr sock;
+ ASSERT_EQ(0, brpc::Socket::Address(id, &sock));
+
+ // Both requests in one write, so that the server reads them together.
+ butil::IOBuf request_buf;
+ request_buf.append("GET /health HTTP/1.1\r\nHost: a.com\r\n\r\n"
+ "GET /version HTTP/1.1\r\nHost: a.com\r\n\r\n");
+ ASSERT_EQ(0, sock->Write(&request_buf));
+ 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!";
+ }
+
+ // The server may answer in any order.
+ std::multiset<std::string> bodies;
+ butil::IOPortal read_buf;
+ 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) {
+ ASSERT_LT(butil::cpuwide_time_us(), start_time + 1000000L)
+ << "Too long! bodies.size()=" << bodies.size();
+ bthread_usleep(1000);
+ continue;
+ }
+ ASSERT_GT(nr, 0);
Review Comment:
[replied by brpc-oncall robot] Fixed, the loop now retries on `EINTR` as
well.
##########
src/brpc/details/http_message.h:
##########
@@ -55,11 +55,14 @@ class HttpMessage {
butil::IOBuf &body() { return _body; }
// Parse from array, length=0 is treated as EOF.
+ // Parsing stops at the end of the message, bytes after it are left to
+ // the next message.
// Returns bytes parsed, -1 on failure.
ssize_t ParseFromArray(const char *data, const size_t length);
-
+
// 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.
Review Comment:
[replied by brpc-oncall robot] Fixed the typos.
##########
test/brpc_http_message_unittest.cpp:
##########
@@ -605,6 +611,141 @@ TEST(HttpMessageTest, chunked_trailer_is_not_a_header) {
ASSERT_EQ("10.0.0.1", *xff);
}
+// Puts `data' in a single IOBuf block, the way one read() returns pipelined
+// messages that arrive together.
+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));
+}
Review Comment:
[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.
--
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]
