diff --git a/source/common/http/header_map_impl.cc b/source/common/http/header_map_impl.cc index dd836c0d381c..e192438992ee 100644 --- a/source/common/http/header_map_impl.cc +++ b/source/common/http/header_map_impl.cc @@ -19,11 +19,11 @@ constexpr size_t MinDynamicCapacity{32}; // This includes the NULL (StringUtil::itoa technically only needs 21). constexpr size_t MaxIntegerLength{32}; -size_t newCapacity(uint32_t existing_capacity, uint32_t size_to_append) { - return (static_cast(existing_capacity) + size_to_append) * 2; +uint64_t newCapacity(uint32_t existing_capacity, uint32_t size_to_append) { + return (static_cast(existing_capacity) + size_to_append) * 2; } -void validateCapacity(size_t new_capacity) { +void validateCapacity(uint64_t new_capacity) { // If the resizing will cause buffer overflow due to hitting uint32_t::max, an OOM is likely // imminent. Fast-fail rather than allow a buffer overflow attack (issue #1421) RELEASE_ASSERT(new_capacity <= std::numeric_limits::max(), @@ -91,10 +91,10 @@ void HeaderString::append(const char* data, uint32_t size) { // Rather than be too clever and optimize this uncommon case, we dynamically // allocate and copy. type_ = Type::Dynamic; - dynamic_capacity_ = - std::max(MinDynamicCapacity, static_cast((string_length_ + size) * 2)); + const uint64_t new_capacity = newCapacity(string_length_, size); + dynamic_capacity_ = std::max(MinDynamicCapacity, new_capacity); if (dynamic_capacity_ != MinDynamicCapacity) { - validateCapacity(newCapacity(string_length_, size)); + validateCapacity(new_capacity); } char* buf = static_cast(malloc(dynamic_capacity_)); RELEASE_ASSERT(buf != nullptr, ""); @@ -115,7 +115,7 @@ void HeaderString::append(const char* data, uint32_t size) { case Type::Dynamic: { // We can get here either because we didn't fit in inline or we are already dynamic. if (type_ == Type::Inline) { - const size_t new_capacity = newCapacity(string_length_, size); + const uint64_t new_capacity = newCapacity(string_length_, size); validateCapacity(new_capacity); buffer_.dynamic_ = static_cast(malloc(new_capacity)); RELEASE_ASSERT(buffer_.dynamic_ != nullptr, ""); @@ -124,7 +124,7 @@ void HeaderString::append(const char* data, uint32_t size) { type_ = Type::Dynamic; } else { if (size + 1 + string_length_ > dynamic_capacity_) { - const size_t new_capacity = newCapacity(string_length_, size); + const uint64_t new_capacity = newCapacity(string_length_, size); validateCapacity(new_capacity); // Need to reallocate. diff --git a/test/common/router/header_formatter_test.cc b/test/common/router/header_formatter_test.cc index b5e60fe6b6b8..e5baa8041e08 100644 --- a/test/common/router/header_formatter_test.cc +++ b/test/common/router/header_formatter_test.cc @@ -163,11 +163,10 @@ TEST_F(RequestInfoHeaderFormatterTest, UserDefinedHeadersConsideredHarmful) { // This must be an inline header to get the append-in-place semantics. const char* header_name = "connection"; Protobuf::RepeatedPtrField to_add; - int loop_factor = 10; - uint64_t length = std::numeric_limits::max() / loop_factor; + const uint32_t num_header_chunks = 10; + const uint64_t length = std::numeric_limits::max() / num_header_chunks; std::string really_long_string(length + 1, 'a'); - std::cerr << "really long string " << really_long_string.size() << std::endl; - for (int i = 0; i < loop_factor; ++i) { + for (uint32_t i = 0; i < num_header_chunks; ++i) { envoy::api::v2::core::HeaderValueOption* header = to_add.Add(); header->mutable_header()->set_key(header_name); header->mutable_header()->set_value(really_long_string); @@ -176,9 +175,9 @@ TEST_F(RequestInfoHeaderFormatterTest, UserDefinedHeadersConsideredHarmful) { HeaderParserPtr req_header_parser = HeaderParser::configure(to_add); - Http::TestHeaderMapImpl headerMap{{":method", "POST"}}; + Http::TestHeaderMapImpl header_map{{":method", "POST"}}; NiceMock request_info; - EXPECT_DEATH(req_header_parser->evaluateHeaders(headerMap, request_info), + EXPECT_DEATH(req_header_parser->evaluateHeaders(header_map, request_info), "Trying to allocate overly large headers."); } @@ -383,14 +382,14 @@ TEST(HeaderParserTest, TestParseInternal) { HeaderParserPtr req_header_parser = HeaderParser::configure(to_add); - Http::TestHeaderMapImpl headerMap{{":method", "POST"}}; - req_header_parser->evaluateHeaders(headerMap, request_info); + Http::TestHeaderMapImpl header_map{{":method", "POST"}}; + req_header_parser->evaluateHeaders(header_map, request_info); std::string descriptor = fmt::format("for test case input: {}", test_case.input_); - EXPECT_TRUE(headerMap.has("x-header")) << descriptor; + EXPECT_TRUE(header_map.has("x-header")) << descriptor; EXPECT_TRUE(test_case.expected_output_) << descriptor; - EXPECT_EQ(test_case.expected_output_.value(), headerMap.get_("x-header")) << descriptor; + EXPECT_EQ(test_case.expected_output_.value(), header_map.get_("x-header")) << descriptor; } } @@ -410,10 +409,10 @@ TEST(HeaderParserTest, EvaluateHeaders) { )EOF"; HeaderParserPtr req_header_parser = HeaderParser::configure(parseRouteFromJson(json).route().request_headers_to_add()); - Http::TestHeaderMapImpl headerMap{{":method", "POST"}}; + Http::TestHeaderMapImpl header_map{{":method", "POST"}}; NiceMock request_info; - req_header_parser->evaluateHeaders(headerMap, request_info); - EXPECT_TRUE(headerMap.has("x-client-ip")); + req_header_parser->evaluateHeaders(header_map, request_info); + EXPECT_TRUE(header_map.has("x-client-ip")); } TEST(HeaderParserTest, EvaluateEmptyHeaders) { @@ -432,15 +431,15 @@ TEST(HeaderParserTest, EvaluateEmptyHeaders) { )EOF"; HeaderParserPtr req_header_parser = HeaderParser::configure(parseRouteFromJson(json).route().request_headers_to_add()); - Http::TestHeaderMapImpl headerMap{{":method", "POST"}}; + Http::TestHeaderMapImpl header_map{{":method", "POST"}}; std::shared_ptr> host( new NiceMock()); NiceMock request_info; auto metadata = std::make_shared(); ON_CALL(request_info, upstreamHost()).WillByDefault(Return(host)); ON_CALL(*host, metadata()).WillByDefault(Return(metadata)); - req_header_parser->evaluateHeaders(headerMap, request_info); - EXPECT_FALSE(headerMap.has("x-key")); + req_header_parser->evaluateHeaders(header_map, request_info); + EXPECT_FALSE(header_map.has("x-key")); } TEST(HeaderParserTest, EvaluateStaticHeaders) { @@ -459,11 +458,11 @@ TEST(HeaderParserTest, EvaluateStaticHeaders) { )EOF"; HeaderParserPtr req_header_parser = HeaderParser::configure(parseRouteFromJson(json).route().request_headers_to_add()); - Http::TestHeaderMapImpl headerMap{{":method", "POST"}}; + Http::TestHeaderMapImpl header_map{{":method", "POST"}}; NiceMock request_info; - req_header_parser->evaluateHeaders(headerMap, request_info); - EXPECT_TRUE(headerMap.has("static-header")); - EXPECT_EQ("static-value", headerMap.get_("static-header")); + req_header_parser->evaluateHeaders(header_map, request_info); + EXPECT_TRUE(header_map.has("static-header")); + EXPECT_EQ("static-value", header_map.get_("static-header")); } TEST(HeaderParserTest, EvaluateCompoundHeaders) { @@ -500,7 +499,7 @@ match: { prefix: "/new_endpoint" } HeaderParserPtr req_header_parser = HeaderParser::configure(parseRouteFromV2Yaml(yaml).route().request_headers_to_add()); - Http::TestHeaderMapImpl headerMap{{":method", "POST"}}; + Http::TestHeaderMapImpl header_map{{":method", "POST"}}; NiceMock request_info; absl::optional protocol = Envoy::Http::Protocol::Http11; ON_CALL(request_info, protocol()).WillByDefault(ReturnPointee(&protocol)); @@ -519,31 +518,31 @@ match: { prefix: "/new_endpoint" } )EOF")); ON_CALL(*host, metadata()).WillByDefault(Return(metadata)); - req_header_parser->evaluateHeaders(headerMap, request_info); + req_header_parser->evaluateHeaders(header_map, request_info); - EXPECT_TRUE(headerMap.has("x-prefix")); - EXPECT_EQ("prefix-127.0.0.1", headerMap.get_("x-prefix")); + EXPECT_TRUE(header_map.has("x-prefix")); + EXPECT_EQ("prefix-127.0.0.1", header_map.get_("x-prefix")); - EXPECT_TRUE(headerMap.has("x-suffix")); - EXPECT_EQ("127.0.0.1-suffix", headerMap.get_("x-suffix")); + EXPECT_TRUE(header_map.has("x-suffix")); + EXPECT_EQ("127.0.0.1-suffix", header_map.get_("x-suffix")); - EXPECT_TRUE(headerMap.has("x-both")); - EXPECT_EQ("prefix-127.0.0.1-suffix", headerMap.get_("x-both")); + EXPECT_TRUE(header_map.has("x-both")); + EXPECT_EQ("prefix-127.0.0.1-suffix", header_map.get_("x-both")); - EXPECT_TRUE(headerMap.has("x-escaping-1")); - EXPECT_EQ("%127.0.0.1%", headerMap.get_("x-escaping-1")); + EXPECT_TRUE(header_map.has("x-escaping-1")); + EXPECT_EQ("%127.0.0.1%", header_map.get_("x-escaping-1")); - EXPECT_TRUE(headerMap.has("x-escaping-2")); - EXPECT_EQ("%%%", headerMap.get_("x-escaping-2")); + EXPECT_TRUE(header_map.has("x-escaping-2")); + EXPECT_EQ("%%%", header_map.get_("x-escaping-2")); - EXPECT_TRUE(headerMap.has("x-multi")); - EXPECT_EQ("HTTP/1.1 from 127.0.0.1", headerMap.get_("x-multi")); + EXPECT_TRUE(header_map.has("x-multi")); + EXPECT_EQ("HTTP/1.1 from 127.0.0.1", header_map.get_("x-multi")); - EXPECT_TRUE(headerMap.has("x-multi-back-to-back")); - EXPECT_EQ("HTTP/1.1127.0.0.1", headerMap.get_("x-multi-back-to-back")); + EXPECT_TRUE(header_map.has("x-multi-back-to-back")); + EXPECT_EQ("HTTP/1.1127.0.0.1", header_map.get_("x-multi-back-to-back")); - EXPECT_TRUE(headerMap.has("x-metadata")); - EXPECT_EQ("value", headerMap.get_("x-metadata")); + EXPECT_TRUE(header_map.has("x-metadata")); + EXPECT_EQ("value", header_map.get_("x-metadata")); } TEST(HeaderParserTest, EvaluateHeadersWithAppendFalse) { @@ -585,29 +584,29 @@ TEST(HeaderParserTest, EvaluateHeadersWithAppendFalse) { HeaderParserPtr req_header_parser = Router::HeaderParser::configure(route_action.request_headers_to_add()); - Http::TestHeaderMapImpl headerMap{ + Http::TestHeaderMapImpl header_map{ {":method", "POST"}, {"static-header", "old-value"}, {"x-client-ip", "0.0.0.0"}}; NiceMock request_info; const SystemTime start_time(std::chrono::microseconds(1522796769123456)); EXPECT_CALL(request_info, startTime()).Times(3).WillRepeatedly(Return(start_time)); - req_header_parser->evaluateHeaders(headerMap, request_info); - EXPECT_TRUE(headerMap.has("static-header")); - EXPECT_EQ("static-value", headerMap.get_("static-header")); - EXPECT_TRUE(headerMap.has("x-client-ip")); - EXPECT_EQ("127.0.0.1", headerMap.get_("x-client-ip")); - EXPECT_TRUE(headerMap.has("x-request-start")); - EXPECT_EQ("1522796769123", headerMap.get_("x-request-start")); - EXPECT_TRUE(headerMap.has("x-request-start-default")); - EXPECT_EQ("2018-04-03T23:06:09.123Z", headerMap.get_("x-request-start-default")); - EXPECT_TRUE(headerMap.has("x-request-start-range")); + req_header_parser->evaluateHeaders(header_map, request_info); + EXPECT_TRUE(header_map.has("static-header")); + EXPECT_EQ("static-value", header_map.get_("static-header")); + EXPECT_TRUE(header_map.has("x-client-ip")); + EXPECT_EQ("127.0.0.1", header_map.get_("x-client-ip")); + EXPECT_TRUE(header_map.has("x-request-start")); + EXPECT_EQ("1522796769123", header_map.get_("x-request-start")); + EXPECT_TRUE(header_map.has("x-request-start-default")); + EXPECT_EQ("2018-04-03T23:06:09.123Z", header_map.get_("x-request-start-default")); + EXPECT_TRUE(header_map.has("x-request-start-range")); EXPECT_EQ("123456000, 1, 12, 123, 1234, 12345, 123456, 1234560, 12345600, 123456000", - headerMap.get_("x-request-start-range")); + header_map.get_("x-request-start-range")); typedef std::map CountMap; CountMap counts; - headerMap.iterate( + header_map.iterate( [](const Http::HeaderEntry& header, void* cb_v) -> Http::HeaderMap::Iterate { CountMap* m = static_cast(cb_v); std::string key = std::string{header.key().c_str()}; @@ -662,29 +661,29 @@ match: { prefix: "/new_endpoint" } const auto route = parseRouteFromV2Yaml(yaml).route(); HeaderParserPtr resp_header_parser = HeaderParser::configure(route.response_headers_to_add(), route.response_headers_to_remove()); - Http::TestHeaderMapImpl headerMap{{":method", "POST"}, {"x-safe", "safe"}, {"x-nope", "nope"}}; + Http::TestHeaderMapImpl header_map{{":method", "POST"}, {"x-safe", "safe"}, {"x-nope", "nope"}}; NiceMock request_info; // Initialize start_time as 2018-04-03T23:06:09.123Z in microseconds. const SystemTime start_time(std::chrono::microseconds(1522796769123456)); EXPECT_CALL(request_info, startTime()).Times(7).WillRepeatedly(Return(start_time)); - resp_header_parser->evaluateHeaders(headerMap, request_info); - EXPECT_TRUE(headerMap.has("x-client-ip")); - EXPECT_TRUE(headerMap.has("x-request-start-multiple")); - EXPECT_TRUE(headerMap.has("x-safe")); - EXPECT_FALSE(headerMap.has("x-nope")); - EXPECT_TRUE(headerMap.has("x-request-start")); - EXPECT_EQ("1522796769.123", headerMap.get_("x-request-start")); + resp_header_parser->evaluateHeaders(header_map, request_info); + EXPECT_TRUE(header_map.has("x-client-ip")); + EXPECT_TRUE(header_map.has("x-request-start-multiple")); + EXPECT_TRUE(header_map.has("x-safe")); + EXPECT_FALSE(header_map.has("x-nope")); + EXPECT_TRUE(header_map.has("x-request-start")); + EXPECT_EQ("1522796769.123", header_map.get_("x-request-start")); EXPECT_EQ("1522796769.123 2018-04-03T23:06:09.123Z 1522796769", - headerMap.get_("x-request-start-multiple")); - EXPECT_TRUE(headerMap.has("x-request-start-f")); - EXPECT_EQ("f", headerMap.get_("x-request-start-f")); - EXPECT_TRUE(headerMap.has("x-request-start-default")); - EXPECT_EQ("2018-04-03T23:06:09.123Z", headerMap.get_("x-request-start-default")); - EXPECT_TRUE(headerMap.has("x-request-start-range")); + header_map.get_("x-request-start-multiple")); + EXPECT_TRUE(header_map.has("x-request-start-f")); + EXPECT_EQ("f", header_map.get_("x-request-start-f")); + EXPECT_TRUE(header_map.has("x-request-start-default")); + EXPECT_EQ("2018-04-03T23:06:09.123Z", header_map.get_("x-request-start-default")); + EXPECT_TRUE(header_map.has("x-request-start-range")); EXPECT_EQ("123456000, 1, 12, 123, 1234, 12345, 123456, 1234560, 12345600, 123456000", - headerMap.get_("x-request-start-range")); + header_map.get_("x-request-start-range")); } } // namespace Router