From f22e22e7940b736f9e2e428edeed90f08db33c96 Mon Sep 17 00:00:00 2001
From: Alyssa Wilk <alyssar@chromium.org>
Date: Mon, 11 Jun 2018 15:11:07 -0400
Subject: [PATCH] Revert "http: delaying filter stack creation until full
 headers have been received (#3574)"

This reverts commit 1d14b9e46cf6daaff0f02348588a36fbfe8ef65f.

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
---
 source/common/http/conn_manager_impl.cc    | 17 ++++++-----------
 source/common/http/conn_manager_impl.h     |  2 --
 test/common/http/conn_manager_impl_test.cc |  6 ++++--
 3 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc
index a0f5d184785ea..6b47b3d8bf2fa 100644
--- a/source/common/http/conn_manager_impl.cc
+++ b/source/common/http/conn_manager_impl.cc
@@ -189,6 +189,11 @@ StreamDecoder& ConnectionManagerImpl::newStream(StreamEncoder& response_encoder)
   new_stream->response_encoder_ = &response_encoder;
   new_stream->response_encoder_->getStream().addCallbacks(*new_stream);
   new_stream->buffer_limit_ = new_stream->response_encoder_->getStream().bufferLimit();
+  config_.filterFactory().createFilterChain(*new_stream);
+  // Make sure new streams are apprised that the underlying connection is blocked.
+  if (read_callbacks_->connection().aboveHighWatermark()) {
+    new_stream->callHighWatermarkCallbacks();
+  }
   new_stream->moveIntoList(std::move(new_stream), streams_);
   return **streams_.begin();
 }
@@ -440,10 +445,8 @@ const Network::Connection* ConnectionManagerImpl::ActiveStream::connection() {
 }
 
 void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, bool end_stream) {
-  request_headers_ = std::move(headers);
-  createFilterChain();
-
   maybeEndDecode(end_stream);
+  request_headers_ = std::move(headers);
 
   ENVOY_STREAM_LOG(debug, "request headers complete (end_stream={}):\n{}", *this, end_stream,
                    *request_headers_);
@@ -1111,14 +1114,6 @@ void ConnectionManagerImpl::ActiveStream::setBufferLimit(uint32_t new_limit) {
   }
 }
 
-void ConnectionManagerImpl::ActiveStream::createFilterChain() {
-  connection_manager_.config_.filterFactory().createFilterChain(*this);
-  // Make sure new streams are apprised that the underlying connection is blocked.
-  if (connection_manager_.read_callbacks_->connection().aboveHighWatermark()) {
-    callHighWatermarkCallbacks();
-  }
-}
-
 void ConnectionManagerImpl::ActiveStreamFilterBase::commonContinue() {
   // TODO(mattklein123): Raise an error if this is called during a callback.
   if (!canContinue()) {
diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h
index 2bfa88ceeb295..9cefbfd254ef4 100644
--- a/source/common/http/conn_manager_impl.h
+++ b/source/common/http/conn_manager_impl.h
@@ -360,8 +360,6 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
 
     // Possibly increases buffer_limit_ to the value of limit.
     void setBufferLimit(uint32_t limit);
-    // Set up the Encoder/Decoder filter chain.
-    void createFilterChain();
 
     ConnectionManagerImpl& connection_manager_;
     Router::ConfigConstSharedPtr snapped_route_config_;
diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc
index 21c7343626ec1..50b7761e66a62 100644
--- a/test/common/http/conn_manager_impl_test.cc
+++ b/test/common/http/conn_manager_impl_test.cc
@@ -1712,13 +1712,14 @@ TEST_F(HttpConnectionManagerImplTest, DownstreamDisconnect) {
     data.drain(2);
   }));
 
-  EXPECT_CALL(filter_factory_, createFilterChain(_)).Times(0);
+  setupFilterChain(1, 0);
 
   // Kick off the incoming data.
   Buffer::OwnedImpl fake_input("1234");
   conn_manager_->onData(fake_input, false);
 
   // Now raise a remote disconnection, we should see the filter get reset called.
+  EXPECT_CALL(*decoder_filters_[0], onDestroy());
   conn_manager_->onEvent(Network::ConnectionEvent::RemoteClose);
 }
 
@@ -1731,9 +1732,10 @@ TEST_F(HttpConnectionManagerImplTest, DownstreamProtocolError) {
     throw CodecProtocolException("protocol error");
   }));
 
-  EXPECT_CALL(filter_factory_, createFilterChain(_)).Times(0);
+  setupFilterChain(1, 0);
 
   // A protocol exception should result in reset of the streams followed by a local close.
+  EXPECT_CALL(*decoder_filters_[0], onDestroy());
   EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::FlushWrite));
 
   // Kick off the incoming data.