Fix the crash caused by continuing the filter after recreating the st…

…ream Signed-off-by: zty98751 <[email protected]>
envoyproxy · Dec 12, 2024 · 922551b · 922551b
1 parent 602a2b9
commit 922551b
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 2 deletions.
diff --git a/source/common/http/filter_manager.cc b/source/common/http/filter_manager.cc
@@ -344,13 +344,13 @@ bool ActiveStreamDecoderFilter::canContinue() {
   // continue to further filters. A concrete example of this is a filter buffering data, the
   // last data frame comes in and the filter continues, but the final buffering takes the stream
   // over the high watermark such that a 413 is returned.
-  return !parent_.stopDecoderFilterChain();
+  return !parent_.stopDecoderFilterChain() && !parent_.state_.recreated_stream_;
 }
 
 bool ActiveStreamEncoderFilter::canContinue() {
   // As with ActiveStreamDecoderFilter::canContinue() make sure we do not
   // continue if a local reply has been sent.
-  return !parent_.state_.encoder_filter_chain_complete_;
+  return !parent_.state_.encoder_filter_chain_complete_ && !parent_.state_.recreated_stream_;
 }
 
 Buffer::InstancePtr ActiveStreamDecoderFilter::createBuffer() {

diff --git a/test/common/http/conn_manager_impl_test_2.cc b/test/common/http/conn_manager_impl_test_2.cc
@@ -2399,6 +2399,105 @@ TEST_F(HttpConnectionManagerImplTest, AddDataWithStopAndContinue) {
   encoder_filters_[2]->callbacks_->continueEncoding();
 }
 
+// This test verifies that when recreateStream is executed during the decodeHeader
+// phase and returns StopIteration, executing continueDecoding does not proceed with the processing
+// of subsequent filters.
+TEST_F(HttpConnectionManagerImplTest, CannotContinueDecodingAfterRecreateStream) {
+  setup(false, "");
+  decoder_filters_.push_back(new NiceMock<MockStreamDecoderFilter>());
+  decoder_filters_.push_back(new NiceMock<MockStreamDecoderFilter>());
+
+  EXPECT_CALL(filter_factory_, createFilterChain(_))
+      .WillOnce(Invoke([this](FilterChainManager& manager) -> bool {
+        bool applied_filters = false;
+        if (log_handler_.get()) {
+          auto factory = createLogHandlerFactoryCb(log_handler_);
+          manager.applyFilterFactoryCb({}, factory);
+          applied_filters = true;
+        }
+        for (int i = 0; i < 2; i++) {
+          auto factory =
+              createDecoderFilterFactoryCb(StreamDecoderFilterSharedPtr{decoder_filters_[i]});
+          manager.applyFilterFactoryCb({}, factory);
+          applied_filters = true;
+        }
+        return applied_filters;
+      }))
+      .WillOnce(Return(true));
+
+  EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, true))
+      .WillOnce(InvokeWithoutArgs([&]() -> FilterHeadersStatus {
+        decoder_filters_[0]->callbacks_->recreateStream(nullptr);
+        return FilterHeadersStatus::StopIteration;
+      }));
+
+  // Kick off the request.
+  startRequest(true);
+
+  // Should not continue headers of filter 1.
+  EXPECT_CALL(*decoder_filters_[1], decodeHeaders(_, true)).Times(0);
+  decoder_filters_[0]->callbacks_->continueDecoding();
+  filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::RemoteClose);
+}
+
+// This test verifies that when recreateStream is executed during the encodeHeader
+// phase and returns StopIteration, executing continueEncoding does not proceed with the processing
+// of subsequent filters.
+TEST_F(HttpConnectionManagerImplTest, CannotContinueEncodingAfterRecreateStream) {
+  setup(false, "");
+  decoder_filters_.push_back(new NiceMock<MockStreamDecoderFilter>());
+  decoder_filters_.push_back(new NiceMock<MockStreamDecoderFilter>());
+  encoder_filters_.push_back(new NiceMock<MockStreamEncoderFilter>());
+  encoder_filters_.push_back(new NiceMock<MockStreamEncoderFilter>());
+
+  EXPECT_CALL(filter_factory_, createFilterChain(_))
+      .WillOnce(Invoke([this](FilterChainManager& manager) -> bool {
+        bool applied_filters = false;
+        if (log_handler_.get()) {
+          auto factory = createLogHandlerFactoryCb(log_handler_);
+          manager.applyFilterFactoryCb({}, factory);
+          applied_filters = true;
+        }
+        for (int i = 0; i < 2; i++) {
+          auto factory =
+              createDecoderFilterFactoryCb(StreamDecoderFilterSharedPtr{decoder_filters_[i]});
+          manager.applyFilterFactoryCb({}, factory);
+          applied_filters = true;
+        }
+        for (int i = 0; i < 2; i++) {
+          auto factory =
+              createEncoderFilterFactoryCb(StreamEncoderFilterSharedPtr{encoder_filters_[i]});
+          manager.applyFilterFactoryCb({}, factory);
+          applied_filters = true;
+        }
+        return applied_filters;
+      }))
+      .WillOnce(Return(true));
+
+  EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, true))
+      .WillOnce(Return(FilterHeadersStatus::Continue));
+  EXPECT_CALL(*decoder_filters_[1], decodeHeaders(_, true))
+      .WillOnce(Return(FilterHeadersStatus::StopIteration));
+
+  // Kick off the request.
+  startRequest(true);
+
+  EXPECT_CALL(*encoder_filters_[1], encodeHeaders(_, true))
+      .WillOnce(InvokeWithoutArgs([&]() -> FilterHeadersStatus {
+        decoder_filters_[1]->callbacks_->recreateStream(nullptr);
+        return FilterHeadersStatus::StopIteration;
+      }));
+
+  decoder_filters_[1]->callbacks_->streamInfo().setResponseCodeDetails("");
+  decoder_filters_[1]->callbacks_->encodeHeaders(
+      ResponseHeaderMapPtr{new TestResponseHeaderMapImpl{{":status", "200"}}}, true, "details");
+
+  // Should not continue headers of filter 0.
+  EXPECT_CALL(*encoder_filters_[0], encodeHeaders(_, true)).Times(0);
+  encoder_filters_[1]->callbacks_->continueEncoding();
+  filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::RemoteClose);
+}
+
 // Use filter direct decode/encodeData() calls without trailers.
 TEST_F(HttpConnectionManagerImplTest, FilterDirectDecodeEncodeDataNoTrailers) {
   setup();