grpc_json_transcoder: remove Trailer headers from http/1 responses (#…

…4359)

When clients connect using http/1, Trailer headers are prohibited unless the transfer encoding is chunked, which it shouldn't be. Remove these headers in this case.

Risk Level: Low
Testing: Integration testing has been done to ensure that these headers are in fact removed.
Docs Changes: N/A
Release Notes: N/A
Fixes: #4240

Signed-off-by: Joshua Rubin <joshua@rubixconsulting.com>

source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.cc

@@ -398,6 +398,12 @@ Http::FilterTrailersStatus JsonTranscoderFilter::encodeTrailers(Http::HeaderMap&
     response_headers_->insertGrpcMessage().value(*grpc_message_header);
   }
 
+  // remove Trailer headers if the client connection was http/1
+  if (encoder_callbacks_->requestInfo().protocol() != Http::Protocol::Http2) {
+    static const Http::LowerCaseString trailer_key = Http::LowerCaseString("trailer");
+    response_headers_->remove(trailer_key);
+  }
+
   response_headers_->insertContentLength().value(
       encoder_callbacks_->encodingBuffer() ? encoder_callbacks_->encodingBuffer()->length() : 0);
   return Http::FilterTrailersStatus::Continue;

test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc

@@ -100,6 +100,8 @@ class GrpcJsonTranscoderIntegrationTest
         response_headers.insertGrpcMessage().value(grpc_status.error_message());
         upstream_request_->encodeHeaders(response_headers, true);
       } else {
+        response_headers.addCopy(Http::LowerCaseString("trailer"), "Grpc-Status");
+        response_headers.addCopy(Http::LowerCaseString("trailer"), "Grpc-Message");
         upstream_request_->encodeHeaders(response_headers, false);
         for (const auto& response_message_str : grpc_response_messages) {
           ResponseType response_message;
@@ -119,6 +121,14 @@ class GrpcJsonTranscoderIntegrationTest
 
     response->waitForEndStream();
     EXPECT_TRUE(response->complete());
+
+    if (response->headers().get(Http::LowerCaseString("transfer-encoding")) == nullptr ||
+        strncmp(
+            response->headers().get(Http::LowerCaseString("transfer-encoding"))->value().c_str(),
+            "chunked", strlen("chunked")) != 0) {
+      EXPECT_EQ(response->headers().get(Http::LowerCaseString("trailer")), nullptr);
+    }
+
     response_headers.iterate(
         [](const Http::HeaderEntry& entry, void* context) -> Http::HeaderMap::Iterate {
           IntegrationStreamDecoder* response = static_cast<IntegrationStreamDecoder*>(context);