RESOLVED FIXED 45377
Errors in LayoutTests/http/tests/security/xss-DENIED-mime-type-execute-as-html.html
https://bugs.webkit.org/show_bug.cgi?id=45377
Summary Errors in LayoutTests/http/tests/security/xss-DENIED-mime-type-execute-as-htm...
Sergio Villar Senin
Reported 2010-09-08 03:35:28 PDT
There are several errors in LayoutTests/http/tests/security/xss-DENIED-mime-type-execute-as-html.html * in the .html encodeURIComponent should be used instead of escape (because the URIs contain '+') http://www.w3schools.com/jsref/jsref_encodeURIComponent.asp * in the PHP urldecode cannot be used with $_GET as it's already decoded http://php.net/manual/en/function.urldecode.php
Attachments
Fix for the bug (2.31 KB, patch)
2010-09-08 03:38 PDT, Sergio Villar Senin
abarth: review+
abarth: commit-queue-
Fix the URL encoding/decoding mess in the bug (2.65 KB, patch)
2010-09-08 04:05 PDT, Sergio Villar Senin
no flags
Sergio Villar Senin
Comment 1 2010-09-08 03:38:47 PDT
Created attachment 66866 [details] Fix for the bug
Sergio Villar Senin
Comment 2 2010-09-08 03:39:55 PDT
Adding Adam to Cc: as bug was discussed with him on IRC
Adam Barth
Comment 3 2010-09-08 03:46:05 PDT
Comment on attachment 66866 [details] Fix for the bug View in context: https://bugs.webkit.org/attachment.cgi?id=66866&action=prettypatch Please add more detail to the ChangeLog before landing. > LayoutTests/ChangeLog:7 > + Errors in LayoutTests/http/tests/security/xss-DENIED-mime-type-execute-as-html.html > + https://bugs.webkit.org/show_bug.cgi?id=45377 > + It would be better if your ChangeLog explains what problem this patch solves. "Errors" is a pretty general problem. You explained it to me over IRC, but someone reading this ChangeLog will be pretty mystified. > LayoutTests/http/tests/security/resources/send-mime-types.php:2 > - $mime_type = urldecode($_GET["mt"]); > + $mime_type = $_GET["mt"]; Yeah, the old code is pretty confused, huh? > LayoutTests/http/tests/security/xss-DENIED-mime-type-execute-as-html.html:19 > - ifr.src = "http://localhost:8000/security/resources/send-mime-types.php?mt=" + escape(mime_types[i]); > + ifr.src = "http://localhost:8000/security/resources/send-mime-types.php?mt=" + encodeURIComponent(mime_types[i]); Did this change actually make a difference? It seems like the other change is probably the operative one.
Sergio Villar Senin
Comment 4 2010-09-08 03:50:45 PDT
(In reply to comment #3) > (From update of attachment 66866 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=66866&action=prettypatch > > Please add more detail to the ChangeLog before landing. > > > LayoutTests/ChangeLog:7 > > + Errors in LayoutTests/http/tests/security/xss-DENIED-mime-type-execute-as-html.html > > + https://bugs.webkit.org/show_bug.cgi?id=45377 > > + > It would be better if your ChangeLog explains what problem this patch solves. "Errors" is a pretty general problem. You explained it to me over IRC, but someone reading this ChangeLog will be pretty mystified. Ok I agree. will add more detail > > > LayoutTests/http/tests/security/resources/send-mime-types.php:2 > > - $mime_type = urldecode($_GET["mt"]); > > + $mime_type = $_GET["mt"]; > Yeah, the old code is pretty confused, huh? Yep :-) > > LayoutTests/http/tests/security/xss-DENIED-mime-type-execute-as-html.html:19 > > - ifr.src = "http://localhost:8000/security/resources/send-mime-types.php?mt=" + escape(mime_types[i]); > > + ifr.src = "http://localhost:8000/security/resources/send-mime-types.php?mt=" + encodeURIComponent(mime_types[i]); > Did this change actually make a difference? It seems like the other change is probably the operative one. Yes it's also a must. Without it the PHP will get a '+' character anyway and then transform it into a blank space when parsing the URL.
Sergio Villar Senin
Comment 5 2010-09-08 04:05:18 PDT
Created attachment 66872 [details] Fix the URL encoding/decoding mess in the bug With a more descriptive ChangeLog entry
WebKit Commit Bot
Comment 6 2010-09-08 08:19:00 PDT
Comment on attachment 66872 [details] Fix the URL encoding/decoding mess in the bug Clearing flags on attachment: 66872 Committed r66985: <http://trac.webkit.org/changeset/66985>
WebKit Commit Bot
Comment 7 2010-09-08 08:19:04 PDT
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 8 2010-09-08 09:30:49 PDT
http://trac.webkit.org/changeset/66985 might have broken GTK Linux 64-bit Debug The following changes are on the blame list: http://trac.webkit.org/changeset/66985 http://trac.webkit.org/changeset/66986
Note You need to log in before you can comment on or make changes to this bug.