Change the port argument of SecurityOrigin::Create() methods to be ui…

…nt16_t

The automatic fallback to a unique origin in SecurityOrigin::Create() is
error prone. A unique origin should be created explicitly. In the first
place, it never gets such invalid values. To clarify that, change the
type of the port argument to uint16_t.

In SchemeHostPort, add DCHECKs to clarify that the results of
GURL::EffectiveIntPort() can never be outside of the uint16_t range.

One of the users of the methods, the CreateFromTupleWithSuborigin()
method is no longer used by anyone. So, just remove it.

Bug: 788289
Change-Id: Ib0abd718d5553f6b3b06388c4f5c684033202500
Reviewed-on: https://chromium-review.googlesource.com/796172
Commit-Queue: Takeshi Yoshino <tyoshino@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523742}

third_party/WebKit/Source/platform/exported/WebSecurityOrigin.cpp

@@ -45,15 +45,6 @@ WebSecurityOrigin WebSecurityOrigin::Create(const WebURL& url) {
   return WebSecurityOrigin(SecurityOrigin::Create(url));
 }
 
-WebSecurityOrigin WebSecurityOrigin::CreateFromTupleWithSuborigin(
-    const WebString& protocol,
-    const WebString& host,
-    int port,
-    const WebString& suborigin) {
-  return WebSecurityOrigin(
-      SecurityOrigin::Create(protocol, host, port, suborigin));
-}
-
 WebSecurityOrigin WebSecurityOrigin::CreateUnique() {
   return WebSecurityOrigin(SecurityOrigin::CreateUnique());
 }

third_party/WebKit/Source/platform/weborigin/SecurityOrigin.cpp

@@ -28,7 +28,9 @@
 
 #include "platform/weborigin/SecurityOrigin.h"
 
+#include <stdint.h>
 #include <memory>
+
 #include "net/base/url_util.h"
 #include "platform/runtime_enabled_features.h"
 #include "platform/weborigin/KURL.h"
@@ -48,8 +50,7 @@
 
 namespace blink {
 
-const int kInvalidPort = 0;
-const int kMaxAllowedPort = 65535;
+const uint16_t kInvalidPort = 0;
 
 static URLSecurityOriginMap* g_url_origin_map = nullptr;
 
@@ -535,10 +536,7 @@ scoped_refptr<SecurityOrigin> SecurityOrigin::CreateFromString(
 
 scoped_refptr<SecurityOrigin> SecurityOrigin::Create(const String& protocol,
                                                      const String& host,
-                                                     int port) {
-  if (port < 0 || port > kMaxAllowedPort)
-    return CreateUnique();
-
+                                                     uint16_t port) {
   DCHECK_EQ(host, DecodeURLEscapeSequences(host));
 
   String port_part = port ? ":" + String::Number(port) : String();
@@ -547,7 +545,7 @@ scoped_refptr<SecurityOrigin> SecurityOrigin::Create(const String& protocol,
 
 scoped_refptr<SecurityOrigin> SecurityOrigin::Create(const String& protocol,
                                                      const String& host,
-                                                     int port,
+                                                     uint16_t port,
                                                      const String& suborigin) {
   scoped_refptr<SecurityOrigin> origin = Create(protocol, host, port);
   if (!suborigin.IsEmpty())

third_party/WebKit/Source/platform/weborigin/SecurityOrigin.h

@@ -29,7 +29,9 @@
 #ifndef SecurityOrigin_h
 #define SecurityOrigin_h
 
+#include <stdint.h>
 #include <memory>
+
 #include "base/gtest_prod_util.h"
 #include "platform/PlatformExport.h"
 #include "platform/weborigin/Suborigin.h"
@@ -53,10 +55,10 @@ class PLATFORM_EXPORT SecurityOrigin : public RefCounted<SecurityOrigin> {
   static scoped_refptr<SecurityOrigin> CreateFromString(const String&);
   static scoped_refptr<SecurityOrigin> Create(const String& protocol,
                                               const String& host,
-                                              int port);
+                                              uint16_t port);
   static scoped_refptr<SecurityOrigin> Create(const String& protocol,
                                               const String& host,
-                                              int port,
+                                              uint16_t port,
                                               const String& suborigin);
   static scoped_refptr<SecurityOrigin> CreateFromUrlOrigin(const url::Origin&);
   url::Origin ToUrlOrigin() const;
@@ -92,10 +94,10 @@ class PLATFORM_EXPORT SecurityOrigin : public RefCounted<SecurityOrigin> {
 
   // Returns 0 if the effective port of this origin is the default for its
   // scheme.
-  unsigned short Port() const { return port_; }
+  uint16_t Port() const { return port_; }
   // Returns the effective port, even if it is the default port for the
   // scheme (e.g. "http" => 80).
-  unsigned short EffectivePort() const { return effective_port_; }
+  uint16_t EffectivePort() const { return effective_port_; }
 
   // Returns true if a given URL is secure, based either directly on its
   // own protocol, or, when relevant, on the protocol of its "inner URL"
@@ -313,8 +315,8 @@ class PLATFORM_EXPORT SecurityOrigin : public RefCounted<SecurityOrigin> {
   String host_;
   String domain_;
   Suborigin suborigin_;
-  unsigned short port_;
-  unsigned short effective_port_;
+  uint16_t port_;
+  uint16_t effective_port_;
   const bool is_unique_;
   bool universal_access_;
   bool domain_was_set_in_dom_;

third_party/WebKit/Source/platform/weborigin/SecurityOriginTest.cpp

@@ -30,6 +30,8 @@
 
 #include "platform/weborigin/SecurityOrigin.h"
 
+#include <stdint.h>
+
 #include "platform/blob/BlobURL.h"
 #include "platform/testing/RuntimeEnabledFeaturesTestHelpers.h"
 #include "platform/weborigin/KURL.h"
@@ -43,7 +45,7 @@
 
 namespace blink {
 
-const int kMaxAllowedPort = 65535;
+const uint16_t kMaxAllowedPort = UINT16_MAX;
 
 class SecurityOriginTest : public ::testing::Test {
  public:
@@ -53,19 +55,8 @@ class SecurityOriginTest : public ::testing::Test {
   }
 };
 
-TEST_F(SecurityOriginTest, InvalidPortsCreateUniqueOrigins) {
-  int ports[] = {-100, -1, kMaxAllowedPort + 1, 1000000};
-
-  for (size_t i = 0; i < WTF_ARRAY_LENGTH(ports); ++i) {
-    scoped_refptr<const SecurityOrigin> origin =
-        SecurityOrigin::Create("http", "example.com", ports[i]);
-    EXPECT_TRUE(origin->IsUnique())
-        << "Port " << ports[i] << " should have generated a unique origin.";
-  }
-}
-
 TEST_F(SecurityOriginTest, ValidPortsCreateNonUniqueOrigins) {
-  int ports[] = {0, 80, 443, 5000, kMaxAllowedPort};
+  uint16_t ports[] = {0, 80, 443, 5000, kMaxAllowedPort};
 
   for (size_t i = 0; i < WTF_ARRAY_LENGTH(ports); ++i) {
     scoped_refptr<const SecurityOrigin> origin =
@@ -436,7 +427,7 @@ TEST_F(SecurityOriginTest, CreateFromTuple) {
   struct TestCase {
     const char* scheme;
     const char* host;
-    unsigned short port;
+    uint16_t port;
     const char* origin;
   } cases[] = {
       {"http", "example.com", 80, "http://example.com"},
@@ -561,7 +552,7 @@ TEST_F(SecurityOriginTest, UrlOriginConversions) {
     const char* const url;
     const char* const scheme;
     const char* const host;
-    unsigned short port;
+    uint16_t port;
   } cases[] = {
       // IP Addresses
       {"http://192.168.9.1/", "http", "192.168.9.1", 80},

third_party/WebKit/public/platform/WebSecurityOrigin.h

@@ -124,14 +124,6 @@ class WebSecurityOrigin {
 #endif
 
  private:
-  // Present only to facilitate conversion from 'url::Origin'; this constructor
-  // shouldn't be used anywhere else.
-  BLINK_PLATFORM_EXPORT static WebSecurityOrigin CreateFromTupleWithSuborigin(
-      const WebString& protocol,
-      const WebString& host,
-      int port,
-      const WebString& suborigin);
-
   WebPrivatePtr<const SecurityOrigin> private_;
 };
 

url/scheme_host_port.cc

@@ -140,8 +140,12 @@ SchemeHostPort::SchemeHostPort(const GURL& url) : port_(0) {
 
   // A valid GURL never returns PORT_INVALID.
   int port = url.EffectiveIntPort();
-  if (port == PORT_UNSPECIFIED)
+  if (port == PORT_UNSPECIFIED) {
     port = 0;
+  } else {
+    DCHECK_GE(port, 0);
+    DCHECK_LE(port, 65535);
+  }
 
   if (!IsValidInput(scheme, host, port, ALREADY_CANONICALIZED))
     return;