Allow url::SchemeHostPort to hold non-file scheme without port

url/scheme_host_port.cc

 // Copyright 2015 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.
 
 #include "url/scheme_host_port.h"
 
 #include <string.h>
 
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
 #include "url/gurl.h"
 #include "url/url_canon.h"
 #include "url/url_canon_stdstring.h"
 #include "url/url_constants.h"
 #include "url/url_util.h"
 
 namespace url {
 
-SchemeHostPort::SchemeHostPort() : port_(0) {
-}
+namespace {
 
-SchemeHostPort::SchemeHostPort(base::StringPiece scheme,
-                               base::StringPiece host,
-                               uint16 port)
-    : scheme_(scheme.data(), scheme.length()),
-      host_(host.data(), host.length()),
-      port_(port) {
+bool IsCanonicalHost(const base::StringPiece& host) {
+  std::string canon_host;
+
   // Try to canonicalize the host (copy/pasted from net/base. :( ).
-  const url::Component raw_host_component(0, static_cast<int>(host.length()));
-  std::string canon_host;
-  url::StdStringCanonOutput canon_host_output(&canon_host);
-  url::CanonHostInfo host_info;
-  url::CanonicalizeHostVerbose(host.data(), raw_host_component,
+  const Component raw_host_component(0, static_cast<int>(host.length()));

[Ryan Sleevi, 2015/08/13 22:32:12]

Same DANGER remark applies below.

[tyoshino (SeeGerritForStatus), 2015/08/14 06:29:01]

Done.

+  StdStringCanonOutput canon_host_output(&canon_host);
+  CanonHostInfo host_info;
+  CanonicalizeHostVerbose(host.data(), raw_host_component,
                                &canon_host_output, &host_info);
 
   if (host_info.out_host.is_nonempty() &&
-      host_info.family != url::CanonHostInfo::BROKEN) {
+      host_info.family != CanonHostInfo::BROKEN) {
     // Success!  Assert that there's no extra garbage.
     canon_host_output.Complete();
     DCHECK_EQ(host_info.out_host.len, static_cast<int>(canon_host.length()));
   } else {
     // Empty host, or canonicalization failed.
     canon_host.clear();
   }
 
-  // Return an invalid SchemeHostPort object if any of the following conditions
-  // hold:
-  //
-  // 1. The provided scheme is non-standard, 'blob:', or 'filesystem:'.
-  // 2. The provided host is non-canonical.
-  // 3. The scheme is 'file' and the port is non-zero.
-  // 4. The scheme is not 'file', and the port is zero or the host is empty.
-  bool isUnsupportedScheme =
-      !url::IsStandard(scheme.data(),
-                       url::Component(0, static_cast<int>(scheme.length()))) ||
-      scheme == kFileSystemScheme || scheme == kBlobScheme;
-  bool isNoncanonicalHost = host != canon_host;
-  bool isFileSchemeWithPort = scheme == kFileScheme && port != 0;
-  bool isNonFileSchemeWithoutPortOrHost =
-      scheme != kFileScheme && (port == 0 || host.empty());
-  if (isUnsupportedScheme || isNoncanonicalHost || isFileSchemeWithPort ||
-      isNonFileSchemeWithoutPortOrHost) {
-    scheme_.clear();
-    host_.clear();
-    port_ = 0;
-  }
+  return host == canon_host;
 }
 
-SchemeHostPort::SchemeHostPort(const GURL& url) : port_(0) {
-  if (!url.is_valid() || !url.IsStandard())
-    return;
+bool IsValidInput(const base::StringPiece& scheme,
+                  const base::StringPiece& host,
+                  uint16 port) {
+  SchemeType scheme_type = SCHEME_WITH_PORT;
+  bool is_standard = GetStandardSchemeType(
+      scheme.data(),
+      Component(0, static_cast<int>(scheme.length())),

[Ryan Sleevi, 2015/08/13 22:32:12]

DANGER: I missed this in the original review, but because |scheme| might have
come from IPC (thus hostile input), it's possible that scheme.length() >
std::numeric_limits<int>::max(), thus creating an invalid component.

It may make more sense to do base::checked_cast<int>(scheme.length()), which
will still crash the browser process on an invalid IPC, but would be strictly
safer.

(Or we could rely on the fact that the IPC filter will have filtered these out
as being messages that are too large, but the nature of this constructor, and
the inherent danger, makes me think we're better off with checked_cast<>'s)

[tyoshino (SeeGerritForStatus), 2015/08/14 06:29:01]

Agreed. Changed to checked_cast.

+      &scheme_type);
+  if (!is_standard)
+    return false;
 
   // These schemes do not follow the generic URL syntax, so we treat them as
   // invalid (scheme, host, port) tuples (even though such URLs' _Origin_ might
   // have a (scheme, host, port) tuple, they themselves do not).
-  if (url.SchemeIsBlob() || url.SchemeIsFileSystem())
+  if (scheme == kFileSystemScheme || scheme == kBlobScheme)
+    return false;
+
+  switch (scheme_type) {
+    case SCHEME_WITH_PORT:
+      // A URL with |scheme| is required to have the host and port (may be
+      // omitted in a serialization if it's the same as the default value).
+      // Return an invalid instance if either of them is not given.
+      if (host.empty() || port == 0)
+        return false;
+
+      if (!IsCanonicalHost(host))
+        return false;
+
+      return true;
+
+    case SCHEME_WITHOUT_PORT:
+      if (port != 0) {
+        // Return an invalid object if the scheme is 'file' and the port is

[Ryan Sleevi, 2015/08/13 22:32:12]

This comment seems out of date/incorrect; that is, it doesn't just apply to
'file' schemed objects.

[tyoshino (SeeGerritForStatus), 2015/08/14 06:29:01]

Updated

+        // non-zero.
+        return false;
+      }
+
+      if (!IsCanonicalHost(host))
+        return false;
+
+      return true;
+
+    case SCHEME_WITHOUT_AUTHORITY:
+      return false;
+
+    default:
+      NOTREACHED();
+      return false;
+  }
+}
+
+}  // namespace
+
+SchemeHostPort::SchemeHostPort() : port_(0) {
+}
+
+SchemeHostPort::SchemeHostPort(base::StringPiece scheme,
+                               base::StringPiece host,
+                               uint16 port)
+    : port_(0) {
+  if (!IsValidInput(scheme, host, port))
     return;
 
-  scheme_ = url.scheme();
-  host_ = url.host();
-  port_ = url.EffectiveIntPort() == url::PORT_UNSPECIFIED
-              ? 0
-              : url.EffectiveIntPort();
+  scheme.CopyToString(&scheme_);
+  host.CopyToString(&host_);
+  port_ = port;
+}
+
+SchemeHostPort::SchemeHostPort(const GURL& url) : port_(0) {
+  if (!url.is_valid())
+    return;
+
+  const std::string& scheme = url.scheme();
+  const std::string& host = url.host();
+
+  // A valid GURL never returns PORT_INVALID.
+  int port = url.EffectiveIntPort();
+  if (port == PORT_UNSPECIFIED)
+    port = 0;
+
+  if (!IsValidInput(scheme, host, port))
+    return;
+
+  scheme_ = scheme;
+  host_ = host;
+  port_ = port;
 }
 
 SchemeHostPort::~SchemeHostPort() {
 }
 
 bool SchemeHostPort::IsInvalid() const {
   return scheme_.empty() && host_.empty() && !port_;
 }
 
 std::string SchemeHostPort::Serialize() const {
   std::string result;
   if (IsInvalid())
     return result;
 
-  bool is_default_port =
-      port_ == url::DefaultPortForScheme(scheme_.data(),
-                                         static_cast<int>(scheme_.length()));
-
   result.append(scheme_);
   result.append(kStandardSchemeSeparator);
   result.append(host_);
 
-  if (scheme_ != kFileScheme && !is_default_port) {
+  if (port_ == 0)
+    return result;
+
+  // Omit the port component if the port matches with the default port
+  // defined for the scheme, if any.
+  int default_port = DefaultPortForScheme(scheme_.data(),
+                                          static_cast<int>(scheme_.length()));
+  if (default_port == PORT_UNSPECIFIED)
+    return result;
+  if (port_ != default_port) {
     result.push_back(':');
     result.append(base::IntToString(port_));
   }
 
   return result;
 }
 
 bool SchemeHostPort::Equals(const SchemeHostPort& other) const {
   return port_ == other.port() && scheme_ == other.scheme() &&
          host_ == other.host();
 }
 
 bool SchemeHostPort::operator<(const SchemeHostPort& other) const {
   if (port_ != other.port_)
     return port_ < other.port_;
   if (scheme_ != other.scheme_)
     return scheme_ < other.scheme_;
   if (host_ != other.host_)
     return host_ < other.host_;
   return false;
 }
 
 }  // namespace url