Changeset 96231 in webkit

Timestamp:

Sep 28, 2011 10:19:08 AM (13 years ago)

Author:

commit-queue@webkit.org

Message:

Revert change which broke displaying end script tags in view-source, instead
deal with any trailing </script> tag included by mistake in the XSSAuditor
itself. Correct tests to detect the missing close tags.
​https://bugs.webkit.org/show_bug.cgi?id=68898

Patch by Tom Sepez <​tsepez@chromium.org> on 2011-09-28
Reviewed by Adam Barth.

Source/WebCore:

• html/parser/HTMLSourceTracker.cpp:

(WebCore::HTMLSourceTracker::end):

• html/parser/HTMLTokenizer.cpp:

(WebCore::HTMLTokenizer::nextToken):

• html/parser/XSSAuditor.cpp:

(WebCore::startsHTMLEndTagAt):
(WebCore::XSSAuditor::snippetForJavaScript):

LayoutTests:

• fast/frames/resources/viewsource-frame-2.html:
• fast/frames/viewsource-plain-text-tags-expected.txt:
• fast/frames/viewsource-plain-text-tags.html:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/frames/resources/viewsource-frame-2.html (modified) (1 diff)
• LayoutTests/fast/frames/viewsource-plain-text-tags-expected.txt (modified) (1 diff)
• LayoutTests/fast/frames/viewsource-plain-text-tags.html (modified) (3 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/parser/HTMLSourceTracker.cpp (modified) (1 diff)
• Source/WebCore/html/parser/HTMLTokenizer.cpp (modified) (1 diff)
• Source/WebCore/html/parser/XSSAuditor.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-09-28  Tom Sepez  <tsepez@chromium.org>
+
+        Revert change which broke displaying end script tags in view-source, instead
+        deal with any trailing </script> tag included by mistake in the XSSAuditor
+        itself.  Correct tests to detect the missing close tags.
+        https://bugs.webkit.org/show_bug.cgi?id=68898
+
+        Reviewed by Adam Barth.
+
+        * fast/frames/resources/viewsource-frame-2.html:
+        * fast/frames/viewsource-plain-text-tags-expected.txt:
+        * fast/frames/viewsource-plain-text-tags.html:
+
 2011-09-28  Antaryami Pandia  <antaryami.pandia@motorola.com>
 

trunk/LayoutTests/fast/frames/resources/viewsource-frame-2.html

@@ -1 +1 @@
 <script>
-<test>
+<testscript>
 </script>
 
 <style>
-<test>
+<teststyle>
 </style>
 
 <xmp>
-<test>
+<testxmp>
 </xmp>
 
 <textarea>
-<test>
+<testtextarea>
 </textarea>

trunk/LayoutTests/fast/frames/viewsource-plain-text-tags-expected.txt

@@ -1 @@
-PASS
+script: PASS PASS PASS style: PASS PASS PASS xmp: PASS PASS PASS textarea: PASS PASS PASS

trunk/LayoutTests/fast/frames/viewsource-plain-text-tags.html

@@ -7 +7 @@
     }
 
-    function report(frame) {
+    function found(text, regexString)
+    {
+        var matches = text.match(new RegExp(regexString, 'g'));
+        if (matches && matches.length === 1) 
+            return 'PASS';
+        else 
+            return 'FAIL';
+    }
+    
+    function testSection(text, name) {
+        // Closing tags are not correctly formated, so don't check their markup.
+        return name + ': ' +
+             found(text, '<span class="webkit-html-tag">&lt;' + name + '&gt') + ' ' +
+             found(text, '<td class="webkit-line-content">&lt;test' + name + '&gt;') + ' ' +
+             found(text, '&lt;/' + name + '&gt;') + '\n';
+    }
+
+    function report(frame)
+    {
         var result = frame.contentDocument.documentElement.innerHTML;
-        var regex = new RegExp("<td class=\"webkit-line-content\">&lt;test&gt;</td>", "g");
-        matches = result.match(regex);
+        var resultText = '';
 
-        if (matches && matches.length === 4)
-            var resultText = "PASS";
-        else
-            var resultText = "FAIL";
+        resultText += testSection(result, 'script');
+        resultText += testSection(result, 'style');
+        resultText += testSection(result, 'xmp');
+        resultText += testSection(result, 'textarea');
 
         if (window.layoutTestController) {
@@ -21 +38 @@
             document.write(resultText);
             document.close();
-
             layoutTestController.notifyDone();
         } else {
@@ -31 +47 @@
 <body>
 <p>You should see a frame in 'view source' mode below.</p>
-<p>None of the "&lt;test&gt;" strings shown below should be colorized like HTML.</p>
+<p>None of the "&lt;testxxx&gt;" strings shown below should be colorized like HTML.</p>
 <hr>
 <div id="result"></div>

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-09-28  Tom Sepez  <tsepez@chromium.org>
+
+        Revert change which broke displaying end script tags in view-source, instead
+        deal with any trailing </script> tag included by mistake in the XSSAuditor
+        itself.  Correct tests to detect the missing close tags.
+        https://bugs.webkit.org/show_bug.cgi?id=68898
+
+        Reviewed by Adam Barth.
+
+        * html/parser/HTMLSourceTracker.cpp:
+        (WebCore::HTMLSourceTracker::end):
+        * html/parser/HTMLTokenizer.cpp:
+        (WebCore::HTMLTokenizer::nextToken):
+        * html/parser/XSSAuditor.cpp:
+        (WebCore::startsHTMLEndTagAt):
+        (WebCore::XSSAuditor::snippetForJavaScript):
+
 2011-09-28  Adam Barth  <abarth@webkit.org>
 

trunk/Source/WebCore/html/parser/HTMLSourceTracker.cpp

@@ -45 +45 @@
     m_cachedSourceForToken = String();
 
-    // FIXME: This work should really be done by the HTMLTokenizer in all cases,
-    // instead of the few cases where it explicitly steps in to correct values
-    // known to be wrong in face of its internal buffering.
-    if (!token.endIndex())
-        token.end(input.current().numberOfCharactersConsumed());
+    // FIXME: This work should really be done by the HTMLTokenizer.
+    token.end(input.current().numberOfCharactersConsumed());
 }
 

trunk/Source/WebCore/html/parser/HTMLTokenizer.cpp

@@ -298 +298 @@
 
     HTML_BEGIN_STATE(ScriptDataState) {
-        if (cc == '<') {
-            // Token might end here. If not, we'll come through here again
-            // and update the end location again.
-            m_token->end(source.numberOfCharactersConsumed());
+        if (cc == '<')
             HTML_ADVANCE_TO(ScriptDataLessThanSignState);
-        }
         else if (cc == InputStreamPreprocessor::endOfFileMarker)
             return emitEndOfFile(source);

trunk/Source/WebCore/html/parser/XSSAuditor.cpp

@@ -85 +85 @@
     return (c == '\n' || c == '\r');
 }
+
+static bool startsHTMLEndTagAt(const String& string, size_t start)
+{
+    return (start + 1 < string.length() && string[start] == '<' && string[start+1] == '/');
+}    
+
 
 static bool startsHTMLCommentAt(const String& string, size_t start)
@@ -584 +590 @@
     }
 
-    // Stop at next comment or when we exceed the maximum length target. After hitting the
-    // length target, we can only stop at a point where we know we are not in the middle of
-    // a %-escape sequence. A simple way to do this is to break on whitespace only.                
+    // Stop at next comment, or at a closing script tag (which may have been included with
+    // the code fragment because of buffering in the HTMLSourceTracker), or when we exceed
+    // the maximum length target. After hitting the length target, we can only stop at a
+    // point where we know we are not in the middle of a %-escape sequence. For the sake of
+    // simplicity, approximate stopping at a close script tag by stopping at any close tag,
+    // and approximate not stopping inside a (possibly multiply encoded) %-esacpe sequence
+    // by breaking on whitespace only. We should have enough text in these cases to avoid
+    // false positives.
     for (foundPosition = startPosition; foundPosition < endPosition; foundPosition++) {
-        if (startsSingleLineCommentAt(string, foundPosition) || startsMultiLineCommentAt(string, foundPosition)) {
+        if (startsSingleLineCommentAt(string, foundPosition) || startsMultiLineCommentAt(string, foundPosition) || startsHTMLEndTagAt(string, foundPosition)) {
             endPosition = foundPosition + 2;
             break;