[webkit-reviews] review granted: [Bug 68092] xssauditor - truncate inline snippets at a reasonable length before comparison : [Attachment 107570] Proposed patch plus new test case

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Thomas Sepez <tsepez at chromium.org>'s
request for review:
Bug 68092: xssauditor - truncate inline snippets at a reasonable length before
comparison
https://bugs.webkit.org/show_bug.cgi?id=68092

Attachment 107570: Proposed patch plus new test case
https://bugs.webkit.org/attachment.cgi?id=107570&action=review

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


> Source/WebCore/html/parser/XSSAuditor.cpp:512
> -    if ((position = snippet.find("=")) != notFound
> -	   && (position = snippet.find(isNotHTMLSpace, position + 1)) !=
notFound
> -	   && (position = snippet.find(isTerminatingCharacter,
isHTMLQuote(snippet[position]) ? position + 1 : position)) != notFound) {
> -	   snippet.truncate(position);
> +    if ((position = decodedSnippet.find("=")) != notFound
> +	   && (position = decodedSnippet.find(isNotHTMLSpace, position + 1)) !=
notFound
> +	   && (position = decodedSnippet.find(isTerminatingCharacter,
isHTMLQuote(decodedSnippet[position]) ? position + 1 : position)) != notFound)
{
> +	   decodedSnippet.truncate(position);

I'm slightly confused.	Are URLs going through this path too?

> Source/WebCore/html/parser/XSSAuditor.cpp:-516
> -    ASSERT(!snippet.isEmpty());

Maybe we should assert the fullyDecoding the decodedSnippet doesn't change it?