commit 541a90cc3bf250ce66fdcc31feae3e78370765b6
author Eric Orth <ericorth@chromium.org> Thu Jan 17 20:11:46 2019
committer Commit Bot <commit-bot@chromium.org> Thu Jan 17 20:11:46 2019
tree 9bed309d3485e5cfa87961a0b5630d429db77b10
parent 4d5ba645b78440b3c2109a029411d17db5be1e18

Coerce more canonicalname resolves to use ProcTask

DnsClient doesn't handle cannonname very well (see
https://crbug.com/872665), so some logic was added a while back to force
ProcTask for such cases by setting HostResolverSource::SYSTEM when the
HOST_RESOLVER_CANONNAME flag is set. But this didn't affect the newer
HostResolver::CreateRequest() API, as that has callers directly set the
source and doesn't directly use ProcTask flags.

Bug: 872665
Change-Id: I709959911ec299118369f1f995ea5802df7a92f9
Reviewed-on: https://chromium-review.googlesource.com/c/1418350
Reviewed-by: Asanka Herath <asanka@chromium.org>
Commit-Queue: Eric Orth <ericorth@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623815}

net/dns/host_resolver_impl.cc

diff --git a/net/dns/host_resolver_impl.cc b/net/dns/host_resolver_impl.cc
index 813a246..fc570e7 100644
--- a/net/dns/host_resolver_impl.cc
+++ b/net/dns/host_resolver_impl.cc
@@ -1740,11 +1740,17 @@
 
     switch (key_.host_resolver_source) {
       case HostResolverSource::ANY:
-        // Default to DnsTask (with allowed fallback to ProcTask for address
-        // queries). But if hostname appears to be an MDNS name (ends in
+        // Force address queries with canonname to use ProcTask to counter poor
+        // CNAME support in DnsTask. See https://crbug.com/872665
+        //
+        // Otherwise, default to DnsTask (with allowed fallback to ProcTask for
+        // address queries). But if hostname appears to be an MDNS name (ends in
         // *.local), go with ProcTask for address queries and MdnsTask for non-
         // address queries.
-        if (!ResemblesMulticastDNSName(key_.hostname)) {
+        if ((key_.host_resolver_flags & HOST_RESOLVER_CANONNAME) &&
+            IsAddressType(key_.dns_query_type)) {
+          StartProcTask();
+        } else if (!ResemblesMulticastDNSName(key_.hostname)) {
           StartDnsTask(IsAddressType(
               key_.dns_query_type) /* allow_fallback_resolution */);
         } else if (IsAddressType(key_.dns_query_type)) {

net/dns/host_resolver_impl_unittest.cc

diff --git a/net/dns/host_resolver_impl_unittest.cc b/net/dns/host_resolver_impl_unittest.cc
index cc08987..4abc892 100644
--- a/net/dns/host_resolver_impl_unittest.cc
+++ b/net/dns/host_resolver_impl_unittest.cc
@@ -5923,6 +5923,7 @@
 
   HostResolver::ResolveHostParameters params;
   params.include_canonical_name = true;
+  params.source = HostResolverSource::DNS;
   ResolveHostResponseHelper response(resolver_->CreateRequest(
       HostPortPair("alias", 80), NetLogWithSource(), params));
   ASSERT_THAT(response.result_error(), IsOk());
@@ -5944,6 +5945,7 @@
 
   HostResolver::ResolveHostParameters params;
   params.include_canonical_name = true;
+  params.source = HostResolverSource::DNS;
   ResolveHostResponseHelper response(resolver_->CreateRequest(
       HostPortPair("alias", 80), NetLogWithSource(), params));
   ASSERT_FALSE(response.complete());
@@ -5965,6 +5967,7 @@
   HostResolver::ResolveHostParameters params;
   params.dns_query_type = DnsQueryType::A;
   params.include_canonical_name = true;
+  params.source = HostResolverSource::DNS;
   ResolveHostResponseHelper response(resolver_->CreateRequest(
       HostPortPair("alias", 80), NetLogWithSource(), params));
   ASSERT_THAT(response.result_error(), IsOk());
@@ -5972,6 +5975,29 @@
             "correct");
 }
 
+// Test that without specifying source, a request that would otherwise be
+// handled by DNS is sent to the system resolver if cannonname is requested.
+TEST_F(HostResolverImplDnsTest, CanonicalNameForcesProc) {
+  // Disable fallback to ensure system resolver is used directly, not via
+  // fallback.
+  set_allow_fallback_to_proctask(false);
+
+  proc_->AddRuleForAllFamilies("nx_succeed", "192.168.1.102",
+                               HOST_RESOLVER_CANONNAME, "canonical");
+  proc_->SignalMultiple(1u);
+
+  ChangeDnsConfig(CreateValidDnsConfig());
+
+  HostResolver::ResolveHostParameters params;
+  params.include_canonical_name = true;
+  ResolveHostResponseHelper response(resolver_->CreateRequest(
+      HostPortPair("nx_succeed", 80), NetLogWithSource(), params));
+  ASSERT_THAT(response.result_error(), IsOk());
+
+  EXPECT_EQ(response.request()->GetAddressResults().value().canonical_name(),
+            "canonical");
+}
+
 TEST_F(HostResolverImplTest, ResolveLocalHostname) {
   AddressList addresses;