Stop CSP from matching independent scheme/port upgrades

third_party/WebKit/Source/core/frame/csp/CSPSourceTest.cpp

Index: third_party/WebKit/Source/core/frame/csp/CSPSourceTest.cpp
diff --git a/third_party/WebKit/Source/core/frame/csp/CSPSourceTest.cpp b/third_party/WebKit/Source/core/frame/csp/CSPSourceTest.cpp
index 36590e967f0e74f4dc1ee7c7612860e6909f09eb..95b45cd0f4492b4f3984693f50bc36ab6e34fbd9 100644
--- a/third_party/WebKit/Source/core/frame/csp/CSPSourceTest.cpp
+++ b/third_party/WebKit/Source/core/frame/csp/CSPSourceTest.cpp
@@ -105,10 +105,10 @@ TEST_F(CSPSourceTest, RedirectMatching) {
   EXPECT_TRUE(
       source.matches(KURL(base, "http://example.com:8000/foo"),
                      ResourceRequest::RedirectStatus::FollowedRedirect));
-  EXPECT_TRUE(
+  // Should not allow upgrade of port or scheme without upgrading both
+  EXPECT_FALSE(
       source.matches(KURL(base, "https://example.com:8000/foo"),
                      ResourceRequest::RedirectStatus::FollowedRedirect));
-
   EXPECT_FALSE(
       source.matches(KURL(base, "http://not-example.com:8000/foo"),
                      ResourceRequest::RedirectStatus::FollowedRedirect));
@@ -164,9 +164,7 @@ TEST_F(CSPSourceTest, SchemeIsEmpty) {
     EXPECT_FALSE(source.matches(KURL(base, "http://a.com")));
     EXPECT_TRUE(source.matches(KURL(base, "https://a.com")));
     EXPECT_FALSE(source.matches(KURL(base, "http-so://a.com")));
-    // TODO(mkwst, arthursonzogni): Maybe it should return true.
-    // See http://crbug.com/692442
-    EXPECT_FALSE(source.matches(KURL(base, "https-so://a.com")));
+    EXPECT_TRUE(source.matches(KURL(base, "https-so://a.com")));

[andypaicu, 2017/02/24 08:41:00]

Added 692442 since this will also fix that bug

[Mike West, 2017/02/24 10:56:28]

Hrm. I'm not actually sure this is a bug. :(

This is dumb, but please add something like `// TODO(jochen): Maybe it should
return false?` since he's now responsible for suborigins. Muwahaha.

[andypaicu, 2017/03/13 10:07:20]

I've added the comment and let Jochen know.

     EXPECT_FALSE(source.matches(KURL(base, "ftp://a.com")));
   }
 
@@ -205,13 +203,12 @@ TEST_F(CSPSourceTest, InsecureHostSchemePortMatchesSecurePort) {
                      CSPSource::NoWildcard, CSPSource::NoWildcard);
     EXPECT_TRUE(source.matches(KURL(base, "http://example.com/")));
     EXPECT_TRUE(source.matches(KURL(base, "http://example.com:80/")));
-    // TODO(mkwst, arthursonzogni): It is weird to upgrade the port without the
-    // sheme. See http://crbug.com/692499
-    EXPECT_TRUE(source.matches(KURL(base, "http://example.com:443/")));
+
+    // Should not allow scheme upgrades unless both port and scheme are upgraded
+    EXPECT_FALSE(source.matches(KURL(base, "http://example.com:443/")));
     EXPECT_TRUE(source.matches(KURL(base, "https://example.com/")));
-    // TODO(mkwst, arthursonzogni): It is weird to upgrade the scheme without
-    // the port. See http://crbug.com/692499
-    EXPECT_TRUE(source.matches(KURL(base, "https://example.com:80/")));
+    EXPECT_FALSE(source.matches(KURL(base, "https://example.com:80/")));
+
     EXPECT_TRUE(source.matches(KURL(base, "https://example.com:443/")));
 
     EXPECT_FALSE(source.matches(KURL(base, "http://example.com:8443/")));
@@ -233,9 +230,21 @@ TEST_F(CSPSourceTest, InsecureHostSchemePortMatchesSecurePort) {
                      CSPSource::NoWildcard, CSPSource::NoWildcard);
     EXPECT_TRUE(source.matches(KURL(base, "http://example.com/")));
     EXPECT_TRUE(source.matches(KURL(base, "https://example.com:443")));
-    // TODO(mkwst, arthursonzogni): It is weird to upgrade the port without the
-    // sheme. See http://crbug.com/692499
-    EXPECT_TRUE(source.matches(KURL(base, "http://example.com:443")));
+    // Should not allow upgrade of port or scheme without upgrading both
+    EXPECT_FALSE(source.matches(KURL(base, "http://example.com:443")));
+  }
+
+  // source port is empty
+  {
+    CSPSource source(csp.get(), "http", "example.com", 0, "/",
+                     CSPSource::NoWildcard, CSPSource::NoWildcard);
+
+    EXPECT_TRUE(source.matches(KURL(base, "http://example.com")));
+    EXPECT_TRUE(source.matches(KURL(base, "https://example.com")));
+    EXPECT_TRUE(source.matches(KURL(base, "https://example.com:443")));
+    // Should not allow upgrade of port or scheme without upgrading both
+    EXPECT_FALSE(source.matches(KURL(base, "https://example.com:80")));
+    EXPECT_FALSE(source.matches(KURL(base, "http://example.com:443")));
   }
 }