http: removing old path for strict header validation (envoyproxy#11954)

Risk Level: low
Testing: n/a
Docs Changes: n/a
Release Notes: yes
Fixes envoyproxy#11932
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: scheler <santosh.cheler@appdynamics.com>

docs/root/version_history/current.rst

@@ -17,6 +17,8 @@ Removed Config or Runtime
 -------------------------
 *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`
 
+* http: removed legacy header sanitization and the runtime guard `envoy.reloadable_features.strict_header_validation`.
+
 New Features
 ------------
 

source/common/http/http1/codec_impl.cc

@@ -455,8 +455,6 @@ ConnectionImpl::ConnectionImpl(Network::Connection& connection, CodecStats& stat
     : connection_(connection), stats_(stats),
       header_key_formatter_(std::move(header_key_formatter)), processing_trailers_(false),
       handling_upgrade_(false), reset_stream_called_(false), deferred_end_stream_headers_(false),
-      strict_header_validation_(
-          Runtime::runtimeFeatureEnabled("envoy.reloadable_features.strict_header_validation")),
       connection_header_sanitization_(Runtime::runtimeFeatureEnabled(
           "envoy.reloadable_features.connection_header_sanitization")),
       enable_trailers_(enable_trailers),
@@ -615,13 +613,11 @@ void ConnectionImpl::onHeaderValue(const char* data, size_t length) {
   }
 
   absl::string_view header_value{data, length};
-  if (strict_header_validation_) {
-    if (!Http::HeaderUtility::headerValueIsValid(header_value)) {
-      ENVOY_CONN_LOG(debug, "invalid header value: {}", connection_, header_value);
-      error_code_ = Http::Code::BadRequest;
-      sendProtocolError(Http1ResponseCodeDetails::get().InvalidCharacters);
-      throw CodecProtocolException("http/1.1 protocol error: header value contains invalid chars");
-    }
+  if (!Http::HeaderUtility::headerValueIsValid(header_value)) {
+    ENVOY_CONN_LOG(debug, "invalid header value: {}", connection_, header_value);
+    error_code_ = Http::Code::BadRequest;
+    sendProtocolError(Http1ResponseCodeDetails::get().InvalidCharacters);
+    throw CodecProtocolException("http/1.1 protocol error: header value contains invalid chars");
   }
 
   header_parsing_state_ = HeaderParsingState::Value;

source/common/http/http1/codec_impl.h

@@ -253,7 +253,6 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
   // HTTP/1 message has been flushed from the parser. This allows raising an HTTP/2 style headers
   // block with end stream set to true with no further protocol data remaining.
   bool deferred_end_stream_headers_ : 1;
-  const bool strict_header_validation_ : 1;
   const bool connection_header_sanitization_ : 1;
   const bool enable_trailers_ : 1;
   const bool reject_unsupported_transfer_encodings_ : 1;

source/common/runtime/runtime_features.cc

@@ -55,7 +55,6 @@ constexpr const char* runtime_features[] = {
     // Enabled
     "envoy.reloadable_features.http1_flood_protection",
     "envoy.reloadable_features.test_feature_true",
-    "envoy.reloadable_features.strict_header_validation",
     "envoy.reloadable_features.connection_header_sanitization",
     "envoy.reloadable_features.strict_authority_validation",
     "envoy.reloadable_features.reject_unsupported_transfer_encodings",

test/common/http/http1/codec_impl_test.cc

@@ -1002,34 +1002,11 @@ TEST_F(Http1ServerConnectionImplTest, HostHeaderTranslation) {
   EXPECT_EQ(0U, buffer.length());
 }
 
-// Ensures that requests with invalid HTTP header values are not rejected
-// when the runtime guard is not enabled for the feature.
-TEST_F(Http1ServerConnectionImplTest, HeaderInvalidCharsRuntimeGuard) {
-  TestScopedRuntime scoped_runtime;
-  // When the runtime-guarded feature is NOT enabled, invalid header values
-  // should be accepted by the codec.
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.strict_header_validation", "false"}});
-
-  initialize();
-
-  MockRequestDecoder decoder;
-  EXPECT_CALL(callbacks_, newStream(_, _)).WillOnce(ReturnRef(decoder));
-
-  Buffer::OwnedImpl buffer(
-      absl::StrCat("GET / HTTP/1.1\r\nHOST: h.com\r\nfoo: ", std::string(1, 3), "\r\n"));
-  auto status = codec_->dispatch(buffer);
-  EXPECT_TRUE(status.ok());
-}
-
 // Ensures that requests with invalid HTTP header values are properly rejected
-// when the runtime guard is enabled for the feature.
 TEST_F(Http1ServerConnectionImplTest, HeaderInvalidCharsRejection) {
   TestScopedRuntime scoped_runtime;
   // When the runtime-guarded feature is enabled, invalid header values
   // should result in a rejection.
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.strict_header_validation", "true"}});
 
   initialize();
 
@@ -1140,27 +1117,6 @@ TEST_F(Http1ServerConnectionImplTest, HeaderInvalidAuthority) {
   EXPECT_EQ("http.invalid_authority", response_encoder->getStream().responseDetails());
 }
 
-// Regression test for http-parser allowing embedded NULs in header values,
-// verify we reject them.
-TEST_F(Http1ServerConnectionImplTest, HeaderEmbeddedNulRejection) {
-  TestScopedRuntime scoped_runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.strict_header_validation", "false"}});
-  initialize();
-
-  InSequence sequence;
-
-  MockRequestDecoder decoder;
-  EXPECT_CALL(callbacks_, newStream(_, _)).WillOnce(ReturnRef(decoder));
-
-  Buffer::OwnedImpl buffer(
-      absl::StrCat("GET / HTTP/1.1\r\nHOST: h.com\r\nfoo: bar", std::string(1, '\0'), "baz\r\n"));
-  EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _));
-  auto status = codec_->dispatch(buffer);
-  EXPECT_TRUE(isCodecProtocolError(status));
-  EXPECT_EQ(status.message(), "http/1.1 protocol error: HPE_INVALID_HEADER_TOKEN");
-}
-
 // Mutate an HTTP GET with embedded NULs, this should always be rejected in some
 // way (not necessarily with "head value contains NUL" though).
 TEST_F(Http1ServerConnectionImplTest, HeaderMutateEmbeddedNul) {