fix: handle request trailers in the test server

docs/root/version_history.md

@@ -15,6 +15,7 @@ Version history
 ### Changelist
 
 - Introducing termination predicates (https://github.com/envoyproxy/nighthawk/pull/167) and https://github.com/envoyproxy/nighthawk/pull/176
+- Fixed the test server so requests that terminate with HTTP trailers receive a response.
 
 0.2 (July 16, 2019)
 =========================
@@ -30,4 +31,4 @@ Version history
 0.1 (May 6, 2019)
 =========================
 
-Initial release.
+Initial release.

source/server/README.md

@@ -95,6 +95,7 @@ filter will interpret only the parts that are relevant to it.
   `true`, then the header is appended.
 - `echo_request_headers` - if set to `true`, then append the dump of request headers to the response
   body.
+- Requests that end with HTTP trailers are supported.
 
 The response options above could be used to test and debug proxy or server configuration, for example, to verify request headers that are added by intermediate proxy:
 

source/server/http_test_server_filter.cc

@@ -72,37 +72,42 @@ void HttpTestServerDecoderFilter::sendReply(const ResponseOptions& options) {
       absl::nullopt, "");
 }
 
+bool HttpTestServerDecoderFilter::maybeSendReply() {
+  if (config_->validateOrSendError(effective_config_.status(), *decoder_callbacks_)) {
+    return false;
+  }
+  sendReply(*effective_config_.value());
+  return true;
+}
+
 Envoy::Http::FilterHeadersStatus
 HttpTestServerDecoderFilter::decodeHeaders(Envoy::Http::RequestHeaderMap& headers,
                                            bool end_stream) {
   effective_config_ =
       computeEffectiveConfiguration(config_->getStartupFilterConfiguration(), headers);
   if (end_stream) {
-    if (!config_->validateOrSendError(effective_config_.status(), *decoder_callbacks_)) {
-      if (effective_config_.value()->echo_request_headers()) {
-        std::stringstream headers_dump;
-        headers_dump << "\nRequest Headers:\n" << headers;
-        request_headers_dump_ = headers_dump.str();
-      }
-      sendReply(*effective_config_.value());
+    if (effective_config_.ok() && effective_config_.value()->echo_request_headers()) {
+      std::stringstream headers_dump;
+      headers_dump << "\nRequest Headers:\n" << headers;
+      request_headers_dump_ = headers_dump.str();
     }
+    maybeSendReply();
   }
   return Envoy::Http::FilterHeadersStatus::StopIteration;
 }
 
 Envoy::Http::FilterDataStatus HttpTestServerDecoderFilter::decodeData(Envoy::Buffer::Instance&,
                                                                       bool end_stream) {
   if (end_stream) {
-    if (!config_->validateOrSendError(effective_config_.status(), *decoder_callbacks_)) {
-      sendReply(*effective_config_.value());
-    }
+    maybeSendReply();
   }
   return Envoy::Http::FilterDataStatus::StopIterationNoBuffer;
 }
 
 Envoy::Http::FilterTrailersStatus
 HttpTestServerDecoderFilter::decodeTrailers(Envoy::Http::RequestTrailerMap&) {
-  return Envoy::Http::FilterTrailersStatus::Continue;
+  maybeSendReply();
+  return Envoy::Http::FilterTrailersStatus::StopIteration;
 }
 
 void HttpTestServerDecoderFilter::setDecoderFilterCallbacks(

source/server/http_test_server_filter.h

@@ -45,6 +45,7 @@ class HttpTestServerDecoderFilter : public Envoy::Http::StreamDecoderFilter {
   void setDecoderFilterCallbacks(Envoy::Http::StreamDecoderFilterCallbacks&) override;
 
 private:
+  bool maybeSendReply();
   void sendReply(const nighthawk::server::ResponseOptions& options);
   const HttpTestServerDecoderFilterConfigSharedPtr config_;
   absl::StatusOr<std::shared_ptr<const nighthawk::server::ResponseOptions>> effective_config_;

test/server/http_filter_integration_test_base.cc

@@ -70,4 +70,22 @@ HttpFilterIntegrationTestBase::getResponse(ResponseOrigin expected_origin) {
   return response;
 }
 
+Envoy::IntegrationStreamDecoderPtr
+HttpFilterIntegrationTestBase::getResponseWithTrailers(ResponseOrigin expected_origin) {
+  cleanupUpstreamAndDownstream();
+  codec_client_ = makeHttpConnection(lookupPort("http"));
+  Envoy::IntegrationStreamDecoderPtr response;
+  auto encoder_decoder = codec_client_->startRequest(request_headers_, false);
+  response = std::move(encoder_decoder.second);
+  Envoy::Http::TestRequestTrailerMapImpl trailers{{"x-test-trailer", "present"}};
+  codec_client_->sendTrailers(encoder_decoder.first, trailers);
+
+  if (expected_origin == ResponseOrigin::UPSTREAM) {
+    ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_));
+    ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_));
+  }
+
+  return response;
+}
+
 } // namespace Nighthawk

test/server/http_filter_integration_test_base.h

@@ -97,6 +97,14 @@ class HttpFilterIntegrationTestBase : public Envoy::HttpIntegrationTest {
    */
   Envoy::IntegrationStreamDecoderPtr getResponse(ResponseOrigin expected_origin);
 
+  /**
+   * Fetch a response after sending request trailers.
+   * @param expected_origin Indicate which component will be expected to reply.
+   * @return Envoy::IntegrationStreamDecoderPtr Pointer to the integration stream decoder, which can
+   * be used to inspect the response.
+   */
+  Envoy::IntegrationStreamDecoderPtr getResponseWithTrailers(ResponseOrigin expected_origin);
+
 private:
   Envoy::Http::TestRequestHeaderMapImpl request_headers_;
 };

test/server/http_test_server_filter_integration_test.cc

@@ -192,6 +192,15 @@ TEST_P(HttpTestServerIntegrationTest, TestEchoHeaders) {
   }
 }
 
+TEST_P(HttpTestServerIntegrationTest, TestRequestTrailers) {
+  initializeFilterConfiguration(kDefaultProto);
+  auto response = getResponseWithTrailers(ResponseOrigin::EXTENSION);
+  ASSERT_TRUE(response->waitForEndStream());
+  ASSERT_TRUE(response->complete());
+  EXPECT_EQ("200", response->headers().Status()->value().getStringView());
+  EXPECT_EQ(std::string(10, 'a'), response->body());
+}
+
 TEST_P(HttpTestServerIntegrationTest, NoNoStaticConfigHeaderConfig) {
   initializeFilterConfiguration(kNoConfigProto);
   Envoy::IntegrationStreamDecoderPtr response = getResponse(ResponseOrigin::EXTENSION);