Add IP address handling to net::IsHostnameNonUnique

net/base/net_util.cc

Index: net/base/net_util.cc
diff --git a/net/base/net_util.cc b/net/base/net_util.cc
index 958e3c3bca82049671f39fda3574b7eca3871664..c1bce6c39cd2571d1f40365306ab89b61b44be6f 100644
--- a/net/base/net_util.cc
+++ b/net/base/net_util.cc
@@ -1391,7 +1391,6 @@ std::string GetHostAndOptionalPort(const GURL& url) {
   return url.host();
 }
 
-// static
 bool IsHostnameNonUnique(const std::string& hostname) {
   // CanonicalizeHost requires surrounding brackets to parse an IPv6 address.
   const std::string host_or_ip = hostname.find(':') != std::string::npos ?
@@ -1404,11 +1403,22 @@ bool IsHostnameNonUnique(const std::string& hostname) {
   if (canonical_name.empty())
     return false;
 
-  // If |hostname| is an IP address, presume it's unique.
-  // TODO(rsleevi): In the future, this should also reject IP addresses in
-  // IANA-reserved ranges.
-  if (host_info.IsIPAddress())
-    return false;
+  // If |hostname| is an IP address, check to see if it's in an IANA-reserved
+  // range.
+  if (host_info.IsIPAddress()) {
+    IPAddressNumber host_addr;
+    if (!ParseIPLiteralToNumber(hostname, &host_addr))

[Ryan Sleevi, 2013/08/08 19:49:31]

CanonicalizeHost is going to put the canonical IP address into
host_info.out_host , which seems to be easier to avoid canonicalization issues
when passing back to ParseIPLiteral. Just FWIW.

See
https://code.google.com/p/chromium/codesearch#chromium/src/url/url_canon_host...

[felt, 2013/08/09 04:53:20]

Done.

+      return false;
+    switch (host_info.family) {
+      case url_canon::CanonHostInfo::IPV4:
+        return IsIPAddressReserved(host_addr, true);
+      case url_canon::CanonHostInfo::IPV6:
+        return IsIPAddressReserved(host_addr, false);
+      case url_canon::CanonHostInfo::NEUTRAL:
+      case url_canon::CanonHostInfo::BROKEN:
+        return false;
+    }
+  }
 
   // Check for a registry controlled portion of |hostname|, ignoring private
   // registries, as they already chain to ICANN-administered registries,
@@ -1425,6 +1435,74 @@ bool IsHostnameNonUnique(const std::string& hostname) {
                   registry_controlled_domains::EXCLUDE_PRIVATE_REGISTRIES);
 }
 
+bool IsIPAddressReserved(const IPAddressNumber& host_addr, bool ipv4) {
+  // We shouldn't compare IPv4 to IPv6 (they have different range
+  // reservations), so we keep separate arrays.
+  // Within each array, adjacent ranges are consolidated when possible.
+  // Sources for info:
+  // www.iana.org/assignments/ipv4-address-space/ipv4-address-space.xhtml
+  // www.iana.org/assignments/ipv6-address-space/ipv6-address-space.xhtml
+  // They're formatted here with the prefix as the last element. For example:
+  // 10.0.0.0/8 becomes 10,0,0,0,8 and fec0::/10 becomes 0xfe,0xc0,0,0,0...,10.
+  const unsigned char reserved_ipv4[][5] = {
+    { 0,0,0,0,8 }, { 10,0,0,0,8 }, { 100,64,0,0,10 }, { 127,0,0,0,8 },
+    { 169,254,0,0,16 }, { 172,16,0,0,12 }, { 192,0,2,0,24 }, { 192,88,99,0,24 },
+    { 192,168,0,0,16 }, { 198,18,0,0,15 }, { 198,51,100,0,24 },
+    { 203,0,113,0,24 }, { 224,0,0,0,3 }
+  };
+  const unsigned char reserved_ipv6[][17] = {
+    { 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,8 },
+    { 0x40,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,2 },
+    { 0x80,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,2 },
+    { 0xc0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,3 },
+    { 0xe0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,4 },
+    { 0xf0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,5 },
+    { 0xf8,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,6 },
+    { 0xfc,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,7 },
+    { 0xfe,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,9 },
+    { 0xfe,0x80,0,0,0,0,0,0,0,0,0,0,0,0,0,0,10 },
+    { 0xfe,0xc0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,10 },
+  };
+
+  // Code altered from IPNumberMatchesPrefix; we don't call it directly to avoid
+  // unnecessary parsing and conversions.
+  size_t arr_size = (ipv4) ?

[Ryan Sleevi, 2013/08/08 19:49:31]

So, my main concern with the ParseCIDR duplication.

It definitely seems like IPNumberMatchesPrefix and this should share the same
code. You're already controlling to ensure that |host_addr| will always match v4
or v6.

Minimally, it seems useful to move the match functionality into common code.
This further eliminates the need for constant ipv4 / ipv6 switching.

[felt, 2013/08/08 19:52:47]

I can do this, but I'll have to convert all of the const arrays into vectors,
which is essentially what the ParseCIDR code does (so then we'd might as well go
back to the older version).

[felt, 2013/08/09 04:53:20]

Done.

+                    arraysize(reserved_ipv4) : arraysize(reserved_ipv6);
+  for (size_t i = 0; i < arr_size; ++i) {
+    bool match = true;
+    int prefix;
+    if (ipv4)
+      prefix = reserved_ipv4[i][4];
+    else
+      prefix = reserved_ipv6[i][16];
+    // Compare all the bytes that fall entirely within the prefix.
+    int num_entire_bytes_in_prefix = prefix / 8;
+    for (int j = 0; j < num_entire_bytes_in_prefix; ++j) {
+      if (ipv4 && (host_addr[j] != reserved_ipv4[i][j])) {
+        match = false;
+      } else if (!ipv4 && (host_addr[j] != reserved_ipv6[i][j])) {
+        match = false;
+      }
+    }
+    // In case the prefix was not a multiple of 8, there will be 1 byte
+    // which is only partially masked.
+    int remaining_bits = prefix % 8;
+    if (remaining_bits != 0) {
+      unsigned char mask = 0xFF << (8 - remaining_bits);
+      int j = num_entire_bytes_in_prefix;
+      if (ipv4 && ((host_addr[j] & mask) != (reserved_ipv4[i][j] & mask))) {
+        match = false;
+      } else if (!ipv4 &&
+                 ((host_addr[j] & mask) != (reserved_ipv6[i][j] & mask))) {
+        match = false;
+      }
+    }
+    if (match)
+      return true;
+  }
+  return false;
+}
+
 // Extracts the address and port portions of a sockaddr.
 bool GetIPAddressFromSockAddr(const struct sockaddr* sock_addr,
                               socklen_t sock_addr_len,