Part 1.1: Is policy list subsumed under subsuming policy?

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

Index: third_party/WebKit/Source/core/frame/csp/CSPSource.cpp
diff --git a/third_party/WebKit/Source/core/frame/csp/CSPSource.cpp b/third_party/WebKit/Source/core/frame/csp/CSPSource.cpp
index 8f15383227bf8708b37b23f6073c0cd5387bf1cc..72073430887f74835c858c08d7fd65c3d6f7a9b4 100644
--- a/third_party/WebKit/Source/core/frame/csp/CSPSource.cpp
+++ b/third_party/WebKit/Source/core/frame/csp/CSPSource.cpp
@@ -42,17 +42,28 @@ bool CSPSource::matches(const KURL& url,
 bool CSPSource::schemeMatches(const KURL& url) const {
   if (m_scheme.isEmpty())
     return m_policy->protocolMatchesSelf(url);
-  if (equalIgnoringCase(m_scheme, "http"))
-    return equalIgnoringCase(url.protocol(), "http") ||
-           equalIgnoringCase(url.protocol(), "https");
-  if (equalIgnoringCase(m_scheme, "ws"))
-    return equalIgnoringCase(url.protocol(), "ws") ||
-           equalIgnoringCase(url.protocol(), "wss");
-  return equalIgnoringCase(url.protocol(), m_scheme);
+  return schemeMatches(url.protocol());
+}
+
+bool CSPSource::schemeMatches(const String& protocol) const {
+  // TODO(amalika): DCHECK schemes are lower case & remove calls to
+  // equalIgnoringCase
+  if (equalIgnoringCase(m_scheme, "http")) {
+    return equalIgnoringCase(protocol, "http") ||
+           equalIgnoringCase(protocol, "https");
+  }
+  if (equalIgnoringCase(m_scheme, "ws")) {
+    return equalIgnoringCase(protocol, "ws") ||
+           equalIgnoringCase(protocol, "wss");
+  }
+  return equalIgnoringCase(protocol, m_scheme);
 }
 
 bool CSPSource::hostMatches(const KURL& url) const {
-  const String& host = url.host();
+  return hostMatches(url.host());
+}
+
+bool CSPSource::hostMatches(const String& host) const {
   Document* document = m_policy->document();
   bool match;
 
@@ -75,10 +86,14 @@ bool CSPSource::hostMatches(const KURL& url) const {
 }
 
 bool CSPSource::pathMatches(const KURL& url) const {
+  return pathMatches(url.path());
+}
+
+bool CSPSource::pathMatches(const String& urlPath) const {
   if (m_path.isEmpty())
     return true;
 
-  String path = decodeURLEscapeSequences(url.path());
+  String path = decodeURLEscapeSequences(urlPath);
 
   if (m_path.endsWith("/"))
     return path.startsWith(m_path);
@@ -87,28 +102,130 @@ bool CSPSource::pathMatches(const KURL& url) const {
 }
 
 bool CSPSource::portMatches(const KURL& url) const {
+  return portMatches(url.port(), url.protocol());
+}
+
+bool CSPSource::portMatches(int port, const String& protocol) const {
   if (m_portWildcard == HasWildcard)
     return true;
 
-  int port = url.port();
-
   if (port == m_port)
     return true;
 
   if (m_port == 80 &&
-      (port == 443 ||
-       (port == 0 && defaultPortForProtocol(url.protocol()) == 443)))
+      (port == 443 || (port == 0 && defaultPortForProtocol(protocol) == 443)))
     return true;
 
   if (!port)
-    return isDefaultPortForProtocol(m_port, url.protocol());
+    return isDefaultPortForProtocol(m_port, protocol);
 
   if (!m_port)
-    return isDefaultPortForProtocol(port, url.protocol());
+    return isDefaultPortForProtocol(port, protocol);
+
+  return false;
+}
+
+bool CSPSource::isSimilar(CSPSource* other) {
+  bool schemesMatch = schemeMatches(other->m_scheme) ||
+                      (equalIgnoringCase(m_scheme, "https") &&

[Mike West, 2016/10/26 11:40:29]

Nit: DCHECK_EQ(m_scheme, m_scheme.lower()), and replace `equalIgnoringCase` with
`==`.

+                       equalIgnoringCase(other->m_scheme, "http")) ||
+                      (equalIgnoringCase(m_scheme, "wss") &&
+                       equalIgnoringCase(other->m_scheme, "ws"));
+  bool schemesOnly = isSchemeOnly() || other->isSchemeOnly();
+  bool hostsMatch =
+      schemesOnly || equalIgnoringCase(m_host, other->m_host) ||
+      (m_hostWildcard == HasWildcard && hostMatches(other->m_host)) ||
+      (other->m_hostWildcard == HasWildcard && other->hostMatches(m_host));

[Mike West, 2016/10/26 11:40:29]

Isn't the wildcard work done in `hostMatches()`? Can you just use that?

[amalika, 2016/10/27 09:50:47]

Unfortunately, this does not work when both hosts have wildcards. If both do,
then we need to check that and still compare if the hosts are equal ( host ==
m_host ). We can do that because both will also contain stars so they have to
perfectly match anyways. 

But I simplified it still adding this additional case!

+  bool portsMatch = schemesOnly || m_portWildcard == HasWildcard ||
+                    other->m_portWildcard == HasWildcard ||
+                    portMatches(other->m_port, other->m_scheme);

[Mike West, 2016/10/26 11:40:29]

Isn't the wildcard work done in `portMatches()`?

[amalika, 2016/10/27 09:50:48]

It does not work for when there are wildcards in both ports considered. To
handle this case, it is sufficient to just check whether one of them has a
wildcard (in which case it would match anything) or if it doesnt have a
wildcard, then we can just use already existing methods.

+  bool pathsMatch = other->m_path.isEmpty() || other->m_path == "/" ||

[Mike West, 2016/10/26 11:40:29]

Nit: This matches if `schemesOnly` too, right? If so, you can return right after
calculating `schemesOnly` and `schemesMatch`, and simplify the remaining
comparisons.

+                    pathMatches(other->m_path);
+  if (schemesMatch && hostsMatch && portsMatch && pathsMatch)
+    return true;
 
   return false;
 }
 
+CSPSource* CSPSource::getCommon(CSPSource* other) {
+  if (!isSimilar(other))
+    return nullptr;
+
+  bool preferABasedOnScheme = false;
+  // If the schemes match but their lengths are not equal, that means one of the
+  // schemes is 'https' or 'wss'
+  if (m_scheme.length() != other->m_scheme.length()) {
+    String aScheme = m_scheme.lower();
+    preferABasedOnScheme =
+        aScheme.length() > 3 ? (aScheme == "https") : (aScheme == "wss");
+  }
+
+  bool preferABasedOnPort = (m_port && !other->m_port);
+  bool preferABasedOnPath = (!(m_path.isEmpty() || m_path == "/") &&
+                             (other->m_path.isEmpty() || other->m_path == "/"));
+
+  String scheme = preferABasedOnScheme ? m_scheme : other->m_scheme;
+  String host = (m_hostWildcard == HasWildcard) ? other->m_host : m_host;
+  String path = preferABasedOnPath ? m_path : other->m_path;
+  int port = preferABasedOnPort ? m_port : other->m_port;
+  WildcardDisposition hostWildcard =
+      (m_hostWildcard == HasWildcard) ? other->m_hostWildcard : m_hostWildcard;
+  WildcardDisposition portWildcard =
+      (m_portWildcard == HasWildcard) ? other->m_portWildcard : m_portWildcard;
+  return new CSPSource(m_policy, scheme, host, port, path, hostWildcard,
+                       portWildcard);
+}
+
+bool CSPSource::isSubsumed(CSPSource* other) {
+  if (!isSimilar(other))
+    return false;
+
+  if (other->isSchemeOnly()) {
+    if (other->m_scheme.length() == m_scheme.length())
+      return true;
+    return m_scheme.length() == 3 || m_scheme.length() == 5 ? true : false;
+  }
+
+  if (isSchemeOnly())
+    return false;
+
+  if ((m_hostWildcard == HasWildcard && other->m_hostWildcard == NoWildcard) ||
+      (m_portWildcard == HasWildcard && other->m_portWildcard == NoWildcard)) {
+    return false;
+  }
+
+  bool preferABasedOnScheme = false, preferBBasedOnScheme = false;
+  // If the schemes match but their lengths are not equal, that means one of the
+  // schemes is 'https' or 'wss'
+  if (m_scheme.length() != other->m_scheme.length()) {
+    String aScheme = m_scheme.lower();
+    preferABasedOnScheme =
+        aScheme.length() > 3 ? (aScheme == "https") : (aScheme == "wss");
+    preferBBasedOnScheme = !preferABasedOnScheme;
+  }
+
+  bool preferABasedOnPort = (m_port && !other->m_port),
+       preferBBasedOnPort = (!m_port && other->m_port);
+  bool preferABasedOnPath = (!(m_path.isEmpty() || m_path == "/") &&
+                             (other->m_path.isEmpty() || other->m_path == "/")),
+       preferBBasedOnPath =
+           ((m_path.isEmpty() || m_path == "/") &&
+            !(other->m_path.isEmpty() || other->m_path == "/"));
+
+  bool preferA =
+      preferABasedOnPort || preferABasedOnPath || preferABasedOnScheme;
+  bool preferB =
+      preferBBasedOnPort || preferBBasedOnPath || preferBBasedOnScheme;
+
+  if (preferA & preferB)
+    return false;
+
+  if (preferA || preferB)
+    return preferA ? true : false;
+
+  return true;
+}
+
 bool CSPSource::isSchemeOnly() const {
   return m_host.isEmpty();
 }