This is an automated email from the ASF dual-hosted git repository.
chenBright 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 fb9413c3 Reject HTTP/1 header forms that desync framing, drop chunked
trailers (#3521)
fb9413c3 is described below
commit fb9413c310cdab2396fb1ee562dea3c412d18451
Author: Bright Chen <[email protected]>
AuthorDate: Sat Sep 5 16:02:31 2026 +0800
Reject HTTP/1 header forms that desync framing, drop chunked trailers
(#3521)
---
src/brpc/details/http_message.cpp | 22 ++++
src/brpc/details/http_parser.cpp | 112 ++++++++++++++++--
src/brpc/details/http_parser.h | 6 +
test/brpc_http_message_unittest.cpp | 219 +++++++++++++++++++++++++++++++++++-
4 files changed, 348 insertions(+), 11 deletions(-)
diff --git a/src/brpc/details/http_message.cpp
b/src/brpc/details/http_message.cpp
index 45f77bab..0cb4f783 100644
--- a/src/brpc/details/http_message.cpp
+++ b/src/brpc/details/http_message.cpp
@@ -33,6 +33,13 @@ namespace brpc {
DEFINE_bool(allow_chunked_length, false,
"Allow both Transfer-Encoding and Content-Length headers are
present.");
+DEFINE_bool(http_allow_obs_fold, false,
+ "Allow obsolete line folding (RFC 7230 3.2.4) of Content-Length "
+ "and Transfer-Encoding, which front proxies may parse
differently.");
+DEFINE_bool(http_strict_header_token, false,
+ "Reject a space inside any header field name (RFC 7230 3.2.6), "
+ "not just inside the names that decide framing. Names carrying no "
+ "framing semantics are accepted by default for compatibility.");
DEFINE_bool(allow_http_1_1_request_without_host, true,
"Allow HTTP/1.1 request without host which violates the HTTP/1.1
specification.");
DEFINE_bool(http_verbose, false,
@@ -83,6 +90,15 @@ int HttpMessage::on_status(http_parser *parser, const char
*, const size_t) {
// change the order of these field values when a message is forwarded.
int HttpMessage::on_header_field(http_parser *parser,
const char *at, const size_t length) {
+ if (parser->flags & F_TRAILING) {
+ // Drop chunked trailers instead of merging them into the header map.
+ // A front-end that strips or rewrites headers (Authorization,
+ // X-Forwarded-For, ...) only looks at the header section, so anything
+ // accepted here would be a header smuggled past it. HTTP/1 trailers
+ // have no reader API in bRPC anyway. Supporting them later should add
+ // a separate map rather than reuse this one.
+ return 0;
+ }
HttpMessage *http_message = (HttpMessage *)parser->data;
if (http_message->_stage != HTTP_ON_HEADER_FIELD) {
http_message->_stage = HTTP_ON_HEADER_FIELD;
@@ -94,6 +110,10 @@ int HttpMessage::on_header_field(http_parser *parser,
int HttpMessage::on_header_value(http_parser *parser,
const char *at, const size_t length) {
+ if (parser->flags & F_TRAILING) {
+ // See the comment in on_header_field().
+ return 0;
+ }
HttpMessage *http_message = (HttpMessage *)parser->data;
bool first_entry = false;
if (http_message->_stage != HTTP_ON_HEADER_VALUE) {
@@ -454,6 +474,8 @@ HttpMessage::HttpMessage(bool read_body_progressively,
, _read_body_progressively(read_body_progressively) {
http_parser_init(&_parser, HTTP_BOTH);
_parser.allow_chunked_length = 1;
+ _parser.allow_obs_fold = FLAGS_http_allow_obs_fold;
+ _parser.strict_header_token = FLAGS_http_strict_header_token;
_parser.data = this;
}
diff --git a/src/brpc/details/http_parser.cpp b/src/brpc/details/http_parser.cpp
index 2c68cffc..e983683d 100644
--- a/src/brpc/details/http_parser.cpp
+++ b/src/brpc/details/http_parser.cpp
@@ -407,6 +407,8 @@ enum header_states
, h_connection
, h_content_length
+ , h_content_length_num
+ , h_content_length_ws
, h_transfer_encoding
, h_upgrade
@@ -435,9 +437,13 @@ const char* http_parser_header_state_name(unsigned int
header_state) {
case h_matching_upgrade: return "h_matching_upgrade";
case h_connection: return "h_connection";
case h_content_length: return "h_content_length";
+ case h_content_length_num: return "h_content_length_num";
+ case h_content_length_ws: return "h_content_length_ws";
case h_transfer_encoding: return "h_transfer_encoding";
case h_upgrade: return "h_upgrade";
+ case h_matching_transfer_encoding_token_start: return
"h_matching_transfer_encoding_token_start";
case h_matching_transfer_encoding_chunked: return
"h_matching_transfer_encoding_chunked";
+ case h_matching_transfer_encoding_token: return
"h_matching_transfer_encoding_token";
case h_matching_connection_keep_alive: return
"h_matching_connection_keep_alive";
case h_matching_connection_close: return "h_matching_connection_close";
case h_transfer_encoding_chunked: return "h_transfer_encoding_chunked";
@@ -447,6 +453,26 @@ const char* http_parser_header_state_name(unsigned int
header_state) {
return "h_unknown";
}
+/* States in which the value being parsed decides where the body ends. Folding
+ * such a value (RFC 7230 3.2.4 obs-fold) is never legitimate and makes us
+ * disagree with front proxies on the framing, which is a smuggling primitive.
+ */
+static int is_framing_header_state(unsigned int header_state) {
+ switch (header_state) {
+ case h_content_length:
+ case h_content_length_num:
+ case h_content_length_ws:
+ case h_transfer_encoding:
+ case h_matching_transfer_encoding_token_start:
+ case h_matching_transfer_encoding_chunked:
+ case h_matching_transfer_encoding_token:
+ case h_transfer_encoding_chunked:
+ return 1;
+ default:
+ return 0;
+ }
+}
+
const char* http_parser_type_name(enum http_parser_type type) {
switch (type) {
case HTTP_REQUEST: return "HTTP_REQUEST";
@@ -477,6 +503,10 @@ enum http_host_state
#define IS_ALPHA(c) (LOWER(c) >= 'a' && LOWER(c) <= 'z')
#define IS_NUM(c) ((c) >= '0' && (c) <= '9')
#define IS_ALPHANUM(c) (IS_ALPHA(c) || IS_NUM(c))
+/* RFC 7230 3.2.3: OWS = *( SP / HTAB ). Pass the raw byte, never the LOWER()ed
+ * one: LOWER('\t') is ')', so a tab never compares equal to '\t' afterwards.
+ */
+#define IS_OWS(ch) ((ch) == ' ' || (ch) == '\t')
#define IS_HEX(c) (IS_NUM(c) || (LOWER(c) >= 'a' && LOWER(c) <= 'f'))
#define IS_MARK(c) ((c) == '-' || (c) == '_' || (c) == '.' || \
(c) == '!' || (c) == '~' || (c) == '*' || (c) == '\'' || (c) == '(' || \
@@ -489,6 +519,12 @@ enum http_host_state
* characeters seem to be OK.
*/
#define TOKEN(c) ((c == ' ') ? ' ' : tokens[(unsigned char)c])
+/* tokens[] already leaves SP out, as RFC 7230 3.2.6 does, so the strict form
is
+ * the plain lookup and TOKEN() is the leniency layered on top of it. The name
+ * matches upstream, where the table keeps SP and STRICT_TOKEN() is what drops
+ * it, so both forks spell the same two behaviours the same way.
+ */
+#define STRICT_TOKEN(c) (tokens[(unsigned char)c])
#define IS_HOST_CHAR(c) \
(IS_ALPHANUM(c) || (c) == '.' || (c) == '-' || (c) == '_')
/* NOTE(gejun): Enable utf8 which is compatible with ASCII. Services in baidu
@@ -702,6 +738,8 @@ size_t http_parser_execute (http_parser *parser,
const char *status_mark = 0;
const unsigned int lenient = parser->lenient_http_headers;
const unsigned int allow_chunked_length = parser->allow_chunked_length;
+ const unsigned int allow_obs_fold = parser->allow_obs_fold;
+ const unsigned int strict_header_token = parser->strict_header_token;
/* We're in an error state. Don't bother doing anything. */
if (HTTP_PARSER_ERRNO(parser) != HPE_OK) {
@@ -1369,7 +1407,7 @@ size_t http_parser_execute (http_parser *parser,
goto reexecute_byte;
}
- c = TOKEN(ch);
+ c = strict_header_token ? STRICT_TOKEN(ch) : TOKEN(ch);
if (!c) {
SET_ERRNO(HPE_INVALID_HEADER_TOKEN);
@@ -1407,7 +1445,7 @@ size_t http_parser_execute (http_parser *parser,
case s_header_field:
{
- c = TOKEN(ch);
+ c = strict_header_token ? STRICT_TOKEN(ch) : TOKEN(ch);
if (c) {
switch (parser->header_state) {
@@ -1504,7 +1542,19 @@ size_t http_parser_execute (http_parser *parser,
case h_content_length:
case h_transfer_encoding:
case h_upgrade:
- if (ch != ' ') parser->header_state = h_general;
+ /* TOKEN() accepts ' ' (see the NOTE above it), so without this
+ * `Content-Length : 5` would keep header_state and be used for
+ * framing while the field name we report is "Content-Length ".
+ * RFC 7230 3.2.4 forbids whitespace before the colon; front
+ * proxies that follow it disagree with us, which is a smuggling
+ * primitive. Only these four names are strict, lenient names
+ * elsewhere stay accepted.
+ */
+ if (ch == ' ') {
+ SET_ERRNO(HPE_INVALID_HEADER_TOKEN);
+ goto error;
+ }
+ parser->header_state = h_general;
break;
default:
@@ -1580,6 +1630,12 @@ size_t http_parser_execute (http_parser *parser,
parser->flags |= F_CONTENTLENGTH;
parser->content_length = ch - '0';
+ parser->header_state = h_content_length_num;
+ break;
+
+ /* when obsolete line folding is encountered for content length
+ * continue to the s_header_value state */
+ case h_content_length_ws:
break;
case h_connection:
@@ -1633,10 +1689,18 @@ size_t http_parser_execute (http_parser *parser,
break;
case h_content_length:
+ if (IS_OWS(ch)) break;
+ parser->header_state = h_content_length_num;
+ /* FALLTHROUGH */
+
+ case h_content_length_num:
{
uint64_t t;
- if (ch == ' ') break;
+ if (IS_OWS(ch)) {
+ parser->header_state = h_content_length_ws;
+ break;
+ }
if (!IS_NUM(ch)) {
SET_ERRNO(HPE_INVALID_CONTENT_LENGTH);
@@ -1657,14 +1721,25 @@ size_t http_parser_execute (http_parser *parser,
break;
}
+ /* Trailing OWS ends the number. A digit after it means the value was
+ * something like "1 3", which we used to read as 13 while proxies
may
+ * read 1 or reject the message. cf. CVE-2022-32213.
+ */
+ case h_content_length_ws:
+ if (IS_OWS(ch)) break;
+ SET_ERRNO(HPE_INVALID_CONTENT_LENGTH);
+ goto error;
+
/* Transfer-Encoding: chunked */
case h_matching_transfer_encoding_token_start:
/* looking for 'Transfer-Encoding: chunked' */
if ('c' == c) {
parser->header_state = h_matching_transfer_encoding_chunked;
- } else if (TOKEN(c)) {
- /* NOTE(gejun): Not use strict mode for these macros since the
additional
- * characeters seem to be OK.
+ } else if (STRICT_TOKEN(c)) {
+ /* TOKEN() here made the OWS arm below unreachable for SP, so the
+ * space in `Transfer-Encoding: gzip, chunked` started a token
+ * instead of being skipped and chunked was never matched. Use
the
+ * strict form unconditionally, as upstream does.
*/
/* TODO(indutny): similar code below does this, but why?
@@ -1673,7 +1748,7 @@ size_t http_parser_execute (http_parser *parser,
* `STRICT_TOKEN`
*/
parser->header_state = h_matching_transfer_encoding_token;
- } else if (c == ' ' || c == '\t') {
+ } else if (IS_OWS(ch)) {
/* Skip lws */
} else {
parser->header_state = h_general;
@@ -1720,12 +1795,12 @@ size_t http_parser_execute (http_parser *parser,
break;
case h_transfer_encoding_chunked:
- if (ch != ' ') parser->header_state =
h_matching_transfer_encoding_token;
+ if (!IS_OWS(ch)) parser->header_state =
h_matching_transfer_encoding_token;
break;
case h_connection_keep_alive:
case h_connection_close:
- if (ch != ' ') parser->header_state = h_general;
+ if (!IS_OWS(ch)) parser->header_state = h_general;
break;
default:
@@ -1747,6 +1822,14 @@ size_t http_parser_execute (http_parser *parser,
case s_header_value_lws:
{
if (ch == ' ' || ch == '\t') {
+ if (!allow_obs_fold &&
is_framing_header_state(parser->header_state)) {
+ SET_ERRNO(HPE_INVALID_HEADER_TOKEN);
+ goto error;
+ }
+ if (parser->header_state == h_content_length_num) {
+ /* treat obsolete line folding as space */
+ parser->header_state = h_content_length_ws;
+ }
parser->state = s_header_value_start;
goto reexecute_byte;
}
@@ -1780,6 +1863,15 @@ size_t http_parser_execute (http_parser *parser,
case s_header_value_discard_lws:
{
if (ch == ' ' || ch == '\t') {
+ /* The value is still empty, so `Transfer-Encoding:\r\n chunked` and
+ * `Content-Length:\r\n 5` come through here rather than through
+ * s_header_value_lws. Both states keep header_state across the fold,
+ * so both have to refuse it.
+ */
+ if (!allow_obs_fold &&
is_framing_header_state(parser->header_state)) {
+ SET_ERRNO(HPE_INVALID_HEADER_TOKEN);
+ goto error;
+ }
parser->state = s_header_value_discard_ws;
break;
} else {
diff --git a/src/brpc/details/http_parser.h b/src/brpc/details/http_parser.h
index 0ea4efd0..b09c912b 100644
--- a/src/brpc/details/http_parser.h
+++ b/src/brpc/details/http_parser.h
@@ -216,6 +216,12 @@ struct http_parser {
* `Content-Length` and
* `Transfer-Encoding: chunked` set */
unsigned int lenient_http_headers : 1;
+ unsigned int allow_obs_fold : 1; /* Allow obsolete line folding (RFC 7230
+ * 3.2.4) of `Content-Length` and
+ * `Transfer-Encoding` */
+ unsigned int strict_header_token : 1; /* Reject SP inside any header field
+ * name, not just the ones that decide
+ * framing (RFC 7230 3.2.6) */
uint32_t nread; /* # bytes read in various scenarios */
uint64_t content_length; /* # bytes in body. `(uint64_t) -1` (all bits one)
diff --git a/test/brpc_http_message_unittest.cpp
b/test/brpc_http_message_unittest.cpp
index 951c9108..57e98cca 100644
--- a/test/brpc_http_message_unittest.cpp
+++ b/test/brpc_http_message_unittest.cpp
@@ -18,6 +18,7 @@
// Date 2014/10/24 16:44:30
#include <gtest/gtest.h>
+#include <gflags/gflags.h>
#include <google/protobuf/descriptor.h>
#include "brpc/server.h"
@@ -29,6 +30,8 @@ namespace brpc {
DECLARE_bool(allow_chunked_length);
DECLARE_bool(allow_http_1_1_request_without_host);
+DECLARE_bool(http_allow_obs_fold);
+DECLARE_bool(http_strict_header_token);
int main(int argc, char* argv[]) {
testing::InitGoogleTest(&argc, argv);
@@ -340,6 +343,8 @@ TEST(HttpMessageTest, parse_http_set_cookie) {
}
TEST(HttpMessageTest, cl_and_te) {
+ GFLAGS_NAMESPACE::FlagSaver flag_saver;
+
// https://datatracker.ietf.org/doc/html/rfc2616#section-14.41
// If multiple encodings have been applied to an entity, the transfer-
// codings MUST be listed in the order in which they were applied.
@@ -426,6 +431,218 @@ TEST(HttpMessageTest, cl_and_te) {
}
}
+brpc::http_errno ParseHttpErrno(const char* buf) {
+ butil::IOBuf data;
+ data.append(buf);
+ brpc::HttpMessage http_message;
+ if (http_message.ParseFromIOBuf(data) == (ssize_t)data.size() &&
+ http_message.Completed()) {
+ return brpc::HPE_OK;
+ }
+ brpc::http_errno err = (brpc::http_errno)http_message._parser.http_errno;
+ return err != brpc::HPE_OK ? err : brpc::HPE_UNKNOWN;
+}
+
+TEST(HttpMessageTest, space_before_colon_of_framing_headers) {
+ GFLAGS_NAMESPACE::FlagSaver flag_saver;
+ brpc::FLAGS_http_strict_header_token = false;
+
+ const char* rejected[] = {
+ "POST / HTTP/1.1\r\nHost: a.com\r\nContent-Length : 5\r\n\r\nhello",
+ "POST / HTTP/1.1\r\nHost: a.com\r\nTransfer-Encoding :
chunked\r\n\r\n0\r\n\r\n",
+ "GET / HTTP/1.1\r\nHost: a.com\r\nConnection : close\r\n\r\n",
+ "GET / HTTP/1.1\r\nHost: a.com\r\nUpgrade : h2c\r\n\r\n",
+ };
+ for (size_t i = 0; i < arraysize(rejected); ++i) {
+ ASSERT_EQ(brpc::HPE_INVALID_HEADER_TOKEN, ParseHttpErrno(rejected[i]))
+ << rejected[i];
+ }
+
+ // Names without framing semantics keep the historical leniency.
+ ASSERT_EQ(brpc::HPE_OK, ParseHttpErrno(
+ "POST / HTTP/1.1\r\nHost: a.com\r\nX-Foo : bar\r\n"
+ "Content-Length: 5\r\n\r\nhello"));
+}
+
+TEST(HttpMessageTest, strict_header_token) {
+ GFLAGS_NAMESPACE::FlagSaver flag_saver;
+ brpc::FLAGS_http_strict_header_token = false;
+
+ const char* lenient_only[] = {
+ "POST / HTTP/1.1\r\nHost: a.com\r\nX-Foo : bar\r\n"
+ "Content-Length: 5\r\n\r\nhello",
+ // A space anywhere in the name, not just before the colon.
+ "GET / HTTP/1.1\r\nHost: a.com\r\nX Foo: bar\r\n\r\n",
+ // Names that only look like a framing one until they diverge.
+ "GET / HTTP/1.1\r\nHost: a.com\r\nContent-Type : text/plain\r\n\r\n",
+ };
+ for (size_t i = 0; i < arraysize(lenient_only); ++i) {
+ ASSERT_EQ(brpc::HPE_OK, ParseHttpErrno(lenient_only[i]))
+ << lenient_only[i];
+ }
+
+ brpc::FLAGS_http_strict_header_token = true;
+ for (size_t i = 0; i < arraysize(lenient_only); ++i) {
+ ASSERT_EQ(brpc::HPE_INVALID_HEADER_TOKEN,
+ ParseHttpErrno(lenient_only[i])) << lenient_only[i];
+ }
+ // Well-formed names are unaffected.
+ ASSERT_EQ(brpc::HPE_OK, ParseHttpErrno("POST / HTTP/1.1\r\n"
+ "Host: a.com\r\nX-Foo: bar\r\n"
+ "Content-Length: 5\r\n\r\n"
+ "hello"));
+}
+
+TEST(HttpMessageTest, transfer_encoding_list_with_space_after_comma) {
+ butil::IOBuf response;
+ response.append("HTTP/1.1 200 OK\r\n"
+ "Transfer-Encoding: gzip, chunked\r\n\r\n"
+ "5\r\nhello\r\n0\r\n\r\n");
+ brpc::HttpMessage http_message;
+ ASSERT_EQ((ssize_t)response.size(), http_message.ParseFromIOBuf(response))
+ << http_message._parser;
+ ASSERT_TRUE(http_message.Completed());
+ ASSERT_TRUE(http_message._parser.flags & brpc::F_CHUNKED)
+ << http_message._parser;
+ ASSERT_EQ("hello", http_message.body().to_string());
+
+ // A request would have been rejected outright by RFC 7230 3.3.3 before,
+ // because uses_transfer_encoding was set while F_CHUNKED was not.
+ ASSERT_EQ(brpc::HPE_OK, ParseHttpErrno("POST / HTTP/1.1\r\n"
+ "Host: a.com\r\n"
+ "Transfer-Encoding: gzip,
chunked\r\n\r\n"
+ "0\r\n\r\n"));
+}
+
+TEST(HttpMessageTest, content_length_with_interior_space) {
+ ASSERT_EQ(brpc::HPE_INVALID_CONTENT_LENGTH, ParseHttpErrno("POST /
HTTP/1.1\r\n"
+ "Host:
a.com\r\n"
+
"Content-Length: 1 3\r\n\r\n"
+
"0123456789abc"));
+
+ // Trailing OWS is legal and does not change the value.
+ butil::IOBuf data;
+ data.append("POST / HTTP/1.1\r\nHost: a.com\r\nContent-Length: 13
\r\n\r\n0123456789abc");
+ brpc::HttpMessage http_message;
+ ASSERT_EQ((ssize_t)data.size(), http_message.ParseFromIOBuf(data))
+ << http_message._parser;
+ ASSERT_TRUE(http_message.Completed());
+ ASSERT_EQ("0123456789abc", http_message.body().to_string());
+}
+
+TEST(HttpMessageTest, obs_fold_of_framing_headers) {
+ GFLAGS_NAMESPACE::FlagSaver flag_saver;
+ brpc::FLAGS_http_allow_obs_fold = false;
+
+ const char* folded_te_value = "POST / HTTP/1.1\r\n"
+ "Host: a.com\r\n"
+ "Transfer-Encoding:\r\n chunked\r\n\r\n"
+ "0\r\n\r\n";
+ // Folding inside the value restarts the chunked matcher, so this one never
+ // reached F_CHUNKED, but we still report "chun ked" where a proxy that
+ // unfolds reports "chunked".
+ const char* folded_te_token = "GET / HTTP/1.1\r\n"
+ "Host: a.com\r\n"
+ "Transfer-Encoding: chun\r\n ked\r\n\r\n";
+ const char* folded_cl_value = "POST / HTTP/1.1\r\n"
+ "Host: a.com\r\n"
+ "Content-Length:\r\n 13\r\n\r\n"
+ "0123456789abc";
+ const char* folded_cl_digits = "POST / HTTP/1.1\r\n"
+ "Host: a.com\r\n"
+ "Content-Length: 1\r\n 3\r\n\r\n"
+ "0123456789abc";
+
+ ASSERT_EQ(brpc::HPE_INVALID_HEADER_TOKEN, ParseHttpErrno(folded_te_value));
+ ASSERT_EQ(brpc::HPE_INVALID_HEADER_TOKEN, ParseHttpErrno(folded_te_token));
+ ASSERT_EQ(brpc::HPE_INVALID_HEADER_TOKEN, ParseHttpErrno(folded_cl_value));
+ ASSERT_EQ(brpc::HPE_INVALID_HEADER_TOKEN,
ParseHttpErrno(folded_cl_digits));
+
+ // Folding a header that decides nothing about framing is still accepted.
+ ASSERT_EQ(brpc::HPE_OK, ParseHttpErrno(
+ "POST / HTTP/1.1\r\nHost: a.com\r\nX-Foo: a\r\n b\r\n"
+ "Content-Length: 5\r\n\r\nhello"));
+
+ brpc::FLAGS_http_allow_obs_fold = true;
+ ASSERT_EQ(brpc::HPE_OK, ParseHttpErrno(folded_te_value));
+ ASSERT_EQ(brpc::HPE_OK, ParseHttpErrno(folded_cl_value));
+ // Unfolding turns this into `Content-Length: 1 3`, which stays invalid.
+ ASSERT_EQ(brpc::HPE_INVALID_CONTENT_LENGTH,
+ ParseHttpErrno(folded_cl_digits));
+ // Unfolding leaves a Transfer-Encoding that is not chunked, which RFC 7230
+ // 3.3.3 makes unparseable in a request whatever the fold policy is.
+ ASSERT_EQ(brpc::HPE_INVALID_TRANSFER_ENCODING,
+ ParseHttpErrno(folded_te_token));
+}
+
+TEST(HttpMessageTest, chunked_trailer_is_not_a_header) {
+ butil::IOBuf request;
+ request.append("POST / HTTP/1.1\r\n"
+ "Host: a.com\r\n"
+ "X-Forwarded-For: 10.0.0.1\r\n"
+ "Transfer-Encoding: chunked\r\n\r\n"
+ "5\r\nhello\r\n"
+ "0\r\n"
+ "X-Injected: evil\r\n"
+ "Authorization: Bearer stolen\r\n"
+ "X-Forwarded-For: 6.6.6.6\r\n"
+ "\r\n");
+ brpc::HttpMessage http_message;
+ ASSERT_EQ((ssize_t)request.size(), http_message.ParseFromIOBuf(request))
+ << http_message._parser;
+ ASSERT_TRUE(http_message.Completed());
+ // The body is unaffected by dropping the trailer.
+ ASSERT_EQ("hello", http_message.body().to_string());
+
+ brpc::HttpHeader& header = http_message.header();
+ ASSERT_EQ(nullptr, header.GetHeader("X-Injected"));
+ ASSERT_EQ(nullptr, header.GetHeader("Authorization"));
+ // A trailer repeating a real header must not be appended to it either,
+ // which is where a comma-folded name like X-Forwarded-For would land.
+ const std::string* xff = header.GetHeader("X-Forwarded-For");
+ ASSERT_TRUE(xff != nullptr);
+ ASSERT_EQ("10.0.0.1", *xff);
+}
+
+TEST(HttpMessageTest, htab_is_ows_in_header_values) {
+ // Trailing OWS after the number, alone and mixed with SP.
+ ASSERT_EQ(brpc::HPE_OK, ParseHttpErrno("POST / HTTP/1.1\r\n"
+ "Host: a.com\r\n"
+ "Content-Length: 13\t\r\n\r\n"
+ "0123456789abc"));
+ ASSERT_EQ(brpc::HPE_OK, ParseHttpErrno("POST / HTTP/1.1\r\n"
+ "Host: a.com\r\n"
+ "Content-Length: 13 \t\r\n\r\n"
+ "0123456789abc"));
+ // A tab between digits ends the number just like a space, so the value
+ // stays invalid rather than reading as 13.
+ ASSERT_EQ(brpc::HPE_INVALID_CONTENT_LENGTH,
+ ParseHttpErrno("POST / HTTP/1.1\r\n"
+ "Host: a.com\r\n"
+ "Content-Length: 1\t3\r\n\r\n"
+ "0123456789abc"));
+
+ // Tabs around a transfer-coding and after `Connection: close` used to
leave
+ // the matcher in a state that never set the flag. The Connection case
+ // parsed without an error, so only the flag catches it.
+ struct { const char* buf; unsigned int flag; } flagged[] = {
+ {"POST / HTTP/1.1\r\nHost: a.com\r\n"
+ "Transfer-Encoding: gzip,\tchunked\r\n\r\n0\r\n\r\n",
brpc::F_CHUNKED},
+ {"POST / HTTP/1.1\r\nHost: a.com\r\n"
+ "Transfer-Encoding: chunked\t\r\n\r\n0\r\n\r\n", brpc::F_CHUNKED},
+ {"GET / HTTP/1.1\r\nHost: a.com\r\nConnection: close\t\r\n\r\n",
brpc::F_CONNECTION_CLOSE},
+ };
+ for (size_t i = 0; i < arraysize(flagged); ++i) {
+ butil::IOBuf data;
+ data.append(flagged[i].buf);
+ brpc::HttpMessage http_message;
+ ASSERT_EQ((ssize_t)data.size(), http_message.ParseFromIOBuf(data))
+ << http_message._parser;
+ ASSERT_TRUE(http_message._parser.flags & flagged[i].flag)
+ << http_message._parser;
+ }
+}
+
TEST(HttpMessageTest, find_method_property_by_uri) {
brpc::Server server;
ASSERT_EQ(0, server.AddService(new test::EchoService(),
@@ -799,6 +1016,7 @@ TEST(HttpMessageTest,
serialize_content_type_with_crlf_is_not_injected) {
}
TEST(HttpMessageTest, http_1_1_request_without_host) {
+ GFLAGS_NAMESPACE::FlagSaver flag_saver;
brpc::FLAGS_allow_http_1_1_request_without_host = false;
{
butil::IOBuf request;
@@ -830,7 +1048,6 @@ TEST(HttpMessageTest, http_1_1_request_without_host) {
ASSERT_TRUE(http_message.Completed());
ASSERT_EQ("text/plain", http_message.header().content_type());
}
- brpc::FLAGS_allow_http_1_1_request_without_host = true;
}
} //namespace
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
