Add source and cache selection to HostResolver::CreateRequest.

ResolveHostParameters::allow_cached_response is a direct replacement for
RequestInfo::allow_cached_response.  Defaulting to true, it controls
whether or not a result can come from the host cache.

ResolveHostParameters::source allows specifying a source (system vs
async resolver) for a request.  This is a replacement for setting the
HOST_RESOLVER_SYSTEM_ONLY flag or directly using the async resolver
classes.

Just adding to C++ net/ API for now.  Mojo version in next CL.

Bug: 846423
Change-Id: Ie945745f595302ae745ffdab355ae1b13a993c4a
Reviewed-on: https://chromium-review.googlesource.com/1185216
Commit-Queue: Eric Orth <ericorth@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585641}

net/dns/BUILD.gn

@@ -53,6 +53,7 @@ source_set("dns") {
       "host_resolver_impl.cc",
       "host_resolver_proc.cc",
       "host_resolver_proc.h",
+      "host_resolver_source.h",
       "mapped_host_resolver.cc",
       "notify_watcher_mac.cc",
       "notify_watcher_mac.h",

net/dns/host_cache.h

@@ -22,11 +22,12 @@
 #include "net/base/expiring_cache.h"
 #include "net/base/net_export.h"
 #include "net/dns/dns_util.h"
+#include "net/dns/host_resolver_source.h"
 
 namespace base {
 class ListValue;
 class TickClock;
-}
+}  // namespace base
 
 namespace net {
 
@@ -34,28 +35,35 @@ namespace net {
 class NET_EXPORT HostCache {
  public:
   struct Key {
-    Key(const std::string& hostname, AddressFamily address_family,
-        HostResolverFlags host_resolver_flags)
+    Key(const std::string& hostname,
+        AddressFamily address_family,
+        HostResolverFlags host_resolver_flags,
+        HostResolverSource host_resolver_source = HostResolverSource::ANY)
         : hostname(hostname),
           address_family(address_family),
-          host_resolver_flags(host_resolver_flags) {}
+          host_resolver_flags(host_resolver_flags),
+          host_resolver_source(host_resolver_source) {}
 
     Key()
-        : address_family(ADDRESS_FAMILY_UNSPECIFIED), host_resolver_flags(0) {}
+        : address_family(ADDRESS_FAMILY_UNSPECIFIED),
+          host_resolver_flags(0),
+          host_resolver_source(HostResolverSource::ANY) {}
 
     bool operator<(const Key& other) const {
       // The order of comparisons of |Key| fields is arbitrary, thus
       // |address_family| and |host_resolver_flags| are compared before
       // |hostname| under assumption that integer comparisons are faster than
       // string comparisons.
-      return std::tie(address_family, host_resolver_flags, hostname) <
+      return std::tie(address_family, host_resolver_flags, hostname,
+                      host_resolver_source) <
              std::tie(other.address_family, other.host_resolver_flags,
-                      other.hostname);
+                      other.hostname, other.host_resolver_source);
     }
 
     std::string hostname;
     AddressFamily address_family;
     HostResolverFlags host_resolver_flags;
+    HostResolverSource host_resolver_source;
   };
 
   struct NET_EXPORT EntryStaleness {

net/dns/host_resolver.cc

@@ -198,26 +198,44 @@ HostResolver::ResolveHostParameters
 HostResolver::RequestInfoToResolveHostParameters(
     const HostResolver::RequestInfo& request_info,
     RequestPriority priority) {
+  // Flag that should only be set internally, not used in input.
+  DCHECK(!(request_info.host_resolver_flags() &
+           HOST_RESOLVER_DEFAULT_FAMILY_SET_DUE_TO_NO_IPV6));
+
   ResolveHostParameters parameters;
 
   parameters.dns_query_type =
       AddressFamilyToDnsQueryType(request_info.address_family());
   parameters.initial_priority = priority;
+  parameters.source = FlagsToSource(request_info.host_resolver_flags());
+  parameters.allow_cached_response = request_info.allow_cached_response();
+  parameters.include_canonical_name =
+      request_info.host_resolver_flags() & HOST_RESOLVER_CANONNAME;
+  parameters.loopback_only =
+      request_info.host_resolver_flags() & HOST_RESOLVER_LOOPBACK_ONLY;
   parameters.is_speculative = request_info.is_speculative();
 
   return parameters;
 }
 
+// static
+HostResolverSource HostResolver::FlagsToSource(HostResolverFlags flags) {
+  if (flags & HOST_RESOLVER_SYSTEM_ONLY)
+    return HostResolverSource::SYSTEM;
+
+  return HostResolverSource::ANY;
+}
+
 // static
 HostResolverFlags HostResolver::ParametersToHostResolverFlags(
     const ResolveHostParameters& parameters) {
   HostResolverFlags flags = 0;
-  if (parameters.include_canonical_name) {
+  if (parameters.source == HostResolverSource::SYSTEM)
+    flags |= HOST_RESOLVER_SYSTEM_ONLY;
+  if (parameters.include_canonical_name)
     flags |= HOST_RESOLVER_CANONNAME;
-  }
-  if (parameters.loopback_only) {
+  if (parameters.loopback_only)
     flags |= HOST_RESOLVER_LOOPBACK_ONLY;
-  }
   return flags;
 }
 

net/dns/host_resolver.h

@@ -20,6 +20,7 @@
 #include "net/base/request_priority.h"
 #include "net/dns/dns_config_service.h"
 #include "net/dns/host_cache.h"
+#include "net/dns/host_resolver_source.h"
 
 namespace base {
 class Value;
@@ -197,6 +198,16 @@ class NET_EXPORT HostResolver {
     // The initial net priority for the host resolution request.
     RequestPriority initial_priority = RequestPriority::DEFAULT_PRIORITY;
 
+    // The source to use for resolved addresses. Default allows the resolver to
+    // pick an appropriate source. Only affects use of big external sources (eg
+    // calling the system for resolution or using DNS). Even if a source is
+    // specified, results can still come from cache, resolving "localhost" or
+    // IP literals, etc.
+    HostResolverSource source = HostResolverSource::ANY;
+
+    // If |false|, results will not come from the host cache.
+    bool allow_cached_response = true;
+
     // If |true|, requests that the resolver include AddressList::canonical_name
     // in the results. If the resolver can do so without significant
     // performance impact, canonical_name may still be included even if
@@ -358,6 +369,7 @@ class NET_EXPORT HostResolver {
   static ResolveHostParameters RequestInfoToResolveHostParameters(
       const RequestInfo& request_info,
       RequestPriority priority);
+  static HostResolverSource FlagsToSource(HostResolverFlags flags);
   static HostResolverFlags ParametersToHostResolverFlags(
       const ResolveHostParameters& parameters);
 

net/dns/host_resolver_impl.cc

@@ -619,28 +619,11 @@ class HostResolverImpl::RequestImpl
               const HostPortPair& request_host,
               const base::Optional<ResolveHostParameters>& optional_parameters,
               base::WeakPtr<HostResolverImpl> resolver)
-      : RequestImpl(source_net_log,
-                    request_host,
-                    optional_parameters,
-                    0 /* host_resolver_flags */,
-                    true /* allow_cached_response */,
-                    resolver) {}
-
-  // Overload for use by the legacy Resolve() API. Has more advanced parameters
-  // not yet supported by the CreateRequest() API.
-  RequestImpl(const NetLogWithSource& source_net_log,
-              const HostPortPair& request_host,
-              const base::Optional<ResolveHostParameters>& optional_parameters,
-              HostResolverFlags host_resolver_flags,
-              bool allow_cached_response,
-              base::WeakPtr<HostResolverImpl> resolver)
       : source_net_log_(source_net_log),
         request_host_(request_host),
-        allow_cached_response_(allow_cached_response),
         parameters_(optional_parameters ? optional_parameters.value()
                                         : ResolveHostParameters()),
-        host_resolver_flags_(host_resolver_flags |
-                             ParametersToHostResolverFlags(parameters_)),
+        host_resolver_flags_(ParametersToHostResolverFlags(parameters_)),
         priority_(parameters_.initial_priority),
         job_(nullptr),
         resolver_(resolver),
@@ -727,8 +710,6 @@ class HostResolverImpl::RequestImpl
 
   const HostPortPair& request_host() const { return request_host_; }
 
-  bool allow_cached_response() const { return allow_cached_response_; }
-
   const ResolveHostParameters& parameters() const { return parameters_; }
 
   HostResolverFlags host_resolver_flags() const { return host_resolver_flags_; }
@@ -752,7 +733,6 @@ class HostResolverImpl::RequestImpl
   const NetLogWithSource source_net_log_;
 
   const HostPortPair request_host_;
-  const bool allow_cached_response_;
   const ResolveHostParameters parameters_;
   const HostResolverFlags host_resolver_flags_;
 
@@ -1374,7 +1354,6 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job,
         priority_tracker_(priority),
         proc_task_runner_(std::move(proc_task_runner)),
         had_non_speculative_request_(false),
-        had_dns_config_(false),
         num_occupied_job_slots_(0),
         dns_task_error_(OK),
         tick_clock_(tick_clock),
@@ -1627,8 +1606,6 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job,
 
     net_log_.AddEvent(NetLogEventType::HOST_RESOLVER_IMPL_JOB_STARTED);
 
-    had_dns_config_ = resolver_->HaveDnsConfig();
-
     start_time_ = tick_clock_->NowTicks();
     base::TimeDelta queue_time = start_time_ - creation_time_;
     base::TimeDelta queue_time_after_change =
@@ -1638,16 +1615,28 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job,
     DNS_HISTOGRAM_BY_PRIORITY("Net.DNS.JobQueueTimeAfterChange", priority(),
                               queue_time_after_change);
 
-    bool system_only =
-        (key_.host_resolver_flags & HOST_RESOLVER_SYSTEM_ONLY) != 0;
+    switch (key_.host_resolver_source) {
+      case HostResolverSource::ANY:
+        if (resolver_->HaveDnsConfig() &&
+            !ResemblesMulticastDNSName(key_.hostname)) {
+          StartDnsTask();
+        } else {
+          StartProcTask();
+        }
+        break;
+      case HostResolverSource::SYSTEM:
+        StartProcTask();
+        break;
+      case HostResolverSource::DNS:
+        // DNS source should not be requested unless the resolver is configured
+        // to handle it.
+        DCHECK(resolver_->HaveDnsConfig());
 
-    // Caution: Job::Start must not complete synchronously.
-    if (!system_only && had_dns_config_ &&
-        !ResemblesMulticastDNSName(key_.hostname)) {
-      StartDnsTask();
-    } else {
-      StartProcTask();
+        StartDnsTask();
+        break;
     }
+
+    // Caution: Job::Start must not complete synchronously.
   }
 
   // TODO(szym): Since DnsTransaction does not consume threads, we can increase
@@ -1998,9 +1987,6 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job,
 
   bool had_non_speculative_request_;
 
-  // Distinguishes measurements taken while DnsClient was fully configured.
-  bool had_dns_config_;
-
   // Number of slots occupied by this Job in resolver's PrioritizedDispatcher.
   unsigned num_occupied_job_slots_;
 
@@ -2158,7 +2144,6 @@ int HostResolverImpl::Resolve(const RequestInfo& info,
   auto request = std::make_unique<RequestImpl>(
       source_net_log, info.host_port_pair(),
       RequestInfoToResolveHostParameters(info, priority),
-      info.host_resolver_flags(), info.allow_cached_response(),
       weak_ptr_factory_.GetWeakPtr());
   auto wrapped_request =
       std::make_unique<LegacyRequestImpl>(std::move(request));
@@ -2188,9 +2173,9 @@ int HostResolverImpl::ResolveFromCache(const RequestInfo& info,
   Key key;
   int rv = ResolveLocally(
       info.host_port_pair(), AddressFamilyToDnsQueryType(info.address_family()),
-      info.host_resolver_flags(), info.allow_cached_response(),
-      false /* allow_stale */, nullptr /* stale_info */, source_net_log,
-      addresses, &key);
+      FlagsToSource(info.host_resolver_flags()), info.host_resolver_flags(),
+      info.allow_cached_response(), false /* allow_stale */,
+      nullptr /* stale_info */, source_net_log, addresses, &key);
 
   LogFinishRequest(source_net_log, rv);
   return rv;
@@ -2211,8 +2196,9 @@ int HostResolverImpl::ResolveStaleFromCache(
   Key key;
   int rv = ResolveLocally(
       info.host_port_pair(), AddressFamilyToDnsQueryType(info.address_family()),
-      info.host_resolver_flags(), info.allow_cached_response(),
-      true /* allow_stale */, stale_info, source_net_log, addresses, &key);
+      FlagsToSource(info.host_resolver_flags()), info.host_resolver_flags(),
+      info.allow_cached_response(), true /* allow_stale */, stale_info,
+      source_net_log, addresses, &key);
   LogFinishRequest(source_net_log, rv);
   return rv;
 }
@@ -2349,9 +2335,9 @@ int HostResolverImpl::Resolve(RequestImpl* request) {
   Key key;
   int rv = ResolveLocally(
       request->request_host(), request->parameters().dns_query_type,
-      request->host_resolver_flags(), request->allow_cached_response(),
-      false /* allow_stale */, nullptr /* stale_info */,
-      request->source_net_log(), &addresses, &key);
+      request->parameters().source, request->host_resolver_flags(),
+      request->parameters().allow_cached_response, false /* allow_stale */,
+      nullptr /* stale_info */, request->source_net_log(), &addresses, &key);
   if (rv == OK && !request->parameters().is_speculative) {
     request->set_address_results(
         EnsurePortOnAddressList(addresses, request->request_host().port()));
@@ -2372,6 +2358,7 @@ int HostResolverImpl::Resolve(RequestImpl* request) {
 
 int HostResolverImpl::ResolveLocally(const HostPortPair& host,
                                      DnsQueryType dns_query_type,
+                                     HostResolverSource source,
                                      HostResolverFlags flags,
                                      bool allow_cache,
                                      bool allow_stale,
@@ -2391,7 +2378,7 @@ int HostResolverImpl::ResolveLocally(const HostPortPair& host,
 
   // Build a key that identifies the request in the cache and in the
   // outstanding jobs map.
-  *key = GetEffectiveKeyForRequest(host.host(), dns_query_type, flags,
+  *key = GetEffectiveKeyForRequest(host.host(), dns_query_type, source, flags,
                                    ip_address_ptr, source_net_log);
 
   DCHECK(allow_stale == !!stale_info);
@@ -2619,6 +2606,7 @@ std::unique_ptr<HostResolverImpl::Job> HostResolverImpl::RemoveJob(Job* job) {
 HostResolverImpl::Key HostResolverImpl::GetEffectiveKeyForRequest(
     const std::string& hostname,
     DnsQueryType dns_query_type,
+    HostResolverSource source,
     HostResolverFlags flags,
     const IPAddress* ip_address,
     const NetLogWithSource& net_log) {
@@ -2639,7 +2627,7 @@ HostResolverImpl::Key HostResolverImpl::GetEffectiveKeyForRequest(
     effective_flags |= HOST_RESOLVER_DEFAULT_FAMILY_SET_DUE_TO_NO_IPV6;
   }
 
-  return Key(hostname, effective_address_family, effective_flags);
+  return Key(hostname, effective_address_family, effective_flags, source);
 }
 
 bool HostResolverImpl::IsIPv6Reachable(const NetLogWithSource& net_log) {

net/dns/host_resolver_impl.h

@@ -231,6 +231,7 @@ class NET_EXPORT HostResolverImpl
   // and |stale_info| must be null.
   int ResolveLocally(const HostPortPair& host,
                      DnsQueryType requested_address_family,
+                     HostResolverSource source,
                      HostResolverFlags flags,
                      bool allow_cache,
                      bool allow_stale,
@@ -286,6 +287,7 @@ class NET_EXPORT HostResolverImpl
   // family when the request leaves it unspecified.
   Key GetEffectiveKeyForRequest(const std::string& hostname,
                                 DnsQueryType dns_query_type,
+                                HostResolverSource source,
                                 HostResolverFlags flags,
                                 const IPAddress* ip_address,
                                 const NetLogWithSource& net_log);

net/dns/host_resolver_impl_unittest.cc

@@ -877,6 +877,26 @@ TEST_F(HostResolverImplTest, ResolveIPLiteralWithHostResolverSystemOnly) {
   EXPECT_TRUE(req->HasAddress(kIpLiteral, 80));
 }
 
+TEST_F(HostResolverImplTest,
+       ResolveIPLiteralWithHostResolverSystemOnly_ResolveHost) {
+  const char kIpLiteral[] = "178.78.32.1";
+  // Add a mapping to tell if the resolver proc was called (if it was called,
+  // then the result will be the remapped value. Otherwise it will be the IP
+  // literal).
+  proc_->AddRuleForAllFamilies(kIpLiteral, "183.45.32.1");
+
+  HostResolver::ResolveHostParameters parameters;
+  parameters.source = HostResolverSource::SYSTEM;
+  ResolveHostResponseHelper response(resolver_->CreateRequest(
+      HostPortPair(kIpLiteral, 80), NetLogWithSource(), parameters));
+
+  // IP literal resolution is expected to take precedence over source, so the
+  // result is expected to be the input IP, not the result IP from the proc rule
+  EXPECT_THAT(response.result_error(), IsOk());
+  EXPECT_THAT(response.request()->GetAddressResults().value().endpoints(),
+              testing::ElementsAre(CreateExpected(kIpLiteral, 80)));
+}
+
 TEST_F(HostResolverImplTest, EmptyListMeansNameNotResolved) {
   proc_->AddRuleForAllFamilies("just.testing", "");
   proc_->SignalMultiple(1u);
@@ -1575,8 +1595,6 @@ TEST_F(HostResolverImplTest, StartWithinCallback_ResolveHost) {
   EXPECT_THAT(new_response->result_error(), IsOk());
 }
 
-// TODO(crbug.com/821021): Create a ResolveHost test once bypassing the cache is
-// supported.
 TEST_F(HostResolverImplTest, BypassCache) {
   struct MyHandler : public Handler {
     void Handle(Request* req) override {
@@ -1611,6 +1629,29 @@ TEST_F(HostResolverImplTest, BypassCache) {
   EXPECT_EQ(2u, proc_->GetCaptureList().size());
 }
 
+TEST_F(HostResolverImplTest, BypassCache_ResolveHost) {
+  proc_->SignalMultiple(2u);
+
+  ResolveHostResponseHelper initial_response(resolver_->CreateRequest(
+      HostPortPair("a", 80), NetLogWithSource(), base::nullopt));
+  EXPECT_THAT(initial_response.result_error(), IsOk());
+  EXPECT_EQ(1u, proc_->GetCaptureList().size());
+
+  ResolveHostResponseHelper cached_response(resolver_->CreateRequest(
+      HostPortPair("a", 80), NetLogWithSource(), base::nullopt));
+  EXPECT_THAT(cached_response.result_error(), IsOk());
+  // Expect no increase to calls to |proc_| because result was cached.
+  EXPECT_EQ(1u, proc_->GetCaptureList().size());
+
+  HostResolver::ResolveHostParameters parameters;
+  parameters.allow_cached_response = false;
+  ResolveHostResponseHelper cache_bypassed_response(resolver_->CreateRequest(
+      HostPortPair("a", 80), NetLogWithSource(), parameters));
+  EXPECT_THAT(cache_bypassed_response.result_error(), IsOk());
+  // Expect call to |proc_| because cache was bypassed.
+  EXPECT_EQ(2u, proc_->GetCaptureList().size());
+}
+
 // Test that IP address changes flush the cache but initial DNS config reads do
 // not.
 TEST_F(HostResolverImplTest, FlushCacheOnIPAddressChange) {
@@ -3801,6 +3842,8 @@ TEST_F(HostResolverImplDnsTest, BypassDnsTask) {
     EXPECT_EQ(OK, requests_[i]->WaitForResult()) << i;
 }
 
+// Test that hosts ending in ".local" or ".local." are resolved using the system
+// resolver.
 TEST_F(HostResolverImplDnsTest, BypassDnsTask_ResolveHost) {
   ChangeDnsConfig(CreateValidDnsConfig());
 
@@ -3834,8 +3877,31 @@ TEST_F(HostResolverImplDnsTest, BypassDnsTask_ResolveHost) {
     EXPECT_THAT(responses[i]->result_error(), IsOk());
 }
 
-// TODO(crbug.com/821021): Create a ResolveHost version of this test once
-// system-only resolves are supported.
+// Test that DNS task is always used when explicitly requested as the source,
+// even with a case that would normally bypass it eg hosts ending in ".local".
+TEST_F(HostResolverImplDnsTest, DnsNotBypassedWhenDnsSource) {
+  // Ensure DNS task requests will succeed and system (proc) requests will fail.
+  ChangeDnsConfig(CreateValidDnsConfig());
+  proc_->AddRuleForAllFamilies(std::string(), std::string());
+
+  HostResolver::ResolveHostParameters dns_parameters;
+  dns_parameters.source = HostResolverSource::DNS;
+
+  ResolveHostResponseHelper dns_response(resolver_->CreateRequest(
+      HostPortPair("ok", 80), NetLogWithSource(), dns_parameters));
+  ResolveHostResponseHelper dns_local_response(resolver_->CreateRequest(
+      HostPortPair("ok.local", 80), NetLogWithSource(), dns_parameters));
+  ResolveHostResponseHelper normal_local_response(resolver_->CreateRequest(
+      HostPortPair("ok.local", 80), NetLogWithSource(), base::nullopt));
+
+  proc_->SignalMultiple(3u);
+
+  EXPECT_THAT(dns_response.result_error(), IsOk());
+  EXPECT_THAT(dns_local_response.result_error(), IsOk());
+  EXPECT_THAT(normal_local_response.result_error(),
+              IsError(ERR_NAME_NOT_RESOLVED));
+}
+
 TEST_F(HostResolverImplDnsTest, SystemOnlyBypassesDnsTask) {
   ChangeDnsConfig(CreateValidDnsConfig());
 
@@ -3855,6 +3921,25 @@ TEST_F(HostResolverImplDnsTest, SystemOnlyBypassesDnsTask) {
   EXPECT_THAT(requests_[1]->WaitForResult(), IsOk());
 }
 
+TEST_F(HostResolverImplDnsTest, SystemOnlyBypassesDnsTask_ResolveHost) {
+  // Ensure DNS task requests will succeed and system (proc) requests will fail.
+  ChangeDnsConfig(CreateValidDnsConfig());
+  proc_->AddRuleForAllFamilies(std::string(), std::string());
+
+  ResolveHostResponseHelper dns_response(resolver_->CreateRequest(
+      HostPortPair("ok", 80), NetLogWithSource(), base::nullopt));
+
+  HostResolver::ResolveHostParameters parameters;
+  parameters.source = HostResolverSource::SYSTEM;
+  ResolveHostResponseHelper system_response(resolver_->CreateRequest(
+      HostPortPair("ok", 80), NetLogWithSource(), parameters));
+
+  proc_->SignalMultiple(2u);
+
+  EXPECT_THAT(dns_response.result_error(), IsOk());
+  EXPECT_THAT(system_response.result_error(), IsError(ERR_NAME_NOT_RESOLVED));
+}
+
 TEST_F(HostResolverImplDnsTest, DisableDnsClientOnPersistentFailure) {
   ChangeDnsConfig(CreateValidDnsConfig());
 

net/dns/host_resolver_source.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef NET_DNS_HOST_RESOLVER_SOURCE_H_
+#define NET_DNS_HOST_RESOLVER_SOURCE_H_
+
+// Enumeration to specify the allowed results source for HostResolver
+// requests.
+enum class HostResolverSource {
+  // Resolver will pick an appropriate source. Results could come from DNS,
+  // MulticastDNS, HOSTS file, etc.
+  ANY,
+
+  // Results will only be retrieved from the system or OS, eg via the
+  // getaddrinfo() system call.
+  SYSTEM,
+
+  // Results will only come from DNS queries.
+  DNS,
+
+  // TODO(crbug.com/846423): Add MDNS support.
+};
+
+#endif  // NET_DNS_HOST_RESOLVER_SOURCE_H_