Summary: | CSP doesn't work for a wide variety of cases | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Boris Smus <smus> | ||||
Component: | WebCore Misc. | Assignee: | Adam Barth <abarth> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, apf, sam, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
URL: | http://erlend.oftedal.no/blog/csp/readiness/latest.php | ||||||
Attachments: |
|
Description
Boris Smus
2011-09-27 11:44:02 PDT
> Do we have a CSP unit test suite? Our CSP tests are here: http://trac.webkit.org/browser/trunk/LayoutTests/http/tests/security/contentSecurityPolicy I'm not sure why we're doing so badly on the oftedal test suite. I'll investigate. This looks like an issue with our port matching code in CSPSource. In the case that the source's port is set to 0 (for no port specified) and the URL to match has no port specified (also set to 0), the code checks if 0 is the default port for the protocol. int port = url.port(); return port ? port == m_port : isDefaultPortForProtocol(m_port, url.protocol()); In this case, we should be returning true, something like this should probably work: bool portMatches(const KURL& url) const { if (m_portHasWildcard) return true; int port = url.port(); if (port == m_port) return true; if (!port) return isDefaultPortForProtocol(m_port, m_scheme); if (!m_port) return isDefaultPortForProtocol(port, m_scheme); return false; } Fixing that issue, it looks like we pass all the same tests that Firefox does. We probably have no tests for this case, since our testing harness requires using a non-default port :(. Ah, that makes a lot of sense. Thanks Sam. Are you planning to post a patch, or should I? (Also, any thoughts on how we can test this bug?) Created attachment 109035 [details]
Patch
Comment on attachment 109035 [details] Patch Clearing flags on attachment: 109035 Committed r96239: <http://trac.webkit.org/changeset/96239> All reviewed patches have been landed. Closing bug. |