diff --git a/src/brpc/details/http_message.cpp b/src/brpc/details/http_message.cpp index 0cb4f78378..14a81e553c 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 b20a05727c..6d9b2b8714 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 6c58f71aa5..64c9864d68 100644 --- a/src/brpc/policy/http2_rpc_protocol.h +++ b/src/brpc/policy/http2_rpc_protocol.h @@ -234,7 +234,9 @@ class H2StreamContext : public HttpContext { // 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 283473df9e..a0b49662fe 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 2881a8e54a..326bdb0039 100644 --- a/src/brpc/uri.cpp +++ b/src/brpc/uri.cpp @@ -17,9 +17,8 @@ #include // isalnum - #include - +#include #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 an 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 7edac4002d..a42cf88f12 100644 --- a/src/brpc/uri.h +++ b/src/brpc/uri.h @@ -56,7 +56,7 @@ class URI { // You can copy a URI. URI(); - ~URI(); + ~URI() = default; // Exchange internal fields with another URI. void Swap(URI &rhs); @@ -99,8 +99,9 @@ class URI { 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 57e98ccab0..90c9dbddae 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 f23bbfb73a..0033a0d947 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; } @@ -698,10 +700,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\"}"; @@ -860,11 +862,11 @@ class DownloadServiceImpl : public ::test::DownloadService { }; 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); @@ -884,11 +886,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; @@ -1007,10 +1009,10 @@ TEST_F(HttpTest, read_long_body_progressively) { std::numeric_limits::max()); butil::intrusive_ptr 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; @@ -1053,12 +1055,12 @@ TEST_F(HttpTest, read_long_body_progressively) { TEST_F(HttpTest, read_short_body_progressively) { butil::intrusive_ptr 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; @@ -1090,11 +1092,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; @@ -1119,11 +1121,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 reader(new TimeoutReadBody); { @@ -1153,11 +1155,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 reader(new TimeoutReadBody); { @@ -1186,11 +1188,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; @@ -1216,11 +1218,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; @@ -1245,10 +1247,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; @@ -1276,10 +1278,10 @@ TEST_F(HttpTest, read_progressively_after_cntl_destroys) { std::numeric_limits::max()); butil::intrusive_ptr 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; @@ -1322,10 +1324,10 @@ TEST_F(HttpTest, read_progressively_after_long_delay) { DownloadServiceImpl svc(DONE_BEFORE_CREATE_PA, std::numeric_limits::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; @@ -1370,10 +1372,10 @@ TEST_F(HttpTest, read_progressively_after_long_delay) { TEST_F(HttpTest, skip_progressive_reading) { DownloadServiceImpl svc(DONE_BEFORE_CREATE_PA, std::numeric_limits::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; @@ -1409,12 +1411,12 @@ class AlwaysFailRead : public brpc::ProgressiveReader { }; TEST_F(HttpTest, failed_on_read_one_part) { - const int port = 8923; brpc::Server server; DownloadServiceImpl svc(DONE_BEFORE_CREATE_PA, std::numeric_limits::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; @@ -1435,12 +1437,12 @@ TEST_F(HttpTest, failed_on_read_one_part) { TEST_F(HttpTest, broken_socket_stops_progressive_reading) { butil::intrusive_ptr reader; - const int port = 8923; brpc::Server server; DownloadServiceImpl svc(DONE_BEFORE_CREATE_PA, std::numeric_limits::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; @@ -1558,14 +1560,14 @@ class UploadServiceImpl : public ::test::UploadService { }; 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; @@ -1597,14 +1599,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; @@ -1635,10 +1637,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; @@ -1940,6 +1942,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 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 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 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 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 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 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 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 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 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 @@ -2098,29 +2325,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"; @@ -2357,7 +2584,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); @@ -2394,23 +2622,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; @@ -2454,10 +2691,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; @@ -2526,10 +2763,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; @@ -2564,10 +2801,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; @@ -2641,11 +2878,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; @@ -2769,8 +3006,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 b9d6b6508e..b1be2c55b3 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 #include #include "brpc/uri.h" +namespace brpc { +DECLARE_uint32(http_max_query_count); +} + TEST(URITest, everything) { brpc::URI uri; std::string uri_str = " foobar://user:passwd@www.baidu.com: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