diff --git a/envoy/http/codec.h b/envoy/http/codec.h index 99c696964c58..236a4da6f250 100644 --- a/envoy/http/codec.h +++ b/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; }; /** diff --git a/mobile/library/common/http/client.h b/mobile/library/common/http/client.h index 68f7eab06750..4a2d459df034 100644 --- a/mobile/library/common/http/client.h +++ b/mobile/library/common/http/client.h @@ -163,6 +163,7 @@ class Client : public Logger::Loggable { PANIC("not implemented"); } bool streamErrorOnInvalidHttpMessage() const override { return false; } + void setRequestDecoder(RequestDecoder& /*decoder*/) override{}; void encodeMetadata(const MetadataMapVector&) override { PANIC("not implemented"); } diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index f9b3c21de475..2faa6fa82097 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/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. diff --git a/source/common/http/http1/codec_impl.h b/source/common/http/http1/codec_impl.h index ec2cdc51826a..1e1bd36dc131 100644 --- a/source/common/http/http1/codec_impl.h +++ b/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; diff --git a/source/common/http/http2/codec_impl.cc b/source/common/http/http2/codec_impl.cc index 812e3d88f211..e03f8e097e71 100644 --- a/source/common/http/http2/codec_impl.cc +++ b/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()); diff --git a/source/common/http/http2/codec_impl.h b/source/common/http/http2/codec_impl.h index ed7808c285bf..1176c6058a78 100644 --- a/source/common/http/http2/codec_impl.h +++ b/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 headers_or_trailers_; bool streamErrorOnInvalidHttpMessage() const override { return parent_.stream_error_on_invalid_http_messaging_; } + + private: + RequestDecoder* request_decoder_{}; }; using ServerStreamImplPtr = std::unique_ptr; diff --git a/source/common/quic/envoy_quic_server_stream.h b/source/common/quic/envoy_quic_server_stream.h index edd3aa60887c..3a2ae67c316d 100644 --- a/source/common/quic/envoy_quic_server_stream.h +++ b/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; diff --git a/test/mocks/http/stream_encoder.h b/test/mocks/http/stream_encoder.h index c15da88f170d..efe3626af3ab 100644 --- a/test/mocks/http/stream_encoder.h +++ b/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));