Wire 'blink::Platform::isHostnameReservedIPAddress()' to 'net::IsReservedIPAddress()'.

content/child/blink_platform_impl.cc

Index: content/child/blink_platform_impl.cc
diff --git a/content/child/blink_platform_impl.cc b/content/child/blink_platform_impl.cc
index ca4a2400c60eb79c371b66a080785b73e42683c9..17f52a8b27dba47fe1e4522c4fd1e97ddfd5ac1b 100644
--- a/content/child/blink_platform_impl.cc
+++ b/content/child/blink_platform_impl.cc
@@ -42,6 +42,7 @@
 #include "net/base/data_url.h"
 #include "net/base/mime_util.h"
 #include "net/base/net_errors.h"
+#include "net/base/net_util.h"
 #include "third_party/WebKit/public/platform/WebConvertableToTraceFormat.h"
 #include "third_party/WebKit/public/platform/WebData.h"
 #include "third_party/WebKit/public/platform/WebString.h"
@@ -439,6 +440,41 @@ WebURLError BlinkPlatformImpl::cancelledError(
   return WebURLLoaderImpl::CreateError(unreachableURL, false, net::ERR_ABORTED);
 }
 
+bool BlinkPlatformImpl::isHostnameReservedIPAddress(
+    const blink::WebString& hostname) const {
+  // CanonicalizeHost requires surrounding brackets to parse an IPv6 address:
+  // these are provided by 'blink::SecurityOrigin::host()', so we don't have to
+  // add them here.
+  std::string host = hostname.utf8();
+  url::CanonHostInfo host_info;
+  std::string canonical_name = net::CanonicalizeHost(host, &host_info);

[Ryan Sleevi, 2014/08/05 00:01:38]

Is there no way to pre-canonicalize? This seems a relatively expensive thing to
be calling regularly, especially if Blink has an already-canonicalized form.

[Mike West, 2014/08/05 07:20:29]

That's fair. We're getting the hostname from either a SecurityOrigin or a URL,
so I think we can safely assume that the hostname is already canonical.

+
+  // If it's not an IP address or canonicalization fails, it's not a reserved
+  // IP address.
+  if (canonical_name.empty() || !host_info.IsIPAddress())
+    return false;
+
+  host = host.substr(host_info.out_host.begin, host_info.out_host.len);

[Ryan Sleevi, 2014/08/05 00:01:38]

BUG: host_info.out_host is "location of the host within the canoncalized output"
-
https://code.google.com/p/chromium/codesearch#chromium/src/url/url_canon.h&rc...
- but you're using the non-canonical input.

Use canonical_name.substr()

[Mike West, 2014/08/05 07:20:29]

I can drop this entirely.

+  if (host_info.family == url::CanonHostInfo::IPV6) {
+    // ParseIPLiteralToNumber requires that IPv6 addresses _not_ have brackets,
+    // so we strip them here. Hooray for consistency.

[Ryan Sleevi, 2014/08/05 00:01:37]

URLs vs not-URLs ;)

[Mike West, 2014/08/05 07:20:29]

Acknowledged.

+    host = host.substr(1, host.size() - 2);
+  }
+
+  net::IPAddressNumber host_addr;
+  if (!net::ParseIPLiteralToNumber(host, &host_addr))

[Ryan Sleevi, 2014/08/05 00:01:37]

If you do find it unavoidable to call this every time, we should update this api
to base::StringPiece() instead of requiring multiple copies. Then, your
.substr() calls (line 461, 457) are not so expensive, even if you can
(generally) rely on the small string optimization.

[Mike West, 2014/08/05 07:20:29]

What do you think of the current patchset, which drops the substr() calls
entirely, given what we know about the source?

+    return false;
+
+  switch (host_info.family) {
+    case url::CanonHostInfo::IPV4:
+    case url::CanonHostInfo::IPV6:
+      return net::IsIPAddressReserved(host_addr);
+    case url::CanonHostInfo::NEUTRAL:
+    case url::CanonHostInfo::BROKEN:
+      return false;
+  }
+}
+
 blink::WebThread* BlinkPlatformImpl::createThread(const char* name) {
   return new WebThreadImpl(name);
 }