Bug 97538

Summary: CSP paths: Ignore invalid path components, rather than dropping the source completely.
Product: WebKit Reporter: Mike West <mkwst>
Component: WebCore Misc.Assignee: Mike West <mkwst>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, webkit.review.bot
Priority: P2 Keywords: WebExposed
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 85558    
Attachments:
Description Flags
Patch
none
Patch none

Mike West
Reported 2012-09-25 01:43:48 PDT
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
Attachments
Patch (15.69 KB, patch)
2012-09-25 01:50 PDT, Mike West 		no flags
DetailsFormatted DiffDiff
Patch (15.71 KB, patch)
2012-09-25 09:35 PDT, Mike West 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mike West
Comment 1 2012-09-25 01:50:30 PDT
Created attachment 165557 [details] Patch
Adam Barth
Comment 2 2012-09-25 09:15:04 PDT
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 3 2012-09-25 09:29:59 PDT
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
Comment 4 2012-09-25 09:35:07 PDT
Created attachment 165629 [details] Patch
Mike West
Comment 5 2012-09-25 09:37:16 PDT
Comment on attachment 165629 [details] Patch CQ?, assuming the bots don't mind the extra ASSERT? :)
Adam Barth
Comment 6 2012-09-25 09:46:13 PDT
> 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 7 2012-09-25 10:24:01 PDT
Comment on attachment 165629 [details] Patch Clearing flags on attachment: 165629 Committed r129525: <http://trac.webkit.org/changeset/129525>
WebKit Review Bot
Comment 8 2012-09-25 10:24:04 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.