diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index a1b02e01877e..c64bf8f07fa6 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -14,7 +14,6 @@ #include "envoy/config/core/v3/base.pb.h" #include "envoy/config/route/v3/route_components.pb.h" #include "envoy/config/typed_metadata.h" -#include "envoy/event/deferred_deletable.h" #include "envoy/http/codec.h" #include "envoy/http/codes.h" #include "envoy/http/conn_pool.h" @@ -1257,8 +1256,9 @@ class GenericConnectionPoolCallbacks { * * It is similar logically to RequestEncoder, only without the getStream interface. */ -class GenericUpstream : public Event::DeferredDeletable { +class GenericUpstream { public: + virtual ~GenericUpstream() = default; /** * Encode a data frame. * @param data supplies the data to encode. The data may be moved by the encoder. diff --git a/source/common/http/status.cc b/source/common/http/status.cc index af8d9ecd90f7..1204a9ac9e4f 100644 --- a/source/common/http/status.cc +++ b/source/common/http/status.cc @@ -25,8 +25,6 @@ absl::string_view statusCodeToString(StatusCode code) { return "CodecClientError"; case StatusCode::InboundFramesWithEmptyPayload: return "InboundFramesWithEmptyPayloadError"; - case StatusCode::StreamAlreadyReset: - return "StreamAlreadyReset"; } NOT_REACHED_GCOVR_EXCL_LINE; } @@ -115,13 +113,6 @@ Status inboundFramesWithEmptyPayloadError() { return status; } -Status streamAlreadyReset() { - absl::Status status(absl::StatusCode::kInternal, - "Attempted to proxy headers after stream has been reset."); - storePayload(status, EnvoyStatusPayload(StatusCode::StreamAlreadyReset)); - return status; -} - // Methods for checking and extracting error information StatusCode getStatusCode(const Status& status) { return status.ok() ? StatusCode::Ok : getPayload(status).status_code_; diff --git a/source/common/http/status.h b/source/common/http/status.h index 6bb73851b9f2..97c7dac96ca1 100644 --- a/source/common/http/status.h +++ b/source/common/http/status.h @@ -74,11 +74,6 @@ enum class StatusCode : int { * Indicates that peer sent too many consecutive DATA frames with empty payload. */ InboundFramesWithEmptyPayload = 5, - - /** - * Indicates that we attempted to proxy upstream headers to a stream that has already been reset. - */ - StreamAlreadyReset = 6, }; using Status = absl::Status; @@ -99,7 +94,6 @@ Status bufferFloodError(absl::string_view message); Status prematureResponseError(absl::string_view message, Http::Code http_code); Status codecClientError(absl::string_view message); Status inboundFramesWithEmptyPayloadError(); -Status streamAlreadyReset(); /** * Returns Envoy::StatusCode of the given status object. @@ -115,7 +109,6 @@ ABSL_MUST_USE_RESULT bool isBufferFloodError(const Status& status); ABSL_MUST_USE_RESULT bool isPrematureResponseError(const Status& status); ABSL_MUST_USE_RESULT bool isCodecClientError(const Status& status); ABSL_MUST_USE_RESULT bool isInboundFramesWithEmptyPayloadError(const Status& status); -ABSL_MUST_USE_RESULT bool isStreamAlreadyReset(const Status& status); /** * Returns Http::Code value of the PrematureResponseError status. diff --git a/source/common/router/upstream_request.cc b/source/common/router/upstream_request.cc index e63d30ff59c7..5a20b8a84154 100644 --- a/source/common/router/upstream_request.cc +++ b/source/common/router/upstream_request.cc @@ -481,8 +481,8 @@ void UpstreamRequest::clearRequestEncoder() { // Before clearing the encoder, unsubscribe from callbacks. if (upstream_) { parent_.callbacks()->removeDownstreamWatermarkCallbacks(downstream_watermark_manager_); - parent_.callbacks()->dispatcher().deferredDelete(std::move(upstream_)); } + upstream_.reset(); } void UpstreamRequest::DownstreamWatermarkManager::onAboveWriteBufferHighWatermark() { diff --git a/source/extensions/upstreams/http/tcp/upstream_request.cc b/source/extensions/upstreams/http/tcp/upstream_request.cc index 8d0d4b25a4b5..bc2f1b76f4b2 100644 --- a/source/extensions/upstreams/http/tcp/upstream_request.cc +++ b/source/extensions/upstreams/http/tcp/upstream_request.cc @@ -11,7 +11,6 @@ #include "common/http/header_map_impl.h" #include "common/http/headers.h" #include "common/http/message_impl.h" -#include "common/http/status.h" #include "common/network/transport_socket_options_impl.h" #include "common/router/router.h" @@ -46,10 +45,6 @@ void TcpUpstream::encodeData(Buffer::Instance& data, bool end_stream) { Envoy::Http::Status TcpUpstream::encodeHeaders(const Envoy::Http::RequestHeaderMap&, bool end_stream) { - if (!upstream_request_) { - return Envoy::Http::streamAlreadyReset(); - } - // Headers should only happen once, so use this opportunity to add the proxy // proto header, if configured. ASSERT(upstream_request_->routeEntry().connectConfig().has_value()); @@ -91,16 +86,7 @@ void TcpUpstream::resetStream() { } void TcpUpstream::onUpstreamData(Buffer::Instance& data, bool end_stream) { - if (!upstream_request_) { - return; - } - upstream_request_->decodeData(data, end_stream); - // This ensures that if we get a reset after end_stream we won't propagate two - // "end streams" to the upstream_request_. - if (end_stream) { - upstream_request_ = nullptr; - } } void TcpUpstream::onEvent(Network::ConnectionEvent event) { diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 663f3f326685..30cea338d1d0 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -118,10 +118,6 @@ class RouterTestBase : public testing::Test { // Allow any number of setTrackedObject calls for the dispatcher strict mock. EXPECT_CALL(callbacks_.dispatcher_, setTrackedObject(_)).Times(AnyNumber()); } - ~RouterTestBase() override { - EXPECT_CALL(callbacks_.dispatcher_, clearDeferredDeleteList()); - callbacks_.dispatcher_.clearDeferredDeleteList(); - } void expectResponseTimerCreate() { response_timeout_ = new Event::MockTimer(&callbacks_.dispatcher_); @@ -282,7 +278,6 @@ class RouterTestBase : public testing::Test { .WillOnce(Invoke([expected_count](Http::ResponseHeaderMap& headers, bool) { EXPECT_EQ(expected_count, atoi(std::string(headers.getEnvoyAttemptCountValue()).c_str())); })); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeHeaders(std::move(response_headers), true); EXPECT_TRUE(verifyHostUpstreamStats(1, 0)); EXPECT_EQ(1U, @@ -507,7 +502,6 @@ TEST_F(RouterTest, MissingRequiredHeaders) { "filter_removed_required_headers{missing required header: :method}")) .WillOnce(testing::InvokeWithoutArgs([] {})); router_.decodeHeaders(headers, true); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.onDestroy(); } @@ -731,7 +725,6 @@ TEST_F(RouterTest, AddCookie) { Http::ResponseHeaderMapPtr response_headers( new Http::TestResponseHeaderMapImpl{{":status", "200"}}); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeHeaders(std::move(response_headers), true); EXPECT_EQ(callbacks_.details(), "via_upstream"); // When the router filter gets reset we should cancel the pool request. @@ -785,7 +778,6 @@ TEST_F(RouterTest, AddCookieNoDuplicate) { Http::ResponseHeaderMapPtr response_headers( new Http::TestResponseHeaderMapImpl{{":status", "200"}, {"set-cookie", "foo=baz"}}); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeHeaders(std::move(response_headers), true); // When the router filter gets reset we should cancel the pool request. router_.onDestroy(); @@ -846,7 +838,6 @@ TEST_F(RouterTest, AddMultipleCookies) { Http::ResponseHeaderMapPtr response_headers( new Http::TestResponseHeaderMapImpl{{":status", "200"}}); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeHeaders(std::move(response_headers), true); router_.onDestroy(); } @@ -1009,7 +1000,6 @@ TEST_F(RouterTest, ResponseCodeDetailsSetByUpstream) { Http::ResponseHeaderMapPtr response_headers( new Http::TestResponseHeaderMapImpl{{":status", "200"}}); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeHeaders(std::move(response_headers), true); EXPECT_TRUE(verifyHostUpstreamStats(1, 0)); } @@ -1042,7 +1032,6 @@ TEST_F(RouterTest, EnvoyUpstreamServiceTime) { .WillOnce(Invoke([](Http::HeaderMap& headers, bool) { EXPECT_FALSE(headers.get(Http::Headers::get().EnvoyUpstreamServiceTime).empty()); })); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeHeaders(std::move(response_headers), true); EXPECT_TRUE(verifyHostUpstreamStats(1, 0)); } @@ -1104,7 +1093,6 @@ TEST_F(RouterTest, EnvoyAttemptCountInRequestUpdatedInRetries) { new Http::TestResponseHeaderMapImpl{{":status", "503"}}); EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putHttpResponseCode(503)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeHeaders(std::move(response_headers1), true); EXPECT_TRUE(verifyHostUpstreamStats(0, 1)); @@ -1134,7 +1122,6 @@ TEST_F(RouterTest, EnvoyAttemptCountInRequestUpdatedInRetries) { new Http::TestResponseHeaderMapImpl{{":status", "200"}}); EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putHttpResponseCode(200)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeHeaders(std::move(response_headers2), true); EXPECT_TRUE(verifyHostUpstreamStats(1, 1)); } @@ -1238,7 +1225,6 @@ TEST_F(RouterTest, EnvoyAttemptCountInResponseWithRetries) { Http::TestRequestHeaderMapImpl headers{{"x-envoy-retry-on", "5xx"}, {"x-envoy-internal", "true"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -1280,7 +1266,6 @@ TEST_F(RouterTest, EnvoyAttemptCountInResponseWithRetries) { // Because a retry happened the number of attempts in the response headers should be 2. EXPECT_EQ(2, atoi(std::string(headers.getEnvoyAttemptCountValue()).c_str())); })); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeHeaders(std::move(response_headers2), true); EXPECT_TRUE(verifyHostUpstreamStats(1, 1)); } @@ -1329,7 +1314,6 @@ void RouterTestBase::testAppendCluster(absl::optional clu EXPECT_FALSE(cluster_header.empty()); EXPECT_EQ("fake_cluster", cluster_header[0]->value().getStringView()); })); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeHeaders(std::move(response_headers), true); EXPECT_TRUE(verifyHostUpstreamStats(1, 0)); } @@ -1395,7 +1379,6 @@ void RouterTestBase::testAppendUpstreamHost( EXPECT_FALSE(host_address_header.empty()); EXPECT_EQ("10.0.0.5:9211", host_address_header[0]->value().getStringView()); })); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeHeaders(std::move(response_headers), true); EXPECT_TRUE(verifyHostUpstreamStats(1, 0)); } @@ -1509,7 +1492,6 @@ TEST_F(RouterTestSuppressEnvoyHeaders, EnvoyUpstreamServiceTime) { Http::TestRequestHeaderMapImpl headers; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -1554,7 +1536,6 @@ TEST_F(RouterTest, NoRetriesOverflow) { new Http::TestResponseHeaderMapImpl{{":status", "503"}}); EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putHttpResponseCode(503)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeHeaders(std::move(response_headers1), true); EXPECT_TRUE(verifyHostUpstreamStats(0, 1)); @@ -1583,7 +1564,6 @@ TEST_F(RouterTest, NoRetriesOverflow) { new Http::TestResponseHeaderMapImpl{{":status", "503"}}); EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putHttpResponseCode(503)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeHeaders(std::move(response_headers2), true); EXPECT_TRUE(verifyHostUpstreamStats(0, 2)); } @@ -1617,7 +1597,6 @@ TEST_F(RouterTest, ResetDuringEncodeHeaders) { absl::optional(absl::nullopt))); EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putResult(Upstream::Outlier::Result::LocalOriginConnectFailed, _)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -1661,7 +1640,6 @@ TEST_F(RouterTest, UpstreamTimeout) { EXPECT_CALL(*router_.retry_state_, shouldRetryReset(_, _)).Times(0); EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putResult(Upstream::Outlier::Result::LocalOriginTimeout, _)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_timeout_->invokeCallback(); EXPECT_EQ(1U, @@ -1714,7 +1692,6 @@ TEST_F(RouterTest, TimeoutBudgetHistogramStat) { new Http::TestResponseHeaderMapImpl{{":status", "200"}}); response_decoder->decodeHeaders(std::move(response_headers), false); test_time_.advanceTimeWait(std::chrono::milliseconds(80)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeData(data, true); } @@ -1759,7 +1736,6 @@ TEST_F(RouterTest, TimeoutBudgetHistogramStatFailure) { new Http::TestResponseHeaderMapImpl{{":status", "500"}}); response_decoder->decodeHeaders(std::move(response_headers), false); test_time_.advanceTimeWait(std::chrono::milliseconds(80)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeData(data, true); } @@ -1801,7 +1777,6 @@ TEST_F(RouterTest, TimeoutBudgetHistogramStatOnlyGlobal) { new Http::TestResponseHeaderMapImpl{{":status", "200"}}); response_decoder->decodeHeaders(std::move(response_headers), false); test_time_.advanceTimeWait(std::chrono::milliseconds(80)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeData(data, true); } @@ -1849,7 +1824,6 @@ TEST_F(RouterTest, TimeoutBudgetHistogramStatDuringRetries) { new Http::TestResponseHeaderMapImpl{{":status", "504"}}); EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putHttpResponseCode(504)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); response_decoder1->decodeHeaders(std::move(response_headers1), true); EXPECT_TRUE(verifyHostUpstreamStats(0, 1)); @@ -1924,7 +1898,6 @@ TEST_F(RouterTest, TimeoutBudgetHistogramStatDuringGlobalTimeout) { {"x-envoy-upstream-rq-timeout-ms", "400"}, {"x-envoy-upstream-rq-per-try-timeout-ms", "320"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); router_.decodeHeaders(headers, false); Buffer::OwnedImpl data; router_.decodeData(data, true); @@ -2021,7 +1994,6 @@ TEST_F(RouterTest, GrpcOkTrailersOnly) { Http::TestRequestHeaderMapImpl headers{{"content-type", "application/grpc"}, {"grpc-timeout", "20S"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -2052,7 +2024,6 @@ TEST_F(RouterTest, GrpcAlreadyExistsTrailersOnly) { Http::TestRequestHeaderMapImpl headers{{"content-type", "application/grpc"}, {"grpc-timeout", "20S"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -2083,7 +2054,6 @@ TEST_F(RouterTest, GrpcOutlierDetectionUnavailableStatusCode) { Http::TestRequestHeaderMapImpl headers{{"content-type", "application/grpc"}, {"grpc-timeout", "20S"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -2123,7 +2093,6 @@ TEST_F(RouterTest, GrpcInternalTrailersOnly) { new Http::TestResponseHeaderMapImpl{{":status", "200"}, {"grpc-status", "13"}}); EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putHttpResponseCode(500)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeHeaders(std::move(response_headers), true); EXPECT_TRUE(verifyHostUpstreamStats(0, 1)); } @@ -2147,7 +2116,6 @@ TEST_F(RouterTest, GrpcDataEndStream) { Http::TestRequestHeaderMapImpl headers{{"content-type", "application/grpc"}, {"grpc-timeout", "20S"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -2182,7 +2150,6 @@ TEST_F(RouterTest, GrpcReset) { Http::TestRequestHeaderMapImpl headers{{"content-type", "application/grpc"}, {"grpc-timeout", "20S"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -2218,7 +2185,6 @@ TEST_F(RouterTest, GrpcOk) { Http::TestRequestHeaderMapImpl headers{{"content-type", "application/grpc"}, {"grpc-timeout", "20S"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -2256,7 +2222,6 @@ TEST_F(RouterTest, GrpcInternal) { Http::TestRequestHeaderMapImpl headers{{"content-type", "application/grpc"}, {"grpc-timeout", "20S"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -2310,7 +2275,6 @@ TEST_F(RouterTest, UpstreamTimeoutWithAltResponse) { EXPECT_CALL( cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putResult(Upstream::Outlier::Result::LocalOriginTimeout, absl::optional(204))); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_timeout_->invokeCallback(); EXPECT_EQ(1U, @@ -2363,7 +2327,6 @@ TEST_F(RouterTest, UpstreamPerTryTimeout) { EXPECT_CALL( cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putResult(Upstream::Outlier::Result::LocalOriginTimeout, absl::optional(504))); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); per_try_timeout_->invokeCallback(); EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_ @@ -2421,7 +2384,6 @@ TEST_F(RouterTest, UpstreamPerTryTimeoutDelayedPoolReady) { EXPECT_CALL(callbacks_, encodeData(_, true)); EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putResult(Upstream::Outlier::Result::LocalOriginTimeout, _)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); per_try_timeout_->invokeCallback(); EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_ @@ -2473,7 +2435,6 @@ TEST_F(RouterTest, UpstreamPerTryTimeoutExcludesNewStream) { callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); EXPECT_CALL(encoder.stream_, resetStream(Http::StreamResetReason::LocalReset)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putResult(Upstream::Outlier::Result::LocalOriginTimeout, _)); EXPECT_CALL(*per_try_timeout_, disableTimer()); @@ -2566,7 +2527,6 @@ TEST_F(RouterTest, HedgedPerTryTimeoutFirstRequestSucceeds) { EXPECT_EQ(headers.Status()->value(), "200"); EXPECT_TRUE(end_stream); })); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); response_decoder1->decodeHeaders(std::move(response_headers), true); EXPECT_TRUE(verifyHostUpstreamStats(1, 0)); @@ -2604,7 +2564,6 @@ TEST_F(RouterTest, HedgedPerTryTimeoutResetsOnBadHeaders) { Http::TestRequestHeaderMapImpl headers{{"x-envoy-upstream-rq-per-try-timeout-ms", "5"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -2691,7 +2650,6 @@ TEST_F(RouterTest, HedgedPerTryTimeoutThirdRequestSucceeds) { Http::TestRequestHeaderMapImpl headers{{"x-envoy-upstream-rq-per-try-timeout-ms", "5"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(3); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -2813,7 +2771,6 @@ TEST_F(RouterTest, RetryOnlyOnceForSameUpstreamRequest) { cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putResult(Upstream::Outlier::Result::LocalOriginTimeout, absl::optional(504))); router_.retry_state_->expectHedgedPerTryTimeoutRetry(); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); per_try_timeout_->invokeCallback(); NiceMock encoder2; @@ -2876,7 +2833,6 @@ TEST_F(RouterTest, BadHeadersDroppedIfPreviousRetryScheduled) { Http::TestRequestHeaderMapImpl headers{{"x-envoy-upstream-rq-per-try-timeout-ms", "5"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); router_.decodeHeaders(headers, true); EXPECT_CALL(encoder1.stream_, resetStream(_)).Times(0); @@ -2947,7 +2903,6 @@ TEST_F(RouterTest, RetryRequestBeforeBody) { Http::TestRequestHeaderMapImpl headers{ {"x-envoy-retry-on", "5xx"}, {"x-envoy-internal", "true"}, {"myheader", "present"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); router_.decodeHeaders(headers, false); router_.retry_state_->expectResetRetry(); @@ -3010,7 +2965,6 @@ TEST_F(RouterTest, RetryRequestDuringBody) { Http::TestRequestHeaderMapImpl headers{ {"x-envoy-retry-on", "5xx"}, {"x-envoy-internal", "true"}, {"myheader", "present"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); router_.decodeHeaders(headers, false); const std::string body1("body1"); Buffer::OwnedImpl buf1(body1); @@ -3088,7 +3042,6 @@ TEST_F(RouterTest, RetryRequestDuringBodyDataBetweenAttemptsNotEndStream) { router_.decodeData(buf1, false); router_.retry_state_->expectResetRetry(); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); encoder1.stream_.resetStream(Http::StreamResetReason::RemoteReset); const std::string body2("body2"); @@ -3117,7 +3070,6 @@ TEST_F(RouterTest, RetryRequestDuringBodyDataBetweenAttemptsNotEndStream) { const std::string body3("body3"); EXPECT_CALL(encoder2, encodeData(BufferStringEqual(body3), true)); Buffer::OwnedImpl buf3(body3); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeData(buf3, true); // Send successful response, verify success. @@ -3162,13 +3114,11 @@ TEST_F(RouterTest, RetryRequestDuringBodyCompleteBetweenAttempts) { router_.decodeData(buf1, false); router_.retry_state_->expectResetRetry(); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); encoder1.stream_.resetStream(Http::StreamResetReason::RemoteReset); // Complete request while there is no upstream request. const std::string body2("body2"); Buffer::OwnedImpl buf2(body2); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeData(buf2, true); NiceMock encoder2; @@ -3231,12 +3181,10 @@ TEST_F(RouterTest, RetryRequestDuringBodyTrailerBetweenAttempts) { router_.decodeData(buf1, false); router_.retry_state_->expectResetRetry(); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); encoder1.stream_.resetStream(Http::StreamResetReason::RemoteReset); // Complete request while there is no upstream request. Http::TestRequestTrailerMapImpl trailers{{"some", "trailer"}}; - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeTrailers(trailers); NiceMock encoder2; @@ -3301,7 +3249,6 @@ TEST_F(RouterTest, RetryRequestDuringBodyBufferLimitExceeded) { router_.decodeData(buf1, false); router_.retry_state_->expectResetRetry(); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); encoder1.stream_.resetStream(Http::StreamResetReason::RemoteReset); // Complete request while there is no upstream request. @@ -3384,7 +3331,6 @@ TEST_F(RouterTest, HedgedPerTryTimeoutGlobalTimeout) { .WillOnce(Invoke([&](Http::ResponseHeaderMap& headers, bool) -> void { EXPECT_EQ(headers.Status()->value(), "504"); })); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); response_timeout_->invokeCallback(); EXPECT_TRUE(verifyHostUpstreamStats(0, 2)); EXPECT_EQ(2, cm_.thread_local_cluster_.conn_pool_.host_->stats_.rq_timeout_.value()); @@ -3416,7 +3362,6 @@ TEST_F(RouterTest, HedgingRetriesExhaustedBadResponse) { Http::TestRequestHeaderMapImpl headers{{"x-envoy-upstream-rq-per-try-timeout-ms", "5"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -3511,7 +3456,6 @@ TEST_F(RouterTest, HedgingRetriesProceedAfterReset) { Http::TestRequestHeaderMapImpl headers{{"x-envoy-upstream-rq-per-try-timeout-ms", "5"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -3594,7 +3538,6 @@ TEST_F(RouterTest, HedgingRetryImmediatelyReset) { Http::TestRequestHeaderMapImpl headers{{"x-envoy-upstream-rq-per-try-timeout-ms", "5"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, false); expectPerTryTimerCreate(); @@ -3677,7 +3620,6 @@ TEST_F(RouterTest, RetryNoneHealthy) { router_.retry_state_->expectResetRetry(); EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putResult(Upstream::Outlier::Result::LocalOriginConnectFailed, _)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); encoder1.stream_.resetStream(Http::StreamResetReason::LocalReset); EXPECT_CALL(cm_.thread_local_cluster_, httpConnPool(_, _, _)).WillOnce(Return(nullptr)); @@ -3721,7 +3663,6 @@ TEST_F(RouterTest, RetryUpstreamReset) { router_.retry_state_->expectResetRetry(); EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putResult(Upstream::Outlier::Result::LocalOriginConnectFailed, _)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); encoder1.stream_.resetStream(Http::StreamResetReason::RemoteReset); // We expect this reset to kick off a new request. @@ -3770,7 +3711,6 @@ TEST_F(RouterTest, NoRetryWithBodyLimit) { EXPECT_CALL(callbacks_.route_->route_entry_, retryShadowBufferLimit()).WillOnce(Return(0)); Http::TestRequestHeaderMapImpl headers{{"x-envoy-retry-on", "5xx"}, {"x-envoy-internal", "true"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, false); // Unlike RetryUpstreamReset above the data won't be buffered as the body exceeds the buffer limit EXPECT_CALL(*router_.retry_state_, enabled()).WillOnce(Return(true)); @@ -3808,7 +3748,6 @@ TEST_F(RouterTest, RetryUpstreamPerTryTimeout) { {"x-envoy-internal", "true"}, {"x-envoy-upstream-rq-per-try-timeout-ms", "5"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -3863,7 +3802,6 @@ TEST_F(RouterTest, RetryUpstreamConnectionFailure) { Http::TestRequestHeaderMapImpl headers{{"x-envoy-retry-on", "5xx"}, {"x-envoy-internal", "true"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); EXPECT_CALL(*router_.retry_state_, onHostAttempted(_)).Times(0); @@ -3921,7 +3859,6 @@ TEST_F(RouterTest, DontResetStartedResponseOnUpstreamPerTryTimeout) { Http::TestRequestHeaderMapImpl headers{{"x-envoy-internal", "true"}, {"x-envoy-upstream-rq-per-try-timeout-ms", "5"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -3981,7 +3918,6 @@ TEST_F(RouterTest, RetryUpstreamResetResponseStarted) { // the encoder again. EXPECT_CALL(callbacks_, sendLocalReply(_, _, _, _, _)).WillOnce(testing::InvokeWithoutArgs([] { })); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); encoder1.stream_.resetStream(Http::StreamResetReason::RemoteReset); // For normal HTTP, once we have a 200 we consider this a success, even if a // later reset occurs. @@ -4031,7 +3967,6 @@ TEST_F(RouterTest, Coalesce100ContinueHeaders) { new Http::TestResponseHeaderMapImpl{{":status", "100"}}); response_decoder->decode100ContinueHeaders(std::move(continue_headers)); } - EXPECT_EQ( 2U, cm_.thread_local_cluster_.cluster_.info_->stats_store_.counter("upstream_rq_100").value()); @@ -4039,7 +3974,6 @@ TEST_F(RouterTest, Coalesce100ContinueHeaders) { // Reset stream and cleanup. EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putResult(Upstream::Outlier::Result::LocalOriginConnectFailed, _)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); encoder1.stream_.resetStream(Http::StreamResetReason::RemoteReset); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -4078,7 +4012,6 @@ TEST_F(RouterTest, RetryUpstreamReset100ContinueResponseStarted) { cm_.thread_local_cluster_.cluster_.info_->stats_store_.counter("upstream_rq_100").value()); EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putResult(Upstream::Outlier::Result::LocalOriginConnectFailed, _)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); encoder1.stream_.resetStream(Http::StreamResetReason::RemoteReset); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -4100,7 +4033,6 @@ TEST_F(RouterTest, RetryUpstream5xx) { Http::TestRequestHeaderMapImpl headers{{"x-envoy-retry-on", "5xx"}, {"x-envoy-internal", "true"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -4157,7 +4089,6 @@ TEST_F(RouterTest, RetryTimeoutDuringRetryDelay) { Http::TestRequestHeaderMapImpl headers{{"x-envoy-retry-on", "5xx"}, {"x-envoy-internal", "true"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -4203,7 +4134,6 @@ TEST_F(RouterTest, MaxStreamDurationValidlyConfiguredWithoutRetryPolicy) { Http::TestRequestHeaderMapImpl headers; HttpTestUtility::addDefaultHeaders(headers); router_.decodeHeaders(headers, false); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); max_stream_duration_timer_->invokeCallback(); router_.onDestroy(); @@ -4231,7 +4161,6 @@ TEST_F(RouterTest, MaxStreamDurationDisabledIfSetToZero) { HttpTestUtility::addDefaultHeaders(headers); router_.decodeHeaders(headers, false); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.onDestroy(); EXPECT_TRUE(verifyHostUpstreamStats(0, 0)); } @@ -4255,7 +4184,6 @@ TEST_F(RouterTest, MaxStreamDurationCallbackNotCalled) { HttpTestUtility::addDefaultHeaders(headers); router_.decodeHeaders(headers, false); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.onDestroy(); EXPECT_TRUE(verifyHostUpstreamStats(0, 0)); } @@ -4281,7 +4209,6 @@ TEST_F(RouterTest, MaxStreamDurationWhenDownstreamAlreadyStartedWithoutRetryPoli Http::ResponseHeaderMapPtr response_headers( new Http::TestResponseHeaderMapImpl{{":status", "200"}}); response_decoder->decodeHeaders(std::move(response_headers), false); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); max_stream_duration_timer_->invokeCallback(); router_.onDestroy(); @@ -4307,8 +4234,6 @@ TEST_F(RouterTest, MaxStreamDurationWithRetryPolicy) { Http::TestRequestHeaderMapImpl headers{{"x-envoy-retry-on", "reset"}, {"x-envoy-internal", "true"}}; HttpTestUtility::addDefaultHeaders(headers); - - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); router_.decodeHeaders(headers, false); router_.retry_state_->expectResetRetry(); @@ -4352,7 +4277,6 @@ TEST_F(RouterTest, RetryTimeoutDuringRetryDelayWithUpstreamRequestNoHost) { Http::TestRequestHeaderMapImpl headers{{"x-envoy-retry-on", "5xx"}, {"x-envoy-internal", "true"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -4412,7 +4336,6 @@ TEST_F(RouterTest, RetryTimeoutDuringRetryDelayWithUpstreamRequestNoHostAltRespo {"x-envoy-internal", "true"}, {"x-envoy-upstream-rq-timeout-alt-response", "204"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -4475,7 +4398,6 @@ TEST_F(RouterTest, RetryUpstream5xxNotComplete) { EXPECT_EQ(Http::FilterDataStatus::StopIterationNoBuffer, router_.decodeData(*body_data, false)); Http::TestRequestTrailerMapImpl trailers{{"some", "trailer"}}; - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); router_.decodeTrailers(trailers); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -4554,7 +4476,6 @@ TEST_F(RouterTest, RetryUpstreamGrpcCancelled) { {"content-type", "application/grpc"}, {"grpc-timeout", "20S"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -4637,7 +4558,6 @@ TEST_F(RouterTest, RetryRespsectsMaxHostSelectionCount) { EXPECT_CALL(encoder1.stream_, resetStream(Http::StreamResetReason::LocalReset)); EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putHttpResponseCode(503)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); response_decoder->decodeHeaders(std::move(response_headers1), false); EXPECT_TRUE(verifyHostUpstreamStats(0, 1)); @@ -4717,7 +4637,6 @@ TEST_F(RouterTest, RetryRespectsRetryHostPredicate) { EXPECT_CALL(encoder1.stream_, resetStream(Http::StreamResetReason::LocalReset)); EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putHttpResponseCode(503)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); response_decoder->decodeHeaders(std::move(response_headers1), false); EXPECT_TRUE(verifyHostUpstreamStats(0, 1)); @@ -4764,7 +4683,6 @@ TEST_F(RouterTest, InternalRedirectRejectedWhenReachingMaxInternalRedirect) { response_decoder_->decodeHeaders(std::move(redirect_headers_), false); Buffer::OwnedImpl data("1234567890"); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder_->decodeData(data, true); EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_ .counter("upstream_internal_redirect_failed_total") @@ -4784,7 +4702,6 @@ TEST_F(RouterTest, InternalRedirectRejectedWithEmptyLocation) { response_decoder_->decodeHeaders(std::move(redirect_headers_), false); Buffer::OwnedImpl data("1234567890"); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder_->decodeData(data, true); EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_ .counter("upstream_internal_redirect_failed_total") @@ -4803,7 +4720,6 @@ TEST_F(RouterTest, InternalRedirectRejectedWithInvalidLocation) { response_decoder_->decodeHeaders(std::move(redirect_headers_), false); Buffer::OwnedImpl data("1234567890"); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder_->decodeData(data, true); EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_ .counter("upstream_internal_redirect_failed_total") @@ -4821,7 +4737,6 @@ TEST_F(RouterTest, InternalRedirectRejectedWithoutCompleteRequest) { response_decoder_->decodeHeaders(std::move(redirect_headers_), false); Buffer::OwnedImpl data("1234567890"); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder_->decodeData(data, true); EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_ .counter("upstream_internal_redirect_failed_total") @@ -4839,7 +4754,6 @@ TEST_F(RouterTest, InternalRedirectRejectedWithoutLocation) { response_decoder_->decodeHeaders(std::move(redirect_headers_), false); Buffer::OwnedImpl data("1234567890"); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder_->decodeData(data, true); EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_ .counter("upstream_internal_redirect_failed_total") @@ -4857,7 +4771,6 @@ TEST_F(RouterTest, InternalRedirectRejectedWithBody) { response_decoder_->decodeHeaders(std::move(redirect_headers_), false); Buffer::OwnedImpl data("1234567890"); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder_->decodeData(data, true); EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_ .counter("upstream_internal_redirect_failed_total") @@ -4867,7 +4780,6 @@ TEST_F(RouterTest, InternalRedirectRejectedWithBody) { TEST_F(RouterTest, CrossSchemeRedirectRejectedByPolicy) { enableRedirects(); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); sendRequest(); redirect_headers_->setLocation("https://www.foo.com"); @@ -4885,7 +4797,6 @@ TEST_F(RouterTest, CrossSchemeRedirectRejectedByPolicy) { TEST_F(RouterTest, InternalRedirectRejectedByPredicate) { enableRedirects(); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); sendRequest(); redirect_headers_->setLocation("http://www.foo.com/some/path"); @@ -4928,7 +4839,6 @@ TEST_F(RouterTest, HttpInternalRedirectSucceeded) { .value()); // In production, the HCM recreateStream would have called this. - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.onDestroy(); EXPECT_EQ(3, callbacks_.streamInfo() .filterState() @@ -4954,7 +4864,6 @@ TEST_F(RouterTest, HttpsInternalRedirectSucceeded) { .value()); // In production, the HCM recreateStream would have called this. - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.onDestroy(); } @@ -4978,7 +4887,6 @@ TEST_F(RouterTest, CrossSchemeRedirectAllowedByPolicy) { .value()); // In production, the HCM recreateStream would have called this. - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.onDestroy(); } @@ -5008,7 +4916,6 @@ TEST_F(RouterTest, Shadow) { Http::TestRequestHeaderMapImpl headers; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, false); Buffer::InstancePtr body_data(new Buffer::OwnedImpl("hello")); @@ -5066,7 +4973,6 @@ TEST_F(RouterTest, AltStatName) { Http::TestRequestHeaderMapImpl headers{{"x-envoy-upstream-alt-stat-name", "alt_stat"}, {"x-envoy-internal", "true"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -5281,7 +5187,6 @@ TEST_F(RouterTest, PropagatesUpstreamFilterState) { Http::TestRequestHeaderMapImpl headers{}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); Http::ResponseHeaderMapPtr response_headers( @@ -5320,7 +5225,6 @@ TEST_F(RouterTest, UpstreamSSLConnection) { Http::ResponseHeaderMapPtr response_headers( new Http::TestResponseHeaderMapImpl{{":status", "200"}}); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeHeaders(std::move(response_headers), true); EXPECT_TRUE(verifyHostUpstreamStats(1, 0)); @@ -5357,7 +5261,6 @@ TEST_F(RouterTest, UpstreamTimingSingleRequest) { test_time_.advanceTimeWait(std::chrono::milliseconds(32)); Buffer::OwnedImpl data; - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeData(data, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -5413,7 +5316,6 @@ TEST_F(RouterTest, UpstreamTimingRetry) { // Check that upstream timing is updated after the first request. Http::TestRequestHeaderMapImpl headers{{"x-envoy-retry-on", "5xx"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); router_.decodeHeaders(headers, false); router_.retry_state_->expectHeadersRetry(); @@ -5520,7 +5422,6 @@ TEST_F(RouterTest, UpstreamTimingTimeout) { response_decoder->decodeHeaders(std::move(response_headers), false); test_time_.advanceTimeWait(std::chrono::milliseconds(99)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_timeout_->invokeCallback(); EXPECT_TRUE(stream_info.firstUpstreamTxByteSent().has_value()); @@ -6074,7 +5975,6 @@ TEST_F(RouterTest, CanaryStatusTrue) { {"x-envoy-upstream-canary", "false"}, {"x-envoy-virtual-cluster", "hello"}}); ON_CALL(*cm_.thread_local_cluster_.conn_pool_.host_, canary()).WillByDefault(Return(true)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeHeaders(std::move(response_headers), true); EXPECT_TRUE(verifyHostUpstreamStats(1, 0)); @@ -6111,7 +6011,6 @@ TEST_F(RouterTest, CanaryStatusFalse) { new Http::TestResponseHeaderMapImpl{{":status", "200"}, {"x-envoy-upstream-canary", "false"}, {"x-envoy-virtual-cluster", "hello"}}); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder->decodeHeaders(std::move(response_headers), true); EXPECT_TRUE(verifyHostUpstreamStats(1, 0)); @@ -6157,7 +6056,6 @@ TEST_F(RouterTest, AutoHostRewriteEnabled) { EXPECT_EQ(host_address_, host->address()); })); EXPECT_CALL(callbacks_.route_->route_entry_, autoHostRewrite()).WillOnce(Return(true)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(incoming_headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -6197,7 +6095,6 @@ TEST_F(RouterTest, AutoHostRewriteDisabled) { EXPECT_EQ(host_address_, host->address()); })); EXPECT_CALL(callbacks_.route_->route_entry_, autoHostRewrite()).WillOnce(Return(false)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(incoming_headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -6284,7 +6181,6 @@ TEST_F(RouterTest, ConnectPauseAndResume) { // Make sure any early data does not go upstream. EXPECT_CALL(encoder, encodeData(_, _)).Times(0); Buffer::OwnedImpl data; - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeData(data, true); // Now send the response headers, and ensure the deferred payload is proxied. @@ -6326,7 +6222,6 @@ TEST_F(RouterTest, ConnectPauseNoResume) { // Make sure any early data does not go upstream. EXPECT_CALL(encoder, encodeData(_, _)).Times(0); Buffer::OwnedImpl data; - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeData(data, true); // Now send the response headers, and ensure the deferred payload is not proxied. @@ -6389,7 +6284,6 @@ class WatermarkTest : public RouterTest { } } void sendResponse() { - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder_->decodeHeaders( Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "200"}}}, true); } @@ -6443,12 +6337,11 @@ TEST_F(WatermarkTest, UpstreamWatermarks) { Buffer::OwnedImpl data; EXPECT_CALL(encoder_, getStream()).Times(2).WillRepeatedly(ReturnRef(stream_)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); response_decoder_->decodeData(data, true); } TEST_F(WatermarkTest, FilterWatermarks) { - EXPECT_CALL(callbacks_, decoderBufferLimit()).WillRepeatedly(Return(10)); + EXPECT_CALL(callbacks_, decoderBufferLimit()).Times(3).WillRepeatedly(Return(10)); router_.setDecoderFilterCallbacks(callbacks_); // Send the headers sans-fin, and don't flag the pool as ready. sendRequest(false, false); @@ -6544,7 +6437,6 @@ TEST_F(WatermarkTest, RetryRequestNotComplete) { // This should not trigger a retry as the retry state has been deleted. EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putResult(Upstream::Outlier::Result::LocalOriginConnectFailed, _)); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); encoder1.stream_.resetStream(Http::StreamResetReason::RemoteReset); EXPECT_EQ(callbacks_.details(), "upstream_reset_before_response_started{remote reset}"); } @@ -6580,7 +6472,6 @@ TEST_F(RouterTestChildSpan, BasicFlow) { EXPECT_CALL(callbacks_.active_span_, spawnChild_(_, "router fake_cluster egress", _)) .WillOnce(Return(child_span)); EXPECT_CALL(callbacks_, tracingConfig()); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -6625,7 +6516,6 @@ TEST_F(RouterTestChildSpan, ResetFlow) { EXPECT_CALL(callbacks_.active_span_, spawnChild_(_, "router fake_cluster egress", _)) .WillOnce(Return(child_span)); EXPECT_CALL(callbacks_, tracingConfig()); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); @@ -6691,7 +6581,6 @@ TEST_F(RouterTestChildSpan, CancelFlow) { EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().Canceled), Eq(Tracing::Tags::get().True))); EXPECT_CALL(*child_span, finishSpan()); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_.onDestroy(); } @@ -6720,7 +6609,6 @@ TEST_F(RouterTestChildSpan, ResetRetryFlow) { EXPECT_CALL(callbacks_.active_span_, spawnChild_(_, "router fake_cluster egress", _)) .WillOnce(Return(child_span_1)); EXPECT_CALL(callbacks_, tracingConfig()); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); router_.decodeHeaders(headers, true); EXPECT_EQ(1U, callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); diff --git a/test/common/router/router_upstream_log_test.cc b/test/common/router/router_upstream_log_test.cc index 5931269988b1..6edf6d6b4b87 100644 --- a/test/common/router/router_upstream_log_test.cc +++ b/test/common/router/router_upstream_log_test.cc @@ -148,7 +148,6 @@ class RouterUpstreamLogTest : public testing::Test { Http::TestRequestHeaderMapImpl headers(request_headers_init); HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)); router_->decodeHeaders(headers, true); EXPECT_CALL(*router_->retry_state_, shouldRetryHeaders(_, _)).WillOnce(Return(RetryStatus::No)); @@ -191,7 +190,6 @@ class RouterUpstreamLogTest : public testing::Test { {"x-envoy-internal", "true"}, {"x-envoy-upstream-rq-per-try-timeout-ms", "5"}}; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_(_)).Times(2); router_->decodeHeaders(headers, true); router_->retry_state_->expectResetRetry(); diff --git a/test/extensions/upstreams/http/tcp/upstream_request_test.cc b/test/extensions/upstreams/http/tcp/upstream_request_test.cc index e0ee478f9306..f6a9c808f377 100644 --- a/test/extensions/upstreams/http/tcp/upstream_request_test.cc +++ b/test/extensions/upstreams/http/tcp/upstream_request_test.cc @@ -186,28 +186,6 @@ TEST_F(TcpUpstreamTest, V2Header) { tcp_upstream_->encodeData(buffer, false); } -// Verifies that a reset after end_stream=true doesn't trigger a callback -// on the router filter. -TEST_F(TcpUpstreamTest, ResetAfterEndStream) { - Buffer::OwnedImpl buffer("something"); - EXPECT_CALL(mock_router_filter_, onUpstreamData(BufferStringEqual("something"), _, true)); - tcp_upstream_->onUpstreamData(buffer, true); - tcp_upstream_->onEvent(Network::ConnectionEvent::RemoteClose); -} - -// Verifies that if we send data after the upstream has been reset nothing crashes. -TEST_F(TcpUpstreamTest, DataAfterReset) { - tcp_upstream_->resetStream(); - Buffer::OwnedImpl buffer("something"); - tcp_upstream_->onUpstreamData(buffer, true); -} - -// Verifies that if we send headers after the upstream has been reset nothing crashes. -TEST_F(TcpUpstreamTest, HeadersAfterReset) { - tcp_upstream_->resetStream(); - EXPECT_FALSE(tcp_upstream_->encodeHeaders(request_, false).ok()); -} - TEST_F(TcpUpstreamTest, TrailersEndStream) { // Swallow the headers. EXPECT_TRUE(tcp_upstream_->encodeHeaders(request_, false).ok());