Bug 97538 – CSP paths: Ignore invalid path components, rather than dropping the source completely.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Mike West]

Offlist, Tanvi expressed concern that WebKit currently drops source expressions on the floor if they contain '#' or '?'. I think this is a mistake in the implementation, as we'd apparently agreed to simply throw a warning in that case[1].

[1]: https://bugs.webkit.org/show_bug.cgi?id=89750#c4

[Mike West]

Created attachment 165557 [details]
Patch

[Adam Barth]

Comment on attachment 165557 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=165557&action=review

> Source/WebCore/page/ContentSecurityPolicy.cpp:1579
> +{

Can you ASSERT that invalidChar is either '#' or '?'

I know you have that assert above, but it's good to have it in this function because the correctness of this function depends on that fact.

[Mike West]

Comment on attachment 165557 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=165557&action=review

>> Source/WebCore/page/ContentSecurityPolicy.cpp:1579
>> +{
> 
> Can you ASSERT that invalidChar is either '#' or '?'
> 
> I know you have that assert above, but it's good to have it in this function because the correctness of this function depends on that fact.

Sure. I'll spin a new patch in a moment, thanks!

[Mike West]

Created attachment 165629 [details]
Patch

[Mike West]

Comment on attachment 165629 [details]
Patch

CQ?, assuming the bots don't mind the extra ASSERT? :)

[Adam Barth]

> CQ?, assuming the bots don't mind the extra ASSERT? :)

The bots test in release, so they're not going to complain.  ;)

[WebKit Review Bot]

Comment on attachment 165629 [details]
Patch

Clearing flags on attachment: 165629

Committed r129525: <http://trac.webkit.org/changeset/129525>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.