diff --git a/source/extensions/http/header_validators/envoy_default/header_validator.cc b/source/extensions/http/header_validators/envoy_default/header_validator.cc index 9d1a848849b0a..78cf8fc724424 100644 --- a/source/extensions/http/header_validators/envoy_default/header_validator.cc +++ b/source/extensions/http/header_validators/envoy_default/header_validator.cc @@ -8,6 +8,7 @@ #include "source/common/runtime/runtime_features.h" #include "source/extensions/http/header_validators/envoy_default/character_tables.h" +#include "absl/container/flat_hash_set.h" #include "absl/container/node_hash_set.h" #include "absl/strings/match.h" @@ -634,19 +635,23 @@ void HeaderValidator::sanitizeHeadersWithUnderscores(::Envoy::Http::HeaderMap& h return; } - std::vector drop_headers; + // Store owning copies because removing one duplicate header can free the + // underlying storage for another matching entry. Using absl::string_view + // would still be unsafe here as the view may dangle after the first removal. + // We only need unique names. + absl::flat_hash_set drop_headers; header_map.iterate([&drop_headers](const ::Envoy::Http::HeaderEntry& header_entry) -> ::Envoy::Http::HeaderMap::Iterate { const absl::string_view header_name = header_entry.key().getStringView(); if (absl::StrContains(header_name, '_')) { - drop_headers.push_back(header_name); + drop_headers.emplace(header_name); } return ::Envoy::Http::HeaderMap::Iterate::Continue; }); ASSERT(drop_headers.empty() || underscore_action == HeaderValidatorConfig::DROP_HEADER); - for (auto& name : drop_headers) { + for (const auto& name : drop_headers) { stats_.incDroppedHeadersWithUnderscores(); header_map.remove(::Envoy::Http::LowerCaseString(name)); } diff --git a/test/extensions/http/header_validators/envoy_default/http1_header_validator_test.cc b/test/extensions/http/header_validators/envoy_default/http1_header_validator_test.cc index e8a6ad73950df..f5324eb6fd474 100644 --- a/test/extensions/http/header_validators/envoy_default/http1_header_validator_test.cc +++ b/test/extensions/http/header_validators/envoy_default/http1_header_validator_test.cc @@ -497,6 +497,23 @@ TEST_F(Http1HeaderValidatorTest, ValidateRequestHeaderMapDropUnderscoreHeaders) {{":scheme", "https"}, {":method", "GET"}, {":path", "/"}, {":authority", "envoy.com"}})); } +TEST_F(Http1HeaderValidatorTest, ValidateRequestHeaderMapDropDuplicateUnderscoreHeaders) { + ::Envoy::Http::TestRequestHeaderMapImpl headers{{":scheme", "https"}, + {":method", "GET"}, + {":path", "/"}, + {":authority", "envoy.com"}, + {"x_foo", "bar"}}; + headers.addCopy("x_foo", "baz"); + auto uhv = createH1(drop_headers_with_underscores_config); + + EXPECT_ACCEPT(uhv->validateRequestHeaders(headers)); + EXPECT_ACCEPT(uhv->transformRequestHeaders(headers)); + EXPECT_EQ( + headers, + TestRequestHeaderMapImpl( + {{":scheme", "https"}, {":method", "GET"}, {":path", "/"}, {":authority", "envoy.com"}})); +} + TEST_F(Http1HeaderValidatorTest, RejectUnderscoreHeadersFromRequestHeadersWhenConfigured) { ::Envoy::Http::TestRequestHeaderMapImpl headers{{":scheme", "https"}, {":method", "GET"}, diff --git a/test/extensions/http/header_validators/envoy_default/http2_header_validator_test.cc b/test/extensions/http/header_validators/envoy_default/http2_header_validator_test.cc index 75a817eb3745c..58094adcfbce7 100644 --- a/test/extensions/http/header_validators/envoy_default/http2_header_validator_test.cc +++ b/test/extensions/http/header_validators/envoy_default/http2_header_validator_test.cc @@ -260,6 +260,23 @@ TEST_F(Http2HeaderValidatorTest, ValidateRequestHeaderMapDropUnderscoreHeaders) {{":scheme", "https"}, {":method", "GET"}, {":path", "/"}, {":authority", "envoy.com"}})); } +TEST_F(Http2HeaderValidatorTest, ValidateRequestHeaderMapDropDuplicateUnderscoreHeaders) { + ::Envoy::Http::TestRequestHeaderMapImpl headers{{":scheme", "https"}, + {":method", "GET"}, + {":path", "/"}, + {":authority", "envoy.com"}, + {"x_foo", "bar"}}; + headers.addCopy("x_foo", "baz"); + auto uhv = createH2ServerUhv(drop_headers_with_underscores_config); + + EXPECT_ACCEPT(uhv->validateRequestHeaders(headers)); + EXPECT_ACCEPT(uhv->transformRequestHeaders(headers)); + EXPECT_EQ( + headers, + ::Envoy::Http::TestRequestHeaderMapImpl( + {{":scheme", "https"}, {":method", "GET"}, {":path", "/"}, {":authority", "envoy.com"}})); +} + TEST_F(Http2HeaderValidatorTest, ValidateRequestHeaderMapRejectUnderscoreHeaders) { ::Envoy::Http::TestRequestHeaderMapImpl headers{{":scheme", "https"}, {":method", "GET"},