diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 22711b6123dd..3ab0f76bfb13 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -17,6 +17,8 @@ Removed Config or Runtime ------------------------- *Normally occurs at the end of the* :ref:`deprecation period ` +* http: removed legacy header sanitization and the runtime guard `envoy.reloadable_features.strict_header_validation`. + New Features ------------ diff --git a/source/common/http/http1/codec_impl.cc b/source/common/http/http1/codec_impl.cc index 96dd8a939ea7..d622fe69016e 100644 --- a/source/common/http/http1/codec_impl.cc +++ b/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; diff --git a/source/common/http/http1/codec_impl.h b/source/common/http/http1/codec_impl.h index d2f8c47d7c8a..994873701f85 100644 --- a/source/common/http/http1/codec_impl.h +++ b/source/common/http/http1/codec_impl.h @@ -253,7 +253,6 @@ class ConnectionImpl : public virtual Connection, protected Logger::LoggablemergeValues( - {{"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) {