CSP should let sites both enforce one policy and monitor another

[Adam Barth]

This is to help sites test out a new policy while they already have a policy.

[Adam Barth]

Created attachment 140338 [details]
Patch

[Build Bot]

Comment on attachment 140338 [details]
Patch

Attachment 140338 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12633291

[Adam Barth]

I think the mac build failure isn't real.

[Eric Seidel (no email)]

Comment on attachment 140338 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=140338&action=review

> Source/WebCore/page/ContentSecurityPolicy.cpp:921
> +    for (PolicyList::const_iterator iter = m_policies.begin(); iter != m_policies.end(); ++iter) {
> +        if (!(*iter)->allowJavaScriptURLs())
> +            return false;
> +    }
> +    return true;

Really?  Can't a helper or templates save us here?  Some sort of function pointer?

[Adam Barth]

> Really?  Can't a helper or templates save us here?  Some sort of function pointer?

We can do it with macros, but that's pretty ugly.  There might be a way to do it with templates...  Do you know how?

[Darin Adler]

Comment on attachment 140338 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=140338&action=review

> Source/WebCore/page/ContentSecurityPolicy.cpp:892
> +    for (PolicyList::const_iterator iter = other->m_policies.begin(); iter != other->m_policies.end(); ++iter)
> +        didReceiveHeader((*iter)->header(), (*iter)->headerType());

Our usual name for this is “it” rather than “iter” although I don’t think either is great.

Normally we iterate a vector using indices rather than iterators, and use iterators only for compatibility with generic algorithms.

>> Source/WebCore/page/ContentSecurityPolicy.cpp:921
>> +    return true;
> 
> Really?  Can't a helper or templates save us here?  Some sort of function pointer?

I think we can do this cleanly with a template.

> Source/WebCore/page/ContentSecurityPolicy.h:57
> +    // FIXME: These functions are wrong becuase they assume that there is only one header.

What’s the plan for those clients?

> Source/WebCore/page/ContentSecurityPolicy.h:79
> +    typedef Vector<OwnPtr<CSPDirectiveList> > PolicyList;

PolicyVector? Unless the term list is a term of art here.

[Adam Barth]

(In reply to comment #6)
> (From update of attachment 140338 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=140338&action=review
> 
> > Source/WebCore/page/ContentSecurityPolicy.h:57
> > +    // FIXME: These functions are wrong becuase they assume that there is only one header.
> 
> What’s the plan for those clients?

I'm going to change these functions to return a vector.  There's some amount of plumbing involved, which is why I'm saving it for a future patch.

[Adam Barth]

Comment on attachment 140338 [details]
Patch

/me will attempt to templatize.

[Adam Barth]

Created attachment 140438 [details]
Patch

[Adam Barth]

Created attachment 140443 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 140443 [details]
Patch

Infinitely better.  THank you.

[WebKit Review Bot]

Comment on attachment 140443 [details]
Patch

Clearing flags on attachment: 140443

Committed r116254: <http://trac.webkit.org/changeset/116254>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.