Summary: | CSP should let sites both enforce one policy and monitor another | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Barth <abarth> | ||||||||
Component: | WebCore Misc. | Assignee: | Adam Barth <abarth> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | eric, mkwst, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 53572 | ||||||||||
Attachments: |
|
Description
Adam Barth
2012-05-03 17:43:55 PDT
Created attachment 140338 [details]
Patch
Comment on attachment 140338 [details] Patch Attachment 140338 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12633291 I think the mac build failure isn't real. 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? > 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?
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. (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. Comment on attachment 140338 [details]
Patch
/me will attempt to templatize.
Created attachment 140438 [details]
Patch
Created attachment 140443 [details]
Patch
Comment on attachment 140443 [details]
Patch
Infinitely better. THank you.
Comment on attachment 140443 [details] Patch Clearing flags on attachment: 140443 Committed r116254: <http://trac.webkit.org/changeset/116254> All reviewed patches have been landed. Closing bug. |