RESOLVED FIXED 85561
CSP should let sites both enforce one policy and monitor another 
https://bugs.webkit.org/show_bug.cgi?id=85561
Summary CSP should let sites both enforce one policy and monitor another
Adam Barth
Reported 2012-05-03 17:43:55 PDT
This is to help sites test out a new policy while they already have a policy.
Attachments
Patch (20.90 KB, patch)
2012-05-04 15:15 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch (20.13 KB, patch)
2012-05-06 15:06 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch (20.10 KB, patch)
2012-05-06 16:15 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Adam Barth
Comment 1 2012-05-04 15:15:13 PDT
Created attachment 140338 [details] Patch
Build Bot
Comment 2 2012-05-04 15:35:16 PDT
Comment on attachment 140338 [details] Patch Attachment 140338 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12633291
Adam Barth
Comment 3 2012-05-04 16:10:24 PDT
I think the mac build failure isn't real.
Eric Seidel (no email)
Comment 4 2012-05-04 16:11:21 PDT
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
Comment 5 2012-05-04 16:15:45 PDT
> 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 6 2012-05-04 17:58:18 PDT
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
Comment 7 2012-05-04 18:37:53 PDT
(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 8 2012-05-06 13:49:31 PDT
Comment on attachment 140338 [details] Patch /me will attempt to templatize.
Adam Barth
Comment 9 2012-05-06 15:06:09 PDT
Created attachment 140438 [details] Patch
Adam Barth
Comment 10 2012-05-06 16:15:41 PDT
Created attachment 140443 [details] Patch
Eric Seidel (no email)
Comment 11 2012-05-06 16:18:24 PDT
Comment on attachment 140443 [details] Patch Infinitely better. THank you.
WebKit Review Bot
Comment 12 2012-05-06 17:52:51 PDT
Comment on attachment 140443 [details] Patch Clearing flags on attachment: 140443 Committed r116254: <http://trac.webkit.org/changeset/116254>
WebKit Review Bot
Comment 13 2012-05-06 17:52:56 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.