<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[201624] trunk</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/201624">201624</a></dd>
<dt>Author</dt> <dd>oliver@apple.com</dd>
<dt>Date</dt> <dd>2016-06-02 16:07:48 -0700 (Thu, 02 Jun 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>JS parser incorrectly handles invalid utf8 in error messages.
https://bugs.webkit.org/show_bug.cgi?id=158128

Reviewed by Saam Barati.

Source/JavaScriptCore:

The bug here was caused by us using PrintStream's toString method
to produce the error message for a parse error, even though toString
may produce a null string in the event of invalid utf8 that causes
the error in first case. So when we try to create an error message
containing the invalid character code, we set m_errorMessage to the
null string, as that signals &quot;no error&quot; we don't stop parsing, and
everything goes down hill from there.

Now we use the new toStringWithLatin1Fallback so that we can always
produce an error message, even if it contains invalid unicode. We
also add an additional fallback so that we can guarantee an error
message is set even if we're given a null string. There's a debug
mode assertion to prevent anyone accidentally attempting to clear
the message via setErrorMessage.

* parser/Parser.cpp:
(JSC::Parser&lt;LexerType&gt;::logError):
* parser/Parser.h:
(JSC::Parser::setErrorMessage):

Source/WTF:

Add a new toStringWithLatin1Fallback that simply uses
String::fromUTF8WithLatin1Fallback, so we can avoid the
standard String::fromUTF8 null return.

* wtf/StringPrintStream.cpp:
(WTF::StringPrintStream::toStringWithLatin1Fallback):
* wtf/StringPrintStream.h:

LayoutTests:

Add a testcase.

* js/invalid-utf8-in-syntax-error-expected.txt: Added.
* js/script-tests/invalid-utf8-in-syntax-error.js: Added.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreparserParsercpp">trunk/Source/JavaScriptCore/parser/Parser.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreparserParserh">trunk/Source/JavaScriptCore/parser/Parser.h</a></li>
<li><a href="#trunkSourceWTFChangeLog">trunk/Source/WTF/ChangeLog</a></li>
<li><a href="#trunkSourceWTFwtfStringPrintStreamcpp">trunk/Source/WTF/wtf/StringPrintStream.cpp</a></li>
<li><a href="#trunkSourceWTFwtfStringPrintStreamh">trunk/Source/WTF/wtf/StringPrintStream.h</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsjsinvalidutf8insyntaxerrorexpectedtxt">trunk/LayoutTests/js/invalid-utf8-in-syntax-error-expected.txt</a></li>
<li><a href="#trunkLayoutTestsjsscripttestsinvalidutf8insyntaxerrorjs">trunk/LayoutTests/js/script-tests/invalid-utf8-in-syntax-error.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (201623 => 201624)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2016-06-02 22:43:25 UTC (rev 201623)
+++ trunk/LayoutTests/ChangeLog        2016-06-02 23:07:48 UTC (rev 201624)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2016-06-02  Oliver Hunt  &lt;oliver@apple.com&gt;
+
+        JS parser incorrectly handles invalid utf8 in error messages.
+        https://bugs.webkit.org/show_bug.cgi?id=158128
+
+        Reviewed by Saam Barati.
+
+        Add a testcase.
+
+        * js/invalid-utf8-in-syntax-error-expected.txt: Added.
+        * js/script-tests/invalid-utf8-in-syntax-error.js: Added.
+
</ins><span class="cx"> 2016-06-02  Michael Saboff  &lt;msaboff@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         REGRESSION(r200694): %ThrowTypeError% is not unique
</span></span></pre></div>
<a id="trunkLayoutTestsjsinvalidutf8insyntaxerrorexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/js/invalid-utf8-in-syntax-error-expected.txt (0 => 201624)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/js/invalid-utf8-in-syntax-error-expected.txt                                (rev 0)
+++ trunk/LayoutTests/js/invalid-utf8-in-syntax-error-expected.txt        2016-06-02 23:07:48 UTC (rev 201624)
</span><span class="lines">@@ -0,0 +1,10 @@
</span><ins>+Ensures that we correctly propagate the error message for lexer errors containing invalid utf8 code sequences
+
+On success, you will see a series of &quot;PASS&quot; messages, followed by &quot;TEST COMPLETE&quot;.
+
+
+PASS ({f(&quot;\x{DEAD}&quot;)}) threw exception SyntaxError: Unexpected string literal &quot;íº­&quot;. Expected a parameter pattern or a ')' in parameter list..
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestsjsscripttestsinvalidutf8insyntaxerrorjs"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/js/script-tests/invalid-utf8-in-syntax-error.js (0 => 201624)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/js/script-tests/invalid-utf8-in-syntax-error.js                                (rev 0)
+++ trunk/LayoutTests/js/script-tests/invalid-utf8-in-syntax-error.js        2016-06-02 23:07:48 UTC (rev 201624)
</span><span class="lines">@@ -0,0 +1,6 @@
</span><ins>+description('Ensures that we correctly propagate the error message for lexer errors containing invalid utf8 code sequences');
+
+shouldThrow('({f(&quot;\udead&quot;)})');
+
+var successfullyParsed = true;
+
</ins></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (201623 => 201624)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2016-06-02 22:43:25 UTC (rev 201623)
+++ trunk/Source/JavaScriptCore/ChangeLog        2016-06-02 23:07:48 UTC (rev 201624)
</span><span class="lines">@@ -1,3 +1,30 @@
</span><ins>+2016-06-02  Oliver Hunt  &lt;oliver@apple.com&gt;
+
+        JS parser incorrectly handles invalid utf8 in error messages.
+        https://bugs.webkit.org/show_bug.cgi?id=158128
+
+        Reviewed by Saam Barati.
+
+        The bug here was caused by us using PrintStream's toString method
+        to produce the error message for a parse error, even though toString
+        may produce a null string in the event of invalid utf8 that causes
+        the error in first case. So when we try to create an error message
+        containing the invalid character code, we set m_errorMessage to the
+        null string, as that signals &quot;no error&quot; we don't stop parsing, and
+        everything goes down hill from there.
+
+        Now we use the new toStringWithLatin1Fallback so that we can always
+        produce an error message, even if it contains invalid unicode. We
+        also add an additional fallback so that we can guarantee an error
+        message is set even if we're given a null string. There's a debug
+        mode assertion to prevent anyone accidentally attempting to clear
+        the message via setErrorMessage.
+
+        * parser/Parser.cpp:
+        (JSC::Parser&lt;LexerType&gt;::logError):
+        * parser/Parser.h:
+        (JSC::Parser::setErrorMessage):
+
</ins><span class="cx"> 2016-06-02  Saam Barati  &lt;sbarati@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Teach bytecode liveness about the debugger
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreparserParsercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/parser/Parser.cpp (201623 => 201624)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/parser/Parser.cpp        2016-06-02 22:43:25 UTC (rev 201623)
+++ trunk/Source/JavaScriptCore/parser/Parser.cpp        2016-06-02 23:07:48 UTC (rev 201624)
</span><span class="lines">@@ -93,7 +93,7 @@
</span><span class="cx">         return;
</span><span class="cx">     StringPrintStream stream;
</span><span class="cx">     printUnexpectedTokenText(stream);
</span><del>-    setErrorMessage(stream.toString());
</del><ins>+    setErrorMessage(stream.toStringWithLatin1Fallback());
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> template &lt;typename LexerType&gt; template &lt;typename A&gt;
</span><span class="lines">@@ -107,7 +107,7 @@
</span><span class="cx">         stream.print(&quot;. &quot;);
</span><span class="cx">     }
</span><span class="cx">     stream.print(value1, &quot;.&quot;);
</span><del>-    setErrorMessage(stream.toString());
</del><ins>+    setErrorMessage(stream.toStringWithLatin1Fallback());
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> template &lt;typename LexerType&gt; template &lt;typename A, typename B&gt;
</span><span class="lines">@@ -121,7 +121,7 @@
</span><span class="cx">         stream.print(&quot;. &quot;);
</span><span class="cx">     }
</span><span class="cx">     stream.print(value1, value2, &quot;.&quot;);
</span><del>-    setErrorMessage(stream.toString());
</del><ins>+    setErrorMessage(stream.toStringWithLatin1Fallback());
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> template &lt;typename LexerType&gt; template &lt;typename A, typename B, typename C&gt;
</span><span class="lines">@@ -135,7 +135,7 @@
</span><span class="cx">         stream.print(&quot;. &quot;);
</span><span class="cx">     }
</span><span class="cx">     stream.print(value1, value2, value3, &quot;.&quot;);
</span><del>-    setErrorMessage(stream.toString());
</del><ins>+    setErrorMessage(stream.toStringWithLatin1Fallback());
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> template &lt;typename LexerType&gt; template &lt;typename A, typename B, typename C, typename D&gt;
</span><span class="lines">@@ -149,7 +149,7 @@
</span><span class="cx">         stream.print(&quot;. &quot;);
</span><span class="cx">     }
</span><span class="cx">     stream.print(value1, value2, value3, value4, &quot;.&quot;);
</span><del>-    setErrorMessage(stream.toString());
</del><ins>+    setErrorMessage(stream.toStringWithLatin1Fallback());
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> template &lt;typename LexerType&gt; template &lt;typename A, typename B, typename C, typename D, typename E&gt;
</span><span class="lines">@@ -163,7 +163,7 @@
</span><span class="cx">         stream.print(&quot;. &quot;);
</span><span class="cx">     }
</span><span class="cx">     stream.print(value1, value2, value3, value4, value5, &quot;.&quot;);
</span><del>-    setErrorMessage(stream.toString());
</del><ins>+    setErrorMessage(stream.toStringWithLatin1Fallback());
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> template &lt;typename LexerType&gt; template &lt;typename A, typename B, typename C, typename D, typename E, typename F&gt;
</span><span class="lines">@@ -177,7 +177,7 @@
</span><span class="cx">         stream.print(&quot;. &quot;);
</span><span class="cx">     }
</span><span class="cx">     stream.print(value1, value2, value3, value4, value5, value6, &quot;.&quot;);
</span><del>-    setErrorMessage(stream.toString());
</del><ins>+    setErrorMessage(stream.toStringWithLatin1Fallback());
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> template &lt;typename LexerType&gt; template &lt;typename A, typename B, typename C, typename D, typename E, typename F, typename G&gt;
</span><span class="lines">@@ -191,7 +191,7 @@
</span><span class="cx">         stream.print(&quot;. &quot;);
</span><span class="cx">     }
</span><span class="cx">     stream.print(value1, value2, value3, value4, value5, value6, value7, &quot;.&quot;);
</span><del>-    setErrorMessage(stream.toString());
</del><ins>+    setErrorMessage(stream.toStringWithLatin1Fallback());
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> template &lt;typename LexerType&gt;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreparserParserh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/parser/Parser.h (201623 => 201624)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/parser/Parser.h        2016-06-02 22:43:25 UTC (rev 201623)
+++ trunk/Source/JavaScriptCore/parser/Parser.h        2016-06-02 23:07:48 UTC (rev 201624)
</span><span class="lines">@@ -1276,7 +1276,10 @@
</span><span class="cx"> 
</span><span class="cx">     void setErrorMessage(const String&amp; message)
</span><span class="cx">     {
</span><ins>+        ASSERT_WITH_MESSAGE(!message.isEmpty(), &quot;Attempted to set the empty string as an error message. Likely caused by invalid UTF8 used when creating the message.&quot;);
</ins><span class="cx">         m_errorMessage = message;
</span><ins>+        if (m_errorMessage.isEmpty())
+            m_errorMessage = ASCIILiteral(&quot;Unparseable script&quot;);
</ins><span class="cx">     }
</span><span class="cx">     
</span><span class="cx">     NEVER_INLINE void logError(bool);
</span></span></pre></div>
<a id="trunkSourceWTFChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/ChangeLog (201623 => 201624)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WTF/ChangeLog        2016-06-02 22:43:25 UTC (rev 201623)
+++ trunk/Source/WTF/ChangeLog        2016-06-02 23:07:48 UTC (rev 201624)
</span><span class="lines">@@ -1,3 +1,18 @@
</span><ins>+2016-06-02  Oliver Hunt  &lt;oliver@apple.com&gt;
+
+        JS parser incorrectly handles invalid utf8 in error messages.
+        https://bugs.webkit.org/show_bug.cgi?id=158128
+
+        Reviewed by Saam Barati.
+
+        Add a new toStringWithLatin1Fallback that simply uses
+        String::fromUTF8WithLatin1Fallback, so we can avoid the
+        standard String::fromUTF8 null return.
+
+        * wtf/StringPrintStream.cpp:
+        (WTF::StringPrintStream::toStringWithLatin1Fallback):
+        * wtf/StringPrintStream.h:
+
</ins><span class="cx"> 2016-06-02  Keith Miller  &lt;keith_miller@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Unreviewed, reland r201532. The associated regressions have been fixed
</span></span></pre></div>
<a id="trunkSourceWTFwtfStringPrintStreamcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/wtf/StringPrintStream.cpp (201623 => 201624)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WTF/wtf/StringPrintStream.cpp        2016-06-02 22:43:25 UTC (rev 201623)
+++ trunk/Source/WTF/wtf/StringPrintStream.cpp        2016-06-02 23:07:48 UTC (rev 201624)
</span><span class="lines">@@ -100,6 +100,12 @@
</span><span class="cx">     return String::fromUTF8(m_buffer, m_next);
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+String StringPrintStream::toStringWithLatin1Fallback()
+{
+    ASSERT(m_next == strlen(m_buffer));
+    return String::fromUTF8WithLatin1Fallback(m_buffer, m_next);
+}
+
</ins><span class="cx"> void StringPrintStream::increaseSize(size_t newSize)
</span><span class="cx"> {
</span><span class="cx">     ASSERT_WITH_SECURITY_IMPLICATION(newSize &gt; m_size);
</span></span></pre></div>
<a id="trunkSourceWTFwtfStringPrintStreamh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/wtf/StringPrintStream.h (201623 => 201624)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WTF/wtf/StringPrintStream.h        2016-06-02 22:43:25 UTC (rev 201623)
+++ trunk/Source/WTF/wtf/StringPrintStream.h        2016-06-02 23:07:48 UTC (rev 201624)
</span><span class="lines">@@ -43,6 +43,7 @@
</span><span class="cx">     
</span><span class="cx">     WTF_EXPORT_PRIVATE CString toCString();
</span><span class="cx">     WTF_EXPORT_PRIVATE String toString();
</span><ins>+    WTF_EXPORT_PRIVATE String toStringWithLatin1Fallback();
</ins><span class="cx">     WTF_EXPORT_PRIVATE void reset();
</span><span class="cx">     
</span><span class="cx"> private:
</span></span></pre>
</div>
</div>

</body>
</html>