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 <[email protected]>
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: [email protected]
For additional commands, e-mail: [email protected]
