Bug 171310 - We incorrectly allow escaped characters in keyword tokens
Summary: We incorrectly allow escaped characters in keyword tokens
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: GSkachkov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-25 19:08 PDT by Saam Barati
Modified: 2017-06-12 11:24 PDT (History)
13 users (show)

See Also:


Attachments
Patch (3.02 KB, patch)
2017-06-09 14:12 PDT, GSkachkov
no flags Details | Formatted Diff | Diff
Patch (28.28 KB, patch)
2017-06-10 02:50 PDT, GSkachkov
no flags Details | Formatted Diff | Diff
Patch (30.84 KB, patch)
2017-06-10 14:17 PDT, GSkachkov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2017-04-25 19:08:35 PDT
e.g, we parse: "i\u006E" as "in"
Comment 1 GSkachkov 2017-06-09 10:01:08 PDT
There is a link to spec https://tc39.github.io/ecma262/#sec-reserved-words
Comment 2 GSkachkov 2017-06-09 14:12:23 PDT
Created attachment 312488 [details]
Patch

WIP: test is coming
Comment 3 Build Bot 2017-06-09 14:52:43 PDT
Comment on attachment 312488 [details]
Patch

Attachment 312488 [details] did not pass jsc-ews (mac):
Output: http://webkit-queues.webkit.org/results/3902642

New failing tests:
stress/error-messages-for-in-operator-should-not-crash.js.ftl-no-cjit-no-inline-validate
stress/error-messages-for-in-operator-should-not-crash.js.dfg-eager
stress/reserved-word-with-escape.js.no-ftl
stress/destructuring-assignment-syntax.js.dfg-eager-no-cjit-validate
stress/reserved-word-with-escape.js.ftl-no-cjit-b3o1
stress/error-messages-for-in-operator-should-not-crash.js.ftl-eager
stress/error-messages-for-in-operator-should-not-crash.js.ftl-eager-no-cjit
stress/reserved-word-with-escape.js.no-llint
stress/reserved-word-with-escape.js.ftl-eager
stress/destructuring-assignment-syntax.js.ftl-eager-no-cjit-b3o1
stress/error-messages-for-in-operator-should-not-crash.js.dfg-maximal-flush-validate-no-cjit
stress/destructuring-assignment-syntax.js.ftl-no-cjit-validate-sampling-profiler
stress/error-messages-for-in-operator-should-not-crash.js.ftl-eager-no-cjit-b3o1
stress/error-messages-for-in-operator-should-not-crash.js.default
stress/reserved-word-with-escape.js.dfg-maximal-flush-validate-no-cjit
stress/error-messages-for-in-operator-should-not-crash.js.ftl-no-cjit-no-put-stack-validate
stress/destructuring-assignment-syntax.js.no-ftl
stress/error-messages-for-in-operator-should-not-crash.js.dfg-eager-no-cjit-validate
stress/destructuring-assignment-syntax.js.no-cjit-collect-continuously
stress/destructuring-assignment-syntax.js.ftl-eager-no-cjit
stress/destructuring-assignment-syntax.js.no-cjit-validate-phases
stress/reserved-word-with-escape.js.ftl-eager-no-cjit-b3o1
stress/reserved-word-with-escape.js.ftl-no-cjit-no-put-stack-validate
stress/destructuring-assignment-syntax.js.no-llint
stress/destructuring-assignment-syntax.js.default
stress/destructuring-assignment-syntax.js.ftl-no-cjit-no-put-stack-validate
stress/reserved-word-with-escape.js.dfg-eager-no-cjit-validate
stress/error-messages-for-in-operator-should-not-crash.js.ftl-no-cjit-small-pool
stress/error-messages-for-in-operator-should-not-crash.js.no-llint
ChakraCore.yaml/ChakraCore/test/Basics/IdsWithEscapes.js.default
stress/destructuring-assignment-syntax.js.ftl-no-cjit-small-pool
stress/error-messages-for-in-operator-should-not-crash.js.no-cjit-collect-continuously
stress/reserved-word-with-escape.js.default
stress/destructuring-assignment-syntax.js.ftl-no-cjit-b3o1
stress/reserved-word-with-escape.js.no-cjit-collect-continuously
stress/error-messages-for-in-operator-should-not-crash.js.ftl-no-cjit-validate-sampling-profiler
stress/error-messages-for-in-operator-should-not-crash.js.no-cjit-validate-phases
stress/reserved-word-with-escape.js.ftl-no-cjit-validate-sampling-profiler
stress/destructuring-assignment-syntax.js.dfg-eager
stress/reserved-word-with-escape.js.dfg-eager
stress/error-messages-for-in-operator-should-not-crash.js.no-ftl
stress/destructuring-assignment-syntax.js.dfg-maximal-flush-validate-no-cjit
stress/destructuring-assignment-syntax.js.ftl-eager
stress/reserved-word-with-escape.js.ftl-eager-no-cjit
stress/reserved-word-with-escape.js.ftl-no-cjit-small-pool
stress/destructuring-assignment-syntax.js.ftl-no-cjit-no-inline-validate
stress/reserved-word-with-escape.js.ftl-no-cjit-no-inline-validate
stress/reserved-word-with-escape.js.no-cjit-validate-phases
stress/error-messages-for-in-operator-should-not-crash.js.ftl-no-cjit-b3o1
Comment 4 GSkachkov 2017-06-10 02:50:44 PDT
Created attachment 312568 [details]
Patch

Patch
Comment 5 Build Bot 2017-06-10 03:32:13 PDT
Comment on attachment 312568 [details]
Patch

Attachment 312568 [details] did not pass jsc-ews (mac):
Output: http://webkit-queues.webkit.org/results/3906552

New failing tests:
ChakraCore.yaml/ChakraCore/test/Basics/IdsWithEscapes.js.default
Comment 6 GSkachkov 2017-06-10 14:17:49 PDT
Created attachment 312579 [details]
Patch

Fix tests
Comment 7 Yusuke Suzuki 2017-06-12 06:57:22 PDT
Comment on attachment 312579 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=312579&action=review

> Source/JavaScriptCore/ChangeLog:10
> +        Current patch implements this requirements.

The spec text says

"The ReservedWord definitions are specified as literal sequences of specific SourceCharacter elements. A code point in a ReservedWord cannot be expressed by a \ UnicodeEscapeSequence."

Does it mean that fals\x65 is allowed?
Comment 8 GSkachkov 2017-06-12 10:13:56 PDT
Comment on attachment 312579 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=312579&action=review

>> Source/JavaScriptCore/ChangeLog:10
>> +        Current patch implements this requirements.
> 
> The spec text says
> 
> "The ReservedWord definitions are specified as literal sequences of specific SourceCharacter elements. A code point in a ReservedWord cannot be expressed by a \ UnicodeEscapeSequence."
> 
> Does it mean that fals\x65 is allowed?

No, I think that it is not allowed. 
According to spec ReservedWord is "A reserved word is an `IdentifierName` that cannot be used as an Identifier.", and the IdentifierName according to the https://tc39.github.io/ecma262/#prod-IdentifierName allows to use  \UnicodeEscapeSequence and UnicodeIDStart, $, _, so this NOTE means that we remove from the list of allowed symbols `\UnicodeEscapeSequence`, but it does not mean that we can use other symbols expect UnicodeIDStart/UnicodeIDContinue, $, _
Comment 9 Yusuke Suzuki 2017-06-12 10:53:30 PDT
Comment on attachment 312579 [details]
Patch

r=me
Comment 10 WebKit Commit Bot 2017-06-12 11:24:03 PDT
Comment on attachment 312579 [details]
Patch

Clearing flags on attachment: 312579

Committed r218111: <http://trac.webkit.org/changeset/218111>
Comment 11 WebKit Commit Bot 2017-06-12 11:24:05 PDT
All reviewed patches have been landed.  Closing bug.