`X-WebKit-CSP: script-src http://example.com/;` should work correctly. See http://crbug.com/128493 and the `ext-host-source` definition in the spec (https://dvcs.w3.org/hg/content-security-policy/raw-file/tip/csp-1.0-specification.html#source-list).
Created attachment 147961 [details] First pass: needs more tests.
I'd like to take this in steps, if you don't mind Adam. The attached patch is a first pass at adjusting the parseSource method to accept paths, but has a more or less no-op `parsePath` method, and throws away the path component. I think it's complex enough that it deserves it's own review. I think it also needs more tests... suggestions for cases I've missed are welcome. :) The next step would be to save the path (after properly parsing it), and use it for comparisons.
Comment on attachment 147961 [details] First pass: needs more tests. View in context: https://bugs.webkit.org/attachment.cgi?id=147961&action=review > Source/WebCore/page/ContentSecurityPolicy.cpp:316 > if (!parseHost(beginHost, position, host, hostHasWildcard)) > return false; > return true; Can't we just say "return parseHost(beginHost, position, host, hostHasWildcard);" in these cases? > Source/WebCore/page/ContentSecurityPolicy.cpp:356 > + skipWhile<isNotColonOrSlash>(position, end); Why isNotColonOrSlash here? > Source/WebCore/page/ContentSecurityPolicy.cpp:462 > + ASSERT(!path); Does that work? I would have thought you'd need to say ASSERT(path.isEmpty()) > LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-05.html:17 > + ['yes', 'script-src http://127.0.0.1:*/path', 'resources/script.js'], I would add some tests with ? and # parts of the URL. I'd also add some tests that include spaces and ; in the path to show that we're not too aggressive in eating up characters.
Comment on attachment 147961 [details] First pass: needs more tests. View in context: https://bugs.webkit.org/attachment.cgi?id=147961&action=review I'll fix these up in a patch shortly. Thanks for the quick review! >> Source/WebCore/page/ContentSecurityPolicy.cpp:316 >> return true; > > Can't we just say "return parseHost(beginHost, position, host, hostHasWildcard);" in these cases? We can! I only did it this way because I thought it fit better stylistically, but I didn't like it. :) >> Source/WebCore/page/ContentSecurityPolicy.cpp:356 >> + skipWhile<isNotColonOrSlash>(position, end); > > Why isNotColonOrSlash here? Hrm. I guess I can replace this with skipUtil(position, end, '/'). Relatedly: is `skipUtil` intentional? I want to change it to skipU_n_til every time I see it. >> Source/WebCore/page/ContentSecurityPolicy.cpp:462 >> + ASSERT(!path); > > Does that work? I would have thought you'd need to say ASSERT(path.isEmpty()) I think you're right. I'm not sure why it worked either. >> LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-05.html:17 >> + ['yes', 'script-src http://127.0.0.1:*/path', 'resources/script.js'], > > I would add some tests with ? and # parts of the URL. I'd also add some tests that include spaces and ; in the path to show that we're not too aggressive in eating up characters. What's the expected behavior in those cases? I think we'd end up with a path of `/path?query=value`, as both '?' and '#' fall into `<VCHAR except ";" and ",">`. Is that accurate?
> Relatedly: is `skipUtil` intentional? I want to change it to skipU_n_til every time I see it. That's just a typo. I fail at spelling.
> What's the expected behavior in those cases? I think we'd end up with a path of `/path?query=value`, as both '?' and '#' fall into `<VCHAR except ";" and ",">`. Is that accurate? Yeah, the "path" will eat up all those characters. We'll probably end up ignoring everything after the ? or the # though.
Created attachment 147982 [details] abarth feedback.
Created attachment 147983 [details] adding abarth as reviewer.
That was fast. :) I'm renaming this bug to "Ignore paths in Content Security Policy sources rather than failing to parse them." and will cover any future work on paths in another. This patch brings us up to 1.0, but the path behavior isn't really specced out yet... Should I set that next bug to block the CSP 1.1 meta bug?
I commented on the next bug.
Comment on attachment 147983 [details] adding abarth as reviewer. Clearing flags on attachment: 147983 Committed r120540: <http://trac.webkit.org/changeset/120540>
All reviewed patches have been landed. Closing bug.