test: moving per connection arguments to be per upstream (#15518)

As quic connections are created early, we can't set header limits in waitForHttpConnection
Part of #14829

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

test/extensions/filters/network/sni_dynamic_forward_proxy/proxy_filter_integration_test.cc

@@ -117,9 +117,7 @@ TEST_P(SniDynamicProxyFilterIntegrationTest, UpstreamTls) {
 
   codec_client_ = makeHttpConnection(
       makeSslClientConnection(Ssl::ClientSslTransportOptions().setSni("localhost")));
-  ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(
-      *dispatcher_, fake_upstream_connection_, TestUtility::DefaultTimeout, max_request_headers_kb_,
-      max_request_headers_count_));
+  ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_));
 
   const Http::TestRequestHeaderMapImpl request_headers{
       {":method", "POST"},

test/integration/base_integration_test.h

@@ -376,6 +376,14 @@ class BaseIntegrationTest : protected Logger::Loggable<Logger::Id::testing> {
   bool enableHalfClose() { return upstream_config_.enable_half_close_; }
 
   const FakeUpstreamConfig& upstreamConfig() { return upstream_config_; }
+  void setMaxRequestHeadersKb(uint32_t value) { upstream_config_.max_request_headers_kb_ = value; }
+  void setMaxRequestHeadersCount(uint32_t value) {
+    upstream_config_.max_request_headers_count_ = value;
+  }
+  void setHeadersWithUnderscoreAction(
+      envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction value) {
+    upstream_config_.headers_with_underscores_action_ = value;
+  }
 
   void setServerBufferFactory(Buffer::WatermarkFactorySharedPtr proxy_buffer_factory) {
     ASSERT(!test_server_, "Proxy buffer factory must be set before test server creation");

test/integration/eds_integration_test.cc

@@ -182,8 +182,7 @@ TEST_P(EdsIntegrationTest, Http2HcClusterRewarming) {
 
   // We need to do a bunch of work to get a hold of second hc connection.
   FakeHttpConnectionPtr fake_upstream_connection;
-  auto result = fake_upstreams_[0]->waitForHttpConnection(
-      *dispatcher_, fake_upstream_connection, TestUtility::DefaultTimeout, max_request_headers_kb_);
+  auto result = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection);
   RELEASE_ASSERT(result, result.message());
 
   FakeStreamPtr upstream_request;

test/integration/fake_upstream.cc

@@ -321,6 +321,7 @@ FakeHttpConnection::FakeHttpConnection(
     envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
         headers_with_underscores_action)
     : FakeConnectionBase(shared_connection, time_system), type_(type) {
+  ASSERT(max_request_headers_count != 0);
   if (type == Type::HTTP1) {
     Http::Http1Settings http1_settings;
     // For the purpose of testing, we always have the upstream encode the trailers if any
@@ -516,7 +517,7 @@ FakeUpstream::FakeUpstream(Network::TransportSocketFactoryPtr&& transport_socket
       socket_factory_(std::make_shared<FakeListenSocketFactory>(socket_)),
       api_(Api::createApiForTest(stats_store_)), time_system_(config.time_system_),
       dispatcher_(api_->allocateDispatcher("fake_upstream")),
-      handler_(new Server::ConnectionHandlerImpl(*dispatcher_, 0)),
+      handler_(new Server::ConnectionHandlerImpl(*dispatcher_, 0)), config_(config),
       read_disable_on_new_connection_(true), enable_half_close_(config.enable_half_close_),
       listener_(*this, http_type_ == FakeHttpConnection::Type::HTTP3),
       filter_chain_(Network::Test::createEmptyFilterChain(std::move(transport_socket_factory))) {
@@ -555,12 +556,10 @@ bool FakeUpstream::createNetworkFilterChain(Network::Connection& connection,
   // Normally we don't associate a logical network connection with a FakeHttpConnection  until
   // waitForHttpConnection is called, but QUIC needs to be set up as packets come in, so we do
   // not lazily create for HTTP/3
-  // TODO(#14829) handle the case where waitForHttpConnection is called with
-  // non-default arguments for max request headers and protocol options.
   if (http_type_ == FakeHttpConnection::Type::HTTP3) {
     quic_connections_.push_back(std::make_unique<FakeHttpConnection>(
-        *this, consumeConnection(), http_type_, time_system_, Http::DEFAULT_MAX_REQUEST_HEADERS_KB,
-        Http::DEFAULT_MAX_HEADERS_COUNT, envoy::config::core::v3::HttpProtocolOptions::ALLOW));
+        *this, consumeConnection(), http_type_, time_system_, config_.max_request_headers_kb_,
+        config_.max_request_headers_count_, config_.headers_with_underscores_action_));
     quic_connections_.back()->initialize();
   }
   return true;
@@ -585,11 +584,9 @@ void FakeUpstream::threadRoutine() {
   }
 }
 
-AssertionResult FakeUpstream::waitForHttpConnection(
-    Event::Dispatcher& client_dispatcher, FakeHttpConnectionPtr& connection, milliseconds timeout,
-    uint32_t max_request_headers_kb, uint32_t max_request_headers_count,
-    envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
-        headers_with_underscores_action) {
+AssertionResult FakeUpstream::waitForHttpConnection(Event::Dispatcher& client_dispatcher,
+                                                    FakeHttpConnectionPtr& connection,
+                                                    milliseconds timeout) {
   {
     absl::MutexLock lock(&lock_);
 
@@ -621,12 +618,11 @@ AssertionResult FakeUpstream::waitForHttpConnection(
       return AssertionFailure() << "Timed out waiting for new connection.";
     }
   }
-
   return runOnDispatcherThreadAndWait([&]() {
     absl::MutexLock lock(&lock_);
     connection = std::make_unique<FakeHttpConnection>(
-        *this, consumeConnection(), http_type_, time_system_, max_request_headers_kb,
-        max_request_headers_count, headers_with_underscores_action);
+        *this, consumeConnection(), http_type_, time_system_, config_.max_request_headers_kb_,
+        config_.max_request_headers_count_, config_.headers_with_underscores_action_);
     connection->initialize();
     return AssertionSuccess();
   });

test/integration/fake_upstream.h

@@ -544,6 +544,10 @@ struct FakeUpstreamConfig {
   bool enable_half_close_{};
   absl::optional<UdpConfig> udp_fake_upstream_;
   envoy::config::core::v3::Http2ProtocolOptions http2_options_;
+  uint32_t max_request_headers_kb_ = Http::DEFAULT_MAX_REQUEST_HEADERS_KB;
+  uint32_t max_request_headers_count_ = Http::DEFAULT_MAX_HEADERS_COUNT;
+  envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
+      headers_with_underscores_action_ = envoy::config::core::v3::HttpProtocolOptions::ALLOW;
 };
 
 /**
@@ -577,13 +581,9 @@ class FakeUpstream : Logger::Loggable<Logger::Id::testing>,
 
   // Returns the new connection via the connection argument.
   ABSL_MUST_USE_RESULT
-  testing::AssertionResult waitForHttpConnection(
-      Event::Dispatcher& client_dispatcher, FakeHttpConnectionPtr& connection,
-      std::chrono::milliseconds timeout = TestUtility::DefaultTimeout,
-      uint32_t max_request_headers_kb = Http::DEFAULT_MAX_REQUEST_HEADERS_KB,
-      uint32_t max_request_headers_count = Http::DEFAULT_MAX_HEADERS_COUNT,
-      envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
-          headers_with_underscores_action = envoy::config::core::v3::HttpProtocolOptions::ALLOW);
+  testing::AssertionResult
+  waitForHttpConnection(Event::Dispatcher& client_dispatcher, FakeHttpConnectionPtr& connection,
+                        std::chrono::milliseconds timeout = TestUtility::DefaultTimeout);
 
   ABSL_MUST_USE_RESULT
   testing::AssertionResult
@@ -796,6 +796,7 @@ class FakeUpstream : Logger::Loggable<Logger::Id::testing>,
   // consumed_connections_. This allows later the Connection destruction (when the FakeUpstream is
   // deleted) on the same thread that allocated the connection.
   std::list<SharedConnectionWrapperPtr> consumed_connections_ ABSL_GUARDED_BY(lock_);
+  const FakeUpstreamConfig config_;
   bool read_disable_on_new_connection_;
   const bool enable_half_close_;
   FakeListener listener_;

test/integration/http_integration.cc

@@ -383,8 +383,7 @@ absl::optional<uint64_t> HttpIntegrationTest::waitForNextUpstreamConnection(
   while (!result) {
     upstream_index = upstream_index % upstream_indices.size();
     result = fake_upstreams_[upstream_indices[upstream_index]]->waitForHttpConnection(
-        *dispatcher_, fake_upstream_connection, std::chrono::milliseconds(5),
-        max_request_headers_kb_, max_request_headers_count_);
+        *dispatcher_, fake_upstream_connection, std::chrono::milliseconds(5));
     if (result) {
       return upstream_index;
     } else if (!bound.withinBound()) {
@@ -1053,7 +1052,7 @@ void HttpIntegrationTest::testLargeRequestUrl(uint32_t url_size, uint32_t max_he
   config_helper_.addConfigModifier(
       [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
               hcm) -> void { hcm.mutable_max_request_headers_kb()->set_value(max_headers_size); });
-  max_request_headers_kb_ = max_headers_size;
+  setMaxRequestHeadersKb(max_headers_size);
 
   Http::TestRequestHeaderMapImpl big_headers{{":method", "GET"},
                                              {":path", "/" + std::string(url_size * 1024, 'a')},
@@ -1097,8 +1096,8 @@ void HttpIntegrationTest::testLargeRequestHeaders(uint32_t size, uint32_t count,
         hcm.mutable_common_http_protocol_options()->mutable_max_headers_count()->set_value(
             max_count);
       });
-  max_request_headers_kb_ = max_size;
-  max_request_headers_count_ = max_count;
+  setMaxRequestHeadersKb(max_size);
+  setMaxRequestHeadersCount(max_count);
 
   Http::TestRequestHeaderMapImpl big_headers{
       {":method", "GET"}, {":path", "/test/long/url"}, {":scheme", "http"}, {":authority", "host"}};
@@ -1141,7 +1140,7 @@ void HttpIntegrationTest::testLargeRequestTrailers(uint32_t size, uint32_t max_s
   config_helper_.addConfigModifier(
       [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
               hcm) -> void { hcm.mutable_max_request_headers_kb()->set_value(max_size); });
-  max_request_headers_kb_ = max_size;
+  setMaxRequestHeadersKb(max_size);
   Http::TestRequestTrailerMapImpl request_trailers{{"trailer", "trailer"}};
   request_trailers.addCopy("big", std::string(size * 1024, 'a'));
 
@@ -1178,15 +1177,15 @@ void HttpIntegrationTest::testLargeRequestTrailers(uint32_t size, uint32_t max_s
 void HttpIntegrationTest::testManyRequestHeaders(std::chrono::milliseconds time) {
   // This test uses an Http::HeaderMapImpl instead of an Http::TestHeaderMapImpl to avoid
   // time-consuming asserts when using a large number of headers.
-  max_request_headers_kb_ = 96;
-  max_request_headers_count_ = 10005;
+  setMaxRequestHeadersKb(96);
+  setMaxRequestHeadersCount(10005);
 
   config_helper_.addConfigModifier(
       [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
               hcm) -> void {
-        hcm.mutable_max_request_headers_kb()->set_value(max_request_headers_kb_);
+        hcm.mutable_max_request_headers_kb()->set_value(upstreamConfig().max_request_headers_kb_);
         hcm.mutable_common_http_protocol_options()->mutable_max_headers_count()->set_value(
-            max_request_headers_count_);
+            upstreamConfig().max_request_headers_count_);
       });
 
   auto big_headers = Http::createHeaderMap<Http::RequestHeaderMapImpl>(

test/integration/http_integration.h

@@ -257,8 +257,6 @@ class HttpIntegrationTest : public BaseIntegrationTest {
       {":method", "GET"}, {":path", "/test/long/url"}, {":scheme", "http"}, {":authority", "host"}};
   // The codec type for the client-to-Envoy connection
   Http::CodecClient::Type downstream_protocol_{Http::CodecClient::Type::HTTP1};
-  uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_KB};
-  uint32_t max_request_headers_count_{Http::DEFAULT_MAX_HEADERS_COUNT};
   std::string access_log_name_;
   testing::NiceMock<Random::MockRandomGenerator> random_;
 };

test/integration/protocol_integration_test.cc

@@ -1311,7 +1311,7 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingMany) {
         hcm.mutable_common_http_protocol_options()->mutable_max_headers_count()->set_value(
             max_count);
       });
-  max_request_headers_count_ = max_count;
+  setMaxRequestHeadersCount(max_count);
 
   initialize();
 
@@ -1544,7 +1544,7 @@ TEST_P(DownstreamProtocolIntegrationTest, ManyRequestTrailersAccepted) {
             max_count);
       });
   config_helper_.addConfigModifier(setEnableUpstreamTrailersHttp1());
-  max_request_headers_count_ = max_count;
+  setMaxRequestHeadersCount(max_count);
   Http::TestRequestTrailerMapImpl request_trailers;
   for (int i = 0; i < 150; i++) {
     request_trailers.addCopy("trailer", std::string(1, 'a'));
@@ -1588,16 +1588,16 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeRequestTrailersRejected) {
 // time-consuming byte size verification that will cause this test to timeout.
 TEST_P(DownstreamProtocolIntegrationTest, ManyTrailerHeaders) {
   EXCLUDE_UPSTREAM_HTTP3;
-  max_request_headers_kb_ = 96;
-  max_request_headers_count_ = 20005;
+  setMaxRequestHeadersKb(96);
+  setMaxRequestHeadersCount(20005);
 
   config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1());
   config_helper_.addConfigModifier(
       [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
               hcm) -> void {
-        hcm.mutable_max_request_headers_kb()->set_value(max_request_headers_kb_);
+        hcm.mutable_max_request_headers_kb()->set_value(upstreamConfig().max_request_headers_kb_);
         hcm.mutable_common_http_protocol_options()->mutable_max_headers_count()->set_value(
-            max_request_headers_count_);
+            upstreamConfig().max_request_headers_count_);
       });
 
   auto request_trailers = Http::RequestTrailerMapImpl::create();
@@ -1947,9 +1947,7 @@ TEST_P(ProtocolIntegrationTest, TestDownstreamResetIdleTimeout) {
 
   auto encoder_decoder = codec_client_->startRequest(default_request_headers_);
 
-  EXPECT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_,
-                                                        TestUtility::DefaultTimeout,
-                                                        max_request_headers_kb_));
+  EXPECT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_));
 
   EXPECT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_));
 

test/integration/redirect_integration_test.cc

@@ -19,6 +19,8 @@ constexpr char kTestHeaderKey[] = "test-header";
 class RedirectIntegrationTest : public HttpProtocolIntegrationTest {
 public:
   void initialize() override {
+    setMaxRequestHeadersKb(60);
+    setMaxRequestHeadersCount(100);
     envoy::config::route::v3::RetryPolicy retry_policy;
 
     auto pass_through = config_helper_.createVirtualHost("pass.through.internal.redirect");
@@ -61,9 +63,8 @@ class RedirectIntegrationTest : public HttpProtocolIntegrationTest {
     // connections because connection reuse may or may not be triggered.
     while (new_stream == nullptr) {
       FakeHttpConnectionPtr new_connection = nullptr;
-
       AssertionResult result = fake_upstreams_[0]->waitForHttpConnection(
-          *dispatcher_, new_connection, std::chrono::milliseconds(50), 60, 100);
+          *dispatcher_, new_connection, std::chrono::milliseconds(50));
       if (result) {
         ASSERT(new_connection);
         upstream_connections_.push_back(std::move(new_connection));