Skip to content

dns ios: attempt to filter out unroutable addresses category #18418

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattklein123 merged 7 commits into from
Oct 7, 2021
Merged

dns ios: attempt to filter out unroutable addresses category #18418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0be92ab
dns ios: attempt to filter out unroutable addresses category
Reflejo Oct 4, 2021
f98ed9a
Add 'routable' to the spelling dictionary
Reflejo Oct 5, 2021
b694de6
Apply suggestions from code review
Reflejo Oct 5, 2021
aa40bda
fix tests
Oct 6, 2021
5c219ec
segment test
Oct 6, 2021
61565c5
update tests
Oct 6, 2021
3303722
fmt
Oct 6, 2021
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: 13 additions & 1 deletion source/common/network/apple_dns_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,19 @@ DNSServiceErrorType AppleDnsResolverImpl::PendingResolution::dnsServiceGetAddrIn
break;
case DnsLookupFamily::Auto:
case DnsLookupFamily::V4Preferred:
protocol = kDNSServiceProtocol_IPv4 | kDNSServiceProtocol_IPv6;
/* We want to make sure we don't get any address that is not routable. Passing 0
* to apple's `DNSServiceGetAddrInfo` will make a best attempt to filter out IPv6
* or IPv4 addresses depending on what's routable, per Apple's documentation:
*
* If neither flag is set, the system will apply an intelligent heuristic, which
* is (currently) that it will attempt to look up both, except:
* If "hostname" is a wide-area unicast DNS hostname (i.e. not a ".local." name) but
* this host has no routable IPv6 address, then the call will not try to look up IPv6
* addresses for "hostname", since any addresses it found would be unlikely to be of
* any use anyway. Similarly, if this host has no routable IPv4 address, the call will
* not try to look up IPv4 addresses for "hostname".
*/
protocol = 0;
break;
}

Expand Down
117 changes: 60 additions & 57 deletions test/common/network/apple_dns_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,36 @@ class AppleDnsImplTest : public testing::Test {
EXPECT_EQ(expected_status, status);
if (expected_results) {
EXPECT_FALSE(results.empty());
absl::optional<bool> is_v4{};
for (const auto& result : results) {
if (lookup_family == DnsLookupFamily::V4Only ||
lookup_family == DnsLookupFamily::V4Preferred) {
switch (lookup_family) {
case DnsLookupFamily::V4Only:
EXPECT_NE(nullptr, result.address_->ip()->ipv4());
} else if (lookup_family == DnsLookupFamily::V6Only ||
lookup_family == DnsLookupFamily::Auto) {
break;
case DnsLookupFamily::V6Only:
EXPECT_NE(nullptr, result.address_->ip()->ipv6());
break;
// In CI these modes could return either V4 or V6 with the non-mocked API calls. But
// regardless of the family all returned addresses need to be one _or_ the other.
case DnsLookupFamily::V4Preferred:
case DnsLookupFamily::Auto:
// Set the expectation for subsequent responses based on the first one.
if (!is_v4.has_value()) {
if (result.address_->ip()->ipv4()) {
is_v4 = true;
} else {
is_v4 = false;
}
}

if (is_v4.value()) {
EXPECT_NE(nullptr, result.address_->ip()->ipv4());
} else {
EXPECT_NE(nullptr, result.address_->ip()->ipv6());
}
break;
default:
NOT_REACHED_GCOVR_EXCL_LINE;
}
}
}
Expand Down Expand Up @@ -149,19 +172,25 @@ TEST_F(AppleDnsImplTest, LocalLookup) {
dispatcher_->run(Event::Dispatcher::RunType::Block);
}

TEST_F(AppleDnsImplTest, DnsIpAddressVersion) {
TEST_F(AppleDnsImplTest, DnsIpAddressVersionAuto) {
EXPECT_NE(nullptr, resolveWithExpectations("google.com", DnsLookupFamily::Auto,
DnsResolver::ResolutionStatus::Success, true));
dispatcher_->run(Event::Dispatcher::RunType::Block);
}

TEST_F(AppleDnsImplTest, DnsIpAddressVersionV4Preferred) {
EXPECT_NE(nullptr, resolveWithExpectations("google.com", DnsLookupFamily::V4Preferred,
DnsResolver::ResolutionStatus::Success, true));
dispatcher_->run(Event::Dispatcher::RunType::Block);
}

TEST_F(AppleDnsImplTest, DnsIpAddressVersionV4Only) {
EXPECT_NE(nullptr, resolveWithExpectations("google.com", DnsLookupFamily::V4Only,
DnsResolver::ResolutionStatus::Success, true));
dispatcher_->run(Event::Dispatcher::RunType::Block);
}

TEST_F(AppleDnsImplTest, DnsIpAddressVersionV6Only) {
EXPECT_NE(nullptr, resolveWithExpectations("google.com", DnsLookupFamily::V6Only,
DnsResolver::ResolutionStatus::Success, true));
dispatcher_->run(Event::Dispatcher::RunType::Block);
Expand Down Expand Up @@ -305,10 +334,8 @@ class AppleDnsImplFakeApiTest : public testing::Test {
Network::Address::Ipv4Instance address(&addr4);
absl::Notification dns_callback_executed;

EXPECT_CALL(dns_service_,
dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0,
kDNSServiceProtocol_IPv4 | kDNSServiceProtocol_IPv6,
StrEq(hostname.c_str()), _, _))
EXPECT_CALL(dns_service_, dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0, 0,
StrEq(hostname.c_str()), _, _))
.WillOnce(DoAll(
// Have the API call synchronously call the provided callback.
WithArgs<5, 6>(Invoke([&](DNSServiceGetAddrInfoReply callback, void* context) -> void {
Expand Down Expand Up @@ -349,10 +376,8 @@ class AppleDnsImplFakeApiTest : public testing::Test {
DNSServiceGetAddrInfoReply reply_callback;
absl::Notification dns_callback_executed;

EXPECT_CALL(dns_service_,
dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0,
kDNSServiceProtocol_IPv4 | kDNSServiceProtocol_IPv6,
StrEq(hostname.c_str()), _, _))
EXPECT_CALL(dns_service_, dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0, 0,
StrEq(hostname.c_str()), _, _))
.WillOnce(DoAll(SaveArg<5>(&reply_callback), Return(kDNSServiceErr_NoError)));

EXPECT_CALL(dns_service_, dnsServiceRefSockFD(_)).WillOnce(Return(0));
Expand Down Expand Up @@ -429,10 +454,8 @@ TEST_F(AppleDnsImplFakeApiTest, ErrorInSocketAccess) {
DNSServiceGetAddrInfoReply reply_callback;
absl::Notification dns_callback_executed;

EXPECT_CALL(dns_service_,
dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0,
kDNSServiceProtocol_IPv4 | kDNSServiceProtocol_IPv6,
StrEq(hostname.c_str()), _, _))
EXPECT_CALL(dns_service_, dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0, 0,
StrEq(hostname.c_str()), _, _))
.WillOnce(DoAll(SaveArg<5>(&reply_callback), Return(kDNSServiceErr_NoError)));

EXPECT_CALL(dns_service_, dnsServiceRefSockFD(_)).WillOnce(Return(-1));
Expand Down Expand Up @@ -466,10 +489,8 @@ TEST_F(AppleDnsImplFakeApiTest, InvalidFileEvent) {
DNSServiceGetAddrInfoReply reply_callback;
absl::Notification dns_callback_executed;

EXPECT_CALL(dns_service_,
dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0,
kDNSServiceProtocol_IPv4 | kDNSServiceProtocol_IPv6,
StrEq(hostname.c_str()), _, _))
EXPECT_CALL(dns_service_, dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0, 0,
StrEq(hostname.c_str()), _, _))
.WillOnce(DoAll(SaveArg<5>(&reply_callback), Return(kDNSServiceErr_NoError)));

EXPECT_CALL(dns_service_, dnsServiceRefSockFD(_)).WillOnce(Return(0));
Expand Down Expand Up @@ -503,10 +524,8 @@ TEST_F(AppleDnsImplFakeApiTest, ErrorInProcessResult) {
DNSServiceGetAddrInfoReply reply_callback;
absl::Notification dns_callback_executed;

EXPECT_CALL(dns_service_,
dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0,
kDNSServiceProtocol_IPv4 | kDNSServiceProtocol_IPv6,
StrEq(hostname.c_str()), _, _))
EXPECT_CALL(dns_service_, dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0, 0,
StrEq(hostname.c_str()), _, _))
.WillOnce(DoAll(SaveArg<5>(&reply_callback), Return(kDNSServiceErr_NoError)));

EXPECT_CALL(dns_service_, dnsServiceRefSockFD(_)).WillOnce(Return(0));
Expand Down Expand Up @@ -558,10 +577,8 @@ TEST_F(AppleDnsImplFakeApiTest, QuerySynchronousCompletion) {
Network::Address::Ipv4Instance address(&addr4);
absl::Notification dns_callback_executed;

EXPECT_CALL(dns_service_,
dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0,
kDNSServiceProtocol_IPv4 | kDNSServiceProtocol_IPv6,
StrEq(hostname.c_str()), _, _))
EXPECT_CALL(dns_service_, dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0, 0,
StrEq(hostname.c_str()), _, _))
.WillOnce(DoAll(
// Have the API call synchronously call the provided callback.
WithArgs<5, 6>(Invoke([&](DNSServiceGetAddrInfoReply callback, void* context) -> void {
Expand Down Expand Up @@ -618,10 +635,8 @@ TEST_F(AppleDnsImplFakeApiTest, MultipleAddresses) {
DNSServiceGetAddrInfoReply reply_callback;
absl::Notification dns_callback_executed;

EXPECT_CALL(dns_service_,
dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0,
kDNSServiceProtocol_IPv4 | kDNSServiceProtocol_IPv6,
StrEq(hostname.c_str()), _, _))
EXPECT_CALL(dns_service_, dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0, 0,
StrEq(hostname.c_str()), _, _))
.WillOnce(DoAll(SaveArg<5>(&reply_callback), Return(kDNSServiceErr_NoError)));

EXPECT_CALL(dns_service_, dnsServiceRefSockFD(_)).WillOnce(Return(0));
Expand Down Expand Up @@ -684,10 +699,8 @@ TEST_F(AppleDnsImplFakeApiTest, MultipleAddressesSecondOneFails) {
DNSServiceGetAddrInfoReply reply_callback;
absl::Notification dns_callback_executed;

EXPECT_CALL(dns_service_,
dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0,
kDNSServiceProtocol_IPv4 | kDNSServiceProtocol_IPv6,
StrEq(hostname.c_str()), _, _))
EXPECT_CALL(dns_service_, dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0, 0,
StrEq(hostname.c_str()), _, _))
.WillOnce(DoAll(SaveArg<5>(&reply_callback), Return(kDNSServiceErr_NoError)));

EXPECT_CALL(dns_service_, dnsServiceRefSockFD(_)).WillOnce(Return(0));
Expand Down Expand Up @@ -734,10 +747,8 @@ TEST_F(AppleDnsImplFakeApiTest, MultipleQueries) {
absl::Notification dns_callback_executed2;

// Start first query.
EXPECT_CALL(dns_service_,
dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0,
kDNSServiceProtocol_IPv4 | kDNSServiceProtocol_IPv6,
StrEq(hostname.c_str()), _, _))
EXPECT_CALL(dns_service_, dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0, 0,
StrEq(hostname.c_str()), _, _))
.WillOnce(DoAll(SaveArg<5>(&reply_callback), Return(kDNSServiceErr_NoError)));

EXPECT_CALL(dns_service_, dnsServiceRefSockFD(_)).WillOnce(Return(0));
Expand Down Expand Up @@ -805,10 +816,8 @@ TEST_F(AppleDnsImplFakeApiTest, MultipleQueriesOneFails) {
absl::Notification dns_callback_executed2;

// Start first query.
EXPECT_CALL(dns_service_,
dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0,
kDNSServiceProtocol_IPv4 | kDNSServiceProtocol_IPv6,
StrEq(hostname.c_str()), _, _))
EXPECT_CALL(dns_service_, dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0, 0,
StrEq(hostname.c_str()), _, _))
.WillOnce(DoAll(SaveArg<5>(&reply_callback), Return(kDNSServiceErr_NoError)));

EXPECT_CALL(dns_service_, dnsServiceRefSockFD(_)).WillOnce(Return(0));
Expand Down Expand Up @@ -869,10 +878,8 @@ TEST_F(AppleDnsImplFakeApiTest, ResultWithOnlyNonAdditiveReplies) {
DNSServiceGetAddrInfoReply reply_callback;
absl::Notification dns_callback_executed;

EXPECT_CALL(dns_service_,
dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0,
kDNSServiceProtocol_IPv4 | kDNSServiceProtocol_IPv6,
StrEq(hostname.c_str()), _, _))
EXPECT_CALL(dns_service_, dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0, 0,
StrEq(hostname.c_str()), _, _))
.WillOnce(DoAll(SaveArg<5>(&reply_callback), Return(kDNSServiceErr_NoError)));

EXPECT_CALL(dns_service_, dnsServiceRefSockFD(_)).WillOnce(Return(0));
Expand Down Expand Up @@ -904,10 +911,8 @@ TEST_F(AppleDnsImplFakeApiTest, ResultWithNullAddress) {
Network::Address::Ipv4Instance address(&addr4);
DNSServiceGetAddrInfoReply reply_callback;

EXPECT_CALL(dns_service_,
dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0,
kDNSServiceProtocol_IPv4 | kDNSServiceProtocol_IPv6,
StrEq(hostname.c_str()), _, _))
EXPECT_CALL(dns_service_, dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0, 0,
StrEq(hostname.c_str()), _, _))
.WillOnce(DoAll(SaveArg<5>(&reply_callback), Return(kDNSServiceErr_NoError)));

EXPECT_CALL(dns_service_, dnsServiceRefSockFD(_)).WillOnce(Return(0));
Expand Down Expand Up @@ -941,10 +946,8 @@ TEST_F(AppleDnsImplFakeApiTest, DeallocateOnDestruction) {
DNSServiceGetAddrInfoReply reply_callback;
absl::Notification dns_callback_executed;

EXPECT_CALL(dns_service_,
dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0,
kDNSServiceProtocol_IPv4 | kDNSServiceProtocol_IPv6,
StrEq(hostname.c_str()), _, _))
EXPECT_CALL(dns_service_, dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0, 0,
StrEq(hostname.c_str()), _, _))
.WillOnce(DoAll(
SaveArg<5>(&reply_callback),
WithArgs<0>(Invoke([](DNSServiceRef* ref) -> void { *ref = new _DNSServiceRef_t{}; })),
Expand Down
1 change: 1 addition & 0 deletions tools/spelling/spelling_dictionary.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1300,3 +1300,4 @@ crlf
ep
suri
transid
routable