This is an automated email from the ASF dual-hosted git repository. wwbmmm pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push: new fcb483ed Fix HTTP HEAD method (#2366) fcb483ed is described below commit fcb483ed4e99d4a6d1ea7cc28917efd2b31ede07 Author: Bright Chen <chenguangmin...@foxmail.com> AuthorDate: Tue Sep 19 20:06:04 2023 +0800 Fix HTTP HEAD method (#2366) --- src/brpc/details/http_message.cpp | 12 +++++- src/brpc/details/http_message.h | 7 +++- src/brpc/policy/http_rpc_protocol.cpp | 15 ++++++- src/brpc/policy/http_rpc_protocol.h | 9 +++-- src/brpc/socket.cpp | 1 + src/brpc/socket.h | 6 +++ test/brpc_http_message_unittest.cpp | 43 +++++++++++++++++--- test/brpc_http_rpc_protocol_unittest.cpp | 67 +++++++++++++++++++++++++++++--- test/echo.proto | 4 ++ 9 files changed, 145 insertions(+), 19 deletions(-) diff --git a/src/brpc/details/http_message.cpp b/src/brpc/details/http_message.cpp index d9c590c5..bb6116d4 100644 --- a/src/brpc/details/http_message.cpp +++ b/src/brpc/details/http_message.cpp @@ -177,6 +177,14 @@ int HttpMessage::on_headers_complete(http_parser *parser) { uri.SetHostAndPort(*host_header); } } + + + // If server receives a response to a HEAD request, returns 1 and then + // the parser will interpret that as saying that this message has no body. + if (parser->type == HTTP_RESPONSE && + http_message->request_method() == HTTP_METHOD_HEAD) { + return 1; + } return 0; } @@ -392,9 +400,11 @@ const http_parser_settings g_parser_settings = { &HttpMessage::on_message_complete_cb }; -HttpMessage::HttpMessage(bool read_body_progressively) +HttpMessage::HttpMessage(bool read_body_progressively, + HttpMethod request_method) : _parsed_length(0) , _stage(HTTP_ON_MESSAGE_BEGIN) + , _request_method(request_method) , _read_body_progressively(read_body_progressively) , _body_reader(NULL) , _cur_value(NULL) diff --git a/src/brpc/details/http_message.h b/src/brpc/details/http_message.h index 2b9471a1..ca978b5b 100644 --- a/src/brpc/details/http_message.h +++ b/src/brpc/details/http_message.h @@ -46,7 +46,8 @@ class HttpMessage { public: // If read_body_progressively is true, the body will be read progressively // by using SetBodyReader(). - HttpMessage(bool read_body_progressively = false); + explicit HttpMessage(bool read_body_progressively = false, + HttpMethod request_method = HTTP_METHOD_GET); ~HttpMessage(); const butil::IOBuf &body() const { return _body; } @@ -64,6 +65,8 @@ public: bool Completed() const { return _stage == HTTP_ON_MESSAGE_COMPLETE; } HttpParserStage stage() const { return _stage; } + HttpMethod request_method() const { return _request_method; } + HttpHeader &header() { return _header; } const HttpHeader &header() const { return _header; } size_t parsed_length() const { return _parsed_length; } @@ -74,6 +77,7 @@ public: static int on_status(http_parser*, const char *, const size_t); static int on_header_field(http_parser *, const char *, const size_t); static int on_header_value(http_parser *, const char *, const size_t); + // Returns -1 on error, 0 on success, 1 on success and skip body. static int on_headers_complete(http_parser *); static int on_body_cb(http_parser*, const char *, const size_t); static int on_message_complete_cb(http_parser *); @@ -102,6 +106,7 @@ private: HttpParserStage _stage; std::string _url; + HttpMethod _request_method; HttpHeader _header; bool _read_body_progressively; // For mutual exclusion between on_body and SetBodyReader. diff --git a/src/brpc/policy/http_rpc_protocol.cpp b/src/brpc/policy/http_rpc_protocol.cpp index 58e89ee0..9120ba58 100644 --- a/src/brpc/policy/http_rpc_protocol.cpp +++ b/src/brpc/policy/http_rpc_protocol.cpp @@ -684,6 +684,10 @@ void PackHttpRequest(butil::IOBuf* buf, // may not echo back this field. But we send it anyway. accessor.get_sending_socket()->set_correlation_id(correlation_id); + // Store http request method into Socket since http response parser needs it, + // and skips response body if request method is HEAD. + accessor.get_sending_socket()->set_http_request_method(header->method()); + MakeRawHttpRequest(buf, header, cntl->remote_side(), &cntl->request_attachment()); if (FLAGS_http_verbose) { @@ -934,7 +938,12 @@ HttpResponseSender::~HttpResponseSender() { } } else { butil::IOBuf* content = NULL; - if (cntl->Failed() || !cntl->has_progressive_writer()) { + if ((cntl->Failed() || !cntl->has_progressive_writer()) && + // https://datatracker.ietf.org/doc/html/rfc7231#section-4.3.2 + // The HEAD method is identical to GET except that the server MUST NOT + // send a message body in the response (i.e., the response terminates at + // the end of the header section). + req_header->method() != HTTP_METHOD_HEAD) { content = &cntl->response_attachment(); } butil::IOBuf res_buf; @@ -1097,7 +1106,9 @@ ParseResult ParseHttpMessage(butil::IOBuf *source, Socket *socket, // source is likely to be empty. return MakeParseError(PARSE_ERROR_NOT_ENOUGH_DATA); } - http_imsg = new (std::nothrow) HttpContext(socket->is_read_progressive()); + http_imsg = new (std::nothrow) HttpContext( + socket->is_read_progressive(), + socket->http_request_method()); if (http_imsg == NULL) { LOG(FATAL) << "Fail to new HttpContext"; return MakeParseError(PARSE_ERROR_NO_RESOURCE); diff --git a/src/brpc/policy/http_rpc_protocol.h b/src/brpc/policy/http_rpc_protocol.h index d7337221..05c037b1 100644 --- a/src/brpc/policy/http_rpc_protocol.h +++ b/src/brpc/policy/http_rpc_protocol.h @@ -85,9 +85,10 @@ class HttpContext : public ReadableProgressiveAttachment , public InputMessageBase , public HttpMessage { public: - HttpContext(bool read_body_progressively) + explicit HttpContext(bool read_body_progressively, + HttpMethod request_method = HTTP_METHOD_GET) : InputMessageBase() - , HttpMessage(read_body_progressively) + , HttpMessage(read_body_progressively, request_method) , _is_stage2(false) { // add one ref for Destroy butil::intrusive_ptr<HttpContext>(this).detach(); @@ -106,12 +107,12 @@ public: bool is_stage2() const { return _is_stage2; } // @InputMessageBase - void DestroyImpl() { + void DestroyImpl() override { RemoveOneRefForStage2(); } // @ReadableProgressiveAttachment - void ReadProgressiveAttachmentBy(ProgressiveReader* r) { + void ReadProgressiveAttachmentBy(ProgressiveReader* r) override { return SetBodyReader(r); } diff --git a/src/brpc/socket.cpp b/src/brpc/socket.cpp index a634dd37..da1c8e5a 100644 --- a/src/brpc/socket.cpp +++ b/src/brpc/socket.cpp @@ -465,6 +465,7 @@ Socket::Socket(Forbidden) , _stream_set(NULL) , _total_streams_unconsumed_size(0) , _ninflight_app_health_check(0) + , _http_request_method(HTTP_METHOD_GET) { CreateVarsOnce(); pthread_mutex_init(&_id_wait_list_mutex, NULL); diff --git a/src/brpc/socket.h b/src/brpc/socket.h index 74f7360e..0e43122c 100644 --- a/src/brpc/socket.h +++ b/src/brpc/socket.h @@ -38,6 +38,7 @@ #include "brpc/socket_id.h" // SocketId #include "brpc/socket_message.h" // SocketMessagePtr #include "bvar/bvar.h" +#include "http_method.h" namespace brpc { namespace policy { @@ -577,6 +578,9 @@ public: bthread_keytable_pool_t* keytable_pool() const { return _keytable_pool; } + void set_http_request_method(const HttpMethod& method) { _http_request_method = method; } + HttpMethod http_request_method() const { return _http_request_method; } + private: DISALLOW_COPY_AND_ASSIGN(Socket); @@ -915,6 +919,8 @@ private: // Refer to `SocketKeepaliveOptions' for details. // non-NULL means that keepalive is on. std::shared_ptr<SocketKeepaliveOptions> _keepalive_options; + + HttpMethod _http_request_method; }; } // namespace brpc diff --git a/test/brpc_http_message_unittest.cpp b/test/brpc_http_message_unittest.cpp index 7eccf7cd..57fd4b1e 100644 --- a/test/brpc_http_message_unittest.cpp +++ b/test/brpc_http_message_unittest.cpp @@ -226,6 +226,39 @@ TEST(HttpMessageTest, parse_from_iobuf) { ASSERT_EQ("text/plain", http_message.header().content_type()); } + +TEST(HttpMessageTest, parse_http_head_response) { + char response1[1024] = "HTTP/1.1 200 OK\r\n" + "Content-Type: text/plain\r\n" + "Content-Length: 1024\r\n" + "\r\n"; + butil::IOBuf request; + request.append(response1); + + brpc::HttpMessage http_message(false, brpc::HTTP_METHOD_HEAD); + ASSERT_TRUE(http_message.ParseFromIOBuf(request) >= 0); + ASSERT_TRUE(http_message.Completed()) << http_message.stage(); + ASSERT_EQ("text/plain", http_message.header().content_type()); + const std::string* content_length = http_message.header().GetHeader("Content-Length"); + ASSERT_NE(nullptr, content_length); + ASSERT_EQ("1024", *content_length); + + + char response2[1024] = "HTTP/1.1 200 OK\r\n" + "Content-Type: text/plain\r\n" + "Transfer-Encoding: chunked\r\n" + "\r\n"; + butil::IOBuf request2; + request2.append(response2); + brpc::HttpMessage http_message2(false, brpc::HTTP_METHOD_HEAD); + ASSERT_TRUE(http_message2.ParseFromIOBuf(request2) >= 0); + ASSERT_TRUE(http_message2.Completed()) << http_message2.stage(); + ASSERT_EQ("text/plain", http_message2.header().content_type()); + const std::string* transfer_encoding = http_message2.header().GetHeader("Transfer-Encoding"); + ASSERT_NE(nullptr, transfer_encoding); + ASSERT_EQ("chunked", *transfer_encoding); +} + TEST(HttpMessageTest, find_method_property_by_uri) { brpc::Server server; ASSERT_EQ(0, server.AddService(new test::EchoService(), @@ -393,15 +426,15 @@ TEST(HttpMessageTest, serialize_http_response) { // content is cleared. CHECK(content.empty()); + // null content + header.SetHeader("Content-Length", "100"); + MakeRawHttpResponse(&response, &header, NULL); + ASSERT_EQ("HTTP/1.1 200 OK\r\nContent-Length: 100\r\nFoo: Bar\r\n\r\n", response) << butil::ToPrintable(response); + // user-set content-length is ignored. content.append("data2"); - header.SetHeader("Content-Length", "100"); MakeRawHttpResponse(&response, &header, &content); ASSERT_EQ("HTTP/1.1 200 OK\r\nContent-Length: 5\r\nFoo: Bar\r\n\r\ndata2", response); - - // null content - MakeRawHttpResponse(&response, &header, NULL); - ASSERT_EQ("HTTP/1.1 200 OK\r\nFoo: Bar\r\n\r\n", response); } } //namespace diff --git a/test/brpc_http_rpc_protocol_unittest.cpp b/test/brpc_http_rpc_protocol_unittest.cpp index 0698651e..988b3b31 100644 --- a/test/brpc_http_rpc_protocol_unittest.cpp +++ b/test/brpc_http_rpc_protocol_unittest.cpp @@ -23,18 +23,15 @@ #include <google/protobuf/stubs/logging.h> #include <string> #include <sys/ioctl.h> -#include <sys/types.h> #include <sys/socket.h> #include <gtest/gtest.h> #include <gflags/gflags.h> -#include <google/protobuf/descriptor.h> #include <google/protobuf/text_format.h> #include <unistd.h> +#include <butil/strings/string_number_conversions.h> #include "brpc/http_method.h" #include "butil/iobuf.h" #include "butil/logging.h" -#include "butil/time.h" -#include "butil/macros.h" #include "butil/files/scoped_file.h" #include "butil/fd_guard.h" #include "butil/file_util.h" @@ -43,7 +40,6 @@ #include "brpc/server.h" #include "brpc/channel.h" #include "brpc/policy/most_common_message.h" -#include "brpc/controller.h" #include "echo.pb.h" #include "brpc/policy/http_rpc_protocol.h" #include "brpc/policy/http2_rpc_protocol.h" @@ -51,7 +47,6 @@ #include "json2pb/json_to_pb.h" #include "brpc/details/method_status.h" #include "brpc/rpc_dump.h" -#include "bvar/collector.h" namespace brpc { DECLARE_bool(rpc_dump); @@ -78,6 +73,8 @@ namespace { static const std::string EXP_REQUEST = "hello"; static const std::string EXP_RESPONSE = "world"; +static const std::string EXP_RESPONSE_CONTENT_LENGTH = "1024"; +static const std::string EXP_RESPONSE_TRANSFER_ENCODING = "chunked"; static const std::string MOCK_CREDENTIAL = "mock credential"; static const std::string MOCK_USER = "mock user"; @@ -1781,4 +1778,62 @@ TEST_F(HttpTest, spring_protobuf_text_content_type) { ASSERT_EQ(EXP_RESPONSE, res.message()); } +class HttpServiceImpl : public ::test::HttpService { + public: + void Head(::google::protobuf::RpcController* cntl_base, + const ::test::HttpRequest*, + ::test::HttpResponse*, + ::google::protobuf::Closure* done) override { + brpc::ClosureGuard done_guard(done); + brpc::Controller* cntl = + static_cast<brpc::Controller*>(cntl_base); + ASSERT_EQ(cntl->http_request().method(), brpc::HTTP_METHOD_HEAD); + const std::string* index = cntl->http_request().GetHeader("x-db-index"); + ASSERT_NE(nullptr, index); + int i; + ASSERT_TRUE(butil::StringToInt(*index, &i)); + cntl->http_response().set_content_type("text/plain"); + if (i % 2 == 0) { + cntl->http_response().SetHeader("Content-Length", + EXP_RESPONSE_CONTENT_LENGTH); + } else { + cntl->http_response().SetHeader("Transfer-Encoding", + EXP_RESPONSE_TRANSFER_ENCODING); + } + } +}; + +TEST_F(HttpTest, http_head) { + const int port = 8923; + brpc::Server server; + HttpServiceImpl svc; + EXPECT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE)); + EXPECT_EQ(0, server.Start(port, NULL)); + + brpc::Channel channel; + brpc::ChannelOptions options; + options.protocol = brpc::PROTOCOL_HTTP; + ASSERT_EQ(0, channel.Init(butil::EndPoint(butil::my_ip(), port), &options)); + for (int i = 0; i < 100; ++i) { + brpc::Controller cntl; + cntl.http_request().set_method(brpc::HTTP_METHOD_HEAD); + cntl.http_request().uri().set_path("/HttpService/Head"); + cntl.http_request().SetHeader("x-db-index", butil::IntToString(i)); + channel.CallMethod(NULL, &cntl, NULL, NULL, NULL); + + ASSERT_FALSE(cntl.Failed()) << cntl.ErrorText(); + if (i % 2 == 0) { + const std::string* content_length + = cntl.http_response().GetHeader("content-length"); + ASSERT_NE(nullptr, content_length); + ASSERT_EQ(EXP_RESPONSE_CONTENT_LENGTH, *content_length); + } else { + const std::string* transfer_encoding + = cntl.http_response().GetHeader("Transfer-Encoding"); + ASSERT_NE(nullptr, transfer_encoding); + ASSERT_EQ(EXP_RESPONSE_TRANSFER_ENCODING, *transfer_encoding); + } + } +} + } //namespace diff --git a/test/echo.proto b/test/echo.proto index a027516e..6de4db6c 100644 --- a/test/echo.proto +++ b/test/echo.proto @@ -88,6 +88,10 @@ service NacosNamingService { rpc List(HttpRequest) returns (HttpResponse); }; +service HttpService { + rpc Head(HttpRequest) returns (HttpResponse); +} + enum State0 { STATE0_NUM_0 = 0; STATE0_NUM_1 = 1; --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For additional commands, e-mail: dev-h...@brpc.apache.org