Summary: | CSP: Implement frame-ancestors directive | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||||
Component: | WebCore Misc. | Assignee: | Daniel Bates <dbates> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | aestes, ap, bfulgham, cdumez, commit-queue, japhet, mkwst, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar, WebExposed | ||||||
Version: | WebKit Local Build | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=94836 | ||||||||
Attachments: |
|
Description
Daniel Bates
2016-02-17 11:12:27 PST
Created attachment 273630 [details]
Patch and Layout Tests
Created attachment 273636 [details]
Patch and Layout Tests
Comment on attachment 273636 [details] Patch and Layout Tests View in context: https://bugs.webkit.org/attachment.cgi?id=273636&action=review r=me. > Source/WebCore/loader/FrameLoader.cpp:691 > + m_frame.document()->contentSecurityPolicy()->didReceiveHeaders(m_documentLoader->response(), ContentSecurityPolicy::ReportParsingErrors::No); So we are using an implicit cast here? I'm not sure that is an improvement. > Source/WebCore/page/csp/ContentSecurityPolicy.h:165 > + // We can never have both a script execution context and a frame. Seems like we should ASSERT on this invariant, perhaps in places where we set one of these values. That is not needed for the current patch, but it's something to consider in the future. (In reply to comment #4) > > Source/WebCore/loader/FrameLoader.cpp:691 > > + m_frame.document()->contentSecurityPolicy()->didReceiveHeaders(m_documentLoader->response(), ContentSecurityPolicy::ReportParsingErrors::No); > > So we are using an implicit cast here? I'm not sure that is an improvement. > Will revert. I will also mark the constructor of ContentSecurityPolicyResponseHeaders explicit and update all other callers. I take it you feel that it is makes the code more understandable to see the explicit construction of a ContentSecurityPolicyResponseHeaders object from a ResourceResponse object. > > Source/WebCore/page/csp/ContentSecurityPolicy.h:165 > > + // We can never have both a script execution context and a frame. > > Seems like we should ASSERT on this invariant, perhaps in places where we > set one of these values. No need to use a run-time assert. This invariant is enforced by the public interface of ContentSecurityPolicy. It is not possible to create a ContentSecurityPolicy object that has both a script execution context and a frame because you can only instantiate one or the other via two different constructors and ContentSecurityPolicy does not expose a setter for either ContentSecurityPolicy::m_scriptExecutionContext or ContentSecurityPolicy::m_frame. Committed r197972: <http://trac.webkit.org/changeset/197972> |