Bug 68898 - Regression: View Source not showing closing script tags
Summary: Regression: View Source not showing closing script tags
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Thomas Sepez
Depends on:
Reported: 2011-09-27 06:26 PDT by Keishi Hattori
Modified: 2011-09-28 10:18 PDT (History)
3 users (show)

See Also:

Patching XSSAuditor (8.71 KB, patch)
2011-09-27 15:24 PDT, Thomas Sepez
tsepez: commit-queue-
Details | Formatted Diff | Diff
Patch + be more grammatical (8.71 KB, patch)
2011-09-27 15:29 PDT, Thomas Sepez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keishi Hattori 2011-09-27 06:26:58 PDT
http://trac.webkit.org/changeset/95451 caused some</script> to disappear from the view-source mode.

Test: Visit "view-source:http://www.roseindia.net/tutorialfiles/26911.Script_Tag.htm" and see that </script> is missing.

Comment 1 Adam Barth 2011-09-27 09:16:05 PDT
Tom, your patch to SourceTracker probably broke this.  I thought we had tests covering this case, but I guess we don't.
Comment 2 Thomas Sepez 2011-09-27 15:24:29 PDT
Created attachment 108912 [details]
Patching XSSAuditor

Most prudent fix is to revert the truncation of the token in the HTMLTokenizer, and beware of the trailing close script tag.  We now have XSSAuditor::javaScriptForSnippet() which is well suited to doing this itself.  My patch at the tokenizer level fails badly when document.write re-introduces more text around the insertion point.  I'd like to get this resolved, and see if we can't get proper tokenization later.  The patch to the XSSAuditor is still relevant even in a world where the tokenizer appears correct as a second line of defense.  The tests are still running locally, I'll flip commit-queue back to ? when they pass, but it would be good to get comments now.
Comment 3 Thomas Sepez 2011-09-27 15:29:29 PDT
Created attachment 108914 [details]
Patch + be more grammatical
Comment 4 Adam Barth 2011-09-27 15:32:28 PDT
Comment on attachment 108914 [details]
Patch + be more grammatical

Ok.  I guess discretion is the better part of valor.  :)
Comment 5 Thomas Sepez 2011-09-28 10:00:02 PDT
Comment on attachment 108914 [details]
Patch + be more grammatical

Tests OK. Setting c-c: ?
Comment 6 WebKit Review Bot 2011-09-28 10:18:15 PDT
Comment on attachment 108914 [details]
Patch + be more grammatical

Clearing flags on attachment: 108914

Committed r96231: <http://trac.webkit.org/changeset/96231>
Comment 7 WebKit Review Bot 2011-09-28 10:18:19 PDT
All reviewed patches have been landed.  Closing bug.