[webkit-reviews] review granted: [Bug 89281] Content Security Policy should support paths. : [Attachment 147961] First pass: needs more tests.

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Mike West <mkwst at chromium.org>'s
request for review:
Bug 89281: Content Security Policy should support paths.
https://bugs.webkit.org/show_bug.cgi?id=89281

Attachment 147961: First pass: needs more tests.
https://bugs.webkit.org/attachment.cgi?id=147961&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
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.ht
ml: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.