Add some flags to mojo DNS API.

Adding include_canonical_name and loopback_only to the optional
parameters struct.  These will be equivalent to HOST_RESOLVER_CANONNAME
and HOST_RESOLVER_LOOPBACK_ONLY from the old API.  And at least for now,
they'll be implemented by converting to the old flags for simple
compatibility with the current implementation of HostCache::Key.

Added canonical name support to the mojo AddressList as it was missing.

Bug: 821021
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I7ff288f05d6a13e1c6c3c293d9f7a1064f011380
Reviewed-on: https://chromium-review.googlesource.com/1179996
Commit-Queue: Eric Orth <ericorth@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584500}

net/dns/host_resolver.cc

@@ -208,6 +208,19 @@ HostResolver::RequestInfoToResolveHostParameters(
   return parameters;
 }
 
+// static
+HostResolverFlags HostResolver::ParametersToHostResolverFlags(
+    const ResolveHostParameters& parameters) {
+  HostResolverFlags flags = 0;
+  if (parameters.include_canonical_name) {
+    flags |= HOST_RESOLVER_CANONNAME;
+  }
+  if (parameters.loopback_only) {
+    flags |= HOST_RESOLVER_LOOPBACK_ONLY;
+  }
+  return flags;
+}
+
 HostResolver::HostResolver() = default;
 
 }  // namespace net

net/dns/host_resolver.h

@@ -197,6 +197,16 @@ class NET_EXPORT HostResolver {
     // The initial net priority for the host resolution request.
     RequestPriority initial_priority = RequestPriority::DEFAULT_PRIORITY;
 
+    // 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
+    // parameter is set to |false|.
+    bool include_canonical_name = false;
+
+    // Hint to the resolver that resolution is only being requested for loopback
+    // hosts.
+    bool loopback_only = false;
+
     // Set |true| iff the host resolve request is only being made speculatively
     // to fill the cache and the result addresses will not be used. The request
     // will receive special logging/observer treatment, and the result addresses
@@ -348,6 +358,8 @@ class NET_EXPORT HostResolver {
   static ResolveHostParameters RequestInfoToResolveHostParameters(
       const RequestInfo& request_info,
       RequestPriority priority);
+  static HostResolverFlags ParametersToHostResolverFlags(
+      const ResolveHostParameters& parameters);
 
  protected:
   HostResolver();

net/dns/host_resolver_impl.cc

@@ -636,10 +636,11 @@ class HostResolverImpl::RequestImpl
               base::WeakPtr<HostResolverImpl> resolver)
       : source_net_log_(source_net_log),
         request_host_(request_host),
-        host_resolver_flags_(host_resolver_flags),
         allow_cached_response_(allow_cached_response),
         parameters_(optional_parameters ? optional_parameters.value()
                                         : ResolveHostParameters()),
+        host_resolver_flags_(host_resolver_flags |
+                             ParametersToHostResolverFlags(parameters_)),
         priority_(parameters_.initial_priority),
         job_(nullptr),
         resolver_(resolver),
@@ -726,12 +727,12 @@ class HostResolverImpl::RequestImpl
 
   const HostPortPair& request_host() const { return request_host_; }
 
-  HostResolverFlags host_resolver_flags() const { return host_resolver_flags_; }
-
   bool allow_cached_response() const { return allow_cached_response_; }
 
   const ResolveHostParameters& parameters() const { return parameters_; }
 
+  HostResolverFlags host_resolver_flags() const { return host_resolver_flags_; }
+
   RequestPriority priority() const { return priority_; }
   void set_priority(RequestPriority priority) { priority_ = priority; }
 
@@ -751,9 +752,9 @@ class HostResolverImpl::RequestImpl
   const NetLogWithSource source_net_log_;
 
   const HostPortPair request_host_;
-  const HostResolverFlags host_resolver_flags_;
   const bool allow_cached_response_;
   const ResolveHostParameters parameters_;
+  const HostResolverFlags host_resolver_flags_;
 
   RequestPriority priority_;
 

net/dns/host_resolver_impl_unittest.cc

@@ -76,14 +76,17 @@ HostResolverImpl::ProcTaskParams DefaultParams(
 class MockHostResolverProc : public HostResolverProc {
  public:
   struct ResolveKey {
-    ResolveKey(const std::string& hostname, AddressFamily address_family)
-        : hostname(hostname), address_family(address_family) {}
+    ResolveKey(const std::string& hostname,
+               AddressFamily address_family,
+               HostResolverFlags flags)
+        : hostname(hostname), address_family(address_family), flags(flags) {}
     bool operator<(const ResolveKey& other) const {
-      return std::tie(address_family, hostname) <
-             std::tie(other.address_family, other.hostname);
+      return std::tie(address_family, hostname, flags) <
+             std::tie(other.address_family, other.hostname, other.flags);
     }
     std::string hostname;
     AddressFamily address_family;
+    HostResolverFlags flags;
   };
 
   typedef std::vector<ResolveKey> CaptureList;
@@ -123,28 +126,35 @@ class MockHostResolverProc : public HostResolverProc {
     slots_available_.Broadcast();
   }
 
-  void AddRule(const std::string& hostname, AddressFamily family,
-               const AddressList& result) {
+  void AddRule(const std::string& hostname,
+               AddressFamily family,
+               const AddressList& result,
+               HostResolverFlags flags = 0) {
     base::AutoLock lock(lock_);
-    rules_[ResolveKey(hostname, family)] = result;
+    rules_[ResolveKey(hostname, family, flags)] = result;
   }
 
-  void AddRule(const std::string& hostname, AddressFamily family,
-               const std::string& ip_list) {
+  void AddRule(const std::string& hostname,
+               AddressFamily family,
+               const std::string& ip_list,
+               HostResolverFlags flags = 0,
+               const std::string& canonical_name = "") {
     AddressList result;
-    int rv = ParseAddressList(ip_list, std::string(), &result);
+    int rv = ParseAddressList(ip_list, canonical_name, &result);
     DCHECK_EQ(OK, rv);
-    AddRule(hostname, family, result);
+    AddRule(hostname, family, result, flags);
   }
 
   void AddRuleForAllFamilies(const std::string& hostname,
-                             const std::string& ip_list) {
+                             const std::string& ip_list,
+                             HostResolverFlags flags = 0,
+                             const std::string& canonical_name = "") {
     AddressList result;
-    int rv = ParseAddressList(ip_list, std::string(), &result);
+    int rv = ParseAddressList(ip_list, canonical_name, &result);
     DCHECK_EQ(OK, rv);
-    AddRule(hostname, ADDRESS_FAMILY_UNSPECIFIED, result);
-    AddRule(hostname, ADDRESS_FAMILY_IPV4, result);
-    AddRule(hostname, ADDRESS_FAMILY_IPV6, result);
+    AddRule(hostname, ADDRESS_FAMILY_UNSPECIFIED, result, flags);
+    AddRule(hostname, ADDRESS_FAMILY_IPV4, result, flags);
+    AddRule(hostname, ADDRESS_FAMILY_IPV6, result, flags);
   }
 
   int Resolve(const std::string& hostname,
@@ -153,7 +163,8 @@ class MockHostResolverProc : public HostResolverProc {
               AddressList* addrlist,
               int* os_error) override {
     base::AutoLock lock(lock_);
-    capture_list_.push_back(ResolveKey(hostname, address_family));
+    capture_list_.push_back(
+        ResolveKey(hostname, address_family, host_resolver_flags));
     ++num_requests_waiting_;
     requests_waiting_.Broadcast();
     {
@@ -170,7 +181,7 @@ class MockHostResolverProc : public HostResolverProc {
       DCHECK_EQ(OK, rv);
       return OK;
     }
-    ResolveKey key(hostname, address_family);
+    ResolveKey key(hostname, address_family, host_resolver_flags);
     if (rules_.count(key) == 0)
       return ERR_NAME_NOT_RESOLVED;
     *addrlist = rules_[key];
@@ -2891,6 +2902,46 @@ TEST_F(HostResolverImplTest, InputObjectsBoundToCallback_Async) {
   EXPECT_TRUE(result_request);
 }
 
+TEST_F(HostResolverImplTest, IncludeCanonicalName) {
+  proc_->AddRuleForAllFamilies("just.testing", "192.168.1.42",
+                               HOST_RESOLVER_CANONNAME, "canon.name");
+  proc_->SignalMultiple(2u);
+
+  HostResolver::ResolveHostParameters parameters;
+  parameters.include_canonical_name = true;
+  ResolveHostResponseHelper response(resolver_->CreateRequest(
+      HostPortPair("just.testing", 80), NetLogWithSource(), parameters));
+  ResolveHostResponseHelper response_no_flag(resolver_->CreateRequest(
+      HostPortPair("just.testing", 80), NetLogWithSource(), base::nullopt));
+
+  EXPECT_THAT(response.result_error(), IsOk());
+  EXPECT_THAT(response.request()->GetAddressResults().value().endpoints(),
+              testing::ElementsAre(CreateExpected("192.168.1.42", 80)));
+  EXPECT_EQ("canon.name",
+            response.request()->GetAddressResults().value().canonical_name());
+
+  EXPECT_THAT(response_no_flag.result_error(), IsError(ERR_NAME_NOT_RESOLVED));
+}
+
+TEST_F(HostResolverImplTest, LoopbackOnly) {
+  proc_->AddRuleForAllFamilies("otherlocal", "127.0.0.1",
+                               HOST_RESOLVER_LOOPBACK_ONLY);
+  proc_->SignalMultiple(2u);
+
+  HostResolver::ResolveHostParameters parameters;
+  parameters.loopback_only = true;
+  ResolveHostResponseHelper response(resolver_->CreateRequest(
+      HostPortPair("otherlocal", 80), NetLogWithSource(), parameters));
+  ResolveHostResponseHelper response_no_flag(resolver_->CreateRequest(
+      HostPortPair("otherlocal", 80), NetLogWithSource(), base::nullopt));
+
+  EXPECT_THAT(response.result_error(), IsOk());
+  EXPECT_THAT(response.request()->GetAddressResults().value().endpoints(),
+              testing::ElementsAre(CreateExpected("127.0.0.1", 80)));
+
+  EXPECT_THAT(response_no_flag.result_error(), IsError(ERR_NAME_NOT_RESOLVED));
+}
+
 TEST_F(HostResolverImplTest, IsSpeculative_ResolveHost) {
   proc_->AddRuleForAllFamilies("just.testing", "192.168.1.42");
   proc_->SignalMultiple(1u);
@@ -4462,7 +4513,8 @@ TEST_F(HostResolverImplDnsTest, IPv4EmptyFallback_ResolveHost) {
   config.use_local_ipv6 = false;
   ChangeDnsConfig(config);
 
-  proc_->AddRuleForAllFamilies("empty_fallback", "192.168.0.1");
+  proc_->AddRuleForAllFamilies("empty_fallback", "192.168.0.1",
+                               HOST_RESOLVER_DEFAULT_FAMILY_SET_DUE_TO_NO_IPV6);
   proc_->SignalMultiple(1u);
 
   ResolveHostResponseHelper response(resolver_->CreateRequest(
@@ -4849,6 +4901,8 @@ TEST_F(HostResolverImplDnsTest, NoIPv6OnWifi) {
 
   proc_->AddRule("h1", ADDRESS_FAMILY_UNSPECIFIED, "::3");
   proc_->AddRule("h1", ADDRESS_FAMILY_IPV4, "1.0.0.1");
+  proc_->AddRule("h1", ADDRESS_FAMILY_IPV4, "1.0.0.1",
+                 HOST_RESOLVER_DEFAULT_FAMILY_SET_DUE_TO_NO_IPV6);
   proc_->AddRule("h1", ADDRESS_FAMILY_IPV6, "::2");
 
   CreateRequest("h1", 80, MEDIUM, ADDRESS_FAMILY_UNSPECIFIED);
@@ -4936,6 +4990,8 @@ TEST_F(HostResolverImplDnsTest, NoIPv6OnWifi_ResolveHost) {
 
   proc_->AddRule("h1", ADDRESS_FAMILY_UNSPECIFIED, "::3");
   proc_->AddRule("h1", ADDRESS_FAMILY_IPV4, "1.0.0.1");
+  proc_->AddRule("h1", ADDRESS_FAMILY_IPV4, "1.0.0.1",
+                 HOST_RESOLVER_DEFAULT_FAMILY_SET_DUE_TO_NO_IPV6);
   proc_->AddRule("h1", ADDRESS_FAMILY_IPV6, "::2");
 
   ResolveHostResponseHelper response(resolver_->CreateRequest(

net/dns/mock_host_resolver.cc

@@ -75,10 +75,11 @@ class MockHostResolverBase::RequestImpl
               bool allow_cached_response,
               base::WeakPtr<MockHostResolverBase> resolver)
       : request_host_(request_host),
-        host_resolver_flags_(host_resolver_flags),
         allow_cached_response_(allow_cached_response),
         parameters_(optional_parameters ? optional_parameters.value()
                                         : ResolveHostParameters()),
+        host_resolver_flags_(host_resolver_flags |
+                             ParametersToHostResolverFlags(parameters_)),
         id_(0),
         resolver_(resolver),
         complete_(false) {}
@@ -142,12 +143,12 @@ class MockHostResolverBase::RequestImpl
 
   const HostPortPair& request_host() const { return request_host_; }
 
-  int host_resolver_flags() const { return host_resolver_flags_; }
-
   bool allow_cached_response() const { return allow_cached_response_; }
 
   const ResolveHostParameters& parameters() const { return parameters_; }
 
+  int host_resolver_flags() const { return host_resolver_flags_; }
+
   size_t id() { return id_; }
 
   void set_id(size_t id) {
@@ -161,9 +162,9 @@ class MockHostResolverBase::RequestImpl
 
  private:
   const HostPortPair request_host_;
-  int host_resolver_flags_;
   bool allow_cached_response_;
   const ResolveHostParameters parameters_;
+  int host_resolver_flags_;
 
   base::Optional<AddressList> address_results_;
 
@@ -514,6 +515,17 @@ void RuleBasedHostResolverProc::AddRuleForAddressFamily(
   AddRuleInternal(rule);
 }
 
+void RuleBasedHostResolverProc::AddRuleWithFlags(
+    const std::string& host_pattern,
+    const std::string& replacement,
+    HostResolverFlags flags,
+    const std::string& canonical_name) {
+  DCHECK(!replacement.empty());
+  Rule rule(Rule::kResolverTypeSystem, host_pattern, ADDRESS_FAMILY_UNSPECIFIED,
+            flags, replacement, canonical_name, 0);
+  AddRuleInternal(rule);
+}
+
 void RuleBasedHostResolverProc::AddIPLiteralRule(
     const std::string& host_pattern,
     const std::string& ip_literal,

net/dns/mock_host_resolver.h

@@ -216,6 +216,11 @@ class RuleBasedHostResolverProc : public HostResolverProc {
                                AddressFamily address_family,
                                const std::string& ip_literal);
 
+  void AddRuleWithFlags(const std::string& host_pattern,
+                        const std::string& ip_literal,
+                        HostResolverFlags flags,
+                        const std::string& canonical_name = "");
+
   // Same as AddRule(), but the replacement is expected to be an IPv4 or IPv6
   // literal. This can be used in place of AddRule() to bypass the system's
   // host resolver (the address list will be constructed manually).

net/interfaces/address_list.mojom

@@ -9,4 +9,5 @@ import "net/interfaces/ip_endpoint.mojom";
 // Mirror of net::AddressList.
 struct AddressList {
   array<IPEndPoint> addresses;
+  string canonical_name;
 };

net/interfaces/address_list_mojom_traits.cc

@@ -13,7 +13,15 @@ namespace mojo {
 bool StructTraits<net::interfaces::AddressListDataView, net::AddressList>::Read(
     net::interfaces::AddressListDataView data,
     net::AddressList* out) {
-  return data.ReadAddresses(&out->endpoints());
+  if (!data.ReadAddresses(&out->endpoints()))
+    return false;
+
+  std::string canonical_name;
+  if (!data.ReadCanonicalName(&canonical_name))
+    return false;
+  out->set_canonical_name(canonical_name);
+
+  return true;
 }
 
 }  // namespace mojo

net/interfaces/address_list_mojom_traits.h

@@ -5,6 +5,7 @@
 #ifndef NET_INTERFACES_ADDRESS_LIST_MOJOM_TRAITS_H_
 #define NET_INTERFACES_ADDRESS_LIST_MOJOM_TRAITS_H_
 
+#include <string>
 #include <vector>
 
 #include "mojo/public/cpp/bindings/struct_traits.h"
@@ -19,6 +20,10 @@ struct StructTraits<net::interfaces::AddressListDataView, net::AddressList> {
     return obj.endpoints();
   }
 
+  static const std::string& canonical_name(const net::AddressList& obj) {
+    return obj.canonical_name();
+  }
+
   static bool Read(net::interfaces::AddressListDataView data,
                    net::AddressList* out);
 };

services/network/host_resolver.cc

@@ -31,6 +31,8 @@ ConvertOptionalParameters(
   net::HostResolver::ResolveHostParameters parameters;
   parameters.dns_query_type = mojo_parameters->dns_query_type;
   parameters.initial_priority = mojo_parameters->initial_priority;
+  parameters.include_canonical_name = mojo_parameters->include_canonical_name;
+  parameters.loopback_only = mojo_parameters->loopback_only;
   parameters.is_speculative = mojo_parameters->is_speculative;
   return parameters;
 }

services/network/host_resolver_unittest.cc

@@ -198,6 +198,61 @@ TEST_F(HostResolverTest, InitialPriority) {
   EXPECT_EQ(net::HIGHEST, inner_resolver->last_request_priority());
 }
 
+TEST_F(HostResolverTest, IncludeCanonicalName) {
+  auto inner_resolver = std::make_unique<net::MockHostResolver>();
+  inner_resolver->rules()->AddRuleWithFlags("example.com", "123.0.12.24",
+                                            net::HOST_RESOLVER_CANONNAME,
+                                            "canonicalexample.com");
+  net::NetLog net_log;
+
+  HostResolver resolver(inner_resolver.get(), &net_log);
+
+  mojom::ResolveHostParametersPtr optional_parameters =
+      mojom::ResolveHostParameters::New();
+  optional_parameters->include_canonical_name = true;
+
+  base::RunLoop run_loop;
+  mojom::ResolveHostClientPtr response_client_ptr;
+  TestResolveHostClient response_client(&response_client_ptr, &run_loop);
+
+  resolver.ResolveHost(net::HostPortPair("example.com", 80),
+                       std::move(optional_parameters),
+                       std::move(response_client_ptr));
+  run_loop.Run();
+
+  EXPECT_EQ(net::OK, response_client.result_error());
+  EXPECT_THAT(response_client.result_addresses().value().endpoints(),
+              testing::ElementsAre(CreateExpectedEndPoint("123.0.12.24", 80)));
+  EXPECT_EQ("canonicalexample.com",
+            response_client.result_addresses().value().canonical_name());
+}
+
+TEST_F(HostResolverTest, LoopbackOnly) {
+  auto inner_resolver = std::make_unique<net::MockHostResolver>();
+  inner_resolver->rules()->AddRuleWithFlags("example.com", "127.0.12.24",
+                                            net::HOST_RESOLVER_LOOPBACK_ONLY);
+  net::NetLog net_log;
+
+  HostResolver resolver(inner_resolver.get(), &net_log);
+
+  mojom::ResolveHostParametersPtr optional_parameters =
+      mojom::ResolveHostParameters::New();
+  optional_parameters->loopback_only = true;
+
+  base::RunLoop run_loop;
+  mojom::ResolveHostClientPtr response_client_ptr;
+  TestResolveHostClient response_client(&response_client_ptr, &run_loop);
+
+  resolver.ResolveHost(net::HostPortPair("example.com", 80),
+                       std::move(optional_parameters),
+                       std::move(response_client_ptr));
+  run_loop.Run();
+
+  EXPECT_EQ(net::OK, response_client.result_error());
+  EXPECT_THAT(response_client.result_addresses().value().endpoints(),
+              testing::ElementsAre(CreateExpectedEndPoint("127.0.12.24", 80)));
+}
+
 TEST_F(HostResolverTest, Failure_Sync) {
   auto inner_resolver = std::make_unique<net::MockHostResolver>();
   inner_resolver->rules()->AddSimulatedFailure("example.com");

services/network/public/mojom/host_resolver.mojom

@@ -56,6 +56,16 @@ struct ResolveHostParameters {
   // handle.
   ResolveHostHandle&? control_handle;
 
+  // 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 parameter is set to
+  // |false|.
+  bool include_canonical_name = false;
+
+  // Hint to the resolver that resolution is only being requested for loopback
+  // hosts.
+  bool loopback_only = false;
+
   // Set |true| iff the host resolve request is only being made speculatively
   // to fill the cache and the result addresses will not be used. The request
   // will receive special logging/observer treatment, and