diff --git a/source/common/router/router.cc b/source/common/router/router.cc index cda954c27ca2d..50b3232ad2a7b 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -1299,7 +1299,7 @@ void Filter::onUpstreamHeaders(uint64_t response_code, Http::ResponseHeaderMapPt if (route_entry_->internalRedirectPolicy().enabled() && route_entry_->internalRedirectPolicy().shouldRedirectForResponseCode( static_cast(response_code)) && - setupRedirect(*headers, upstream_request)) { + setupRedirect(*headers)) { return; // If the redirect could not be handled, fail open and let it pass to the // next downstream. @@ -1471,24 +1471,10 @@ void Filter::onUpstreamComplete(UpstreamRequest& upstream_request) { cleanup(); } -bool Filter::setupRedirect(const Http::ResponseHeaderMap& headers, - UpstreamRequest& upstream_request) { +bool Filter::setupRedirect(const Http::ResponseHeaderMap& headers) { ENVOY_STREAM_LOG(debug, "attempting internal redirect", *callbacks_); const Http::HeaderEntry* location = headers.Location(); - // If the internal redirect succeeds, callbacks_->recreateStream() will result in the - // destruction of this filter before the stream is marked as complete, and onDestroy will reset - // the stream. - // - // Normally when a stream is complete we signal this by resetting the upstream but this cannot - // be done in this case because if recreateStream fails, the "failure" path continues to call - // code in onUpstreamHeaders which requires the upstream *not* be reset. To avoid onDestroy - // performing a spurious stream reset in the case recreateStream() succeeds, we explicitly track - // stream completion here and check it in onDestroy. This is annoyingly complicated but is - // better than needlessly resetting streams. - attempting_internal_redirect_with_complete_stream_ = - upstream_request.upstreamTiming().last_upstream_rx_byte_received_ && downstream_end_stream_; - const uint64_t status_code = Http::Utility::getResponseStatus(headers); // Redirects are not supported for streaming requests yet. @@ -1502,8 +1488,6 @@ bool Filter::setupRedirect(const Http::ResponseHeaderMap& headers, return true; } - attempting_internal_redirect_with_complete_stream_ = false; - ENVOY_STREAM_LOG(debug, "Internal redirect failed", *callbacks_); cluster_->stats().upstream_internal_redirect_failed_total_.inc(); return false; diff --git a/source/common/router/router.h b/source/common/router/router.h index f250b3d58e412..e06d577f34cf0 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -285,9 +285,7 @@ class Filter : Logger::Loggable, Filter(FilterConfig& config) : config_(config), final_upstream_request_(nullptr), downstream_100_continue_headers_encoded_(false), downstream_response_started_(false), - downstream_end_stream_(false), is_retry_(false), - attempting_internal_redirect_with_complete_stream_(false), - request_buffer_overflowed_(false) {} + downstream_end_stream_(false), is_retry_(false), request_buffer_overflowed_(false) {} ~Filter() override; @@ -497,7 +495,7 @@ class Filter : Logger::Loggable, // for the remaining upstream requests to return. void resetOtherUpstreams(UpstreamRequest& upstream_request); void sendNoHealthyUpstreamResponse(); - bool setupRedirect(const Http::ResponseHeaderMap& headers, UpstreamRequest& upstream_request); + bool setupRedirect(const Http::ResponseHeaderMap& headers); bool convertRequestHeadersForInternalRedirect(Http::RequestHeaderMap& downstream_headers, const Http::HeaderEntry& internal_redirect, uint64_t status_code); @@ -544,7 +542,6 @@ class Filter : Logger::Loggable, bool downstream_end_stream_ : 1; bool is_retry_ : 1; bool include_attempt_count_in_request_ : 1; - bool attempting_internal_redirect_with_complete_stream_ : 1; bool request_buffer_overflowed_ : 1; bool internal_redirects_with_body_enabled_ : 1; uint32_t attempt_count_{1};