Net: Save CNAME aliases to AddressList with aim of exposing to ad-tag.

The overall project aims to expose CNAME aliases to the
SubresourceFilter to aid in ad tagging and blocking.

This change stores the aliases (which are already
being extracted) in a new field of type
std::vector<std::string> in net::AddressList called `dns_aliases_`.

The first entry of this vector, if it exists, will be
the canonical name, and so this change also removes the previous
`canonical_name_` field and renames the canonical name getters and
setters with camel case, making them no longer inline, as now they have
compound logic.

This CL moreover provides the ability to merge lists of aliases to
the net::HostCache::Entry class, as well as some additional unit tests
and updates to existing ones.

Relevant tests:
net:net_unittests
--gtest_filter=*GetCanonicalNameWhenUnset*
--gtest_filter=*DnsAliases*
--gtest_filter=*CnameChain*TypeA*
--gtest_filter=*MergeEntries*

Bug: 1151047
Change-Id: Ie45cb4284df33053db36b364ea982cbfe3d6a618
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2561274
Commit-Queue: Cammie Smith Barnes <cammie@chromium.org>
Reviewed-by: Eric Orth <ericorth@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834439}

chrome/test/chromedriver/net/adb_client_socket.cc

@@ -500,8 +500,9 @@ void AdbClientSocket::Connect(net::CompletionOnceCallback callback) {
   // on IPv4. So just try IPv4 first, then fall back to IPv6.
   net::IPAddressList list = {net::IPAddress::IPv4Localhost(),
                              net::IPAddress::IPv6Localhost()};
-  net::AddressList ip_list = net::AddressList::CreateFromIPAddressList(
-      list, "localhost");
+  std::vector<std::string> aliases({"localhost"});
+  net::AddressList ip_list =
+      net::AddressList::CreateFromIPAddressList(list, std::move(aliases));
   net::AddressList address_list = net::AddressList::CopyWithPort(
       ip_list, port_);
 

content/browser/renderer_host/pepper/pepper_host_resolver_message_filter.cc

@@ -189,7 +189,7 @@ void PepperHostResolverMessageFilter::OnLookupFinished(
   if (net_result != net::OK) {
     SendResolveError(NetErrorToPepperError(net_result), context);
   } else {
-    const std::string& canonical_name = addresses.value().canonical_name();
+    const std::string& canonical_name = addresses.value().GetCanonicalName();
     NetAddressList net_address_list;
     CreateNetAddressListFromAddressList(addresses.value(), &net_address_list);
     if (net_address_list.empty())

net/base/address_list.cc

@@ -4,12 +4,16 @@
 
 #include "net/base/address_list.h"
 
+#include <iterator>
+#include <string>
 #include <utility>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/containers/flat_map.h"
 #include "base/logging.h"
+#include "base/no_destructor.h"
 #include "base/values.h"
 #include "net/base/sys_addrinfo.h"
 #include "net/log/net_log_capture_mode.h"
@@ -32,6 +36,12 @@ AddressList::AddressList(const IPEndPoint& endpoint) {
   push_back(endpoint);
 }
 
+AddressList::AddressList(const IPEndPoint& endpoint,
+                         std::vector<std::string> aliases)
+    : dns_aliases_(std::move(aliases)) {
+  push_back(endpoint);
+}
+
 // static
 AddressList AddressList::CreateFromIPAddress(const IPAddress& address,
                                              uint16_t port) {
@@ -41,21 +51,23 @@ AddressList AddressList::CreateFromIPAddress(const IPAddress& address,
 // static
 AddressList AddressList::CreateFromIPAddressList(
     const IPAddressList& addresses,
-    const std::string& canonical_name) {
+    std::vector<std::string> aliases) {
   AddressList list;
-  list.set_canonical_name(canonical_name);
   for (auto iter = addresses.begin(); iter != addresses.end(); ++iter) {
     list.push_back(IPEndPoint(*iter, 0));
   }
+  list.SetDnsAliases(std::move(aliases));
   return list;
 }
 
 // static
 AddressList AddressList::CreateFromAddrinfo(const struct addrinfo* head) {
   DCHECK(head);
   AddressList list;
-  if (head->ai_canonname)
-    list.set_canonical_name(std::string(head->ai_canonname));
+  if (head->ai_canonname) {
+    std::vector<std::string> aliases({std::string(head->ai_canonname)});
+    list.SetDnsAliases(std::move(aliases));
+  }
   for (const struct addrinfo* ai = head; ai; ai = ai->ai_next) {
     IPEndPoint ipe;
     // NOTE: Ignoring non-INET* families.
@@ -70,15 +82,49 @@ AddressList AddressList::CreateFromAddrinfo(const struct addrinfo* head) {
 // static
 AddressList AddressList::CopyWithPort(const AddressList& list, uint16_t port) {
   AddressList out;
-  out.set_canonical_name(list.canonical_name());
-  for (size_t i = 0; i < list.size(); ++i)
-    out.push_back(IPEndPoint(list[i].address(), port));
+  out.SetDnsAliases(list.dns_aliases());
+  for (const auto& i : list)
+    out.push_back(IPEndPoint(i.address(), port));
   return out;
 }
 
+// TODO(cammie/ericorth): Consider changing the return value to
+// base::StringPiece (by non-const value), which  is often a better type
+// for passing/returning non-owned strings. In this case, because it could
+// point directly at a string literal/constant, it would allow us to avoid
+// creating a full std::string (and the annoyance of needing NoDestructor)
+// just to store empty.
+const std::string& AddressList::GetCanonicalName() const {
+  static const base::NoDestructor<std::string> nullstring_result;
+  return dns_aliases_.size() >= 1 ? dns_aliases_.front() : *nullstring_result;
+}
+
 void AddressList::SetDefaultCanonicalName() {
   DCHECK(!empty());
-  set_canonical_name(front().ToStringWithoutPort());
+  DCHECK(dns_aliases_.empty());
+  SetDnsAliases({front().ToStringWithoutPort()});
+}
+
+void AddressList::SetDnsAliases(std::vector<std::string> aliases) {
+  // TODO(cammie): Track down the callers who use {""} for `aliases` and
+  // update so that we can enforce by DCHECK below.
+  // The empty canonical name is represented by a empty `dns_aliases_`
+  // vector, so in this case we reset the field.
+  if (aliases == std::vector<std::string>({""})) {
+    dns_aliases_ = std::vector<std::string>();
+    return;
+  }
+
+  dns_aliases_ = std::move(aliases);
+}
+
+void AddressList::AppendDnsAliases(std::vector<std::string> aliases) {
+  DCHECK(aliases != std::vector<std::string>({""}));
+  using iter_t = std::vector<std::string>::iterator;
+
+  dns_aliases_.insert(dns_aliases_.end(),
+                      std::move_iterator<iter_t>(aliases.begin()),
+                      std::move_iterator<iter_t>(aliases.end()));
 }
 
 base::Value AddressList::NetLogParams() const {
@@ -89,7 +135,7 @@ base::Value AddressList::NetLogParams() const {
     list.Append(ip_endpoint.ToString());
 
   dict.SetKey("address_list", std::move(list));
-  dict.SetStringKey("canonical_name", canonical_name());
+  dict.SetStringKey("canonical_name", GetCanonicalName());
   return dict;
 }
 

net/base/address_list.h

@@ -36,27 +36,42 @@ class NET_EXPORT AddressList {
   // Creates an address list for a single IP literal.
   explicit AddressList(const IPEndPoint& endpoint);
 
+  // Creates an address list for a single IP literal and a list of DNS aliases.
+  AddressList(const IPEndPoint& endpoint, std::vector<std::string> aliases);
+
   static AddressList CreateFromIPAddress(const IPAddress& address,
                                          uint16_t port);
 
   static AddressList CreateFromIPAddressList(const IPAddressList& addresses,
-                                             const std::string& canonical_name);
+                                             std::vector<std::string> aliases);
 
-  // Copies the data from |head| and the chained list into an AddressList.
+  // Copies the data from `head` and the chained list into an AddressList.
   static AddressList CreateFromAddrinfo(const struct addrinfo* head);
 
-  // Returns a copy of |list| with port on each element set to |port|.
+  // Returns a copy of `list` with port on each element set to |port|.
   static AddressList CopyWithPort(const AddressList& list, uint16_t port);
 
-  // TODO(szym): Remove all three. http://crbug.com/126134
-  const std::string& canonical_name() const { return canonical_name_; }
+  // TODO(crbug.com/126134): Remove all references to canonical name
+  // in net::AddressList.
+  // Here and below, by "canonical name", we mean the value of the name for
+  // the DNS record that contained the stored-order first IP address stored
+  // by this class. Note that the canonical name, if set, is now stored as
+  // the first entry in the vector `dns_aliases_` below.
+  // Returns the first entry, if it exists, of `dns_aliases_` or an empty
+  // string otherwise.
+  const std::string& GetCanonicalName() const;
+
+  // Sets the first entry of `dns_aliases_` to the literal of the first IP
+  // address on the list. Assumes that `dns_aliases_` is empty.
+  void SetDefaultCanonicalName();
 
-  void set_canonical_name(const std::string& canonical_name) {
-    canonical_name_ = canonical_name;
-  }
+  // The alias chain is preserved in reverse order, from canonical name (i.e.
+  // address record name) through to query name.
+  const std::vector<std::string>& dns_aliases() const { return dns_aliases_; }
 
-  // Sets canonical name to the literal of the first IP address on the list.
-  void SetDefaultCanonicalName();
+  void SetDnsAliases(std::vector<std::string> aliases);
+
+  void AppendDnsAliases(std::vector<std::string> aliases);
 
   // Creates a value representation of the address list, appropriate for
   // inclusion in a NetLog.
@@ -95,8 +110,11 @@ class NET_EXPORT AddressList {
 
  private:
   std::vector<IPEndPoint> endpoints_;
-  // TODO(szym): Remove. http://crbug.com/126134
-  std::string canonical_name_;
+
+  // The first entry, if it exists, is the canonical name.
+  // The alias chain is preserved in reverse order, from canonical name (i.e.
+  // address record name) through to query name.
+  std::vector<std::string> dns_aliases_;
 };
 
 }  // namespace net

net/base/address_list_unittest.cc

@@ -39,11 +39,11 @@ TEST(AddressListTest, Canonical) {
   // Copy the addrinfo struct into an AddressList object and
   // make sure it seems correct.
   AddressList addrlist1 = AddressList::CreateFromAddrinfo(&ai);
-  EXPECT_EQ("canonical.bar.com", addrlist1.canonical_name());
+  EXPECT_EQ("canonical.bar.com", addrlist1.GetCanonicalName());
 
   // Copy the AddressList to another one.
   AddressList addrlist2 = addrlist1;
-  EXPECT_EQ("canonical.bar.com", addrlist2.canonical_name());
+  EXPECT_EQ("canonical.bar.com", addrlist2.GetCanonicalName());
 }
 
 TEST(AddressListTest, CreateFromAddrinfo) {
@@ -132,13 +132,91 @@ TEST(AddressListTest, CreateFromIPAddressList) {
     ip_list.push_back(ip_address);
   }
 
-  AddressList test_list = AddressList::CreateFromIPAddressList(ip_list,
-                                                               kCanonicalName);
+  // Wrap the canonical name in an alias vector.
+  std::vector<std::string> aliases({kCanonicalName});
+
+  AddressList test_list =
+      AddressList::CreateFromIPAddressList(ip_list, std::move(aliases));
   std::string canonical_name;
-  EXPECT_EQ(kCanonicalName, test_list.canonical_name());
+  EXPECT_EQ(kCanonicalName, test_list.GetCanonicalName());
   EXPECT_EQ(base::size(tests), test_list.size());
 }
 
+TEST(AddressListTest, GetCanonicalNameWhenUnset) {
+  const IPAddress kAddress(1, 2, 3, 4);
+  const IPEndPoint kEndpoint(kAddress, 0);
+  AddressList addrlist(kEndpoint);
+
+  EXPECT_TRUE(addrlist.dns_aliases().empty());
+  EXPECT_EQ(addrlist.GetCanonicalName(), "");
+}
+
+TEST(AddressListTest, SetDefaultCanonicalNameThenSetDnsAliases) {
+  const IPAddress kAddress(1, 2, 3, 4);
+  const IPEndPoint kEndpoint(kAddress, 0);
+  AddressList addrlist(kEndpoint);
+
+  addrlist.SetDefaultCanonicalName();
+
+  EXPECT_EQ(addrlist.GetCanonicalName(), "1.2.3.4");
+  EXPECT_THAT(addrlist.dns_aliases(), ElementsAre("1.2.3.4"));
+
+  std::vector<std::string> aliases({"alias1", "alias2", "alias3"});
+  addrlist.SetDnsAliases(std::move(aliases));
+
+  // Setting the aliases after setting the default canonical name
+  // replaces the default canonical name.
+  EXPECT_EQ(addrlist.GetCanonicalName(), "alias1");
+  EXPECT_THAT(addrlist.dns_aliases(),
+              ElementsAre("alias1", "alias2", "alias3"));
+}
+
+TEST(AddressListTest, SetDefaultCanonicalNameThenAppendDnsAliases) {
+  const IPAddress kAddress(1, 2, 3, 4);
+  const IPEndPoint kEndpoint(kAddress, 0);
+  AddressList addrlist(kEndpoint);
+
+  addrlist.SetDefaultCanonicalName();
+
+  EXPECT_EQ(addrlist.GetCanonicalName(), "1.2.3.4");
+  EXPECT_THAT(addrlist.dns_aliases(), ElementsAre("1.2.3.4"));
+
+  std::vector<std::string> aliases({"alias1", "alias2", "alias3"});
+  addrlist.AppendDnsAliases(std::move(aliases));
+
+  // Appending the aliases after setting the default canonical name
+  // does not replace the default canonical name.
+  EXPECT_EQ(addrlist.GetCanonicalName(), "1.2.3.4");
+  EXPECT_THAT(addrlist.dns_aliases(),
+              ElementsAre("1.2.3.4", "alias1", "alias2", "alias3"));
+}
+
+TEST(AddressListTest, DnsAliases) {
+  const IPAddress kAddress(1, 2, 3, 4);
+  const IPEndPoint kEndpoint(kAddress, 0);
+  std::vector<std::string> aliases({"alias1", "alias2", "alias3"});
+  AddressList addrlist(kEndpoint, std::move(aliases));
+
+  EXPECT_EQ(addrlist.GetCanonicalName(), "alias1");
+  EXPECT_THAT(addrlist.dns_aliases(),
+              ElementsAre("alias1", "alias2", "alias3"));
+
+  std::vector<std::string> more_aliases({"alias4", "alias5", "alias6"});
+  addrlist.AppendDnsAliases(std::move(more_aliases));
+
+  EXPECT_EQ(addrlist.GetCanonicalName(), "alias1");
+  EXPECT_THAT(
+      addrlist.dns_aliases(),
+      ElementsAre("alias1", "alias2", "alias3", "alias4", "alias5", "alias6"));
+
+  std::vector<std::string> new_aliases({"alias7", "alias8", "alias9"});
+  addrlist.SetDnsAliases(std::move(new_aliases));
+
+  EXPECT_EQ(addrlist.GetCanonicalName(), "alias7");
+  EXPECT_THAT(addrlist.dns_aliases(),
+              ElementsAre("alias7", "alias8", "alias9"));
+}
+
 TEST(AddressListTest, DeduplicatesEmptyAddressList) {
   AddressList empty;
   empty.Deduplicate();

net/dns/address_info.cc

@@ -150,8 +150,10 @@ bool AddressInfo::IsAllLocalhostOfOneFamily() const {
 AddressList AddressInfo::CreateAddressList() const {
   AddressList list;
   auto canonical_name = GetCanonicalName();
-  if (canonical_name)
-    list.set_canonical_name(*canonical_name);
+  if (canonical_name) {
+    std::vector<std::string> aliases({*canonical_name});
+    list.SetDnsAliases(std::move(aliases));
+  }
   for (auto&& ai : *this) {
     IPEndPoint ipe;
     // NOTE: Ignoring non-INET* families.