diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 3c3e6ba20947..4ef91879bad6 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -85,6 +85,10 @@ minor_behavior_changes: - area: http change: | Removed runtime guard ``envoy.reloadable_features.refresh_rtt_after_request`` and legacy code path. +- area: http + change: | + Changing HTTP/2 semi-colon prefixed headers to being sanitized by Envoy code rather than nghttp2. Should be a functional no-op but + guarded by ``envoy.reloadable_features.sanitize_http2_headers_without_nghttp2``. bug_fixes: # *Changes expected to improve the state of the world and are unlikely to have negative effects* diff --git a/source/common/http/header_utility.cc b/source/common/http/header_utility.cc index 93d682efaa0f..c06a0f68f3b2 100644 --- a/source/common/http/header_utility.cc +++ b/source/common/http/header_utility.cc @@ -201,11 +201,18 @@ bool HeaderUtility::headerValueIsValid(const absl::string_view header_value) { http2::adapter::ObsTextOption::kAllow); } -bool HeaderUtility::headerNameIsValid(const absl::string_view header_key) { +bool HeaderUtility::headerNameIsValid(absl::string_view header_key) { if (!header_key.empty() && header_key[0] == ':') { - // For HTTP/2 pseudo header, use the HTTP/2 semantics for checking validity - return nghttp2_check_header_name(reinterpret_cast(header_key.data()), - header_key.size()) != 0; + if (!Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.sanitize_http2_headers_without_nghttp2")) { + // For HTTP/2 pseudo header, use the HTTP/2 semantics for checking validity + return nghttp2_check_header_name(reinterpret_cast(header_key.data()), + header_key.size()) != 0; + } + header_key.remove_prefix(1); + if (header_key.empty()) { + return false; + } } // For all other header use HTTP/1 semantics. The only difference from HTTP/2 is that // uppercase characters are allowed. This allows HTTP filters to add header with mixed diff --git a/source/common/http/header_utility.h b/source/common/http/header_utility.h index 1a7accd9a868..dcda03881709 100644 --- a/source/common/http/header_utility.h +++ b/source/common/http/header_utility.h @@ -140,7 +140,7 @@ class HeaderUtility { * http://tools.ietf.org/html/rfc7230#section-3.2 * @return bool true if the header name is valid, according to the aforementioned RFC. */ - static bool headerNameIsValid(const absl::string_view header_key); + static bool headerNameIsValid(absl::string_view header_key); /** * Checks if header name contains underscore characters. diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 50bc1655a634..8f39a339e86f 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -85,6 +85,7 @@ RUNTIME_GUARD(envoy_reloadable_features_quic_receive_ecn); // @danzh2010 or @RyanTheOptimist before removing. RUNTIME_GUARD(envoy_reloadable_features_quic_send_server_preferred_address_to_all_clients); RUNTIME_GUARD(envoy_reloadable_features_quic_upstream_reads_fixed_number_packets); +RUNTIME_GUARD(envoy_reloadable_features_sanitize_http2_headers_without_nghttp2); RUNTIME_GUARD(envoy_reloadable_features_sanitize_te); RUNTIME_GUARD(envoy_reloadable_features_send_header_raw_value); RUNTIME_GUARD(envoy_reloadable_features_send_local_reply_when_no_buffer_and_upstream_request); diff --git a/test/common/http/header_utility_test.cc b/test/common/http/header_utility_test.cc index 579340a499c8..971f0e953f68 100644 --- a/test/common/http/header_utility_test.cc +++ b/test/common/http/header_utility_test.cc @@ -1265,6 +1265,28 @@ TEST(HeaderAddTest, HeaderAdd) { }); } +void headerNameIsValid() { + // Isn't valid but passes legacy checks. + EXPECT_FALSE(HeaderUtility::headerNameIsValid(":")); + EXPECT_FALSE(HeaderUtility::headerNameIsValid("::")); + EXPECT_FALSE(HeaderUtility::headerNameIsValid("\n")); + EXPECT_FALSE(HeaderUtility::headerNameIsValid(":\n")); + EXPECT_FALSE(HeaderUtility::headerNameIsValid("asd\n")); + + EXPECT_TRUE(HeaderUtility::headerNameIsValid("asd")); + // Not actually valid but passes legacy Envoy checks. + EXPECT_TRUE(HeaderUtility::headerNameIsValid(":asd")); +} + +TEST(HeaderIsValidTest, HeaderNameIsValid) { headerNameIsValid(); } + +TEST(HeaderIsValidTest, HeaderNameIsValidLegacy) { + TestScopedRuntime scoped_runtime; + scoped_runtime.mergeValues( + {{"envoy.reloadable_features.sanitize_http2_headers_without_nghttp2", "false"}}); + headerNameIsValid(); +} + TEST(HeaderIsValidTest, HeaderNameContainsUnderscore) { EXPECT_FALSE(HeaderUtility::headerNameContainsUnderscore("cookie")); EXPECT_FALSE(HeaderUtility::headerNameContainsUnderscore("x-something"));