Add `max_header_bytes` to OgHttp2Session::Options() and send in initial SETTINGS. This configurable option allows OgHttp2Session's HPACK decoder to adjust its decoding buffer size and max allowed header block size in a similar strategy to Http2Dispatcher [1]. This change is important to allow OgHttp2Session to decode headers following Envoy's `max_headers_kb_` [2]. When locally added to Envoy's Http2Options [3], this change allows the following codec_impl_tests to pass: LargeRequestHeadersAtLimitAccepted, LargeRequestHeadersInvokeResetStream, LargeMethodRequestEncode, LargeRequestHeadersOverDefaultCodecLibraryLimit (arguably no longer needed), LargeRequestHeadersAtMaxConfigurable, LargeRequestHeadersAccepted. [1] e.g., http://google3/gfe/gfe2/http2/http2_backend_dispatcher.cc;l=221-226;rcl=414330428 [2] http://google3/third_party/envoy/src/source/common/http/http2/codec_impl.h;l=549;rcl=414330428 [3] http://google3/third_party/envoy/src/source/common/http/http2/codec_impl.h;l=196;rcl=414009764 PiperOrigin-RevId: 415087562
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc index cc0cdac..6235d34 100644 --- a/http2/adapter/oghttp2_adapter_test.cc +++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -64,7 +64,8 @@ TEST_F(OgHttp2AdapterTest, InitialSettings) { DataSavingVisitor client_visitor; - OgHttp2Adapter::Options client_options{.perspective = Perspective::kClient}; + OgHttp2Adapter::Options client_options{.perspective = Perspective::kClient, + .max_header_list_bytes = 42}; auto client_adapter = OgHttp2Adapter::Create(client_visitor, client_options); DataSavingVisitor server_visitor; @@ -74,8 +75,8 @@ testing::InSequence s; // Client sends the connection preface, including the initial SETTINGS. - EXPECT_CALL(client_visitor, OnBeforeFrameSent(SETTINGS, 0, 6, 0x0)); - EXPECT_CALL(client_visitor, OnFrameSent(SETTINGS, 0, 6, 0x0, 0)); + EXPECT_CALL(client_visitor, OnBeforeFrameSent(SETTINGS, 0, 12, 0x0)); + EXPECT_CALL(client_visitor, OnFrameSent(SETTINGS, 0, 12, 0x0, 0)); { int result = client_adapter->Send(); EXPECT_EQ(0, result); @@ -96,7 +97,7 @@ } // Client processes the server's initial bytes, including initial SETTINGS. - EXPECT_CALL(client_visitor, OnFrameHeader(0, 0, SETTINGS, 0)); + EXPECT_CALL(client_visitor, OnFrameHeader(0, 0, SETTINGS, 0x0)); EXPECT_CALL(client_visitor, OnSettingsStart()); EXPECT_CALL(client_visitor, OnSettingsEnd()); { @@ -105,13 +106,14 @@ } // Server processes the client's initial bytes, including initial SETTINGS. - EXPECT_CALL(server_visitor, OnFrameHeader(0, 6, SETTINGS, 0)); + EXPECT_CALL(server_visitor, OnFrameHeader(0, 12, SETTINGS, 0x0)); EXPECT_CALL(server_visitor, OnSettingsStart()); + EXPECT_CALL(server_visitor, + OnSetting(Http2Setting{Http2KnownSettingsId::ENABLE_PUSH, 0u})) + .Times(2); EXPECT_CALL( server_visitor, - OnSetting(testing::AllOf( - testing::Field(&Http2Setting::id, Http2KnownSettingsId::ENABLE_PUSH), - testing::Field(&Http2Setting::value, 0)))) + OnSetting(Http2Setting{Http2KnownSettingsId::MAX_HEADER_LIST_SIZE, 42u})) .Times(2); EXPECT_CALL(server_visitor, OnSettingsEnd()); { @@ -2091,6 +2093,61 @@ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::DATA})); } +TEST(OgHttp2AdapterServerTest, ServerReceivesMoreHeaderBytesThanConfigured) { + DataSavingVisitor visitor; + OgHttp2Adapter::Options options{.perspective = Perspective::kServer, + .max_header_list_bytes = 42}; + auto adapter = OgHttp2Adapter::Create(visitor, options); + EXPECT_FALSE(adapter->want_write()); + + const std::string frames = + TestFrameSequence() + .ClientPreface() + .Headers(1, + {{":method", "GET"}, + {":scheme", "https"}, + {":authority", "example.com"}, + {":path", "/this/is/request/one"}, + {"from-douglas-de-fermat", + "I have discovered a truly marvelous answer to the life, " + "the universe, and everything that the header setting is " + "too narrow to contain."}}, + /*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, 0x5)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(1)); + EXPECT_CALL(visitor, OnConnectionError(ConnectionError::kParseError)); + + int64_t result = adapter->ProcessBytes(frames); + EXPECT_LT(result, 0); + + EXPECT_TRUE(adapter->want_write()); + + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0)); + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0)); + EXPECT_CALL(visitor, OnBeforeFrameSent(GOAWAY, 0, _, 0x0)); + EXPECT_CALL(visitor, + OnFrameSent(GOAWAY, 0, _, 0x0, + static_cast<int>(Http2ErrorCode::COMPRESSION_ERROR))); + + int send_result = adapter->Send(); + // Some bytes should have been serialized. + EXPECT_EQ(0, send_result); + EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS, + spdy::SpdyFrameType::SETTINGS, + spdy::SpdyFrameType::GOAWAY})); +} + TEST(OgHttp2AdapterServerTest, ServerSubmitsResponseWithDataSourceError) { DataSavingVisitor visitor; OgHttp2Adapter::Options options{.perspective = Perspective::kServer};
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc index 19f5dad..82d9d87 100644 --- a/http2/adapter/oghttp2_session.cc +++ b/http2/adapter/oghttp2_session.cc
@@ -279,6 +279,14 @@ options_(options) { decoder_.set_visitor(&receive_logger_); decoder_.set_extension_visitor(this); + if (options_.max_header_list_bytes) { + // Limit buffering of encoded HPACK data to 2x the decoded limit. + decoder_.GetHpackDecoder()->set_max_decode_buffer_size_bytes( + 2 * *options_.max_header_list_bytes); + // Limit the total bytes accepted for HPACK decoding to 4x the limit. + decoder_.GetHpackDecoder()->set_max_header_block_bytes( + 4 * *options_.max_header_list_bytes); + } if (options_.perspective == Perspective::kServer) { remaining_preface_ = {spdy::kHttp2ConnectionHeaderPrefix, spdy::kHttp2ConnectionHeaderPrefixSize}; @@ -1214,6 +1222,10 @@ // TODO(diannahu): Consider applying server push disabling on SETTINGS ack. settings.push_back({Http2KnownSettingsId::ENABLE_PUSH, 0}); } + if (options_.max_header_list_bytes) { + settings.push_back({Http2KnownSettingsId::MAX_HEADER_LIST_SIZE, + *options_.max_header_list_bytes}); + } return settings; }
diff --git a/http2/adapter/oghttp2_session.h b/http2/adapter/oghttp2_session.h index 670b236..fd7bdc2 100644 --- a/http2/adapter/oghttp2_session.h +++ b/http2/adapter/oghttp2_session.h
@@ -40,6 +40,8 @@ Perspective perspective = Perspective::kClient; // The maximum HPACK table size to use. absl::optional<size_t> max_hpack_encoding_table_capacity = absl::nullopt; + // The maximum number of decoded header bytes that a stream can receive. + absl::optional<uint32_t> max_header_list_bytes = absl::nullopt; // Whether to automatically send PING acks when receiving a PING. bool auto_ping_ack = true; // Whether (as server) to send a RST_STREAM NO_ERROR when sending a fin on