Bug 16695 – JSC allows non-identifier codepoints in identifiers (affects Acid3)

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Eric Seidel (no email)]

JSC allows non-identifier codepoints in identifiers (Acid3 bug)

We don't throw a parse error exception here like we should:

    function () {
      // test 85: ES3 section 7.3 (unicode escapes can't be used to put non-identifier characters into identifiers)
      // and there's no other place for them in the syntax (other than strings, of course)
      var ok = false;
      try {
        eval("var test = { };\ntest.i= 0;\ntest.i\\u002b= 1;\ntest.i;\n");
      } catch (e) {
        ok = true;
      }
      assert(ok, "\\u002b was not considered a parse error in script");
      return 6;
    },

[Alexey Proskuryakov]

FWIW, Firefox fails this test, too.

Strangely, in my copy of ECMA-262 3rd edition, it's section 7.6, not 7.3.

[Darin Adler]

Yes, it's section 7.6 -- I think that's just a mistake in Hixie's comment.

[Darin Adler]

Created attachment 18227 [details]
patch

[Eric Seidel (no email)]

Comment on attachment 18227 [details]
patch

1.  I'm confused by your testing of \u vs \\u at the end
shouldBe("var test = { }; test.i= 0; test.i\u002b= 1; test.i;", "1");
The fact that that is supposed to work and \\u isn't, confuses me, and calls into question all the other \\u tests above
I guess I'm just not sure what '\u' would be interpreted as, and why it's a valid identifier char.
2.  I don't see a test for valid identifier part unicodes used at a identifier start
3.  it looks like the logic for checking \u could be squashed into the two "call sites" instead of making a new state
(which would be faster, and fewer states)

Assuming you address those comments via IRC or here, the patch looks great. :)  Marking r+ since I don't need to see another patch.

[Ian 'Hixie' Hickson]

This:
   shouldBe("var test = { }; test.i= 0; test.i\u002b= 1; test.i;", "1");
...is exactly equivalen to:
   shouldBe("var test = { }; test.i= 0; test.i+= 1; test.i;", "1");
...because the backslash escape sequence is interpreted when the original script is parsed, not when the eval() script is parsed.

[Darin Adler]

(In reply to comment #5)
> This:
>    shouldBe("var test = { }; test.i= 0; test.i\u002b= 1; test.i;", "1");
> ...is exactly equivalen to:
>    shouldBe("var test = { }; test.i= 0; test.i+= 1; test.i;", "1");
> ...because the backslash escape sequence is interpreted when the original
> script is parsed, not when the eval() script is parsed.

Yes. Eric was quoting one of two different test lines. The line above has two backslashes and expects an exception rather than the value 1.

[Darin Adler]

r29075