Oops: "X-XSS-Protection:" OWS "1" ";" "mode" "=" "block" OWS Note that this is the "new" ABNF syntax, so the following would be valid: X-XSS-Protection: 1;mode=block X-XSS-Protection: 1 ; mode =block because LWS (spaces and tabs) are allowed between each token. Also the tokens are case insensitive.

Created attachment 49651 [details] Patch

Created attachment 49653 [details] Patch

In the expected results file "xss-protection-parsing-01-expected.txt" there is a timed out fail message: ... +CONSOLE MESSAGE: line 1: Refused to execute a JavaScript script. Source code of script found within request. + +FAIL: Timed out waiting for notifyDone to be called +This tests our parsing of the X-XSS-Protection header. ...

Comment on attachment 49653 [details] Patch Ah, that's no good.

Created attachment 49678 [details] Patch

This matches some new spec? How should a reviewer evaluate this patch?

> This matches some new spec? How should a reviewer evaluate this patch? It matches what some IE folks asked us to use.

Comment on attachment 49678 [details] Patch > --- a/WebCore/ChangeLog > +++ b/WebCore/ChangeLog > @@ -1,3 +1,27 @@ > +2010-02-27 Adam Barth <abarth@webkit.org> > [...] > + * WebCore.xcodeproj/project.pbxproj: This line should be removed from the change log since this patch does not contain any changes to the WebCore Xcode project file. > -bool XSSAuditor::shouldFullPageBlockForXSSProtectionHeader() const > +XSSProtectionDisposition XSSAuditor::xssProtection() const > { > // If we detect an XSS attack and find the HTTP header "X-XSS-Protection: 12" then > // we will stop loading the page as opposed to ignoring the script. The value "12" > @@ -302,9 +302,7 @@ bool XSSAuditor::shouldFullPageBlockForXSSProtectionHeader() const Either this comment needs to removed or it needs to be updated since we are no longer using the "12" notation to do full-page blocking. > +XSSProtectionDisposition parseXSSProtectionHeader(const String& header) > +{ > + String stippedHeader = header.stripWhiteSpace(); > + > + if (stippedHeader.isEmpty()) > + return XSSProtectionEnabled; > + > + if (stippedHeader[0] == '0') > + return XSSProtectionDisabled; > + > + int length = (int)header.length(); Minor style issue, the explicit cast to type integer (i.e. "(int)") is unnecessary. r=me

Created attachment 51194 [details] Patch for landing

> This line should be removed from the change log since this patch does not > contain any changes to the WebCore Xcode project file. Fixed. > Either this comment needs to removed or it needs to be updated since we are no > longer using the "12" notation to do full-page blocking. Removed. > Minor style issue, the explicit cast to type integer (i.e. "(int)") is > unnecessary. I left it in because it's converting from unsigned to signed and other instances in this file do it too. (I think one of the ports might have a warning-as-error in this case.)

Comment on attachment 51194 [details] Patch for landing Clearing flags on attachment: 51194 Committed r56295: <http://trac.webkit.org/changeset/56295>

All reviewed patches have been landed. Closing bug.

http/tests/security/xssAuditor/xss-protection-parsing-01.html has failed on the Tiger bot ever since this commit. Please fix. :)