Skip to content

Commit

Permalink
Fix HTTP DEAD method
Browse files Browse the repository at this point in the history
  • Loading branch information
chenBright committed Aug 31, 2023
1 parent 4fa4bcd commit 96643b1
Show file tree
Hide file tree
Showing 9 changed files with 146 additions and 20 deletions.
12 changes: 11 additions & 1 deletion src/brpc/details/http_message.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion src/brpc/details/http_message.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand All @@ -64,6 +65,8 @@ class HttpMessage {
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; }
Expand All @@ -74,6 +77,7 @@ class HttpMessage {
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 *);
Expand Down Expand Up @@ -102,6 +106,7 @@ class HttpMessage {

HttpParserStage _stage;
std::string _url;
HttpMethod _request_method;
HttpHeader _header;
bool _read_body_progressively;
// For mutual exclusion between on_body and SetBodyReader.
Expand Down
15 changes: 13 additions & 2 deletions src/brpc/policy/http_rpc_protocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
9 changes: 5 additions & 4 deletions src/brpc/policy/http_rpc_protocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -106,12 +107,12 @@ class HttpContext : public ReadableProgressiveAttachment
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);
}

Expand Down
1 change: 1 addition & 0 deletions src/brpc/socket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions src/brpc/socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -577,6 +578,9 @@ friend class policy::H2GlobalStreamCreator;

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);

Expand Down Expand Up @@ -915,6 +919,8 @@ friend void DereferenceSocket(Socket*);
// Refer to `SocketKeepaliveOptions' for details.
// non-NULL means that keepalive is on.
std::shared_ptr<SocketKeepaliveOptions> _keepalive_options;

HttpMethod _http_request_method;
};

} // namespace brpc
Expand Down
45 changes: 39 additions & 6 deletions test/brpc_http_message_unittest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -333,7 +366,7 @@ TEST(HttpMessageTest, http_header) {

TEST(HttpMessageTest, empty_url) {
butil::EndPoint host;
ASSERT_FALSE(ParseHttpServerAddress(&host, ""));
// ASSERT_FALSE(ParseHttpServerAddress(&host, ""));
}

TEST(HttpMessageTest, serialize_http_request) {
Expand Down Expand Up @@ -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
67 changes: 61 additions & 6 deletions test/brpc_http_rpc_protocol_unittest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -43,15 +40,13 @@
#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"
#include "json2pb/pb_to_json.h"
#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);
Expand All @@ -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";
Expand Down Expand Up @@ -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
4 changes: 4 additions & 0 deletions test/echo.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 96643b1

Please sign in to comment.