HTTP: add allow_obs_text configuration to HTTP/2 and HTTP/3 protocol options

api/envoy/config/core/v3/protocol.proto

@@ -541,7 +541,7 @@ message KeepaliveSettings {
       [(validate.rules).duration = {gte {nanos: 1000000}}];
 }
 
-// [#next-free-field: 20]
+// [#next-free-field: 21]
 message Http2ProtocolOptions {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.api.v2.core.Http2ProtocolOptions";
@@ -774,6 +774,10 @@ message Http2ProtocolOptions {
   //   request failures.
   google.protobuf.UInt32Value max_header_field_size_kb = 19
       [(validate.rules).uint32 = {lte: 256 gte: 64}];
+
+  // Whether to disallow obsolete text in header field values.
+  // If not set, it defaults to false.
+  bool disallow_obs_text = 20;
 }
 
 // [#not-implemented-hide:]
@@ -785,7 +789,7 @@ message GrpcProtocolOptions {
 }
 
 // A message which allows using HTTP/3.
-// [#next-free-field: 9]
+// [#next-free-field: 10]
 message Http3ProtocolOptions {
   QuicProtocolOptions quic_protocol_options = 1;
 
@@ -827,6 +831,10 @@ message Http3ProtocolOptions {
   // Disables connection level flow control for HTTP/3 streams. This is useful in situations where the streams share the same connection
   // but originate from different end-clients, so that each stream can make progress independently at non-front-line proxies.
   bool disable_connection_flow_control_for_streams = 8;
+
+  // Whether to disallow obsolete text in header field values.
+  // If not set, it defaults to false for alignment with HTTP/2.
+  bool disallow_obs_text = 9;
 }
 
 // A message to control transformations to the :scheme header

source/common/http/http2/codec_impl.cc

@@ -2082,6 +2082,7 @@ ConnectionImpl::Http2Options::Http2Options(
   og_options_.max_header_field_size = max_headers_kb * 1024;
   og_options_.allow_extended_connect = http2_options.allow_connect();
   og_options_.allow_different_host_and_authority = true;
+  og_options_.allow_obs_text = !http2_options.disallow_obs_text();
   if (!PROTOBUF_GET_WRAPPED_OR_DEFAULT(http2_options, enable_huffman_encoding, true)) {
     if (http2_options.has_hpack_table_size() && http2_options.hpack_table_size().value() == 0) {
       og_options_.compression_option = http2::adapter::OgHttp2Session::Options::DISABLE_COMPRESSION;

source/common/quic/envoy_quic_stream.h

@@ -9,6 +9,7 @@
 #include "envoy/http/codec.h"
 
 #include "source/common/http/codec_helper.h"
+#include "source/common/protobuf/utility.h"
 #include "source/common/quic/envoy_quic_simulated_watermark_buffer.h"
 #include "source/common/quic/envoy_quic_utils.h"
 
@@ -50,6 +51,9 @@ class EnvoyQuicStream : public virtual Http::StreamEncoder,
     if (http3_options_.disable_connection_flow_control_for_streams()) {
       quic_stream_.DisableConnectionFlowControlForThisStream();
     }
+    if (!http3_options_.disallow_obs_text()) {
+      header_validator_.SetObsTextOption(http2::adapter::ObsTextOption::kAllow);
+    }
   }
 
   ~EnvoyQuicStream() override = default;

test/common/http/http2/codec_impl_test.cc

@@ -135,6 +135,12 @@ class Http2CodecImplTestFixture {
     return stream_ids;
   }
 
+  static std::unique_ptr<ConnectionImpl::Http2Options>
+  createHttp2Options(const envoy::config::core::v3::Http2ProtocolOptions& http2_options,
+                     uint32_t max_headers_kb) {
+    return std::make_unique<ConnectionImpl::Http2Options>(http2_options, max_headers_kb);
+  }
+
   static Http2SettingsTuple smallWindowHttp2Settings() {
     return std::make_tuple(CommonUtility::OptionsLimits::DEFAULT_HPACK_TABLE_SIZE,
                            CommonUtility::OptionsLimits::DEFAULT_MAX_CONCURRENT_STREAMS,
@@ -211,11 +217,13 @@ class Http2CodecImplTestFixture {
 
     // Ensure that tests driveToCompletion(). Some tests set `expect_buffered_data_on_teardown_` to
     // indicate that they purposefully leave buffered data.
-    if (expect_buffered_data_on_teardown_) {
-      EXPECT_TRUE(client_wrapper_->buffer_.length() > 0 || server_wrapper_->buffer_.length() > 0);
-    } else {
-      EXPECT_EQ(client_wrapper_->buffer_.length(), 0);
-      EXPECT_EQ(server_wrapper_->buffer_.length(), 0);
+    if (client_wrapper_ != nullptr && server_wrapper_ != nullptr) {
+      if (expect_buffered_data_on_teardown_) {
+        EXPECT_TRUE(client_wrapper_->buffer_.length() > 0 || server_wrapper_->buffer_.length() > 0);
+      } else {
+        EXPECT_EQ(client_wrapper_->buffer_.length(), 0);
+        EXPECT_EQ(server_wrapper_->buffer_.length(), 0);
+      }
     }
   }
 
@@ -570,6 +578,59 @@ class Http2CodecImplTest : public ::testing::TestWithParam<Http2SettingsTestPara
   }
 };
 
+TEST_P(Http2CodecImplTest, DisallowObsTextBehaviorDisallow) {
+  if (skipForUhv() || http2_implementation_ != Http2Impl::Oghttp2) {
+    return;
+  }
+
+  // With disallow_obs_text = true, the request is rejected.
+  server_http2_options_.set_disallow_obs_text(true);
+  initialize();
+
+  InSequence s;
+  TestRequestHeaderMapImpl request_headers;
+  HttpTestUtility::addDefaultHeaders(request_headers);
+
+  HeaderString header_name;
+  header_name.setCopy("custom-header");
+  HeaderString header_value;
+  setHeaderStringUnvalidated(header_value, "foo\x80");
+  request_headers.addViaMove(std::move(header_name), std::move(header_value));
+
+  // We don't expect onResetStream because the error might be detected before the stream is fully
+  // established on the server.
+  EXPECT_TRUE(request_encoder_->encodeHeaders(request_headers, true).ok());
+  driveToCompletion();
+  EXPECT_FALSE(server_wrapper_->status_.ok());
+  // Drain the buffer as we expect a connection error and some data might be left.
+  server_wrapper_->buffer_.drain(server_wrapper_->buffer_.length());
+}
+
+TEST_P(Http2CodecImplTest, DisallowObsTextBehaviorAllow) {
+  if (skipForUhv() || http2_implementation_ != Http2Impl::Oghttp2) {
+    return;
+  }
+
+  // With disallow_obs_text = false, the request is accepted.
+  server_http2_options_.set_disallow_obs_text(false);
+  initialize();
+
+  InSequence s;
+  TestRequestHeaderMapImpl request_headers;
+  HttpTestUtility::addDefaultHeaders(request_headers);
+
+  HeaderString header_name;
+  header_name.setCopy("custom-header");
+  HeaderString header_value;
+  setHeaderStringUnvalidated(header_value, "foo\x80");
+  request_headers.addViaMove(std::move(header_name), std::move(header_value));
+
+  EXPECT_CALL(request_decoder_, decodeHeaders_(_, true));
+  EXPECT_TRUE(request_encoder_->encodeHeaders(request_headers, true).ok());
+  driveToCompletion();
+  EXPECT_TRUE(server_wrapper_->status_.ok());
+}
+
 TEST_P(Http2CodecImplTest, SimpleRequestResponse) {
   initialize();
 

test/common/quic/envoy_quic_server_stream_test.cc

@@ -965,5 +965,30 @@ TEST_F(EnvoyQuicServerStreamTest, DuplicatedPathHeader) {
   quic_stream_->OnStreamHeaderList(true, 0, header_list);
 }
 
+TEST_F(EnvoyQuicServerStreamTest, DisallowObsTextBehavior) {
+  EXPECT_CALL(stream_callbacks_, onResetStream(_, _)).Times(testing::AnyNumber());
+  {
+    envoy::config::core::v3::Http3ProtocolOptions options;
+    options.set_disallow_obs_text(true);
+    auto stream = std::make_unique<EnvoyQuicServerStream>(
+        stream_id_ + 16, &quic_session_, quic::BIDIRECTIONAL, stats_, options,
+        envoy::config::core::v3::HttpProtocolOptions::ALLOW);
+    // \x80 is obsolete text
+    EXPECT_EQ(Http::HeaderUtility::HeaderValidationResult::REJECT,
+              stream->validateHeader("custom-header", "foo\x80"));
+    EXPECT_EQ(Http::HeaderUtility::HeaderValidationResult::ACCEPT,
+              stream->validateHeader("custom-header", "foo"));
+  }
+  {
+    envoy::config::core::v3::Http3ProtocolOptions options;
+    options.set_disallow_obs_text(false);
+    auto stream = std::make_unique<EnvoyQuicServerStream>(
+        stream_id_ + 20, &quic_session_, quic::BIDIRECTIONAL, stats_, options,
+        envoy::config::core::v3::HttpProtocolOptions::ALLOW);
+    EXPECT_EQ(Http::HeaderUtility::HeaderValidationResult::ACCEPT,
+              stream->validateHeader("custom-header", "foo\x80"));
+  }
+}
+
 } // namespace Quic
 } // namespace Envoy

test/integration/default_header_validator_integration_test.cc

@@ -79,8 +79,10 @@ class DownstreamUhvIntegrationTest : public HttpProtocolIntegrationTest {
   std::string additionallyAllowedCharactersInUrlPath() {
     // All codecs allow the following characters that are outside of RFC "<>[]^`{}\|
     std::string additionally_allowed_characters(R"--("<>[]^`{}\|)--");
-    if (downstream_protocol_ == Http::CodecType::HTTP2) {
-      // Both nghttp2 and oghttp2 allow extended ASCII >= 0x80 in path
+    if (downstream_protocol_ == Http::CodecType::HTTP2 ||
+        downstream_protocol_ == Http::CodecType::HTTP3) {
+      // Both nghttp2 and oghttp2 allow extended ASCII >= 0x80 in path.
+      // HTTP/3 also defaults to allowing it for alignment with HTTP/2.
       additionally_allowed_characters += generateExtendedAsciiString();
     }
     return additionally_allowed_characters;