Update Envoy to f95f5391b0b8683081ec786ea946026594955fc6

.bazelrc

@@ -201,8 +201,6 @@ build:remote --spawn_strategy=remote,sandboxed,local
 build:remote --strategy=Javac=remote,sandboxed,local
 build:remote --strategy=Closure=remote,sandboxed,local
 build:remote --strategy=Genrule=remote,sandboxed,local
-# rules_rust is not remote runnable (yet)
-build:remote --strategy=Rustc=sandboxed,local
 build:remote --remote_timeout=7200
 build:remote --auth_enabled=true
 build:remote --remote_download_toplevel

bazel/repositories.bzl

@@ -1,7 +1,7 @@
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 
-ENVOY_COMMIT = "2097fe908f2abb718757dbd4087d793c861d7c5a"  # October 12th, 2020
-ENVOY_SHA = "323360544ee355f0eddea742b31a80a94899090db1d64029cd22880083b311c0"
+ENVOY_COMMIT = "9dce187eca9aa10e98c0ae81368c078384528801"  # October 17th, 2020
+ENVOY_SHA = "fa5a452d52475cfc64c0557c4258e1a91fdceb725131aaa02956e83523b23269"
 
 HDR_HISTOGRAM_C_VERSION = "0.11.2"  # October 12th, 2020
 HDR_HISTOGRAM_C_SHA = "637f28b5f64de2e268131e4e34e6eef0b91cf5ff99167db447d9b2825eae6bad"
@@ -11,7 +11,9 @@ def nighthawk_dependencies():
         name = "envoy",
         sha256 = ENVOY_SHA,
         strip_prefix = "envoy-%s" % ENVOY_COMMIT,
+        # // clang-format off: Envoy's format check: Only repository_locations.bzl may contains URL references
         url = "https://github.com/envoyproxy/envoy/archive/%s.tar.gz" % ENVOY_COMMIT,
+        # // clang-format on
     )
     http_archive(
         name = "dep_hdrhistogram_c",
@@ -50,5 +52,7 @@ cc_library(
   """,
         sha256 = HDR_HISTOGRAM_C_SHA,
         strip_prefix = "HdrHistogram_c-%s" % HDR_HISTOGRAM_C_VERSION,
+        # // clang-format off
         url = "https://github.com/HdrHistogram/HdrHistogram_c/archive/%s.tar.gz" % HDR_HISTOGRAM_C_VERSION,
+        # // clang-format on
     )

source/client/stream_decoder.cc

@@ -21,9 +21,11 @@ void StreamDecoder::decodeHeaders(Envoy::Http::ResponseHeaderMapPtr&& headers, b
   stream_info_.response_code_ = static_cast<uint32_t>(response_code);
   if (!latency_response_header_name_.empty()) {
     const auto timing_header_name = Envoy::Http::LowerCaseString(latency_response_header_name_);
-    const Envoy::Http::HeaderEntry* timing_header = response_headers_->get(timing_header_name);
-    if (timing_header != nullptr) {
-      absl::string_view timing_value = timing_header->value().getStringView();
+    const Envoy::Http::HeaderMap::GetResult& timing_header =
+        response_headers_->get(timing_header_name);
+    if (!timing_header.empty()) {
+      absl::string_view timing_value =
+          timing_header.size() == 1 ? timing_header[0]->value().getStringView() : "multiple values";
       int64_t origin_delta;
       if (absl::SimpleAtoi(timing_value, &origin_delta) && origin_delta >= 0) {
         origin_latency_statistic_.addValue(origin_delta);
@@ -142,6 +144,7 @@ StreamDecoder::streamResetReasonToResponseFlag(Envoy::Http::StreamResetReason re
     return Envoy::StreamInfo::ResponseFlag::LocalReset;
   case Envoy::Http::StreamResetReason::Overflow:
     return Envoy::StreamInfo::ResponseFlag::UpstreamOverflow;
+  case Envoy::Http::StreamResetReason::ConnectError:
   case Envoy::Http::StreamResetReason::RemoteReset:
   case Envoy::Http::StreamResetReason::RemoteRefusedStreamReset:
     return Envoy::StreamInfo::ResponseFlag::UpstreamRemoteReset;

source/server/http_filter_config_base.cc

@@ -13,17 +13,20 @@ FilterConfigurationBase::FilterConfigurationBase(
 
 void FilterConfigurationBase::computeEffectiveConfiguration(
     const Envoy::Http::RequestHeaderMap& headers) {
-  const auto* request_config_header = headers.get(TestServer::HeaderNames::get().TestServerConfig);
-  if (request_config_header) {
+  const auto& request_config_header = headers.get(TestServer::HeaderNames::get().TestServerConfig);
+  if (request_config_header.size() == 1) {
     nighthawk::server::ResponseOptions response_options = *server_config_;
     std::string error_message;
-    if (Configuration::mergeJsonConfig(request_config_header->value().getStringView(),
+    if (Configuration::mergeJsonConfig(request_config_header[0]->value().getStringView(),
                                        response_options, error_message)) {
       effective_config_ =
           std::make_shared<const nighthawk::server::ResponseOptions>(std::move(response_options));
     } else {
       effective_config_ = absl::InvalidArgumentError(error_message);
     }
+  } else if (request_config_header.size() > 1) {
+    effective_config_ = absl::InvalidArgumentError(
+        "Received multiple configuration headers in the request, expected only one.");
   }
 }
 

test/request_source/request_source_plugin_test.cc

@@ -90,7 +90,8 @@ TEST_F(StubRequestSourcePluginTest, CreateRequestSourcePluginCreatesWorkingPlugi
   Nighthawk::RequestGenerator generator = plugin->get();
   Nighthawk::RequestPtr request = generator();
   Nighthawk::HeaderMapPtr header = request->header();
-  EXPECT_EQ(header->get(Envoy::Http::LowerCaseString("test_value"))->value().getStringView(),
+  ASSERT_EQ(header->get(Envoy::Http::LowerCaseString("test_value")).size(), 1);
+  EXPECT_EQ(header->get(Envoy::Http::LowerCaseString("test_value"))[0]->value().getStringView(),
             absl::string_view(std::to_string(test_value)));
 }
 TEST_F(FileBasedRequestSourcePluginTest, CreateEmptyConfigProtoCreatesCorrectType) {

test/server/http_dynamic_delay_filter_integration_test.cc

@@ -48,19 +48,21 @@ name: dynamic-delay
   // Don't send any config request header ...
   getResponse(ResponseOrigin::UPSTREAM);
   // ... we shouldn't observe any delay being requested via the upstream request headers.
-  EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString), nullptr);
+  EXPECT_TRUE(upstream_request_->headers().get(kDelayHeaderString).empty());
 
   // Send a config request header with an empty / default configuration ....
   setRequestLevelConfiguration("{}");
   getResponse(ResponseOrigin::UPSTREAM);
   // ... we shouldn't observe any delay being requested via the upstream request headers.
-  EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString), nullptr);
+  EXPECT_TRUE(upstream_request_->headers().get(kDelayHeaderString).empty());
 
   // Send a config request header requesting a 1.6s delay...
   setRequestLevelConfiguration("{static_delay: \"1.6s\"}");
   getResponse(ResponseOrigin::UPSTREAM);
   // ...we should observe a delay of 1.6s in the upstream request.
-  EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString)->value().getStringView(), "1600");
+  ASSERT_EQ(upstream_request_->headers().get(kDelayHeaderString).size(), 1);
+  EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString)[0]->value().getStringView(),
+            "1600");
 }
 
 // Verify expectations with static/file-based static_delay configuration.
@@ -75,13 +77,17 @@ name: dynamic-delay
   // Without any request-level configuration, we expect the statically configured static delay to
   // apply.
   getResponse(ResponseOrigin::UPSTREAM);
-  EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString)->value().getStringView(), "1330");
+  ASSERT_EQ(upstream_request_->headers().get(kDelayHeaderString).size(), 1);
+  EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString)[0]->value().getStringView(),
+            "1330");
 
   // With an empty request-level configuration, we expect the statically configured static delay to
   // apply.
   setRequestLevelConfiguration("{}");
   getResponse(ResponseOrigin::UPSTREAM);
-  EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString)->value().getStringView(), "1330");
+  ASSERT_EQ(upstream_request_->headers().get(kDelayHeaderString).size(), 1);
+  EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString)[0]->value().getStringView(),
+            "1330");
 
   // Overriding the statically configured static delay via request-level configuration should be
   // reflected in the output.
@@ -92,15 +98,17 @@ name: dynamic-delay
   // However, the seconds part is set to '0', which equates to the default of the underlying int
   // type, and the fact that we are using proto3, which doesn't merge default values.
   // Hence the following expectation will fail, as it yields 1200 instead of the expected 200.
-  // EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString)->value().getStringView(),
+  // EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString)[0]->value().getStringView(),
   // "200");
 
   // Overriding the statically configured static delay via request-level configuration should be
   // reflected in the output.
   setRequestLevelConfiguration("{static_delay: \"2.2s\"}");
   getResponse(ResponseOrigin::UPSTREAM);
   // 2.2 seconds -> 2200 ms.
-  EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString)->value().getStringView(), "2200");
+  ASSERT_EQ(upstream_request_->headers().get(kDelayHeaderString).size(), 1);
+  EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString)[0]->value().getStringView(),
+            "2200");
 }
 
 // Verify expectations with static/file-based concurrency_based_linear_delay configuration.
@@ -116,7 +124,8 @@ name: dynamic-delay
   getResponse(ResponseOrigin::UPSTREAM);
   // Based on the algorithm of concurrency_based_linear_delay, for the first request we expect to
   // observe the configured minimal_delay + concurrency_delay_factor = 0.06s -> 60ms.
-  EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString)->value().getStringView(), "60");
+  ASSERT_EQ(upstream_request_->headers().get(kDelayHeaderString).size(), 1);
+  EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString)[0]->value().getStringView(), "60");
 }
 
 class ComputeTest : public testing::Test {

test/server/http_filter_base_test.cc

@@ -90,5 +90,29 @@ TEST_P(HttpFilterBaseIntegrationTest, EmptyRequestLevelConfigurationShouldFail)
   EXPECT_THAT(response->body(), HasSubstr(kBadConfigErrorSentinel));
 }
 
+TEST_P(HttpFilterBaseIntegrationTest, MultipleValidConfigurationHeadersFails) {
+  // Make sure we fail when two valid configuration headers are send.
+  setRequestLevelConfiguration("{}");
+  appendRequestLevelConfiguration("{}");
+  Envoy::IntegrationStreamDecoderPtr response = getResponse(ResponseOrigin::EXTENSION);
+  ASSERT_TRUE(response->complete());
+  EXPECT_THAT(response->body(),
+              HasSubstr("Received multiple configuration headers in the request"));
+}
+
+TEST_P(HttpFilterBaseIntegrationTest, SingleValidPlusEmptyConfigurationHeadersFails) {
+  // Make sure we fail when both a valid configuration plus an empty configuration header is send.
+  // Note that we could be more flexible and look for the first request header that has a value,
+  // but I think the current behavior is reasonable: just assume the first request header will have
+  // the value we are after, and assume that multiple configuration headers indicate accidental user
+  // error.
+  setRequestLevelConfiguration("{}");
+  appendRequestLevelConfiguration("");
+  Envoy::IntegrationStreamDecoderPtr response = getResponse(ResponseOrigin::EXTENSION);
+  ASSERT_TRUE(response->complete());
+  EXPECT_THAT(response->body(),
+              HasSubstr("Received multiple configuration headers in the request"));
+}
+
 } // namespace
 } // namespace Nighthawk

test/server/http_filter_integration_test_base.cc

@@ -21,6 +21,12 @@ void HttpFilterIntegrationTestBase::setRequestLevelConfiguration(
   setRequestHeader(Server::TestServer::HeaderNames::get().TestServerConfig, request_level_config);
 }
 
+void HttpFilterIntegrationTestBase::appendRequestLevelConfiguration(
+    absl::string_view request_level_config) {
+  appendRequestHeader(Server::TestServer::HeaderNames::get().TestServerConfig,
+                      request_level_config);
+}
+
 void HttpFilterIntegrationTestBase::switchToPostWithEntityBody() {
   setRequestHeader(Envoy::Http::Headers::get().Method,
                    Envoy::Http::Headers::get().MethodValues.Post);
@@ -31,6 +37,11 @@ void HttpFilterIntegrationTestBase::setRequestHeader(
   request_headers_.setCopy(header_name, header_value);
 }
 
+void HttpFilterIntegrationTestBase::appendRequestHeader(
+    const Envoy::Http::LowerCaseString& header_name, absl::string_view header_value) {
+  request_headers_.addCopy(header_name, header_value);
+}
+
 Envoy::IntegrationStreamDecoderPtr
 HttpFilterIntegrationTestBase::getResponse(ResponseOrigin expected_origin) {
   cleanupUpstreamAndDownstream();

test/server/http_filter_integration_test_base.h

@@ -52,6 +52,16 @@ class HttpFilterIntegrationTestBase : public Envoy::HttpIntegrationTest {
    */
   void setRequestLevelConfiguration(absl::string_view request_level_config);
 
+  /**
+   * Make getResponse add request-level configuration. Test server extensions read that
+   * configuration and merge it with their static configuration to determine a final effective
+   * configuration. See TestServerConfig in well_known_headers.h for the up to date header name.
+   *
+   * @param request_level_config Configuration to be delivered by request-header in future calls to
+   * getResponse(). For example: "{response_body_size:1024}".
+   */
+  void appendRequestLevelConfiguration(absl::string_view request_level_config);
+
   /**
    * Switch getResponse() to use the POST request method with an entity body.
    * Doing so will make tests hit a different code paths in extensions.
@@ -67,6 +77,15 @@ class HttpFilterIntegrationTestBase : public Envoy::HttpIntegrationTest {
   void setRequestHeader(const Envoy::Http::LowerCaseString& header_name,
                         absl::string_view header_value);
 
+  /**
+   * Appends a request header value.
+   *
+   * @param header_name Name of the request header to set.
+   * @param header_value Value to set for the request header.
+   */
+  void appendRequestHeader(const Envoy::Http::LowerCaseString& header_name,
+                           absl::string_view header_value);
+
   /**
    * Fetch a response, according to the options specified by the class methods. By default,
    * simulates a GET request with minimal headers.

test/server/http_test_server_filter_integration_test.cc

@@ -38,8 +38,8 @@ class HttpTestServerIntegrationTest : public HttpFilterIntegrationTestBase,
     EXPECT_EQ("200", response->headers().Status()->value().getStringView());
     if (expect_header) {
       auto inserted_header = response->headers().get(Envoy::Http::LowerCaseString("x-supplied-by"));
-      ASSERT_NE(nullptr, inserted_header);
-      EXPECT_EQ("nighthawk-test-server", inserted_header->value().getStringView());
+      ASSERT_EQ(1, inserted_header.size());
+      EXPECT_EQ("nighthawk-test-server", inserted_header[0]->value().getStringView());
     }
     if (response_body_size == 0) {
       EXPECT_EQ(nullptr, response->headers().ContentType());
@@ -107,8 +107,10 @@ TEST_P(HttpTestServerIntegrationTest, TestHeaderConfig) {
   Envoy::IntegrationStreamDecoderPtr response = getResponse(ResponseOrigin::EXTENSION);
   ASSERT_TRUE(response->complete());
   EXPECT_EQ("200", response->headers().Status()->value().getStringView());
-  EXPECT_EQ("bar2",
-            response->headers().get(Envoy::Http::LowerCaseString("foo"))->value().getStringView());
+  ASSERT_EQ(1, response->headers().get(Envoy::Http::LowerCaseString("foo")).size());
+  EXPECT_EQ(
+      "bar2",
+      response->headers().get(Envoy::Http::LowerCaseString("foo"))[0]->value().getStringView());
   EXPECT_EQ(std::string(10, 'a'), response->body());
 }
 
@@ -180,8 +182,10 @@ TEST_P(HttpTestServerIntegrationTest, TestNoStaticConfigHeaderConfig) {
 
   ASSERT_TRUE(response->complete());
   EXPECT_EQ("200", response->headers().Status()->value().getStringView());
-  EXPECT_EQ("bar2",
-            response->headers().get(Envoy::Http::LowerCaseString("foo"))->value().getStringView());
+  ASSERT_EQ(1, response->headers().get(Envoy::Http::LowerCaseString("foo")).size());
+  EXPECT_EQ(
+      "bar2",
+      response->headers().get(Envoy::Http::LowerCaseString("foo"))[0]->value().getStringView());
   EXPECT_EQ("", response->body());
 }
 

test/server/http_time_tracking_filter_integration_test.cc

@@ -44,16 +44,15 @@ TEST_P(HttpTimeTrackingIntegrationTest, ReturnsPositiveLatencyForStaticConfigura
   // As the first request doesn't have a prior one, we should not observe a delta.
   Envoy::IntegrationStreamDecoderPtr response = getResponse(ResponseOrigin::UPSTREAM);
   int64_t latency;
-  const Envoy::Http::HeaderEntry* latency_header_1 =
-      response->headers().get(Envoy::Http::LowerCaseString(kLatencyResponseHeaderName));
-  EXPECT_EQ(latency_header_1, nullptr);
+  EXPECT_EQ(
+      response->headers().get(Envoy::Http::LowerCaseString(kLatencyResponseHeaderName)).size(), 0);
 
   // On the second request we should observe a delta.
   response = getResponse(ResponseOrigin::UPSTREAM);
-  const Envoy::Http::HeaderEntry* latency_header_2 =
+  const Envoy::Http::HeaderMap::GetResult& latency_header =
       response->headers().get(Envoy::Http::LowerCaseString(kLatencyResponseHeaderName));
-  ASSERT_NE(latency_header_2, nullptr);
-  EXPECT_TRUE(absl::SimpleAtoi(latency_header_2->value().getStringView(), &latency));
+  ASSERT_EQ(latency_header.size(), 1);
+  EXPECT_TRUE(absl::SimpleAtoi(latency_header[0]->value().getStringView(), &latency));
   EXPECT_GT(latency, 0);
 }
 
@@ -63,18 +62,18 @@ TEST_P(HttpTimeTrackingIntegrationTest, ReturnsPositiveLatencyForPerRequestConfi
   // As the first request doesn't have a prior one, we should not observe a delta.
   setRequestLevelConfiguration("{}");
   Envoy::IntegrationStreamDecoderPtr response = getResponse(ResponseOrigin::UPSTREAM);
-  EXPECT_EQ(response->headers().get(Envoy::Http::LowerCaseString(kLatencyResponseHeaderName)),
-            nullptr);
+  EXPECT_TRUE(
+      response->headers().get(Envoy::Http::LowerCaseString(kLatencyResponseHeaderName)).empty());
 
   // With request level configuration indicating that the timing header should be emitted,
   // we should be able to observe it.
   setRequestLevelConfiguration(fmt::format("{{{}}}", kDefaultProtoFragment));
   response = getResponse(ResponseOrigin::UPSTREAM);
-  const Envoy::Http::HeaderEntry* latency_header =
+  const Envoy::Http::HeaderMap::GetResult& latency_header =
       response->headers().get(Envoy::Http::LowerCaseString(kLatencyResponseHeaderName));
-  ASSERT_NE(latency_header, nullptr);
+  ASSERT_EQ(latency_header.size(), 1);
   int64_t latency;
-  EXPECT_TRUE(absl::SimpleAtoi(latency_header->value().getStringView(), &latency));
+  EXPECT_TRUE(absl::SimpleAtoi(latency_header[0]->value().getStringView(), &latency));
   // TODO(oschaaf): figure out if we can use simtime here, and verify actual timing matches
   // what we'd expect using that.
   EXPECT_GT(latency, 0);

test/stream_decoder_test.cc

@@ -211,6 +211,9 @@ TEST_F(StreamDecoderTest, StreamResetReasonToResponseFlag) {
   ASSERT_EQ(StreamDecoder::streamResetReasonToResponseFlag(
                 Envoy::Http::StreamResetReason::RemoteRefusedStreamReset),
             Envoy::StreamInfo::ResponseFlag::UpstreamRemoteReset);
+  ASSERT_EQ(
+      StreamDecoder::streamResetReasonToResponseFlag(Envoy::Http::StreamResetReason::ConnectError),
+      Envoy::StreamInfo::ResponseFlag::UpstreamRemoteReset);
 }
 
 // This test parameterization structure carries the response header name that ought to be treated
@@ -245,5 +248,22 @@ TEST_P(LatencyTrackingViaResponseHeaderTest, LatencyTrackingViaResponseHeader) {
   EXPECT_EQ(origin_latency_statistic_.count(), expected_count);
 }
 
+// Test that a single response carrying multiple valid latency response headers does not
+// get tracked. This will also yield a burst of warnings, which we unfortunately cannot
+// easily verify here.
+TEST_F(StreamDecoderTest, LatencyTrackingWithMultipleResponseHeadersFails) {
+  const std::string kLatencyTrackingResponseHeader = "latency-in-response-header";
+  auto decoder = new StreamDecoder(
+      *dispatcher_, time_system_, *this, [](bool, bool) {}, connect_statistic_, latency_statistic_,
+      response_header_size_statistic_, response_body_size_statistic_, origin_latency_statistic_,
+      request_headers_, false, 0, random_generator_, http_tracer_, kLatencyTrackingResponseHeader);
+  Envoy::Http::ResponseHeaderMapPtr headers{
+      new Envoy::Http::TestResponseHeaderMapImpl{{":status", "200"},
+                                                 {kLatencyTrackingResponseHeader, "1"},
+                                                 {kLatencyTrackingResponseHeader, "2"}}};
+  decoder->decodeHeaders(std::move(headers), true);
+  EXPECT_EQ(origin_latency_statistic_.count(), 0);
+}
+
 } // namespace Client
 } // namespace Nighthawk