Errors in LayoutTests/http/tests/security/xss-DENIED-mime-type-execute-as-html.html

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Sergio Villar Senin]

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

[Sergio Villar Senin]

Created attachment 66866 [details]
Fix for the bug

[Sergio Villar Senin]

Adding Adam to Cc: as bug was discussed with him on IRC

[Adam Barth]

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]

(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]

Created attachment 66872 [details]
Fix the URL encoding/decoding mess in the bug

With a more descriptive ChangeLog entry

[WebKit Commit Bot]

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]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

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