codec: Raise max_request_headers_kb limit to 96 KiB

source/common/http/http2/codec_impl.cc

@@ -853,6 +853,8 @@ ConnectionImpl::Http2Options::Http2Options(const Http2Settings& http2_settings)
   // of kept alive HTTP/2 connections.
   nghttp2_option_set_no_closed_streams(options_, 1);
   nghttp2_option_set_no_auto_window_update(options_, 1);
+  nghttp2_option_set_max_send_header_block_length(options_,
+                                                  NGHTTP2_MAX_SEND_HEADER_BLOCK_LENGTH * 1024);
 
   if (http2_settings.hpack_table_size_ != NGHTTP2_DEFAULT_HEADER_TABLE_SIZE) {
     nghttp2_option_set_max_deflate_dynamic_table_size(options_, http2_settings.hpack_table_size_);

source/common/http/http2/codec_impl.h

@@ -33,6 +33,7 @@ const std::string ALPN_STRING = "h2";
 // This is not the full client magic, but it's the smallest size that should be able to
 // differentiate between HTTP/1 and HTTP/2.
 const std::string CLIENT_MAGIC_PREFIX = "PRI * HTTP/2";
+static const uint32_t NGHTTP2_MAX_SEND_HEADER_BLOCK_LENGTH = 100;
 
 /**
  * All stats for the HTTP/2 codec. @see stats_macros.h

test/common/http/http1/codec_impl_test.cc

@@ -1,3 +1,5 @@
+#include <http_parser.h>
+
 #include <memory>
 #include <string>
 
@@ -31,6 +33,7 @@ namespace Http1 {
 class Http1ServerConnectionImplTest : public TestBase {
 public:
   void initialize() {
+    http_parser_set_max_header_size(max_request_headers_kb_ * 1024);
     codec_ = std::make_unique<ServerConnectionImpl>(connection_, callbacks_, codec_settings_);
   }
 
@@ -42,6 +45,9 @@ class Http1ServerConnectionImplTest : public TestBase {
   void expectHeadersTest(Protocol p, bool allow_absolute_url, Buffer::OwnedImpl& buffer,
                          TestHeaderMapImpl& expected_headers);
   void expect400(Protocol p, bool allow_absolute_url, Buffer::OwnedImpl& buffer);
+
+protected:
+  uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_KB};
 };
 
 void Http1ServerConnectionImplTest::expect400(Protocol p, bool allow_absolute_url,
@@ -1007,8 +1013,7 @@ TEST_F(Http1ClientConnectionImplTest, HighwatermarkMultipleResponses) {
       ->onUnderlyingConnectionBelowWriteBufferLowWatermark();
 }
 
-// For issue #1421 regression test that Envoy's HTTP parser applies header limits early.
-TEST_F(Http1ServerConnectionImplTest, TestCodecHeaderLimits) {
+TEST_F(Http1ServerConnectionImplTest, TestLargeHeadersRejected) {
   initialize();
 
   std::string exception_reason;
@@ -1022,16 +1027,31 @@ TEST_F(Http1ServerConnectionImplTest, TestCodecHeaderLimits) {
 
   Buffer::OwnedImpl buffer("GET / HTTP/1.1\r\n");
   codec_->dispatch(buffer);
-  std::string long_string = "foo: " + std::string(1024, 'q') + "\r\n";
-  for (int i = 0; i < 79; ++i) {
-    buffer = Buffer::OwnedImpl(long_string);
-    codec_->dispatch(buffer);
-  }
+  std::string long_string = "big: " + std::string(64 * 1024, 'q') + "\r\n";
   buffer = Buffer::OwnedImpl(long_string);
   EXPECT_THROW_WITH_MESSAGE(codec_->dispatch(buffer), EnvoyException,
                             "http/1.1 protocol error: HPE_HEADER_OVERFLOW");
 }
 
+TEST_F(Http1ServerConnectionImplTest, TestLargeHeadersAcceptedIfConfigured) {
+  max_request_headers_kb_ = 65;
+  initialize();
+
+  NiceMock<Http::MockStreamDecoder> decoder;
+  Http::StreamEncoder* response_encoder = nullptr;
+  EXPECT_CALL(callbacks_, newStream(_, _))
+      .WillOnce(Invoke([&](Http::StreamEncoder& encoder, bool) -> Http::StreamDecoder& {
+        response_encoder = &encoder;
+        return decoder;
+      }));
+
+  Buffer::OwnedImpl buffer("GET / HTTP/1.1\r\n");
+  codec_->dispatch(buffer);
+  std::string long_string = "big: " + std::string(64 * 1024, 'q') + "\r\n";
+  buffer = Buffer::OwnedImpl(long_string);
+  codec_->dispatch(buffer);
+}
+
 } // namespace Http1
 } // namespace Http
 } // namespace Envoy

test/common/http/http2/codec_impl_test.cc

@@ -913,6 +913,54 @@ TEST_P(Http2CodecImplTest, TestLargeHeadersAtLimitAccepted) {
   request_encoder_->encodeHeaders(request_headers, true);
 }
 
+TEST_P(Http2CodecImplTest, TestLargeHeadersOverDefaultCodecLimit) {
+  max_request_headers_kb_ = 100;
+  initialize();
+
+  TestHeaderMapImpl request_headers;
+  HttpTestUtility::addDefaultHeaders(request_headers);
+  std::string long_string = std::string(65 * 1024, 'q');
+  request_headers.addCopy("big", long_string);
+
+  EXPECT_CALL(request_decoder_, decodeHeaders_(_, _)).Times(1);
+  EXPECT_CALL(server_stream_callbacks_, onResetStream(_)).Times(0);
+  request_encoder_->encodeHeaders(request_headers, true);
+}
+
+TEST_P(Http2CodecImplTest, TestManyLargeHeadersWayOverDefaultCodecLimit) {
+  max_request_headers_kb_ = 100;
+  initialize();
+
+  TestHeaderMapImpl request_headers;
+  HttpTestUtility::addDefaultHeaders(request_headers);
+  std::string long_string = std::string(1024, 'q');
+  for (int i = 0; i < 80; i++) {
+    request_headers.addCopy(fmt::format("{}", i), long_string);
+  }
+
+  EXPECT_CALL(request_decoder_, decodeHeaders_(_, _)).Times(1);
+  EXPECT_CALL(server_stream_callbacks_, onResetStream(_)).Times(0);
+  request_encoder_->encodeHeaders(request_headers, true);
+}
+
+TEST_P(Http2CodecImplTest, TestSingleLargeHeadersWayOverDefaultCodecLimit) {
+  // this fails unexpectedly due to the arbitrarily-set NGHTTP2_HD_MAX_NV in lib/nghttp2_hd.h
+  // TODO(auni53) turn up this test once that issue is resolved.
+  return;
+
+  max_request_headers_kb_ = 100;
+  initialize();
+
+  TestHeaderMapImpl request_headers;
+  HttpTestUtility::addDefaultHeaders(request_headers);
+  std::string long_string = std::string(80 * 1024, 'q');
+  request_headers.addCopy("big", long_string);
+
+  EXPECT_CALL(request_decoder_, decodeHeaders_(_, _)).Times(1);
+  EXPECT_CALL(server_stream_callbacks_, onResetStream(_)).Times(0);
+  request_encoder_->encodeHeaders(request_headers, true);
+}
+
 TEST_P(Http2CodecImplTest, TestCodecHeaderCompression) {
   initialize();
 

tools/spelling_dictionary.txt

@@ -427,6 +427,7 @@ gregs
 gzip
 hacky
 handshaker
+hd
 hdr
 healths
 healthz