Part 2.3: Is policy list subsumed under subsuming policy?

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

Index: third_party/WebKit/Source/core/frame/csp/SourceListDirective.cpp
diff --git a/third_party/WebKit/Source/core/frame/csp/SourceListDirective.cpp b/third_party/WebKit/Source/core/frame/csp/SourceListDirective.cpp
index 888a95f4d9d551d1ff86ab3386917b2d380d48a0..5af409b81c582e13e57b728067dd582192cc63c2 100644
--- a/third_party/WebKit/Source/core/frame/csp/SourceListDirective.cpp
+++ b/third_party/WebKit/Source/core/frame/csp/SourceListDirective.cpp
@@ -558,6 +558,15 @@ void SourceListDirective::addSourceHash(
   m_hashAlgorithmsUsed |= algorithm;
 }
 
+void SourceListDirective::addSourceToMap(HashMap<String, CSPSource*>& hashMap,
+                                         CSPSource* source) {
+  hashMap.add(source->getScheme(), source);
+  if (source->getScheme() == "http")
+    hashMap.add("https", source);
+  else if (source->getScheme() == "ws")
+    hashMap.add("wss", source);
+}
+
 bool SourceListDirective::hasSourceMatchInList(
     const KURL& url,
     ResourceRequest::RedirectStatus redirectStatus) const {
@@ -583,17 +592,69 @@ bool SourceListDirective::subsumes(
   return CSPSource::firstSubsumesSecond(m_list, normalizedA);
 }
 
+HashMap<String, CSPSource*> SourceListDirective::getIntersectSchemesOnly(
+    HeapVector<Member<CSPSource>> other) {
+  HashMap<String, CSPSource*> schemesA;
+  for (const auto& sourceA : m_list) {
+    if (sourceA->isSchemeOnly())
+      addSourceToMap(schemesA, sourceA);
+  }
+  // Add schemes only sources if they are present in both `this` and `other`,
+  // allowing upgrading `http` to `https` and `ws` to `wss`.
+  HashMap<String, CSPSource*> intersect;
+  for (const auto& sourceB : other) {
+    if (sourceB->isSchemeOnly()) {
+      if (schemesA.contains(sourceB->getScheme()))
+        addSourceToMap(intersect, sourceB);
+      else if (sourceB->getScheme() == "http" && schemesA.contains("https"))
+        intersect.add("https", schemesA.get("https"));
+      else if (sourceB->getScheme() == "ws" && schemesA.contains("wss"))
+        intersect.add("wss", schemesA.get("wss"));
+    }
+  }
+
+  return intersect;
+}
+
 HeapVector<Member<CSPSource>> SourceListDirective::getIntersectCSPSources(
-    HeapVector<Member<CSPSource>> otherVector) {
+    HeapVector<Member<CSPSource>> other) {
+  HashMap<String, CSPSource*> schemesMap = getIntersectSchemesOnly(other);
   HeapVector<Member<CSPSource>> normalized;
-  for (const auto& aCspSource : m_list) {
-    Member<CSPSource> matchedCspSource(nullptr);
-    for (const auto& bCspSource : otherVector) {
-      if ((matchedCspSource = bCspSource->intersect(aCspSource)))
+  // Add all normalized scheme source expressions.
+  for (auto it = schemesMap.begin(); it != schemesMap.end(); ++it) {

[Mike West, 2016/11/24 14:00:54]

`auto&`? Or do you need a copy? (Can this not be `const`?)

[amalika, 2016/11/24 14:50:33]

It cannot be `const` since we are incrementing the iterator on each iteration.
I just tried `auto&` and got
`
error: non-const lvalue reference to type 'HashTableIteratorAdapter<...>' cannot
bind to a temporary of type 'HashTableIteratorAdapter<...>'
`

+    // We do not add secure versions if insecure schemes are present.
+    if ((it->key != "https" || !schemesMap.contains("http")) &&
+        (it->key != "wss" || !schemesMap.contains("ws"))) {
+      normalized.append(it->value);
+    }
+  }
+
+  for (const auto& sourceA : m_list) {
+    if (schemesMap.contains(sourceA->getScheme()))
+      continue;
+
+    CSPSource* match(nullptr);
+    for (const auto& sourceB : other) {
+      // No need to add a host source expression if it is subsumed by the
+      // matching scheme source expression.
+      if (schemesMap.contains(sourceB->getScheme()))
+        continue;
+      // If sourceA is scheme only but there was no intersection for it in the
+      // `other` list, we add all the sourceB with that scheme.
+      if (sourceA->isSchemeOnly()) {
+        if (CSPSource* localMatch = sourceB->intersect(sourceA))
+          normalized.append(localMatch);
+        continue;
+      }
+      if (sourceB->subsumes(sourceA)) {
+        match = sourceA;
         break;
+      }
+      if (CSPSource* localMatch = sourceB->intersect(sourceA))
+        match = localMatch;
     }
-    if (matchedCspSource)
-      normalized.append(matchedCspSource);
+    if (match)
+      normalized.append(match);
   }
   return normalized;
 }