Bug 850086 - Recognize source URLs inside multiline comments. r=tschneidereit
authorEddy Bruel <ejpbruel@mozilla.com>
Tue, 12 Mar 2013 19:27:57 +0100
changeset 124522 a073d9c494259e802711e93b2cb0fac91e55f8b0
parent 124521 b0a8bd999113003d1a67f9dbd4667230e51ce5a7
child 124523 2d4a7e7ca1184926b11cc936ab0553a5673d479a
push id24473
push usertschneidereit@gmail.com
push dateTue, 12 Mar 2013 19:39:41 +0000
treeherdermozilla-inbound@a073d9c49425 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstschneidereit
bugs850086
milestone22.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 850086 - Recognize source URLs inside multiline comments. r=tschneidereit
js/src/frontend/TokenStream.cpp
js/src/frontend/TokenStream.h
js/src/jit-test/tests/debug/Script-sourceMapURL.js
--- a/js/src/frontend/TokenStream.cpp
+++ b/js/src/frontend/TokenStream.cpp
@@ -690,29 +690,45 @@ CharsMatch(const jschar *p, const char *
     while (*q) {
         if (*p++ != *q++)
             return false;
     }
     return true;
 }
 
 bool
-TokenStream::getAtSourceMappingURL()
+TokenStream::getAtSourceMappingURL(bool isMultiline)
 {
-    /* Match comments of the form "//@ sourceMappingURL=<url>" */
-
-    jschar peeked[19];
+    /* Match comments of the form "//@ sourceMappingURL=<url>" or
+     * "/\* //@ sourceMappingURL=<url> *\/"
+     *
+     * To avoid a crashing bug in IE, several JavaScript transpilers
+     * wrap single line comments containing a source mapping URL
+     * inside a multiline comment to avoid a crashing bug in IE. To
+     * avoid potentially expensive lookahead and backtracking, we
+     * only check for this case if we encounter an '@' character.
+     */
+    jschar peeked[18];
     int32_t c;
 
-    if (peekChars(19, peeked) && CharsMatch(peeked, "@ sourceMappingURL=")) {
-        skipChars(19);
+    if (peekChars(18, peeked) && CharsMatch(peeked, " sourceMappingURL=")) {
+        skipChars(18);
         tokenbuf.clear();
 
         while ((c = peekChar()) && c != EOF && !IsSpaceOrBOM2(c)) {
             getChar();
+            /*
+             * Source mapping URLs can occur in both single- and multiline
+             * comments. If we're currently inside a multiline comment, we also
+             * need to recognize multiline comment terminators.
+             */
+            if (isMultiline && c == '*' && peekChar() == '/') {
+                ungetChar('*');
+                break;
+            }
             tokenbuf.append(c);
         }
 
         if (tokenbuf.empty())
             /* The source map's URL was missing, but not quite an exception that
              * we should stop and drop everything for, though. */
             return true;
 
@@ -1419,17 +1435,17 @@ TokenStream::getTokenInternal()
         tt = matchChar('=') ? TOK_MULASSIGN : TOK_STAR;
         break;
 
       case '/':
         /*
          * Look for a single-line comment.
          */
         if (matchChar('/')) {
-            if (!getAtSourceMappingURL())
+            if (matchChar('@') && !getAtSourceMappingURL(false))
                 goto error;
 
   skipline:
             /* Optimize line skipping if we are not in an HTML comment. */
             if (flags & TSF_IN_HTML_COMMENT) {
                 while ((c = getChar()) != EOF && c != '\n') {
                     if (c == '-' && matchChar('-') && matchChar('>'))
                         flags &= ~TSF_IN_HTML_COMMENT;
@@ -1445,17 +1461,18 @@ TokenStream::getTokenInternal()
 
         /*
          * Look for a multi-line comment.
          */
         if (matchChar('*')) {
             unsigned linenoBefore = lineno;
             while ((c = getChar()) != EOF &&
                    !(c == '*' && matchChar('/'))) {
-                /* Ignore all characters until comment close. */
+                if (c == '@' && !getAtSourceMappingURL(true))
+                   goto error;
             }
             if (c == EOF) {
                 reportError(JSMSG_UNTERMINATED_COMMENT);
                 goto error;
             }
             if (linenoBefore != lineno)
                 updateFlagsForEOL();
             cursor = (cursor - 1) & ntokensMask;
--- a/js/src/frontend/TokenStream.h
+++ b/js/src/frontend/TokenStream.h
@@ -762,17 +762,17 @@ class TokenStream
     int32_t getCharIgnoreEOL();
     void ungetChar(int32_t c);
     void ungetCharIgnoreEOL(int32_t c);
     Token *newToken(ptrdiff_t adjust);
     bool peekUnicodeEscape(int32_t *c);
     bool matchUnicodeEscapeIdStart(int32_t *c);
     bool matchUnicodeEscapeIdent(int32_t *c);
     bool peekChars(int n, jschar *cp);
-    bool getAtSourceMappingURL();
+    bool getAtSourceMappingURL(bool isMultiline);
 
     bool matchChar(int32_t expect) {
         int32_t c = getChar();
         if (c == expect)
             return true;
         ungetChar(c);
         return false;
     }
--- a/js/src/jit-test/tests/debug/Script-sourceMapURL.js
+++ b/js/src/jit-test/tests/debug/Script-sourceMapURL.js
@@ -23,19 +23,28 @@ dbg.onDebuggerStatement = function (fram
     fired = true;
     assertEq(frame.script.sourceMapURL, 'file:///var/bar.js.map');
 };
 g.evaluate('(function () { (function () { debugger; })(); })();',
            {sourceMapURL: 'file:///var/bar.js.map'});
 assertEq(fired, true);
 
 // Comment pragmas
+g.evaluate('function f() {}\n' +
+           '//@ sourceMappingURL=file:///var/quux.js.map');
+assertEq(getSourceMapURL(), 'file:///var/quux.js.map');
 
 g.evaluate('function f() {}\n' +
-           '//@ sourceMappingURL=file:///var/quux.js.map');
+           '/*//@ sourceMappingURL=file:///var/quux.js.map*/');
+assertEq(getSourceMapURL(), 'file:///var/quux.js.map');
+
+g.evaluate('function f() {}\n' +
+           '/*\n' +
+           '//@ sourceMappingURL=file:///var/quux.js.map\n' +
+           '*/');
 assertEq(getSourceMapURL(), 'file:///var/quux.js.map');
 
 // Spaces are disallowed by the URL spec (they should have been
 // percent-encoded).
 g.evaluate('function f() {}\n' +
            '//@ sourceMappingURL=http://example.com/has illegal spaces.map');
 assertEq(getSourceMapURL(), 'http://example.com/has');