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 <chenguangmin...@foxmail.com> 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: dev-unsubscr...@brpc.apache.org For additional commands, e-mail: dev-h...@brpc.apache.org