Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unicode line terminators not supported in string literals #235

Closed
djschilling opened this issue May 4, 2020 · 6 comments
Closed

Unicode line terminators not supported in string literals #235

djschilling opened this issue May 4, 2020 · 6 comments
Assignees

Comments

@djschilling
Copy link

There are 4 Unicode caracters that i know of that are not supported by hermes:

There are probably more, but these i had problems with:

Executing this with hermes leads to these errors:
var foo = "
";

test.js:1:12: error: non-terminated string
var foo = "
";
test.js:1:11: note: string started here
var foo = "
";
test.js:1:17: error: non-terminated string
var foo = "
";
test.js:1:15: note: string started here
var foo = "
";

The unicode Character used in this example is U+2028.

The problem occurred to me using this lib: https://github.com/nodeca/unhomoglyph/blob/master/data.json#L160

@tmikov
Copy link
Contributor

tmikov commented May 4, 2020

Hermes doesn't try to interpret most Unicode characters, so there shouldn't be anything to support. But there could be bugs, of course.

What is the encoding of the input? Can you post the contents of a small example as binary or upload it somewhere?

@djschilling
Copy link
Author

Here is a file that produces the error described above: https://gist.github.com/djschilling/12605071bdfafd5d817ece1d12911d8c

In the browser it seems like its an empty string, but actually there is the Unicode Character U+2028 in the string.

@puzrin
Copy link

puzrin commented May 4, 2020

Do you load js file to hermes directly or it passes server/browser? In second case problem may be related to bad server encoding (or missed charset meta). If related to encoding, there are 2 possibilities:

  • fix encoding (preferred)
  • force app builder use ASCII (usually all minifiers have such option)

@tmikov
Copy link
Contributor

tmikov commented May 4, 2020

This is a subtle one. I didn't realize this immediately, but we consider \u2028 and \u2029 to be line terminators, that's why we are reporting a non-terminated string error. Here is a link to the ES9 spec describing our current behavior:
https://www.ecma-international.org/ecma-262/9.0/index.html#sec-literals-string-literals

This was changed in ES10, so \u2028 and \u2029 are now considered valid in a string, precisely for compatibility with JSON. We have not caught up with ES10 yet, but I think we can easily implement this specifically. Thanks for reporting it!

About the other characters from your list: what errors are you getting with them? When I tried them, they worked fine.

@tmikov tmikov self-assigned this May 4, 2020
@tmikov tmikov changed the title Unicode support not complete? Unicode line terminators not supported in string literals May 4, 2020
@djschilling
Copy link
Author

djschilling commented May 5, 2020

@puzrin

I execute it like that: ./bin/hermes unicode-hermes.js

@tmikov You are right only \u2028 and \u2029 are causing the issue. Thanks for investigating. I'm looking forward to a new version where it will be fixed.

tmikov added a commit to tmikov/hermes that referenced this issue May 5, 2020
Summary:
ES10 allows U+2028 (LINE SEPARATOR), U+2029 (PARAGRAPH SEPARATOR) in
string literals for compatibility with JSON. Remove the existing check.
Add a test.

Github: Closes facebook#235

Reviewed By: avp

Differential Revision: D21386937

fbshipit-source-id: 0cbf2d2ff449c33f6f1f3b42ad28ed3650650b36
tmikov added a commit to tmikov/hermes that referenced this issue May 6, 2020
Summary:
ES10 allows U+2028 (LINE SEPARATOR), U+2029 (PARAGRAPH SEPARATOR) in
string literals for compatibility with JSON. Remove the existing check.
Add a test.

Github: Closes facebook#235

Reviewed By: avp

Differential Revision: D21386937

fbshipit-source-id: 0cbf2d2ff449c33f6f1f3b42ad28ed3650650b36
@tmikov
Copy link
Contributor

tmikov commented May 6, 2020

@djschilling the fix landed and will be included in 0.6.0 as well as in the next patch release 0.5.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants