Bug 121609

Summary: Remove URL decoding in srcset handling
Product: WebKit Reporter: Yoav Weiss <yoav>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, benjamin, cbiesinger, commit-queue, dino, esprehn+autocc, kling, paroga, romain.perier
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 121695    
Attachments:
Description Flags
Patch none

Yoav Weiss
Reported 2013-09-19 07:47:42 PDT
Remove URL decoding in srcset handling
Attachments
Patch (7.06 KB, patch)
2013-09-19 07:53 PDT, Yoav Weiss 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Yoav Weiss
Comment 1 2013-09-19 07:53:42 PDT
Created attachment 212068 [details] Patch
Yoav Weiss
Comment 2 2013-09-19 07:59:26 PDT
As pointed out to me by Blink's Christian Biesinger, the URL decoding in the srcset algorithm can break encoded URLs (both in src and in srcset) Since the new parser is in place (which splits the attribute on white spaces rather than commas), data URIs are handled properly, and decoding URLs doesn't really offer any benefits. Therefore, I removed decoding to avoid breaking these URLs.
Benjamin Poulain
Comment 3 2013-09-19 14:48:38 PDT
Thanks for the follow up.
WebKit Commit Bot
Comment 4 2013-09-19 22:01:55 PDT
Comment on attachment 212068 [details] Patch Clearing flags on attachment: 212068 Committed r156140: <http://trac.webkit.org/changeset/156140>
WebKit Commit Bot
Comment 5 2013-09-19 22:01:58 PDT
All reviewed patches have been landed. Closing bug.
Patrick R. Gansterer
Comment 6 2013-09-20 06:11:38 PDT
(In reply to comment #4) > (From update of attachment 212068 [details]) > Clearing flags on attachment: 212068 > > Committed r156140: <http://trac.webkit.org/changeset/156140> This change broke development on native windows systems. '?' is not a valid character in filenames on windows. Can you fix the filename (by changing the test to a sever side script which checks for the question mark or any other special character in the URL) or roll out the patch?
Yoav Weiss
Comment 7 2013-09-20 06:18:41 PDT
(In reply to comment #6) > (In reply to comment #4) > > (From update of attachment 212068 [details] [details]) > > Clearing flags on attachment: 212068 > > > > Committed r156140: <http://trac.webkit.org/changeset/156140> > > This change broke development on native windows systems. '?' is not a valid character in filenames on windows. > Can you fix the filename (by changing the test to a sever side script which checks for the question mark or any other special character in the URL) or roll out the patch? I'm extremely sorry :( Will fix the filename ASAP
Alexey Proskuryakov
Comment 8 2013-09-20 10:02:54 PDT
Looks like Windows was fixed in <http://trac.webkit.org/changeset/156161>.
Alexey Proskuryakov
Comment 9 2013-09-20 10:27:41 PDT
And then more in <http://trac.webkit.org/r156179>.
Alexey Proskuryakov
Comment 10 2013-09-20 10:37:59 PDT
It should be possible to test question marks with a cgi script that intercepts all requests within a directory. We do this in LayoutTests/http/tests/uri/intercept tests.
Note You need to log in before you can comment on or make changes to this bug.