Refactor nonce support.

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

Index: third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp
diff --git a/third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp b/third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..b3a07fb6677a7e0694c0357db605b5681fb02091
--- /dev/null
+++ b/third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp
@@ -0,0 +1,156 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/frame/csp/CSPDirectiveList.h"
+
+#include "core/frame/csp/ContentSecurityPolicy.h"
+#include "core/frame/csp/SourceListDirective.h"
+#include "platform/network/ContentSecurityPolicyParsers.h"
+#include "platform/network/ResourceRequest.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace blink {
+
+class CSPDirectiveListTest : public ::testing::Test {
+public:
+    CSPDirectiveListTest()
+        : csp(ContentSecurityPolicy::create())
+    {
+    }
+
+    CSPDirectiveList* createList(const char* list, ContentSecurityPolicyHeaderType type)
+    {
+        Vector<UChar> characters;
+        String(list).appendTo(characters);
+        const UChar* begin = characters.data();
+        const UChar* end = begin + characters.size();
+
+        return CSPDirectiveList::create(csp, begin, end, type, ContentSecurityPolicyHeaderSourceHTTP);
+    }
+
+protected:
+    Persistent<ContentSecurityPolicy> csp;
+};
+
+TEST_F(CSPDirectiveListTest, IsMatchingNoncePresent)
+{
+    struct TestCase {
+        const char* list;
+        const char* nonce;
+        bool expected;
+    } cases[] = {
+        { "script-src 'self'", "yay", false },
+        { "script-src 'self'", "boo", false },
+        { "script-src 'nonce-yay'", "yay", true },
+        { "script-src 'nonce-yay'", "boo", false },
+        { "script-src 'nonce-yay' 'nonce-boo'", "yay", true },
+        { "script-src 'nonce-yay' 'nonce-boo'", "boo", true },
+
+        // Falls back to 'default-src'
+        { "default-src 'nonce-yay'", "yay", true },
+        { "default-src 'nonce-yay'", "boo", false },
+        { "default-src 'nonce-boo'; script-src 'nonce-yay'", "yay", true },
+        { "default-src 'nonce-boo'; script-src 'nonce-yay'", "boo", false },
+
+        // Unrelated directives do not affect result
+        { "style-src 'nonce-yay'", "yay", false },
+        { "style-src 'nonce-yay'", "boo", false },

[estark, 2016/06/02 18:57:31]

Maybe a test case or two for empty and null strings? e.g.
{ "script-src 'nonce-yay'", "", false }
{ "script-src 'nonce-yay'", String(), false }

[Mike West, 2016/06/04 06:30:56]

Added it into the `for` loop, as these should always return false.

+    };
+
+    for (const auto& test : cases) {
+        // Report-only
+        Member<CSPDirectiveList> policy = createList(test.list, ContentSecurityPolicyHeaderTypeReport);
+        Member<SourceListDirective> scriptSrc = policy->operativeDirective(policy->m_scriptSrc.get());
+        EXPECT_EQ(test.expected, policy->isMatchingNoncePresent(scriptSrc, test.nonce));
+
+        // Enforce
+        policy = createList(test.list, ContentSecurityPolicyHeaderTypeEnforce);
+        scriptSrc = policy->operativeDirective(policy->m_scriptSrc.get());
+        EXPECT_EQ(test.expected, policy->isMatchingNoncePresent(scriptSrc, test.nonce));
+    }
+}
+
+TEST_F(CSPDirectiveListTest, AllowScriptFromSourceNoNonce)
+{
+    struct TestCase {
+        const char* list;
+        const char* url;
+        bool expected;
+    } cases[] = {
+        { "script-src https://example.com", "https://example.com/script.js", true },
+        { "script-src https://example.com/", "https://example.com/script.js", true },
+        { "script-src https://example.com/", "https://example.com/script/script.js", true },
+        { "script-src https://example.com/script", "https://example.com/script.js", false },
+        { "script-src https://example.com/script", "https://example.com/script/script.js", false },
+        { "script-src https://example.com/script/", "https://example.com/script.js", false },
+        { "script-src https://example.com/script/", "https://example.com/script/script.js", true },
+        { "script-src https://example.com", "https://not.example.com/script.js", false },
+        { "script-src https://*.example.com", "https://not.example.com/script.js", true },
+        { "script-src https://*.example.com", "https://example.com/script.js", false },
+
+        // Falls back to default-src:
+        { "default-src https://example.com", "https://example.com/script.js", true },
+        { "default-src https://example.com/", "https://example.com/script.js", true },
+        { "default-src https://example.com/", "https://example.com/script/script.js", true },
+        { "default-src https://example.com/script", "https://example.com/script.js", false },
+        { "default-src https://example.com/script", "https://example.com/script/script.js", false },
+        { "default-src https://example.com/script/", "https://example.com/script.js", false },
+        { "default-src https://example.com/script/", "https://example.com/script/script.js", true },
+        { "default-src https://example.com", "https://not.example.com/script.js", false },
+        { "default-src https://*.example.com", "https://not.example.com/script.js", true },
+        { "default-src https://*.example.com", "https://example.com/script.js", false },
+    };
+
+    for (const auto& test : cases) {
+        SCOPED_TRACE(testing::Message() << "List: `" << test.list << "`, URL: `" << test.url << "`");

[estark, 2016/06/02 18:57:31]

Did you intend to leave this in or was it just for debugging?

[jww, 2016/06/02 23:45:29]

tl;dr I'm pretty sure this is on purpose. I believe it is standard unit testing
messaging that allows you to effectively have a different message for each run
so that if ASSERTS or EXPECTS fail mid-run, you get this message so that you
know where you failed.

[estark, 2016/06/02 23:49:34]

Oh! Cool! TIL. Thanks.

[Mike West, 2016/06/04 06:30:56]

Yeah, if you're combining a bunch of test cases into one test like I'm doing
here, then SCOPED_TRACE is super helpful 3 weeks from now when something breaks,
as it'll dump detail about what's going on, which is nice!

+        KURL scriptSrc = KURL(KURL(), test.url);
+
+        // Report-only
+        Member<CSPDirectiveList> policy = createList(test.list, ContentSecurityPolicyHeaderTypeReport);
+        EXPECT_EQ(test.expected, policy->allowScriptFromSource(scriptSrc, String(), ResourceRequest::RedirectStatus::NoRedirect, ContentSecurityPolicy::SuppressReport));
+
+        // Enforce
+        policy = createList(test.list, ContentSecurityPolicyHeaderTypeEnforce);
+        EXPECT_EQ(test.expected, policy->allowScriptFromSource(scriptSrc, String(), ResourceRequest::RedirectStatus::NoRedirect, ContentSecurityPolicy::SuppressReport));
+    }
+}
+
+TEST_F(CSPDirectiveListTest, AllowScriptFromSourceWithNonce)
+{
+    struct TestCase {
+        const char* list;
+        const char* url;
+        const char* nonce;
+        bool expected;
+    } cases[] = {
+        // Doesn't effect lists without nonces:

[estark, 2016/06/02 18:57:31]

the nittiest of nits (here and below): affect

[Mike West, 2016/06/04 06:30:56]

These words are my nemeses. :/

+        { "script-src https://example.com", "https://example.com/script.js", "yay", true },
+        { "script-src https://example.com", "https://example.com/script.js", "boo", true },
+        { "script-src https://example.com", "https://not.example.com/script.js", "yay", false },
+        { "script-src https://example.com", "https://not.example.com/script.js", "boo", false },

[estark, 2016/06/02 18:57:31]

same suggestion about/empty null strings here perhaps (testing that an empty
nonce doesn't let you bypass the whitelist)

[Mike West, 2016/06/04 06:30:56]

Done.

+
+        // Doesn't effect URLs that match the whitelist.
+        { "script-src https://example.com 'nonce-yay'", "https://example.com/script.js", "yay", true },
+        { "script-src https://example.com 'nonce-yay'", "https://example.com/script.js", "boo", true },
+
+        // Does effect URLs that don't.
+        { "script-src https://example.com 'nonce-yay'", "https://not.example.com/script.js", "yay", true },
+        { "script-src https://example.com 'nonce-yay'", "https://not.example.com/script.js", "boo", false },
+    };
+
+    for (const auto& test : cases) {
+        SCOPED_TRACE(testing::Message() << "List: `" << test.list << "`, URL: `" << test.url << "`");

[estark, 2016/06/02 18:57:31]

ditto, did you mean to leave this in?

+        KURL scriptSrc = KURL(KURL(), test.url);
+
+        // Report-only
+        Member<CSPDirectiveList> policy = createList(test.list, ContentSecurityPolicyHeaderTypeReport);
+        EXPECT_EQ(test.expected, policy->allowScriptFromSource(scriptSrc, String(test.nonce), ResourceRequest::RedirectStatus::NoRedirect, ContentSecurityPolicy::SuppressReport));
+
+        // Enforce
+        policy = createList(test.list, ContentSecurityPolicyHeaderTypeEnforce);
+        EXPECT_EQ(test.expected, policy->allowScriptFromSource(scriptSrc, String(test.nonce), ResourceRequest::RedirectStatus::NoRedirect, ContentSecurityPolicy::SuppressReport));
+    }
+}
+

[estark, 2016/06/02 18:57:31]

Is there a reason you mostly test scripts in this file, and not styles too? You
could omit the "script-" in each test case |list|, and run each test case twice,
once prepending "script-" to the |list| and once prepending "style-".

[Mike West, 2016/06/04 06:30:56]

I've added style tests, thanks!

+} // namespace blink