Adds a mode to HeaderValidator that accepts `obs-text`, and makes it the default for OgHttp2Session. PiperOrigin-RevId: 446033172
diff --git a/quiche/http2/adapter/header_validator.cc b/quiche/http2/adapter/header_validator.cc index 43653a6..cb3edf9 100644 --- a/quiche/http2/adapter/header_validator.cc +++ b/quiche/http2/adapter/header_validator.cc
@@ -40,6 +40,14 @@ } return map; } +CharMap AllowObsText(CharMap map) { + // Characters above 0x80 are allowed in header field values as `obs-text` in + // RFC 7230. + for (char c = 0xff; c >= 0x80; --c) { + map[c] = true; + } + return map; +} bool AllCharsInMap(absl::string_view str, const CharMap& map) { for (char c : str) { @@ -56,10 +64,14 @@ return AllCharsInMap(name, valid_chars); } -bool IsValidHeaderValue(absl::string_view value) { +bool IsValidHeaderValue(absl::string_view value, ObsTextOption option) { static const CharMap valid_chars = BuildValidCharMap(kHttp2HeaderValueAllowedChars); - return AllCharsInMap(value, valid_chars); + static const CharMap valid_chars_with_obs_text = + AllowObsText(BuildValidCharMap(kHttp2HeaderValueAllowedChars)); + return AllCharsInMap(value, option == ObsTextOption::kAllow + ? valid_chars_with_obs_text + : valid_chars); } bool IsValidStatus(absl::string_view status) { @@ -141,7 +153,7 @@ << absl::CEscape(validated_key) << "]"; return HEADER_FIELD_INVALID; } - if (!IsValidHeaderValue(value)) { + if (!IsValidHeaderValue(value, obs_text_option_)) { QUICHE_VLOG(2) << "invalid chars in header value: [" << absl::CEscape(value) << "]"; return HEADER_FIELD_INVALID;
diff --git a/quiche/http2/adapter/header_validator.h b/quiche/http2/adapter/header_validator.h index fb4eecf..c037783 100644 --- a/quiche/http2/adapter/header_validator.h +++ b/quiche/http2/adapter/header_validator.h
@@ -19,11 +19,17 @@ RESPONSE_TRAILER, }; +enum class ObsTextOption : uint8_t { + kAllow, + kDisallow, +}; + class QUICHE_EXPORT_PRIVATE HeaderValidator { public: - HeaderValidator() {} + HeaderValidator() = default; void SetMaxFieldSize(uint32_t field_size) { max_field_size_ = field_size; } + void SetObsTextOption(ObsTextOption option) { obs_text_option_ = option; } // If called, this validator will allow the `:protocol` pseudo-header, as // described in RFC 8441. @@ -59,6 +65,7 @@ std::string path_; absl::optional<size_t> max_field_size_; absl::optional<size_t> content_length_; + ObsTextOption obs_text_option_ = ObsTextOption::kDisallow; bool allow_connect_ = false; };
diff --git a/quiche/http2/adapter/header_validator_test.cc b/quiche/http2/adapter/header_validator_test.cc index 6d25261..444b700 100644 --- a/quiche/http2/adapter/header_validator_test.cc +++ b/quiche/http2/adapter/header_validator_test.cc
@@ -101,6 +101,19 @@ v.ValidateSingleHeader("name", absl::string_view("val\0ue", 6)); EXPECT_EQ(HeaderValidator::HEADER_FIELD_INVALID, status); } + { + // Test that obs-text is disallowed by default. + EXPECT_EQ(HeaderValidator::HEADER_FIELD_INVALID, + v.ValidateSingleHeader("name", "val\xa9ue")); + // Test that obs-text is disallowed when configured. + v.SetObsTextOption(ObsTextOption::kDisallow); + EXPECT_EQ(HeaderValidator::HEADER_FIELD_INVALID, + v.ValidateSingleHeader("name", "val\xa9ue")); + // Test that obs-text is allowed when configured. + v.SetObsTextOption(ObsTextOption::kAllow); + EXPECT_EQ(HeaderValidator::HEADER_OK, + v.ValidateSingleHeader("name", "val\xa9ue")); + } } TEST(HeaderValidatorTest, StatusHasInvalidChar) {
diff --git a/quiche/http2/adapter/nghttp2_adapter_test.cc b/quiche/http2/adapter/nghttp2_adapter_test.cc index f6bf097..5f84e15 100644 --- a/quiche/http2/adapter/nghttp2_adapter_test.cc +++ b/quiche/http2/adapter/nghttp2_adapter_test.cc
@@ -3500,6 +3500,41 @@ SpdyFrameType::PING})); } +TEST(OgHttp2AdapterTest, HeaderValuesWithObsTextAllowed) { + DataSavingVisitor visitor; + auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor); + + const std::string frames = TestFrameSequence() + .ClientPreface() + .Headers(1, + {{":method", "GET"}, + {":scheme", "https"}, + {":authority", "example.com"}, + {":path", "/"}, + {"name", "val\xa1ue"}}, + /*fin=*/true) + .Serialize(); + testing::InSequence s; + + // Client preface (empty SETTINGS) + EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL(visitor, OnSettingsEnd()); + // Stream 1 + EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(1)); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":method", "GET")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/")); + EXPECT_CALL(visitor, OnHeaderForStream(1, "name", "val\xa1ue")); + EXPECT_CALL(visitor, OnEndHeadersForStream(1)); + EXPECT_CALL(visitor, OnEndStream(1)); + + const int64_t result = adapter->ProcessBytes(frames); + EXPECT_EQ(frames.size(), static_cast<size_t>(result)); +} + TEST(NgHttp2AdapterTest, ServerHandlesDataWithPadding) { DataSavingVisitor visitor; auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
diff --git a/quiche/http2/adapter/oghttp2_adapter_test.cc b/quiche/http2/adapter/oghttp2_adapter_test.cc index 81a923b..c98aa26 100644 --- a/quiche/http2/adapter/oghttp2_adapter_test.cc +++ b/quiche/http2/adapter/oghttp2_adapter_test.cc
@@ -66,6 +66,82 @@ TestFrameSequence().ClientPreface().Ping(17).Serialize()); } +TEST(OgHttp2AdapterTest, HeaderValuesWithObsTextAllowedByDefault) { + DataSavingVisitor visitor; + OgHttp2Session::Options options; + options.perspective = Perspective::kServer; + ASSERT_TRUE(options.allow_obs_text); + auto adapter = OgHttp2Adapter::Create(visitor, options); + + const std::string frames = TestFrameSequence() + .ClientPreface() + .Headers(1, + {{":method", "GET"}, + {":scheme", "https"}, + {":authority", "example.com"}, + {":path", "/"}, + {"name", "val\xa1ue"}}, + /*fin=*/true) + .Serialize(); + testing::InSequence s; + + // Client preface (empty SETTINGS) + EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL(visitor, OnSettingsEnd()); + // Stream 1 + EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(1)); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":method", "GET")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/")); + EXPECT_CALL(visitor, OnHeaderForStream(1, "name", "val\xa1ue")); + EXPECT_CALL(visitor, OnEndHeadersForStream(1)); + EXPECT_CALL(visitor, OnEndStream(1)); + + const int64_t result = adapter->ProcessBytes(frames); + EXPECT_EQ(frames.size(), static_cast<size_t>(result)); +} + +TEST(OgHttp2AdapterTest, HeaderValuesWithObsTextDisallowed) { + DataSavingVisitor visitor; + OgHttp2Session::Options options; + options.allow_obs_text = false; + options.perspective = Perspective::kServer; + auto adapter = OgHttp2Adapter::Create(visitor, options); + + const std::string frames = TestFrameSequence() + .ClientPreface() + .Headers(1, + {{":method", "GET"}, + {":scheme", "https"}, + {":authority", "example.com"}, + {":path", "/"}, + {"name", "val\xa1ue"}}, + /*fin=*/true) + .Serialize(); + testing::InSequence s; + + // Client preface (empty SETTINGS) + EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL(visitor, OnSettingsEnd()); + // Stream 1 + EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(1)); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":method", "GET")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/")); + EXPECT_CALL( + visitor, + OnInvalidFrame(1, Http2VisitorInterface::InvalidFrameError::kHttpHeader)); + + const int64_t result = adapter->ProcessBytes(frames); + EXPECT_EQ(frames.size(), static_cast<size_t>(result)); +} + TEST(OgHttp2AdapterTest, InitialSettingsNoExtendedConnect) { DataSavingVisitor client_visitor; OgHttp2Adapter::Options client_options;
diff --git a/quiche/http2/adapter/oghttp2_session.cc b/quiche/http2/adapter/oghttp2_session.cc index af78c76..aa117a6 100644 --- a/quiche/http2/adapter/oghttp2_session.cc +++ b/quiche/http2/adapter/oghttp2_session.cc
@@ -356,6 +356,7 @@ if (options_.max_header_field_size.has_value()) { headers_handler_.SetMaxFieldSize(options_.max_header_field_size.value()); } + headers_handler_.SetAllowObsText(options_.allow_obs_text); } OgHttp2Session::~OgHttp2Session() {}
diff --git a/quiche/http2/adapter/oghttp2_session.h b/quiche/http2/adapter/oghttp2_session.h index 8757f09..916a38f 100644 --- a/quiche/http2/adapter/oghttp2_session.h +++ b/quiche/http2/adapter/oghttp2_session.h
@@ -68,6 +68,9 @@ // in RFC 8441. If true, this endpoint will send the appropriate setting in // initial SETTINGS. bool allow_extended_connect = true; + // Whether to allow `obs-text` (characters from hexadecimal 0x80 to 0xff) in + // header field values. + bool allow_obs_text = true; }; OgHttp2Session(Http2VisitorInterface& visitor, Options options); @@ -271,6 +274,10 @@ void SetMaxFieldSize(uint32_t field_size) { validator_.SetMaxFieldSize(field_size); } + void SetAllowObsText(bool allow) { + validator_.SetObsTextOption(allow ? ObsTextOption::kAllow + : ObsTextOption::kDisallow); + } bool CanReceiveBody() const; private: