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 68432abe Opt Content-Length in response to HEAD request (#2469)
68432abe is described below
commit 68432abe28f6cd64fbf7045127e7436c7062f1d0
Author: Bright Chen <[email protected]>
AuthorDate: Wed Jan 17 10:26:13 2024 +0800
Opt Content-Length in response to HEAD request (#2469)
---
src/brpc/details/http_message.cpp | 38 ++++++++++++++++++++++-------------
src/brpc/policy/http_rpc_protocol.cpp | 8 ++------
test/brpc_http_message_unittest.cpp | 38 +++++++++++++++++++++++++++++------
3 files changed, 58 insertions(+), 26 deletions(-)
diff --git a/src/brpc/details/http_message.cpp
b/src/brpc/details/http_message.cpp
index 252dc8cc..2846b37c 100644
--- a/src/brpc/details/http_message.cpp
+++ b/src/brpc/details/http_message.cpp
@@ -135,12 +135,6 @@ int HttpMessage::on_header_value(http_parser *parser,
int HttpMessage::on_headers_complete(http_parser *parser) {
HttpMessage *http_message = (HttpMessage *)parser->data;
http_message->_stage = HTTP_ON_HEADERS_COMPLETE;
- // Move content-type into the member field.
- const std::string* content_type =
http_message->header().GetHeader("content-type");
- if (content_type) {
- http_message->header().set_content_type(*content_type);
- http_message->header().RemoveHeader("content-type");
- }
if (parser->http_major > 1) {
// NOTE: this checking is a MUST because ProcessHttpResponse relies
// on it to cast InputMessageBase* into different types.
@@ -178,7 +172,6 @@ int HttpMessage::on_headers_complete(http_parser *parser) {
}
}
-
// 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 &&
@@ -633,7 +626,8 @@ void MakeRawHttpResponse(butil::IOBuf* response,
<< h->minor_version() << ' ' << h->status_code()
<< ' ' << h->reason_phrase() << BRPC_CRLF;
bool is_invalid_content = h->status_code() < HTTP_STATUS_OK ||
- h->status_code() == HTTP_STATUS_NO_CONTENT;
+ h->status_code() == HTTP_STATUS_NO_CONTENT;
+ bool is_head_req = h->method() == HTTP_METHOD_HEAD;
if (is_invalid_content) {
// https://www.rfc-editor.org/rfc/rfc7230#section-3.3.1
// A server MUST NOT send a Transfer-Encoding header field in any
@@ -645,11 +639,22 @@ void MakeRawHttpResponse(butil::IOBuf* response,
// with a status code of 1xx (Informational) or 204 (No Content).
h->RemoveHeader("Content-Length");
} else if (content) {
- h->RemoveHeader("Content-Length");
- // Never use "Content-Length" set by user.
- // Always set Content-Length since lighttpd requires the header to be
- // set to 0 for empty content.
- os << "Content-Length: " << content->length() << BRPC_CRLF;
+ const std::string* content_length = h->GetHeader("Content-Length");
+ if (is_head_req) {
+ // Prioritize "Content-Length" set by user.
+ // If "Content-Length" is not set, set it to the length of content.
+ if (!content_length) {
+ os << "Content-Length: " << content->length() << BRPC_CRLF;
+ }
+ } else {
+ if (content_length) {
+ h->RemoveHeader("Content-Length");
+ }
+ // Never use "Content-Length" set by user.
+ // Always set Content-Length since lighttpd requires the header to
be
+ // set to 0 for empty content.
+ os << "Content-Length: " << content->length() << BRPC_CRLF;
+ }
}
if (!h->content_type().empty()) {
os << "Content-Type: " << h->content_type()
@@ -661,7 +666,12 @@ void MakeRawHttpResponse(butil::IOBuf* response,
}
os << BRPC_CRLF; // CRLF before content
os.move_to(*response);
- if (!is_invalid_content && content) {
+
+ // 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).
+ if (!is_invalid_content && !is_head_req && content) {
response->append(butil::IOBuf::Movable(*content));
}
}
diff --git a/src/brpc/policy/http_rpc_protocol.cpp
b/src/brpc/policy/http_rpc_protocol.cpp
index 35aa9a1f..1f9fb5a8 100644
--- a/src/brpc/policy/http_rpc_protocol.cpp
+++ b/src/brpc/policy/http_rpc_protocol.cpp
@@ -945,14 +945,10 @@ HttpResponseSender::~HttpResponseSender() {
}
} else {
butil::IOBuf* content = NULL;
- 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) {
+ if (cntl->Failed() || !cntl->has_progressive_writer()) {
content = &cntl->response_attachment();
}
+ res_header->set_method(req_header->method());
butil::IOBuf res_buf;
MakeRawHttpResponse(&res_buf, res_header, content);
if (FLAGS_http_verbose) {
diff --git a/test/brpc_http_message_unittest.cpp
b/test/brpc_http_message_unittest.cpp
index ff51657e..b48fff61 100644
--- a/test/brpc_http_message_unittest.cpp
+++ b/test/brpc_http_message_unittest.cpp
@@ -423,24 +423,50 @@ TEST(HttpMessageTest, serialize_http_response) {
butil::IOBuf content;
content.append("data");
MakeRawHttpResponse(&response, &header, &content);
- ASSERT_EQ("HTTP/1.1 200 OK\r\nContent-Length: 4\r\nFoo: Bar\r\n\r\ndata",
response);
- // content is cleared.
+ ASSERT_EQ("HTTP/1.1 200 OK\r\nContent-Length: 4\r\nFoo: Bar\r\n\r\ndata",
response)
+ << butil::ToPrintable(response);
+ // Content is cleared.
CHECK(content.empty());
- // null content
+ // 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);
+ 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.
+ // User-set content-length is ignored.
content.append("data2");
MakeRawHttpResponse(&response, &header, &content);
- ASSERT_EQ("HTTP/1.1 200 OK\r\nContent-Length: 5\r\nFoo: Bar\r\n\r\ndata2",
response);
+ ASSERT_EQ("HTTP/1.1 200 OK\r\nContent-Length: 5\r\nFoo: Bar\r\n\r\ndata2",
response)
+ << butil::ToPrintable(response);
+ // User-set content-length and transfer-encoding is ignored when status
code is 204 or 1xx.
+ // 204 No Content.
+ header.SetHeader("Content-Length", "100");
header.SetHeader("Transfer-Encoding", "chunked");
header.set_status_code(brpc::HTTP_STATUS_NO_CONTENT);
MakeRawHttpResponse(&response, &header, &content);
ASSERT_EQ("HTTP/1.1 204 No Content\r\nFoo: Bar\r\n\r\n", response);
+ // 101 Continue
+ header.SetHeader("Content-Length", "100");
+ header.SetHeader("Transfer-Encoding", "chunked");
+ header.set_status_code(brpc::HTTP_STATUS_CONTINUE);
+ MakeRawHttpResponse(&response, &header, &content);
+ ASSERT_EQ("HTTP/1.1 100 Continue\r\nFoo: Bar\r\n\r\n", response)
+ << butil::ToPrintable(response);
+
+ // when request method is HEAD:
+ // 1. There isn't user-set content-length, length of content is used.
+ header.set_method(brpc::HTTP_METHOD_HEAD);
+ header.set_status_code(brpc::HTTP_STATUS_OK);content.append("data2");
+ MakeRawHttpResponse(&response, &header, &content);
+ ASSERT_EQ("HTTP/1.1 200 OK\r\nContent-Length: 5\r\nFoo: Bar\r\n\r\n",
response)
+ << butil::ToPrintable(response);
+ // 2. User-set content-length is not ignored .
+ header.SetHeader("Content-Length", "100");
+ MakeRawHttpResponse(&response, &header, &content);
+ ASSERT_EQ("HTTP/1.1 200 OK\r\nContent-Length: 100\r\nFoo: Bar\r\n\r\n",
response)
+ << butil::ToPrintable(response);
}
} //namespace
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
