Part 3.1: Is policy list subsumed under subsuming policy?

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

Index: third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp
diff --git a/third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp b/third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp
index 4bdfefc6cc7a7147cdca93bbf6659dc25093bb0f..38ee9d234c9a903f1db903d8ef49bf22bb1d6421 100644
--- a/third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp
+++ b/third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp
@@ -1142,6 +1142,148 @@ void CSPDirectiveList::addDirective(const String& name, const String& value) {
   }
 }
 
+SourceListDirectiveVector CSPDirectiveList::getSourceList(
+    const char* name,
+    CSPDirectiveListVector policies) {
+  SourceListDirectiveVector sourceListDirectives;
+  if (name == ContentSecurityPolicy::ScriptSrc) {

[Mike West, 2016/11/14 14:21:23]

Perhaps you could put the loop on the outside of all these `if`s? That is:

```
for (const auto& policy : policies) {
  if (name == ContentSecurityPolicy::FrameSrc) {

  } else if (...) {

  }
}
```

[amalika, 2016/11/15 13:17:18]

I thought it would be better to first get into the right branch based on the
name and then knowing that iterate over policies because otherwise, it would be
less efficient to check name on each iteration.

But let me know if it would be better to put the loop outside for readability
purposes?

[Mike West, 2016/11/17 11:25:15]

We're expecting sites to have some small number of policies; I don't think the
additional work would amount to much.
It seems to me to be easier to read a single `for` loop wrapping a big `if` than
to read a big `if` wrapping many `for` loops. I won't insist on it if you prefer
the current structure, but I think I'd prefer that way of doing things.

+    for (const auto& policy : policies) {
+      if (policy->m_scriptSrc.get()) {
+        sourceListDirectives.append(policy->m_scriptSrc);
+      } else if (policy->m_defaultSrc.get()) {
+        sourceListDirectives.append(policy->m_defaultSrc);
+      }

[Mike West, 2016/11/14 14:21:23]

This could be compressed to something like:

```
if (SourceListDirective* directive = operativeDirective(m_scriptSrc.get()))
  sourceListDirectives.append(*directive);
```

+    }
+  } else if (name == ContentSecurityPolicy::ObjectSrc) {
+    for (const auto& policy : policies) {
+      if (policy->m_objectSrc.get()) {
+        sourceListDirectives.append(policy->m_objectSrc);
+      } else if (policy->m_defaultSrc.get()) {
+        sourceListDirectives.append(policy->m_defaultSrc);
+      }
+    }
+  } else if (name == ContentSecurityPolicy::FrameSrc) {
+    for (const auto& policy : policies) {
+      if (policy->m_frameSrc.get()) {
+        sourceListDirectives.append(policy->m_frameSrc);
+      } else if (policy->m_childSrc.get()) {
+        sourceListDirectives.append(policy->m_childSrc);
+      } else if (policy->m_defaultSrc.get()) {
+        sourceListDirectives.append(policy->m_defaultSrc);
+      }
+    }
+  } else if (name == ContentSecurityPolicy::ImgSrc) {
+    for (const auto& policy : policies) {
+      if (policy->m_imgSrc.get()) {
+        sourceListDirectives.append(policy->m_imgSrc);
+      } else if (policy->m_defaultSrc.get()) {
+        sourceListDirectives.append(policy->m_defaultSrc);
+      }
+    }
+  } else if (name == ContentSecurityPolicy::StyleSrc) {
+    for (const auto& policy : policies) {
+      if (policy->m_styleSrc.get()) {
+        sourceListDirectives.append(policy->m_styleSrc);
+      } else if (policy->m_defaultSrc.get()) {
+        sourceListDirectives.append(policy->m_defaultSrc);
+      }
+    }
+  } else if (name == ContentSecurityPolicy::FontSrc) {
+    for (const auto& policy : policies) {
+      if (policy->m_fontSrc.get()) {
+        sourceListDirectives.append(policy->m_fontSrc);
+      } else if (policy->m_defaultSrc.get()) {
+        sourceListDirectives.append(policy->m_defaultSrc);
+      }
+    }
+  } else if (name == ContentSecurityPolicy::MediaSrc) {
+    for (const auto& policy : policies) {
+      if (policy->m_mediaSrc.get()) {
+        sourceListDirectives.append(policy->m_mediaSrc);
+      } else if (policy->m_defaultSrc.get()) {
+        sourceListDirectives.append(policy->m_defaultSrc);
+      }
+    }
+  } else if (name == ContentSecurityPolicy::ConnectSrc) {
+    for (const auto& policy : policies) {
+      if (policy->m_connectSrc.get()) {
+        sourceListDirectives.append(policy->m_connectSrc);
+      } else if (policy->m_defaultSrc.get()) {
+        sourceListDirectives.append(policy->m_defaultSrc);
+      }
+    }
+  } else if (name == ContentSecurityPolicy::ChildSrc) {
+    for (const auto& policy : policies) {
+      if (policy->m_childSrc.get()) {
+        sourceListDirectives.append(policy->m_childSrc);
+      } else if (policy->m_defaultSrc.get()) {
+        sourceListDirectives.append(policy->m_defaultSrc);
+      }
+    }
+  } else if (name == ContentSecurityPolicy::ManifestSrc) {
+    for (const auto& policy : policies) {
+      if (policy->m_manifestSrc.get()) {
+        sourceListDirectives.append(policy->m_manifestSrc);
+      } else if (policy->m_defaultSrc.get()) {
+        sourceListDirectives.append(policy->m_defaultSrc);
+      }
+    }
+  } else if (name == ContentSecurityPolicy::FrameAncestors) {
+    for (const auto& policy : policies) {
+      if (policy->m_frameAncestors.get()) {
+        sourceListDirectives.append(policy->m_frameAncestors);
+      }
+    }
+  } else if (name == ContentSecurityPolicy::BaseURI) {
+    for (const auto& policy : policies) {
+      if (policy->m_baseURI.get()) {
+        sourceListDirectives.append(policy->m_baseURI);
+      }
+    }
+  } else if (name == ContentSecurityPolicy::FormAction) {
+    for (const auto& policy : policies) {
+      if (policy->m_formAction.get()) {
+        sourceListDirectives.append(policy->m_formAction);
+      }
+    }
+  }
+  return sourceListDirectives;
+}
+
+bool CSPDirectiveList::subsumes(CSPDirectiveListVector other) {
+  const char* directives[] = {
+      // Fetch Directives
+      ContentSecurityPolicy::ChildSrc, ContentSecurityPolicy::ConnectSrc,
+      ContentSecurityPolicy::FontSrc, ContentSecurityPolicy::FrameSrc,
+      ContentSecurityPolicy::ImgSrc, ContentSecurityPolicy::ManifestSrc,
+      ContentSecurityPolicy::MediaSrc, ContentSecurityPolicy::ObjectSrc,
+      ContentSecurityPolicy::ScriptSrc, ContentSecurityPolicy::StyleSrc,
+      // Document Directives
+      ContentSecurityPolicy::BaseURI,
+      // Navigation Directives
+      ContentSecurityPolicy::FrameAncestors, ContentSecurityPolicy::FormAction};
+
+  for (const auto& directive : directives) {
+    // There should only be one SourceListDirective for each dirctive in
+    // Embedding-CSP.
+    SourceListDirectiveVector requiredList =
+        getSourceList(directive, CSPDirectiveListVector(1, this));
+    if (requiredList.size() == 0)
+      continue;
+    SourceListDirective* required = requiredList[0];
+    // Aggregate all serialized source lists of the returned CSP into a vector
+    // based on a directive type, defaulting accordingly (for example, to
+    // `default-src`)
+    SourceListDirectiveVector returned = getSourceList(directive, other);
+    // TODO(amalika): Add checks for plugin-types, sandbox, disown-opener,
+    // navigation-to, worker-src.
+    if (!required->subsumes(returned))
+      return false;
+  }
+
+  return true;
+}
+
 DEFINE_TRACE(CSPDirectiveList) {
   visitor->trace(m_policy);
   visitor->trace(m_pluginTypes);