Add setRequestDecoder to ResponseEncoder interface

envoy/http/codec.h

@@ -45,6 +45,7 @@ const char MaxResponseHeadersCountOverrideKey[] =
     "envoy.reloadable_features.max_response_headers_count";
 
 class Stream;
+class RequestDecoder;
 
 /**
  * Error codes used to convey the reason for a GOAWAY.
@@ -165,6 +166,14 @@ class ResponseEncoder : public virtual StreamEncoder {
    * error.
    */
   virtual bool streamErrorOnInvalidHttpMessage() const PURE;
+
+  /**
+   * Set a new request decoder for this ResponseEncoder. This is helpful in the case of an internal
+   * redirect, in which a new request decoder is created in the context of the same downstream
+   * request.
+   * @param decoder new request decoder.
+   */
+  virtual void setRequestDecoder(RequestDecoder& decoder) PURE;
 };
 
 /**

mobile/library/common/http/client.h

@@ -163,6 +163,7 @@ class Client : public Logger::Loggable<Logger::Id::http> {
       PANIC("not implemented");
     }
     bool streamErrorOnInvalidHttpMessage() const override { return false; }
+    void setRequestDecoder(RequestDecoder& /*decoder*/) override{};
 
     void encodeMetadata(const MetadataMapVector&) override { PANIC("not implemented"); }
 

source/common/http/conn_manager_impl.cc

@@ -1747,9 +1747,6 @@ void ConnectionManagerImpl::ActiveStream::onRequestDataTooLarge() {
 
 void ConnectionManagerImpl::ActiveStream::recreateStream(
     StreamInfo::FilterStateSharedPtr filter_state) {
-  // n.b. we do not currently change the codecs to point at the new stream
-  // decoder because the decoder callbacks are complete. It would be good to
-  // null out that pointer but should not be necessary.
   ResponseEncoder* response_encoder = response_encoder_;
   response_encoder_ = nullptr;
 
@@ -1767,6 +1764,12 @@ void ConnectionManagerImpl::ActiveStream::recreateStream(
   connection_manager_.doEndStream(*this, /*check_for_deferred_close*/ false);
 
   RequestDecoder& new_stream = connection_manager_.newStream(*response_encoder, true);
+  // Set the new RequestDecoder on the ResponseEncoder. Even though all of the decoder callbacks
+  // have already been called at this point, the encoder still needs the new decoder for deferred
+  // logging in some cases.
+  // This doesn't currently work for HTTP/1 as the H/1 ResponseEncoder doesn't hold the active
+  // stream's pointer to the RequestDecoder.
+  response_encoder->setRequestDecoder(new_stream);
   // We don't need to copy over the old parent FilterState from the old StreamInfo if it did not
   // store any objects with a LifeSpan at or above DownstreamRequest. This is to avoid unnecessary
   // heap allocation.

source/common/http/http1/codec_impl.h

@@ -156,6 +156,11 @@ class ResponseEncoderImpl : public StreamEncoderImpl, public ResponseEncoder {
     return stream_error_on_invalid_http_message_;
   }
 
+  // For H/1, ResponseEncoder doesn't hold a pointer to RequestDecoder.
+  // TODO(paulsohn): Enable H/1 codec to get a pointer to the new
+  // request decoder on recreateStream, here or elsewhere.
+  void setRequestDecoder(Http::RequestDecoder& /*decoder*/) override {}
+
   // Http1::StreamEncoderImpl
   void resetStream(StreamResetReason reason) override;
 

source/common/http/http2/codec_impl.cc

@@ -2076,7 +2076,7 @@ Status ServerConnectionImpl::onBeginHeaders(const nghttp2_frame* frame) {
   if (connection_.aboveHighWatermark()) {
     stream->runHighWatermarkCallbacks();
   }
-  stream->request_decoder_ = &callbacks_.newStream(*stream);
+  stream->setRequestDecoder(callbacks_.newStream(*stream));
   stream->stream_id_ = frame->hd.stream_id;
   LinkedList::moveIntoList(std::move(stream), active_streams_);
   adapter_->SetStreamUserData(frame->hd.stream_id, active_streams_.front().get());

source/common/http/http2/codec_impl.h

@@ -497,16 +497,19 @@ class ConnectionImpl : public virtual Connection,
     void encodeTrailers(const ResponseTrailerMap& trailers) override {
       encodeTrailersBase(trailers);
     }
+    void setRequestDecoder(Http::RequestDecoder& decoder) override { request_decoder_ = &decoder; }
 
     // ScopeTrackedObject
     void dumpState(std::ostream& os, int indent_level) const override;
 
-    RequestDecoder* request_decoder_{};
     absl::variant<RequestHeaderMapPtr, RequestTrailerMapPtr> headers_or_trailers_;
 
     bool streamErrorOnInvalidHttpMessage() const override {
       return parent_.stream_error_on_invalid_http_messaging_;
     }
+
+  private:
+    RequestDecoder* request_decoder_{};
   };
 
   using ServerStreamImplPtr = std::unique_ptr<ServerStreamImpl>;

source/common/quic/envoy_quic_server_stream.h

@@ -18,7 +18,7 @@ class EnvoyQuicServerStream : public quic::QuicSpdyServerStreamBase,
                         envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
                             headers_with_underscores_action);
 
-  void setRequestDecoder(Http::RequestDecoder& decoder) { request_decoder_ = &decoder; }
+  void setRequestDecoder(Http::RequestDecoder& decoder) override { request_decoder_ = &decoder; }
 
   // Http::StreamEncoder
   void encode1xxHeaders(const Http::ResponseHeaderMap& headers) override;

test/mocks/http/stream_encoder.h

@@ -48,6 +48,7 @@ class MockResponseEncoder : public ResponseEncoder {
   MOCK_METHOD(void, encode1xxHeaders, (const ResponseHeaderMap& headers));
   MOCK_METHOD(void, encodeHeaders, (const ResponseHeaderMap& headers, bool end_stream));
   MOCK_METHOD(void, encodeTrailers, (const ResponseTrailerMap& trailers));
+  MOCK_METHOD(void, setRequestDecoder, (RequestDecoder & decoder));
 
   // Http::StreamEncoder
   MOCK_METHOD(void, encodeData, (Buffer::Instance & data, bool end_stream));