Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
14 changes: 12 additions & 2 deletions source/common/network/dns_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,18 @@ void DnsResolverImpl::PendingResolution::onAresHostCallback(int status, int time

if (completed_) {
if (!cancelled_) {
dispatcher_.post(
[callback = callback_, al = std::move(address_list)] { callback(std::move(al)); });
try {
callback_(std::move(address_list));
} catch (const EnvoyException& e) {
ENVOY_LOG(critical, "EnvoyException in c-ares callback");
dispatcher_.post([e] { throw e; });
} catch (const std::exception& e) {
ENVOY_LOG(critical, "std::exception in c-ares callback");
dispatcher_.post([s = std::string(e.what())] { throw EnvoyException(s); });
} catch (...) {
ENVOY_LOG(critical, "Unknown exception in c-ares callback");
dispatcher_.post([] { throw EnvoyException("unknown"); });
}
}
if (owned_) {
delete this;
Expand Down
2 changes: 1 addition & 1 deletion source/common/network/dns_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class DnsResolverImpl : public DnsResolver, protected Logger::Loggable<Logger::I

// Caller supplied callback to invoke on query completion or error.
const ResolveCb callback_;
// Dispatcher to post callback_ to.
// Dispatcher to post any callback_ exceptions to.
Event::Dispatcher& dispatcher_;
// Does the object own itself? Resource reclamation occurs via self-deleting
// on query completion or error.
Expand Down
26 changes: 23 additions & 3 deletions test/common/network/dns_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,6 @@ TEST_P(DnsImplTest, LocalLookup) {
[&](const std::list<Address::InstanceConstSharedPtr>&& results) -> void {
address_list = results;
}));
dispatcher_.run(Event::Dispatcher::RunType::NonBlock);
EXPECT_TRUE(hasAddress(address_list, "127.0.0.1"));
EXPECT_FALSE(hasAddress(address_list, "::1"));
}
Expand All @@ -507,7 +506,6 @@ TEST_P(DnsImplTest, LocalLookup) {
address_list = results;
}))
<< error_msg;
dispatcher_.run(Event::Dispatcher::RunType::NonBlock);
EXPECT_TRUE(hasAddress(address_list, "::1")) << error_msg;
EXPECT_FALSE(hasAddress(address_list, "127.0.0.1"));

Expand All @@ -517,7 +515,6 @@ TEST_P(DnsImplTest, LocalLookup) {
address_list = results;
}))
<< error_msg;
dispatcher_.run(Event::Dispatcher::RunType::NonBlock);
EXPECT_FALSE(hasAddress(address_list, "127.0.0.1"));
EXPECT_TRUE(hasAddress(address_list, "::1")) << error_msg;
}
Expand Down Expand Up @@ -557,6 +554,29 @@ TEST_P(DnsImplTest, DnsIpAddressVersionV6) {
EXPECT_TRUE(hasAddress(address_list, "1::2"));
}

// Validate exception behavior during c-ares callbacks.
TEST_P(DnsImplTest, CallbackException) {
// Force immediate resolution, which will trigger a c-ares exception unsafe
// state providing regression coverage for #4307.
EXPECT_EQ(nullptr, resolver_->resolve(
"1.2.3.4", DnsLookupFamily::V4Only,
[&](const std::list<Address::InstanceConstSharedPtr> &&
/*results*/) -> void { throw EnvoyException("Envoy exception"); }));
EXPECT_THROW_WITH_MESSAGE(dispatcher_.run(Event::Dispatcher::RunType::Block), EnvoyException,
"Envoy exception");
EXPECT_EQ(nullptr, resolver_->resolve(
"1.2.3.4", DnsLookupFamily::V4Only,
[&](const std::list<Address::InstanceConstSharedPtr> &&
/*results*/) -> void { throw std::runtime_error("runtime error"); }));
EXPECT_THROW_WITH_MESSAGE(dispatcher_.run(Event::Dispatcher::RunType::Block), EnvoyException,
"runtime error");
EXPECT_EQ(nullptr, resolver_->resolve("1.2.3.4", DnsLookupFamily::V4Only,
[&](const std::list<Address::InstanceConstSharedPtr> &&
/*results*/) -> void { throw std::string(); }));
EXPECT_THROW_WITH_MESSAGE(dispatcher_.run(Event::Dispatcher::RunType::Block), EnvoyException,
"unknown");
}

TEST_P(DnsImplTest, DnsIpAddressVersion) {
std::list<Address::InstanceConstSharedPtr> address_list;
server_->addHosts("some.good.domain", {"1.2.3.4"}, A);
Expand Down