Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,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.
Comment thread
zuercher marked this conversation as resolved.

Removed Config or Runtime
-------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,10 @@ FilterStatus Router::UpstreamRequest::start() {
return FilterStatus::StopIteration;
}

if (upstream_host_ == nullptr) {
return FilterStatus::StopIteration;
}

return FilterStatus::Continue;
}

Expand Down Expand Up @@ -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:
Expand All @@ -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;
}
Expand Down
3 changes: 2 additions & 1 deletion test/extensions/filters/network/thrift_proxy/router_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
9 changes: 6 additions & 3 deletions test/mocks/tcp/mocks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion test/mocks/tcp/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down