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 023dc4f8 Bound the number of http headers and query parameters per
message (#3524)
023dc4f8 is described below
commit 023dc4f8bda4a888004fe2ed2a17ab9940ab6d44
Author: Bright Chen <[email protected]>
AuthorDate: Tue Sep 15 16:54:34 2026 +0800
Bound the number of http headers and query parameters per message (#3524)
---
src/brpc/details/http_message.cpp | 11 +
src/brpc/policy/http2_rpc_protocol.cpp | 62 ++++-
src/brpc/policy/http2_rpc_protocol.h | 17 +-
src/brpc/socket.cpp | 1 +
src/brpc/uri.cpp | 40 +++-
src/brpc/uri.h | 7 +-
test/brpc_http_message_unittest.cpp | 60 +++++
test/brpc_http_rpc_protocol_unittest.cpp | 393 +++++++++++++++++++++++++------
test/brpc_uri_unittest.cpp | 32 +++
9 files changed, 522 insertions(+), 101 deletions(-)
diff --git a/src/brpc/details/http_message.cpp
b/src/brpc/details/http_message.cpp
index 0cb4f783..14a81e55 100644
--- a/src/brpc/details/http_message.cpp
+++ b/src/brpc/details/http_message.cpp
@@ -49,6 +49,9 @@ DEFINE_int32(http_verbose_max_body_length, 512,
DEFINE_bool(http_check_outbound_header_crlf, true,
"Skip outbound http header fields whose name or value contains "
"CR/LF to prevent request/response splitting.");
+DEFINE_uint32(http_max_header_count, 100,
+ "Reject a message carrying more than so many header fields. "
+ "0 lifts the limit.");
DECLARE_int64(socket_max_unwritten_bytes);
DECLARE_uint64(max_body_size);
@@ -131,6 +134,14 @@ int HttpMessage::on_header_value(http_parser *parser,
http_message->_cur_value =
&header.AddHeader(http_message->_cur_header);
}
+
+ if (FLAGS_http_max_header_count > 0 &&
+ header.HeaderCount() > FLAGS_http_max_header_count) {
+ LOG(ERROR) << "Too many headers, max="
+ << FLAGS_http_max_header_count;
+ return -1;
+ }
+
if (http_message->_cur_value && !http_message->_cur_value->empty()) {
http_message->_cur_value->append(
header.HeaderValueDelimiter(http_message->_cur_header));
diff --git a/src/brpc/policy/http2_rpc_protocol.cpp
b/src/brpc/policy/http2_rpc_protocol.cpp
index b20a0572..6d9b2b87 100644
--- a/src/brpc/policy/http2_rpc_protocol.cpp
+++ b/src/brpc/policy/http2_rpc_protocol.cpp
@@ -29,6 +29,7 @@ DECLARE_int32(http_verbose_max_body_length);
DECLARE_int32(health_check_interval);
DECLARE_bool(usercode_in_pthread);
DECLARE_int64(socket_max_unwritten_bytes);
+DECLARE_uint32(http_max_header_count);
namespace policy {
@@ -729,6 +730,11 @@ H2ParseResult H2StreamContext::OnHeaders(
<< ", stream_id=" << frame_head.stream_id;
return MakeH2Error(H2_PROTOCOL_ERROR);
}
+ // The whole block went through the decoder, the connection is in a
+ // consistent state again and only this stream needs to be reset.
+ if (_rejected_error != H2_NO_ERROR) {
+ return MakeH2Error(_rejected_error, stream_id());
+ }
if (frame_head.flags & H2_FLAGS_END_STREAM) {
return OnEndStream();
}
@@ -791,6 +797,10 @@ H2ParseResult H2StreamContext::OnContinuation(
<< ", stream_id=" << frame_head.stream_id;
return MakeH2Error(H2_PROTOCOL_ERROR);
}
+ // See the same check in H2StreamContext::OnHeaders().
+ if (_rejected_error != H2_NO_ERROR) {
+ return MakeH2Error(_rejected_error, stream_id());
+ }
if (_stream_ended) {
return OnEndStream();
}
@@ -1294,7 +1304,8 @@ H2StreamContext::H2StreamContext(bool
read_body_progressively)
, _remote_window_left(0)
, _deferred_window_update(0)
, _correlation_id(INVALID_BTHREAD_ID.value)
- , _decoded_header_list_size(0) {
+ , _decoded_header_list_size(0)
+ , _rejected_error(H2_NO_ERROR) {
header().set_version(2, 0);
#ifndef NDEBUG
get_h2_bvars()->h2_stream_context_count << 1;
@@ -1349,6 +1360,14 @@ int
H2StreamContext::ConsumeHeaders(butil::IOBufBytesIterator& it) {
<< max_header_list_size << ", stream_id=" << _stream_id;
return -1;
}
+ if (_rejected_error != H2_NO_ERROR) {
+ // The stream is already refused, keep feeding the decoder so that
+ // the dynamic table stays in sync with the peer, but stop spending
+ // memory on fields nobody is going to read. A peer that keeps
+ // piling them up still runs into max_header_list_size above, which
+ // escalates to a connection error as it has to.
+ continue;
+ }
const char* const name = pair.name.c_str();
bool matched = false;
if (name[0] == ':') { // reserved names
@@ -1364,10 +1383,12 @@ int
H2StreamContext::ConsumeHeaders(butil::IOBufBytesIterator& it) {
matched = true;
HttpMethod method;
if (!Str2HttpMethod(pair.value.c_str(), &method)) {
- LOG(ERROR) << "Invalid method=" << pair.value;
- return -1;
+ LOG(ERROR) << "Invalid method=" << pair.value
+ << ", stream_id=" << _stream_id;
+ _rejected_error = H2_PROTOCOL_ERROR;
+ } else {
+ h.set_method(method);
}
- h.set_method(method);
}
break;
case 'p':
@@ -1380,11 +1401,16 @@ int
H2StreamContext::ConsumeHeaders(butil::IOBufBytesIterator& it) {
// would take the whole header block, since HPACK does not
// order pseudo-headers and :method may not have arrived.
if (pair.value != "*" && (pair.value.empty() ||
pair.value[0] != '/')) {
- LOG(ERROR) << "Invalid path=" << pair.value;
- return -1;
+ LOG(ERROR) << "Invalid path=" << pair.value
+ << ", stream_id=" << _stream_id;
+ _rejected_error = H2_PROTOCOL_ERROR;
+ } else if (h.uri().SetH2Path(pair.value) != 0) {
+ // Including path/query/fragment. The only way this
+ // fails is too many query parameters.
+ LOG(ERROR) << h.uri().status().error_cstr()
+ << ", stream_id=" << _stream_id;
+ _rejected_error = H2_ENHANCE_YOUR_CALM;
}
- // Including path/query/fragment
- h.uri().SetH2Path(pair.value);
}
break;
case 's':
@@ -1396,24 +1422,34 @@ int
H2StreamContext::ConsumeHeaders(butil::IOBufBytesIterator& it) {
char* endptr = nullptr;
const int sc = strtol(pair.value.c_str(), &endptr, 10);
if (*endptr != '\0') {
- LOG(ERROR) << "Invalid status=" << pair.value;
- return -1;
+ LOG(ERROR) << "Invalid status=" << pair.value
+ << ", stream_id=" << _stream_id;
+ _rejected_error = H2_PROTOCOL_ERROR;
+ } else {
+ h.set_status_code(sc);
}
- h.set_status_code(sc);
}
break;
default:
break;
}
if (!matched) {
- LOG(ERROR) << "Unknown name=`" << name << '\'';
- return -1;
+ LOG(ERROR) << "Unknown pseudo-header=`" << name
+ << "', stream_id=" << _stream_id;
+ _rejected_error = H2_PROTOCOL_ERROR;
}
} else if (name[0] == 'c' &&
strcmp(name + 1, /*c*/"ontent-type") == 0) {
h.set_content_type(pair.value);
} else {
h.AppendHeader(pair.name, pair.value);
+ if (FLAGS_http_max_header_count > 0 &&
+ h.HeaderCount() > FLAGS_http_max_header_count) {
+ LOG(ERROR) << "Too many headers, max="
+ << FLAGS_http_max_header_count
+ << ", stream_id=" << _stream_id;
+ _rejected_error = H2_ENHANCE_YOUR_CALM;
+ }
}
if (FLAGS_http_verbose) {
diff --git a/src/brpc/policy/http2_rpc_protocol.h
b/src/brpc/policy/http2_rpc_protocol.h
index 6c58f71a..64c9864d 100644
--- a/src/brpc/policy/http2_rpc_protocol.h
+++ b/src/brpc/policy/http2_rpc_protocol.h
@@ -234,7 +234,9 @@ public:
// Decode headers in HPACK from *it and set into this->header(). The input
// does not need to complete.
- // Returns 0 on success, -1 otherwise.
+ // Returns 0 on success, -1 on a connection-level error. A message that is
+ // merely malformed or unacceptable does not fail here, it sets
+ // `_rejected_error` instead, see the comment on that field.
int ConsumeHeaders(butil::IOBufBytesIterator& it);
H2ParseResult OnEndStream();
@@ -281,6 +283,19 @@ friend class H2Context;
// (name + value + 32 per field, RFC 7540 section 10.5.1), checked
// against the local max_header_list_size in ConsumeHeaders().
uint64_t _decoded_header_list_size;
+ // Set when this message must be refused although the connection itself is
+ // still healthy: it is malformed (invalid or unknown pseudo-header, RFC
+ // 9113 section 8.1.1 mandates a stream error of type PROTOCOL_ERROR) or it
+ // violates a local limit (too many headers, too many query parameters in
+ // :path). Only the stream is reset so that the other streams keep working,
+ // but the error cannot be raised where it is detected: HPACK keeps a
+ // dynamic table per connection, so leaving the rest of the block undecoded
+ // would desynchronize it from the encoding table of the peer and corrupt
+ // every header block that follows. RFC 9113 section 10.5.1: "The field
+ // block MUST be processed to ensure a consistent connection state, unless
+ // the connection is closed." Hence the rejection is remembered here and
+ // turned into a RST_STREAM once END_HEADERS is reached.
+ H2Error _rejected_error;
butil::IOBuf _remaining_header_fragment;
// Request body which cannot be sent yet due to remote flow control.
// Accessed under H2Context::_stream_mutex.
diff --git a/src/brpc/socket.cpp b/src/brpc/socket.cpp
index 283473df..a0b49662 100644
--- a/src/brpc/socket.cpp
+++ b/src/brpc/socket.cpp
@@ -794,6 +794,7 @@ int Socket::OnCreated(const SocketOptions& options) {
_unwritten_bytes.store(0, butil::memory_order_relaxed);
_keepalive_options = options.keepalive_options;
_tcp_user_timeout_ms = options.tcp_user_timeout_ms;
+ _http_request_method = HTTP_METHOD_GET;
CHECK(nullptr == _write_head.load(butil::memory_order_relaxed));
_is_write_shutdown = false;
int fd = options.fd;
diff --git a/src/brpc/uri.cpp b/src/brpc/uri.cpp
index 2881a8e5..6b10464b 100644
--- a/src/brpc/uri.cpp
+++ b/src/brpc/uri.cpp
@@ -17,9 +17,8 @@
#include <ctype.h> // isalnum
-
#include <unordered_set>
-
+#include <gflags/gflags.h>
#include "brpc/log.h"
#include "brpc/details/http_parser.h" // http_parser_parse_url
#include "brpc/uri.h" // URI
@@ -27,15 +26,16 @@
namespace brpc {
+DEFINE_uint32(http_max_query_count, 1000,
+ "Reject a URL carrying more than so many query parameters. "
+ "0 lifts the limit.");
+
URI::URI()
: _port(-1)
, _query_was_modified(false)
, _initialized_query_map(false)
{}
-URI::~URI() {
-}
-
void URI::Clear() {
_st.reset();
_port = -1;
@@ -64,6 +64,22 @@ void URI::Swap(URI &rhs) {
_query_map.swap(rhs._query_map);
}
+// Counting separators rather than map entries deliberately overestimates: the
+// splitter walks every segment even when the keys repeat, and it is that walk,
+// not the final map size, that the limit is meant to bound.
+static bool TooManyQueries(const std::string& query) {
+ if (FLAGS_http_max_query_count == 0 || query.empty()) {
+ return false;
+ }
+ uint32_t count = 1;
+ for (char i : query) {
+ if (i == '&' && ++count > FLAGS_http_max_query_count) {
+ return true;
+ }
+ }
+ return false;
+}
+
// Parse queries, which is case-sensitive
static void ParseQueries(URI::QueryMap& query_map, const std::string &query) {
query_map.clear();
@@ -238,6 +254,11 @@ int URI::SetHttpURL(const char* url) {
}
}
_query.assign(start, p - start);
+ if (TooManyQueries(_query)) {
+ _st.set_error(EINVAL, "More than %u query parameters in url",
+ FLAGS_http_max_query_count);
+ return -1;
+ }
}
if (*p == '#') {
start = ++p;
@@ -411,7 +432,8 @@ void URI::SetHostAndPort(const std::string& host) {
_host.assign(host_begin, host_end - host_begin);
}
-void URI::SetH2Path(const char* h2_path) {
+int URI::SetH2Path(const char* h2_path) {
+ _st.reset();
_path.clear();
_query.clear();
_fragment.clear();
@@ -427,12 +449,18 @@ void URI::SetH2Path(const char* h2_path) {
start = ++p;
for (; *p && *p != '#'; ++p) {}
_query.assign(start, p - start);
+ if (TooManyQueries(_query)) {
+ _st.set_error(EINVAL, "More than %u query parameters in :path",
+ FLAGS_http_max_query_count);
+ return -1;
+ }
}
if (*p == '#') {
start = ++p;
for (; *p; ++p) {}
_fragment.assign(start, p - start);
}
+ return 0;
}
QueryRemover::QueryRemover(const std::string* str)
diff --git a/src/brpc/uri.h b/src/brpc/uri.h
index 7edac400..a42cf88f 100644
--- a/src/brpc/uri.h
+++ b/src/brpc/uri.h
@@ -56,7 +56,7 @@ public:
// You can copy a URI.
URI();
- ~URI();
+ ~URI() = default;
// Exchange internal fields with another URI.
void Swap(URI &rhs);
@@ -99,8 +99,9 @@ public:
void set_port(int port) { _port = port; }
void SetHostAndPort(const std::string& host_and_optional_port);
// Set path/query/fragment with the input in form of "path?query#fragment"
- void SetH2Path(const char* h2_path);
- void SetH2Path(const std::string& path) { SetH2Path(path.c_str()); }
+ // Returns 0 on success, -1 otherwise and status() is set.
+ int SetH2Path(const char* h2_path);
+ int SetH2Path(const std::string& path) { return SetH2Path(path.c_str()); }
// Get the value of a CASE-SENSITIVE key.
// Returns pointer to the value, nullptr when the key does not exist.
diff --git a/test/brpc_http_message_unittest.cpp
b/test/brpc_http_message_unittest.cpp
index 57e98cca..90c9dbdd 100644
--- a/test/brpc_http_message_unittest.cpp
+++ b/test/brpc_http_message_unittest.cpp
@@ -32,6 +32,7 @@ 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);
+DECLARE_uint32(http_max_header_count);
int main(int argc, char* argv[]) {
testing::InitGoogleTest(&argc, argv);
@@ -643,6 +644,65 @@ TEST(HttpMessageTest, htab_is_ows_in_header_values) {
}
}
+TEST(HttpMessageTest, too_many_headers) {
+ GFLAGS_NAMESPACE::FlagSaver flag_saver;
+ brpc::FLAGS_http_max_header_count = 8;
+
+ // Host counts as well, so 8 distinct names in total are accepted.
+ std::string at_limit = "GET / HTTP/1.1\r\nHost: a.com\r\n";
+ for (int i = 1; i < 8; ++i) {
+ at_limit.append("h" + std::to_string(i) + ": v\r\n");
+ }
+ std::string over_limit = at_limit + "last: v\r\n\r\n";
+ at_limit.append("\r\n");
+ {
+ brpc::HttpMessage http_message;
+ ASSERT_EQ((ssize_t)at_limit.size(),
+ http_message.ParseFromArray(at_limit.data(),
at_limit.size()))
+ << http_message._parser;
+ ASSERT_EQ(8u, http_message.header().HeaderCount());
+ }
+ {
+ brpc::HttpMessage http_message;
+ ASSERT_EQ(-1, http_message.ParseFromArray(over_limit.data(),
+ over_limit.size()));
+ }
+
+ // Repeated names fold into one entry, so they occupy one bucket and are
not
+ // what the limit is aimed at.
+ std::string folded = "GET / HTTP/1.1\r\nHost: a.com\r\n";
+ for (int i = 0; i < 100; ++i) {
+ folded.append("dup: v\r\n");
+ }
+ folded.append("\r\n");
+ {
+ brpc::HttpMessage http_message;
+ ASSERT_EQ((ssize_t)folded.size(),
+ http_message.ParseFromArray(folded.data(), folded.size()))
+ << http_message._parser;
+ ASSERT_EQ(2u, http_message.header().HeaderCount());
+ }
+ // Set-Cookie is the one name that does not fold, so each occurrence is its
+ // own entry and does count.
+ std::string cookies = "GET / HTTP/1.1\r\nHost: a.com\r\n";
+ for (int i = 0; i < 100; ++i) {
+ cookies.append("Set-Cookie: a=b\r\n");
+ }
+ cookies.append("\r\n");
+ {
+ brpc::HttpMessage http_message;
+ ASSERT_EQ(-1, http_message.ParseFromArray(cookies.data(),
cookies.size()));
+ }
+
+ brpc::FLAGS_http_max_header_count = 0;
+ {
+ brpc::HttpMessage http_message;
+ ASSERT_EQ((ssize_t)over_limit.size(),
+ http_message.ParseFromArray(over_limit.data(),
over_limit.size()))
+ << http_message._parser;
+ }
+}
+
TEST(HttpMessageTest, find_method_property_by_uri) {
brpc::Server server;
ASSERT_EQ(0, server.AddService(new test::EchoService(),
diff --git a/test/brpc_http_rpc_protocol_unittest.cpp
b/test/brpc_http_rpc_protocol_unittest.cpp
index 10fb5cc3..136c0f8b 100644
--- a/test/brpc_http_rpc_protocol_unittest.cpp
+++ b/test/brpc_http_rpc_protocol_unittest.cpp
@@ -63,6 +63,8 @@ DECLARE_bool(allow_chunked_length);
DECLARE_int32(max_connection_pool_size);
DECLARE_uint64(max_body_size);
DECLARE_int64(socket_max_unwritten_bytes);
+DECLARE_uint32(http_max_header_count);
+DECLARE_uint32(http_max_query_count);
extern bvar::CollectorSpeedLimit g_rpc_dump_sl;
}
@@ -727,10 +729,10 @@ TEST_F(HttpTest, complete_flow) {
}
TEST_F(HttpTest, chunked_uploading) {
- const int port = 8923;
brpc::Server server;
- EXPECT_EQ(0, server.AddService(&_svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- EXPECT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.AddService(&_svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
// Send request via curl using chunked encoding
const std::string req = "{\"message\":\"hello\"}";
@@ -889,11 +891,11 @@ private:
};
TEST_F(HttpTest, read_chunked_response_normally) {
- const int port = 8923;
brpc::Server server;
DownloadServiceImpl svc;
- EXPECT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- EXPECT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
for (int i = 0; i < 3; ++i) {
svc.set_done_place((DonePlace)i);
@@ -913,11 +915,11 @@ TEST_F(HttpTest, read_chunked_response_normally) {
}
TEST_F(HttpTest, read_failed_chunked_response) {
- const int port = 8923;
brpc::Server server;
DownloadServiceImpl svc;
- EXPECT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- EXPECT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
brpc::Channel channel;
brpc::ChannelOptions options;
@@ -1036,10 +1038,10 @@ TEST_F(HttpTest, read_long_body_progressively) {
std::numeric_limits<size_t>::max());
butil::intrusive_ptr<ReadBody> reader;
{
- const int port = 8923;
brpc::Server server;
- EXPECT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- EXPECT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
{
brpc::Channel channel;
brpc::ChannelOptions options;
@@ -1082,12 +1084,12 @@ TEST_F(HttpTest, read_long_body_progressively) {
TEST_F(HttpTest, read_short_body_progressively) {
butil::intrusive_ptr<ReadBody> reader;
- const int port = 8923;
brpc::Server server;
const int NREP = 10000;
DownloadServiceImpl svc(DONE_BEFORE_CREATE_PA, NREP);
- EXPECT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- EXPECT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
{
brpc::Channel channel;
brpc::ChannelOptions options;
@@ -1119,11 +1121,11 @@ TEST_F(HttpTest, read_short_body_progressively) {
}
TEST_F(HttpTest, progressive_read_timeout_keeps_active_reader_alive) {
- const int port = 8923;
DownloadServiceImpl svc(DONE_BEFORE_CREATE_PA, 8, 100000);
brpc::Server server;
ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- ASSERT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
brpc::Channel channel;
brpc::ChannelOptions options;
@@ -1148,11 +1150,11 @@ TEST_F(HttpTest,
progressive_read_timeout_keeps_active_reader_alive) {
}
TEST_F(HttpTest, progressive_read_timeout_closes_idle_http1_reader_once) {
- const int port = 8923;
DownloadServiceImpl svc(DONE_BEFORE_CREATE_PA, 2, 300000);
brpc::Server server;
ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- ASSERT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
butil::intrusive_ptr<TimeoutReadBody> reader(new TimeoutReadBody);
{
@@ -1182,11 +1184,11 @@ TEST_F(HttpTest,
progressive_read_timeout_closes_idle_http1_reader_once) {
}
TEST_F(HttpTest, progressive_read_timeout_before_first_body_part) {
- const int port = 8923;
DownloadServiceImpl svc(DONE_BEFORE_CREATE_PA, 1, 0, 300000);
brpc::Server server;
ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- ASSERT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
butil::intrusive_ptr<TimeoutReadBody> reader(new TimeoutReadBody);
{
@@ -1215,11 +1217,11 @@ TEST_F(HttpTest,
progressive_read_timeout_before_first_body_part) {
}
TEST_F(HttpTest, progressive_read_timeout_ignores_slow_user_callback) {
- const int port = 8923;
DownloadServiceImpl svc(DONE_BEFORE_CREATE_PA, 3, 50000);
brpc::Server server;
ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- ASSERT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
brpc::Channel channel;
brpc::ChannelOptions options;
@@ -1245,11 +1247,11 @@ TEST_F(HttpTest,
progressive_read_timeout_ignores_slow_user_callback) {
}
TEST_F(HttpTest, progressive_read_timeout_preserves_reader_error) {
- const int port = 8923;
DownloadServiceImpl svc(DONE_BEFORE_CREATE_PA, 10);
brpc::Server server;
ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- ASSERT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
brpc::Channel channel;
brpc::ChannelOptions options;
@@ -1274,10 +1276,10 @@ TEST_F(HttpTest,
progressive_read_timeout_preserves_reader_error) {
}
TEST_F(HttpTest, progressive_read_timeout_rejects_http2) {
- const int port = 8923;
brpc::Server server;
ASSERT_EQ(0, server.AddService(&_svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- ASSERT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
brpc::Channel channel;
brpc::ChannelOptions options;
@@ -1305,10 +1307,10 @@ TEST_F(HttpTest,
read_progressively_after_cntl_destroys) {
std::numeric_limits<size_t>::max());
butil::intrusive_ptr<ReadBody> reader;
{
- const int port = 8923;
brpc::Server server;
- EXPECT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- EXPECT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
{
brpc::Channel channel;
brpc::ChannelOptions options;
@@ -1351,10 +1353,10 @@ TEST_F(HttpTest, read_progressively_after_long_delay) {
DownloadServiceImpl svc(DONE_BEFORE_CREATE_PA,
std::numeric_limits<size_t>::max());
{
- const int port = 8923;
brpc::Server server;
- EXPECT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- EXPECT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
{
brpc::Channel channel;
brpc::ChannelOptions options;
@@ -1399,10 +1401,10 @@ TEST_F(HttpTest, read_progressively_after_long_delay) {
TEST_F(HttpTest, skip_progressive_reading) {
DownloadServiceImpl svc(DONE_BEFORE_CREATE_PA,
std::numeric_limits<size_t>::max());
- const int port = 8923;
brpc::Server server;
- EXPECT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- EXPECT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
brpc::Channel channel;
brpc::ChannelOptions options;
options.protocol = brpc::PROTOCOL_HTTP;
@@ -1438,12 +1440,12 @@ public:
};
TEST_F(HttpTest, failed_on_read_one_part) {
- const int port = 8923;
brpc::Server server;
DownloadServiceImpl svc(DONE_BEFORE_CREATE_PA,
std::numeric_limits<size_t>::max());
- EXPECT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- EXPECT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
brpc::Channel channel;
brpc::ChannelOptions options;
options.protocol = brpc::PROTOCOL_HTTP;
@@ -1464,12 +1466,12 @@ TEST_F(HttpTest, failed_on_read_one_part) {
TEST_F(HttpTest, broken_socket_stops_progressive_reading) {
butil::intrusive_ptr<ReadBody> reader;
- const int port = 8923;
brpc::Server server;
DownloadServiceImpl svc(DONE_BEFORE_CREATE_PA,
std::numeric_limits<size_t>::max());
- EXPECT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- EXPECT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
brpc::Channel channel;
brpc::ChannelOptions options;
@@ -1587,14 +1589,14 @@ private:
};
TEST_F(HttpTest, server_end_read_short_body_progressively) {
- const int port = 8923;
brpc::ServiceOptions opt;
opt.enable_progressive_read = true;
opt.ownership = brpc::SERVER_DOESNT_OWN_SERVICE;
UploadServiceImpl upsvc;
brpc::Server server;
- EXPECT_EQ(0, server.AddService(&upsvc, opt));
- EXPECT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.AddService(&upsvc, opt));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
brpc::Channel channel;
brpc::ChannelOptions options;
@@ -1626,14 +1628,14 @@ TEST_F(HttpTest,
server_end_read_short_body_progressively) {
// Fixme!!! Server progressive reader has a heap-use-after-free bug detected
by ASan.
// For details, see
https://github.com/apache/brpc/issues/2145#issuecomment-2329413363
TEST_F(HttpTest, server_end_read_failed) {
- const int port = 8923;
brpc::ServiceOptions opt;
opt.enable_progressive_read = true;
opt.ownership = brpc::SERVER_DOESNT_OWN_SERVICE;
UploadServiceImpl upsvc;
brpc::Server server;
- EXPECT_EQ(0, server.AddService(&upsvc, opt));
- EXPECT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.AddService(&upsvc, opt));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
brpc::Channel channel;
brpc::ChannelOptions options;
@@ -1664,10 +1666,10 @@ TEST_F(HttpTest, server_end_read_failed) {
#endif // BUTIL_USE_ASAN
TEST_F(HttpTest, http2_sanity) {
- const int port = 8923;
brpc::Server server;
- EXPECT_EQ(0, server.AddService(&_svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- EXPECT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.AddService(&_svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
brpc::Channel channel;
brpc::ChannelOptions options;
@@ -1969,6 +1971,231 @@ TEST_F(HttpTest,
h2_header_list_budget_resets_per_block) {
delete sctx;
}
+// Literal header field with a new name, with both lengths in a single 7-bit
+// prefix octet. `first_octet` selects the representation: 0x00 is "without
+// indexing" (RFC 7541 6.2.2), 0x40 is "with incremental indexing" (6.2.1)
+// which also adds the field to the dynamic table.
+// 0x80 of a length octet is the Huffman flag and a length of 128 or more needs
+// the multi-octet form, so refuse what does not fit instead of emitting a
+// corrupt header block.
+void AppendLiteralHeader(butil::IOBuf* out, const std::string& name,
+ const std::string& value, uint8_t first_octet = 0x00)
{
+ ASSERT_LT(name.size(), 0x80u);
+ ASSERT_LT(value.size(), 0x80u);
+ uint8_t prefix[] = { first_octet, (uint8_t)name.size() };
+ out->append(prefix, sizeof(prefix));
+ out->append(name);
+ uint8_t value_len = (uint8_t)value.size();
+ out->append(&value_len, 1);
+ out->append(value);
+}
+
+// Feed `payload` to `sctx` as one complete HEADERS block, the way
+// H2Context::Consume() would. A non-zero stream_id in the result means the
+// frame handler asked for a RST_STREAM, a zero one means a GOAWAY that closes
+// the whole connection.
+brpc::policy::H2ParseResult ConsumeHeadersBlock(
+ brpc::policy::H2StreamContext* sctx, const butil::IOBuf& payload,
+ int stream_id) {
+ brpc::policy::H2FrameHead head;
+ head.payload_size = payload.size();
+ head.type = brpc::policy::H2_FRAME_HEADERS;
+ head.flags = 0x4; // H2_FLAGS_END_HEADERS
+ head.stream_id = stream_id;
+ butil::IOBufBytesIterator it(payload);
+ return sctx->OnHeaders(it, head, payload.size(), 0);
+}
+
+TEST_F(HttpTest, h2_too_many_headers) {
+ GFLAGS_NAMESPACE::FlagSaver flag_saver;
+ brpc::FLAGS_http_max_header_count = 8;
+
+ brpc::policy::H2Context* ctx =
+ new brpc::policy::H2Context(_socket.get(), nullptr);
+ CHECK_EQ(ctx->Init(), 0);
+ _socket->initialize_parsing_context(&ctx);
+
+ {
+ std::unique_ptr<brpc::policy::H2StreamContext> sctx(
+ new brpc::policy::H2StreamContext(false));
+ sctx->Init(ctx, 1);
+ butil::IOBuf payload;
+ for (int i = 0; i < 8; ++i) {
+ AppendLiteralHeader(&payload, "h" + std::to_string(i), "v");
+ }
+ brpc::policy::H2ParseResult res =
+ ConsumeHeadersBlock(sctx.get(), payload, 1);
+ ASSERT_TRUE(res.is_ok()) << brpc::H2ErrorToString(res.error());
+ ASSERT_EQ(8u, sctx->header().HeaderCount());
+ }
+ {
+ std::unique_ptr<brpc::policy::H2StreamContext> sctx(
+ new brpc::policy::H2StreamContext(false));
+ sctx->Init(ctx, 3);
+ butil::IOBuf payload;
+ for (int i = 0; i < 9; ++i) {
+ AppendLiteralHeader(&payload, "h" + std::to_string(i), "v");
+ }
+ // Refusing the request must not cost the connection its other
+ // streams, so the frame handler asks for a RST_STREAM (non-zero
+ // stream_id) rather than a GOAWAY.
+ brpc::policy::H2ParseResult res =
+ ConsumeHeadersBlock(sctx.get(), payload, 3);
+ ASSERT_FALSE(res.is_ok());
+ ASSERT_EQ(brpc::H2_ENHANCE_YOUR_CALM, res.error());
+ ASSERT_EQ(3, res.stream_id());
+ }
+}
+
+TEST_F(HttpTest, h2_too_many_queries_in_path) {
+ GFLAGS_NAMESPACE::FlagSaver flag_saver;
+ brpc::FLAGS_http_max_query_count = 4;
+
+ brpc::policy::H2Context* ctx =
+ new brpc::policy::H2Context(_socket.get(), nullptr);
+ CHECK_EQ(ctx->Init(), 0);
+ _socket->initialize_parsing_context(&ctx);
+
+ {
+ std::unique_ptr<brpc::policy::H2StreamContext> sctx(
+ new brpc::policy::H2StreamContext(false));
+ sctx->Init(ctx, 1);
+ butil::IOBuf payload;
+ AppendLiteralHeader(&payload, ":path", "/s?a=1&b=2&c=3&d=4");
+ brpc::policy::H2ParseResult res =
+ ConsumeHeadersBlock(sctx.get(), payload, 1);
+ ASSERT_TRUE(res.is_ok()) << brpc::H2ErrorToString(res.error());
+ }
+ {
+ std::unique_ptr<brpc::policy::H2StreamContext> sctx(
+ new brpc::policy::H2StreamContext(false));
+ sctx->Init(ctx, 3);
+ butil::IOBuf payload;
+ AppendLiteralHeader(&payload, ":path", "/s?a=1&b=2&c=3&d=4&e=5");
+ brpc::policy::H2ParseResult res =
+ ConsumeHeadersBlock(sctx.get(), payload, 3);
+ ASSERT_FALSE(res.is_ok());
+ ASSERT_EQ(brpc::H2_ENHANCE_YOUR_CALM, res.error());
+ ASSERT_EQ(3, res.stream_id());
+ }
+}
+
+// A refused header block still has to be fed to the HPACK decoder in full.
+// The dynamic table belongs to the connection, so dropping the tail of a block
+// would leave it out of step with the encoding table of the peer and turn
+// every later block into garbage, which is why RFC 9113 section 10.5.1 says
+// the field block MUST be processed unless the connection is closed.
+TEST_F(HttpTest, h2_refused_header_block_keeps_hpack_in_sync) {
+ GFLAGS_NAMESPACE::FlagSaver flag_saver;
+ brpc::FLAGS_http_max_header_count = 2;
+
+ brpc::policy::H2Context* ctx =
+ new brpc::policy::H2Context(_socket.get(), nullptr);
+ CHECK_EQ(ctx->Init(), 0);
+ _socket->initialize_parsing_context(&ctx);
+
+ // Four headers with incremental indexing, two of them past the limit.
+ std::unique_ptr<brpc::policy::H2StreamContext> sctx(
+ new brpc::policy::H2StreamContext(false));
+ sctx->Init(ctx, 1);
+ butil::IOBuf payload;
+ AppendLiteralHeader(&payload, "a", "1", 0x40);
+ AppendLiteralHeader(&payload, "b", "2", 0x40);
+ AppendLiteralHeader(&payload, "c", "3", 0x40);
+ AppendLiteralHeader(&payload, "d", "4", 0x40);
+ brpc::policy::H2ParseResult res =
+ ConsumeHeadersBlock(sctx.get(), payload, 1);
+ ASSERT_FALSE(res.is_ok());
+ ASSERT_EQ(brpc::H2_ENHANCE_YOUR_CALM, res.error());
+ ASSERT_EQ(1, res.stream_id());
+ // Everything after the offending field is decoded but thrown away.
+ ASSERT_EQ(3u, sctx->header().HeaderCount());
+
+ // The static table ends at index 61, so 62 names the newest dynamic entry.
+ // That is "d" only because decoding ran to the end of the block; had it
+ // stopped at the limit, 62 would still be "c".
+ std::unique_ptr<brpc::policy::H2StreamContext> sctx2(
+ new brpc::policy::H2StreamContext(false));
+ sctx2->Init(ctx, 3);
+ butil::IOBuf indexed;
+ const uint8_t indexed_field[] = { 0x80 | 62 }; // Indexed Header Field
+ indexed.append(indexed_field, sizeof(indexed_field));
+ brpc::policy::H2ParseResult res2 =
+ ConsumeHeadersBlock(sctx2.get(), indexed, 3);
+ ASSERT_TRUE(res2.is_ok()) << brpc::H2ErrorToString(res2.error());
+ const std::string* value = sctx2->header().GetHeader("d");
+ ASSERT_TRUE(value != nullptr);
+ ASSERT_EQ("4", *value);
+}
+
+// RFC 9113 section 8.1.1: "Malformed requests or responses that are detected
+// MUST be treated as a stream error (Section 5.4.2) of type PROTOCOL_ERROR."
+// A bad pseudo-header says nothing about the health of the connection, so it
+// must not cost the other streams theirs. :path has its own case table in
+// HttpTest.http2_reject_path_not_starting_with_slash.
+TEST_F(HttpTest, h2_malformed_pseudo_header_resets_stream_only) {
+ struct MalformedField {
+ const char* name;
+ const char* value;
+ };
+ MalformedField malformed[] = {
+ { ":method", "NOSUCH" },
+ { ":status", "20x" },
+ { ":nosuchheader", "1" }, // 8.3: undefined pseudo-header
+ };
+
+ brpc::policy::H2Context* ctx =
+ new brpc::policy::H2Context(_socket.get(), nullptr);
+ CHECK_EQ(ctx->Init(), 0);
+ _socket->initialize_parsing_context(&ctx);
+
+ int stream_id = 1;
+ for (const auto& bad : malformed) {
+ std::unique_ptr<brpc::policy::H2StreamContext> sctx(
+ new brpc::policy::H2StreamContext(false));
+ sctx->Init(ctx, stream_id);
+ butil::IOBuf payload;
+ AppendLiteralHeader(&payload, bad.name, bad.value);
+ brpc::policy::H2ParseResult res =
+ ConsumeHeadersBlock(sctx.get(), payload, stream_id);
+ std::string desc = std::string(bad.name) + '=' + bad.value;
+ ASSERT_FALSE(res.is_ok()) << desc;
+ ASSERT_EQ(brpc::H2_PROTOCOL_ERROR, res.error())
+ << desc << ": " << brpc::H2ErrorToString(res.error());
+ ASSERT_EQ(stream_id, res.stream_id()) << desc;
+ stream_id += 2;
+ }
+
+ // A malformed block is drained like any other refusal, so a field that
+ // follows the bad pseudo-header still reaches the dynamic table. RFC 9113
+ // section 4.3 leaves no choice here: only a decoding error may take down
+ // the connection, so everything else has to be decoded to the end.
+ std::unique_ptr<brpc::policy::H2StreamContext> sctx(
+ new brpc::policy::H2StreamContext(false));
+ sctx->Init(ctx, stream_id);
+ butil::IOBuf payload;
+ AppendLiteralHeader(&payload, ":path", "foo");
+ AppendLiteralHeader(&payload, "after-the-bad-one", "1", 0x40);
+ brpc::policy::H2ParseResult res =
+ ConsumeHeadersBlock(sctx.get(), payload, stream_id);
+ ASSERT_EQ(brpc::H2_PROTOCOL_ERROR, res.error());
+ ASSERT_EQ(stream_id, res.stream_id());
+
+ stream_id += 2;
+ std::unique_ptr<brpc::policy::H2StreamContext> sctx2(
+ new brpc::policy::H2StreamContext(false));
+ sctx2->Init(ctx, stream_id);
+ butil::IOBuf indexed;
+ const uint8_t indexed_field[] = { 0x80 | 62 }; // newest dynamic entry
+ indexed.append(indexed_field, sizeof(indexed_field));
+ brpc::policy::H2ParseResult res2 =
+ ConsumeHeadersBlock(sctx2.get(), indexed, stream_id);
+ ASSERT_TRUE(res2.is_ok()) << brpc::H2ErrorToString(res2.error());
+ const std::string* value = sctx2->header().GetHeader("after-the-bad-one");
+ ASSERT_TRUE(value != nullptr);
+ ASSERT_EQ("1", *value);
+}
+
TEST_F(HttpTest, h2_oversized_single_headers_block_rejected) {
// A single HEADERS frame whose decoded header list exceeds
// max_header_list_size must be rejected at the block boundary (before
@@ -2127,29 +2354,29 @@ TEST_F(HttpTest, http2_invalid_settings) {
brpc::Server server;
brpc::ServerOptions options;
options.h2_settings.stream_window_size =
brpc::H2Settings::MAX_WINDOW_SIZE + 1;
- ASSERT_EQ(-1, server.Start("127.0.0.1:8924", &options));
+ ASSERT_EQ(-1, server.Start("127.0.0.1:0", &options));
}
{
brpc::Server server;
brpc::ServerOptions options;
options.h2_settings.max_frame_size =
brpc::H2Settings::DEFAULT_MAX_FRAME_SIZE - 1;
- ASSERT_EQ(-1, server.Start("127.0.0.1:8924", &options));
+ ASSERT_EQ(-1, server.Start("127.0.0.1:0", &options));
}
{
brpc::Server server;
brpc::ServerOptions options;
options.h2_settings.max_frame_size =
brpc::H2Settings::MAX_OF_MAX_FRAME_SIZE + 1;
- ASSERT_EQ(-1, server.Start("127.0.0.1:8924", &options));
+ ASSERT_EQ(-1, server.Start("127.0.0.1:0", &options));
}
}
TEST_F(HttpTest, http2_not_closing_socket_when_rpc_timeout) {
- const int port = 8923;
brpc::Server server;
- EXPECT_EQ(0, server.AddService(&_svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- EXPECT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.AddService(&_svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
brpc::Channel channel;
brpc::ChannelOptions options;
options.protocol = "h2";
@@ -2386,7 +2613,8 @@ TEST_F(HttpTest, http2_handle_goaway_streams) {
}
// RFC 9113 8.3.1: :path MUST NOT be empty and MUST begin with '/', the only
-// exception being the asterisk-form that OPTIONS uses.
+// exception being the asterisk-form that OPTIONS uses. Violating that makes
+// the request malformed, which 8.1.1 turns into a stream error.
TEST_F(HttpTest, http2_reject_path_not_starting_with_slash) {
brpc::policy::H2Context* h2_ctx =
new brpc::policy::H2Context(_socket.get(), &_server);
@@ -2423,23 +2651,32 @@ TEST_F(HttpTest,
http2_reject_path_not_starting_with_slash) {
h2_ctx->hpacker().Encode(&appender, header, options);
butil::IOBuf buf;
appender.move_to(buf);
- butil::IOBufBytesIterator it(buf);
brpc::policy::H2StreamContext* h2_msg =
new brpc::policy::H2StreamContext(false);
h2_msg->Init(h2_ctx, stream_id);
+ brpc::policy::H2ParseResult res =
+ ConsumeHeadersBlock(h2_msg, buf, stream_id);
+ if (c.accepted) {
+ ASSERT_TRUE(res.is_ok()) << "path=`" << c.path << "': "
+ << brpc::H2ErrorToString(res.error());
+ } else {
+ // A bad :path makes the request malformed, not the connection
+ // unusable, so only this stream is reset.
+ ASSERT_EQ(brpc::H2_PROTOCOL_ERROR, res.error())
+ << "path=`" << c.path << '\'';
+ ASSERT_EQ(stream_id, res.stream_id()) << "path=`" << c.path <<
'\'';
+ }
stream_id += 2;
- ASSERT_EQ(c.accepted ? 0 : -1, h2_msg->ConsumeHeaders(it))
- << "path=`" << c.path << '\'';
h2_msg->Destroy();
}
}
TEST_F(HttpTest, spring_protobuf_content_type) {
- const int port = 8923;
brpc::Server server;
- EXPECT_EQ(0, server.AddService(&_svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- EXPECT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.AddService(&_svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
brpc::Channel channel;
brpc::ChannelOptions options;
@@ -2483,10 +2720,10 @@ TEST_F(HttpTest, dump_http_request) {
brpc::g_rpc_dump_sl.sampling_range = bvar::COLLECTOR_SAMPLING_BASE;
// init channel
- const int port = 8923;
brpc::Server server;
- EXPECT_EQ(0, server.AddService(&_svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- EXPECT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.AddService(&_svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
brpc::Channel channel;
brpc::ChannelOptions options;
@@ -2555,10 +2792,10 @@ TEST_F(HttpTest, dump_http_request) {
}
TEST_F(HttpTest, proto_text_content_type) {
- const int port = 8923;
brpc::Server server;
- EXPECT_EQ(0, server.AddService(&_svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- EXPECT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.AddService(&_svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
brpc::Channel channel;
brpc::ChannelOptions options;
@@ -2593,10 +2830,10 @@ TEST_F(HttpTest, proto_text_content_type) {
}
TEST_F(HttpTest, proto_json_content_type) {
- const int port = 8923;
brpc::Server server;
- EXPECT_EQ(0, server.AddService(&_svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- EXPECT_EQ(0, server.Start(port, nullptr));
+ ASSERT_EQ(0, server.AddService(&_svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
brpc::Channel channel;
brpc::ChannelOptions options;
@@ -2670,11 +2907,11 @@ class HttpServiceImpl : public ::test::HttpService {
};
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, nullptr));
+ ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ int port = server.listen_address().port;
brpc::Channel channel;
brpc::ChannelOptions options;
@@ -2798,8 +3035,8 @@ void ReadOneResponse(brpc::SocketUniquePtr& sock,
TEST_F(HttpTest, http_expect) {
brpc::Server server;
HttpServiceImpl svc;
- EXPECT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
- EXPECT_EQ(0, server.Start(0, nullptr));
+ ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ ASSERT_EQ(0, server.Start(0, nullptr));
const butil::EndPoint ep = server.listen_address();
brpc::SocketOptions options;
diff --git a/test/brpc_uri_unittest.cpp b/test/brpc_uri_unittest.cpp
index b9d6b650..b1be2c55 100644
--- a/test/brpc_uri_unittest.cpp
+++ b/test/brpc_uri_unittest.cpp
@@ -15,10 +15,15 @@
// specific language governing permissions and limitations
// under the License.
+#include <gflags/gflags.h>
#include <gtest/gtest.h>
#include "brpc/uri.h"
+namespace brpc {
+DECLARE_uint32(http_max_query_count);
+}
+
TEST(URITest, everything) {
brpc::URI uri;
std::string uri_str = "
foobar://user:[email protected]:80/s?wd=uri#frag ";
@@ -347,6 +352,33 @@ TEST(URITest, invalid_query) {
ASSERT_EQ("a-b-c:def", uri.query());
}
+TEST(URITest, too_many_queries) {
+ GFLAGS_NAMESPACE::FlagSaver flag_saver;
+ brpc::FLAGS_http_max_query_count = 4;
+
+ brpc::URI uri;
+ ASSERT_EQ(0, uri.SetHttpURL("http://a.com/s?a=1&b=2&c=3&d=4";)) <<
uri.status();
+ ASSERT_EQ(-1, uri.SetHttpURL("http://a.com/s?a=1&b=2&c=3&d=4&e=5";));
+ ASSERT_STREQ("More than 4 query parameters in url",
uri.status().error_cstr());
+ // Repeated keys collapse into one map entry, but the splitter still walks
+ // every segment, so they count.
+ ASSERT_EQ(-1, uri.SetHttpURL("http://a.com/s?a=1&a=2&a=3&a=4&a=5";));
+ // An empty query is not one parameter.
+ brpc::FLAGS_http_max_query_count = 1;
+ ASSERT_EQ(0, uri.SetHttpURL("http://a.com/s?";)) << uri.status();
+
+ brpc::FLAGS_http_max_query_count = 4;
+ ASSERT_EQ(0, uri.SetH2Path("/s?a=1&b=2&c=3&d=4")) << uri.status();
+ ASSERT_EQ(-1, uri.SetH2Path("/s?a=1&b=2&c=3&d=4&e=5"));
+ ASSERT_STREQ("More than 4 query parameters in :path",
uri.status().error_cstr());
+ // The next path clears the failure rather than inheriting it.
+ ASSERT_EQ(0, uri.SetH2Path("/s?a=1")) << uri.status();
+
+ brpc::FLAGS_http_max_query_count = 0;
+ ASSERT_EQ(0, uri.SetHttpURL("http://a.com/s?a=1&b=2&c=3&d=4&e=5";)) <<
uri.status();
+ ASSERT_EQ(0, uri.SetH2Path("/s?a=1&b=2&c=3&d=4&e=5")) << uri.status();
+}
+
TEST(URITest, high_bit_bytes) {
// Bytes >= 0x80 (e.g. UTF-8 in the host/path) index the +128-biased
// action table. On unsigned-char platforms they would read past the
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
