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