Skip metadata processing after sending local reply (#16154)

Signed-off-by: Yiming Jin <[email protected]>

source/common/http/filter_manager.cc

@@ -545,6 +545,12 @@ void FilterManager::decodeHeaders(ActiveStreamDecoderFilter* filter, RequestHead
       (*entry)->handle_->decodeComplete();
     }
 
+    // Skip processing metadata after sending local reply
+    if (state_.local_complete_ && std::next(entry) != decoder_filters_.end()) {
+      maybeContinueDecoding(continue_data_entry);
+      return;
+    }
+
     const bool new_metadata_added = processNewlyAddedMetadata();
     // If end_stream is set in headers, and a filter adds new metadata, we need to delay end_stream
     // in headers by inserting an empty data frame with end_stream set. The empty data frame is sent
@@ -558,8 +564,7 @@ void FilterManager::decodeHeaders(ActiveStreamDecoderFilter* filter, RequestHead
       addDecodedData(*((*entry).get()), empty_data, true);
     }
 
-    if ((!continue_iteration || state_.local_complete_) &&
-        std::next(entry) != decoder_filters_.end()) {
+    if (!continue_iteration && std::next(entry) != decoder_filters_.end()) {
       // Stop iteration IFF this is not the last filter. If it is the last filter, continue with
       // processing since we need to handle the case where a terminal filter wants to buffer, but
       // a previous filter has added body.

test/integration/BUILD

@@ -453,6 +453,7 @@ envoy_cc_test_library(
         "//test/integration/filters:invalid_header_filter_lib",
         "//test/integration/filters:local_reply_during_encoding_data_filter_lib",
         "//test/integration/filters:local_reply_during_encoding_filter_lib",
+        "//test/integration/filters:local_reply_with_metadata_filter_lib",
         "//test/integration/filters:random_pause_filter_lib",
         "//test/test_common:logging_lib",
         "//test/test_common:utility_lib",

test/integration/filters/BUILD

@@ -69,6 +69,21 @@ envoy_cc_test_library(
     ],
 )
 
+envoy_cc_test_library(
+    name = "local_reply_with_metadata_filter_lib",
+    srcs = [
+        "local_reply_with_metadata_filter.cc",
+    ],
+    deps = [
+        ":common_lib",
+        "//include/envoy/http:filter_interface",
+        "//include/envoy/registry",
+        "//include/envoy/server:filter_config_interface",
+        "//source/extensions/filters/http/common:pass_through_filter_lib",
+        "//test/extensions/filters/http/common:empty_http_filter_config_lib",
+    ],
+)
+
 envoy_cc_test_library(
     name = "continue_headers_only_inject_body",
     srcs = [

test/integration/filters/local_reply_with_metadata_filter.cc

@@ -0,0 +1,40 @@
+#include "envoy/registry/registry.h"
+#include "envoy/server/filter_config.h"
+
+#include "extensions/filters/http/common/pass_through_filter.h"
+
+#include "test/extensions/filters/http/common/empty_http_filter_config.h"
+#include "test/integration/filters/common.h"
+
+#include "gtest/gtest.h"
+
+namespace Envoy {
+
+// A filter that calls Http::FilterHeadersStatus::StopAll and set metadata after a local reply.
+class LocalReplyWithMetadataFilter : public Http::PassThroughFilter {
+public:
+  constexpr static char name[] = "local-reply-with-metadata-filter";
+
+  Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap&, bool) override {
+    Http::MetadataMap metadata_map = {{"headers", "headers"}, {"duplicate", "duplicate"}};
+    Http::MetadataMapPtr metadata_map_ptr = std::make_unique<Http::MetadataMap>(metadata_map);
+    decoder_callbacks_->addDecodedMetadata().emplace_back(std::move(metadata_map_ptr));
+    decoder_callbacks_->sendLocalReply(Envoy::Http::Code::OK, "", nullptr, absl::nullopt,
+                                       "LocalReplyWithMetadataFilter is ready");
+    return Http::FilterHeadersStatus::StopIteration;
+  }
+
+  // Adds new metadata to metadata_map directly
+  Http::FilterMetadataStatus decodeMetadata(Http::MetadataMap& metadata_map) override {
+    metadata_map["data"] = "data";
+    ASSERT(false);
+    return Http::FilterMetadataStatus::Continue;
+  }
+};
+
+constexpr char LocalReplyWithMetadataFilter::name[];
+static Registry::RegisterFactory<SimpleFilterConfig<LocalReplyWithMetadataFilter>,
+                                 Server::Configuration::NamedHttpFilterConfigFactory>
+    register_;
+
+} // namespace Envoy

test/integration/protocol_integration_test.cc

@@ -2314,4 +2314,22 @@ TEST_P(DownstreamProtocolIntegrationTest, HeaderNormalizationRejection) {
   EXPECT_EQ("400", response->headers().getStatusValue());
 }
 
+// Tests a filter that returns a FilterHeadersStatus::Continue after a local reply without
+// processing new metadata generated in decodeHeader
+TEST_P(DownstreamProtocolIntegrationTest, LocalReplyWithMetadata) {
+  config_helper_.addFilter(R"EOF(
+  name: local-reply-with-metadata-filter
+  typed_config:
+    "@type": type.googleapis.com/google.protobuf.Empty
+  )EOF");
+  initialize();
+
+  codec_client_ = makeHttpConnection(lookupPort("http"));
+  // Send a headers only request.
+  auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_);
+  ASSERT_TRUE(response->waitForEndStream());
+  ASSERT_TRUE(response->complete());
+  ASSERT_EQ("200", response->headers().getStatusValue());
+}
+
 } // namespace Envoy