diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 9d1c099e70d4a..9989becbc6db0 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -48,6 +48,7 @@ Bug Fixes * http: made the HeaderValues::prefix() method const. * jwt_authn: supports jwt payload without "iss" field. * rocketmq_proxy network-level filter: fixed an issue involving incorrect header lengths. In debug mode it causes crash and in release mode it causes underflow. +* thrift_proxy: fixed crashing bug on request overflow. * udp_proxy: fixed a crash due to UDP packets being processed after listener removal. Removed Config or Runtime diff --git a/source/extensions/filters/network/thrift_proxy/router/router_impl.cc b/source/extensions/filters/network/thrift_proxy/router/router_impl.cc index 885538c6298a1..f3910f0614bc8 100644 --- a/source/extensions/filters/network/thrift_proxy/router/router_impl.cc +++ b/source/extensions/filters/network/thrift_proxy/router/router_impl.cc @@ -410,6 +410,10 @@ FilterStatus Router::UpstreamRequest::start() { return FilterStatus::StopIteration; } + if (upstream_host_ == nullptr) { + return FilterStatus::StopIteration; + } + return FilterStatus::Continue; } @@ -502,9 +506,8 @@ void Router::UpstreamRequest::onResetStream(ConnectionPool::PoolFailureReason re switch (reason) { case ConnectionPool::PoolFailureReason::Overflow: parent_.callbacks_->sendLocalReply( - AppException( - AppExceptionType::InternalError, - fmt::format("too many connections to '{}'", upstream_host_->address()->asString())), + AppException(AppExceptionType::InternalError, + "thrift upstream request: too many connections"), true); break; case ConnectionPool::PoolFailureReason::LocalConnectionFailure: @@ -519,7 +522,9 @@ void Router::UpstreamRequest::onResetStream(ConnectionPool::PoolFailureReason re parent_.callbacks_->sendLocalReply( AppException( AppExceptionType::InternalError, - fmt::format("connection failure '{}'", upstream_host_->address()->asString())), + fmt::format("connection failure '{}'", (upstream_host_ != nullptr) + ? upstream_host_->address()->asString() + : "to upstream")), true); return; } diff --git a/test/extensions/filters/network/thrift_proxy/router_test.cc b/test/extensions/filters/network/thrift_proxy/router_test.cc index b4e1d37da7b1e..110c10b5d624f 100644 --- a/test/extensions/filters/network/thrift_proxy/router_test.cc +++ b/test/extensions/filters/network/thrift_proxy/router_test.cc @@ -407,7 +407,8 @@ TEST_F(ThriftRouterTest, PoolOverflowFailure) { EXPECT_THAT(app_ex.what(), ContainsRegex(".*too many connections.*")); EXPECT_TRUE(end_stream); })); - context_.cluster_manager_.tcp_conn_pool_.poolFailure(ConnectionPool::PoolFailureReason::Overflow); + context_.cluster_manager_.tcp_conn_pool_.poolFailure(ConnectionPool::PoolFailureReason::Overflow, + true); } TEST_F(ThriftRouterTest, PoolConnectionFailureWithOnewayMessage) { diff --git a/test/mocks/tcp/mocks.cc b/test/mocks/tcp/mocks.cc index 8b86988451afe..e5cb3f6e2b0d9 100644 --- a/test/mocks/tcp/mocks.cc +++ b/test/mocks/tcp/mocks.cc @@ -36,12 +36,15 @@ Envoy::ConnectionPool::MockCancellable* MockInstance::newConnectionImpl(Callback return &handles_.back(); } -void MockInstance::poolFailure(PoolFailureReason reason) { +void MockInstance::poolFailure(PoolFailureReason reason, bool host_null) { Callbacks* cb = callbacks_.front(); callbacks_.pop_front(); handles_.pop_front(); - - cb->onPoolFailure(reason, host_); + if (host_null) { + cb->onPoolFailure(reason, nullptr); + } else { + cb->onPoolFailure(reason, host_); + } } void MockInstance::poolReady(Network::MockClientConnection& conn) { diff --git a/test/mocks/tcp/mocks.h b/test/mocks/tcp/mocks.h index 7b0b5fb349518..9e4182423ed6c 100644 --- a/test/mocks/tcp/mocks.h +++ b/test/mocks/tcp/mocks.h @@ -58,7 +58,7 @@ class MockInstance : public Instance { MOCK_METHOD(Upstream::HostDescriptionConstSharedPtr, host, (), (const)); Envoy::ConnectionPool::MockCancellable* newConnectionImpl(Callbacks& cb); - void poolFailure(PoolFailureReason reason); + void poolFailure(PoolFailureReason reason, bool host_null = false); void poolReady(Network::MockClientConnection& conn); // Invoked when connection_data_, having been assigned via poolReady is released.