CSP Suborigins

Source/core/frame/csp/CSPDirectiveList.cpp

Index: Source/core/frame/csp/CSPDirectiveList.cpp
diff --git a/Source/core/frame/csp/CSPDirectiveList.cpp b/Source/core/frame/csp/CSPDirectiveList.cpp
index d4997139ea2c241433834f1b00cd53e88ee29a24..abd7389b13c3bcf5110d7f8c52d0e135e9be7605 100644
--- a/Source/core/frame/csp/CSPDirectiveList.cpp
+++ b/Source/core/frame/csp/CSPDirectiveList.cpp
@@ -21,6 +21,7 @@ CSPDirectiveList::CSPDirectiveList(ContentSecurityPolicy* policy, ContentSecurit
     , m_headerSource(source)
     , m_reportOnly(false)
     , m_haveSandboxPolicy(false)
+    , m_haveSuboriginPolicy(false)
     , m_reflectedXSSDisposition(ReflectedXSSUnset)
     , m_didSetReferrerPolicy(false)
     , m_referrerPolicy(ReferrerPolicyDefault)
@@ -540,6 +541,20 @@ void CSPDirectiveList::applySandboxPolicy(const String& name, const String& sand
         m_policy->reportInvalidSandboxFlags(invalidTokens);
 }
 
+void CSPDirectiveList::applySuboriginPolicy(const String& name, const String& suboriginPolicy)
+{
+    if (m_haveSuboriginPolicy) {
+        m_policy->reportDuplicateDirective(name);
+        return;
+    }

[Mike West, 2014/10/23 12:59:19]

abarth@ is right: we shouldn't land this with <meta> support. I tried to take
out <meta> support for sandbox this morning, and it's a pain because layout
tests rely on it.

We should do this right from the beginning: please add an `m_headerType` check
here.

[jww, 2015/03/20 22:50:03]

Done.

+    m_haveSuboriginPolicy = true;
+    String invalidTokens;

[Mike West, 2014/10/23 12:59:19]

Nit: Unused?

[jww, 2015/03/20 22:50:02]

Done.

+    String suboriginName = parseSuboriginName(suboriginPolicy);
+    if (!suboriginName.isNull()) {

[Mike West, 2014/10/23 12:59:19]

Again, isNull or isEmpty?

[jww, 2015/03/20 22:50:03]

See earlier comment (NULL means no suborigin; empty should not parse).

+        m_policy->enforceSuborigin(suboriginName);
+    }

[Mike West, 2014/10/23 12:59:19]

Nit: No {} for oneliners.

[jww, 2015/03/20 22:50:03]

Done.

+}
+
 void CSPDirectiveList::parseReflectedXSS(const String& name, const String& value)
 {
     if (m_reflectedXSSDisposition != ReflectedXSSUnset) {
@@ -641,6 +656,37 @@ void CSPDirectiveList::parseReferrer(const String& name, const String& value)
 
 }
 
+String CSPDirectiveList::parseSuboriginName(const String& policy)
+{
+    Vector<UChar> characters;
+    policy.appendTo(characters);
+
+    const UChar* position = characters.data();
+    const UChar* end = position + characters.size();
+
+    // Parse the name of the suborigin (no spaces, single string)
+    skipWhile<UChar, isASCIISpace>(position, end);
+    if (position == end) {
+        m_policy->reportInvalidSuboriginFlags("No suborigin name specified.");
+        return String();

[Mike West, 2014/10/23 12:59:19]

I think s/String()/emptyString()/ would be marginally more efficient.

[jww, 2015/03/20 22:50:03]

As per an earlier comment, we're distinguishing a null String as the case where
there is no suborigin from the empty string, which is an invalid suborigin, this
needs to be String() so we get a null (not empty) string.

+    }
+
+    const UChar* begin = position;
+
+    skipWhile<UChar, isASCIIAlphanumeric>(position, end);
+    if (position != end && !isASCIISpace(*position)) {
+        m_policy->reportInvalidSuboriginFlags("Invalid character \'" + String(position, 1) + "\' in suborigin.");
+        return String();
+    }
+    size_t length = position - begin;
+    skipWhile<UChar, isASCIISpace>(position, end);
+    if (position != end) {
+        m_policy->reportInvalidSuboriginFlags("Invalid whitespace in suborigin. Whitespace may only appear at the beginning and end of a suborigin, and it is ignored.");

[Mike West, 2014/10/23 12:59:19]

1. I think you can clarify this error message: "Whitespace isn't allowed in
suborigin names." perhaps?

2. You need to `return emptyString()` after the error, otherwise you're
returning the first portion of the block.

[jww, 2015/03/20 22:50:03]

Done.
Yikes! Done.

+    }
+
+    return String(begin, length);
+}
+
 void CSPDirectiveList::addDirective(const String& name, const String& value)
 {
     ASSERT(!name.isEmpty());
@@ -683,6 +729,8 @@ void CSPDirectiveList::addDirective(const String& name, const String& value)
         parseReflectedXSS(name, value);
     } else if (equalIgnoringCase(name, ContentSecurityPolicy::Referrer)) {
         parseReferrer(name, value);
+    } else if (equalIgnoringCase(name, ContentSecurityPolicy::Suborigin)) {

[Mike West, 2014/10/23 12:59:19]

This ought to be guarded by the experimental flag.

[jww, 2015/03/20 22:50:03]

Done.

+        applySuboriginPolicy(name, value);
     } else if (m_policy->experimentalFeaturesEnabled()) {
         if (equalIgnoringCase(name, ContentSecurityPolicy::ManifestSrc))
             setCSPDirective<SourceListDirective>(name, value, m_manifestSrc);