Bug 135704 – JSC Lexer is allowing octals 08 and 09 in strict mode functions

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

[Oliver Hunt]

(function() { "use strict"; return 08.9 })()

This should be a syntax error.

[Radar WebKit Bug Importer]

<rdar://problem/17946715>

[Oliver Hunt]

*** Bug 126618 has been marked as a duplicate of this bug. ***

[Mathias Bynens]

(In reply to comment #0)
> (function() { "use strict"; return 08.9 })()
> 
> This should be a syntax error.

For future reference: as per the current spec, it should be a syntax error even in non-strict mode, but unfortunately that ship has sailed. See http://esdiscuss.org/topic/early-error-on-0-followed-by-8-or-9-in-numeric-literals-does-not-seem-to-be-web-compatible for the rationale on why banning it in strict mode might be a good idea for now.

[Diego Pino]

Created attachment 236280 [details]
Patch

[Diego Pino]

Sorry Oliver Hunt, I didn't realized it was assigned to you.

[Oliver Hunt]

Comment on attachment 236280 [details]
Patch

r=me, but we could you add a test case? easiest is probably to add it to js/parser-syntax-check .  Thanks for the patch :)

[Mathias Bynens]

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

> Source/JavaScriptCore/parser/Lexer.cpp:1668
> +            m_lexErrorMessage = "Octal escapes are forbidden in strict mode";

`08` and `09` are not octal escapes, since `8` and `9` are not octal digits.

Better error message (+ commit message):

“Decimal integer literals with a leading zero are forbidden in strict mode”

[Diego Pino]

Created attachment 236299 [details]
Patch

[Oliver Hunt]

Thanks!

[Mathias Bynens]

(In reply to comment #8)
> Created an attachment (id=236299) [details]
> Patch

Commit message (first line) still incorrectly mentions “octals”.

[Build Bot]

Comment on attachment 236299 [details]
Patch

Attachment 236299 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6206489106579456

New failing tests:
js/basic-strict-mode.html

[Build Bot]

Created attachment 236302 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 236299 [details]
Patch

Attachment 236299 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5176058806534144

New failing tests:
js/basic-strict-mode.html

[Build Bot]

Created attachment 236305 [details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-02  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 236299 [details]
Patch

Attachment 236299 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6012677633605632

New failing tests:
js/basic-strict-mode.html

[Build Bot]

Created attachment 236315 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Diego Pino]

Apparently there's a test for octal numbers in strict mode in js/basic-strict-mode. Since I have to resend the patch, should I move the tests I added to js/parser-syntax-check-expected to js/basic-strict-mode?

[Diego Pino]

(In reply to comment #10)
> (In reply to comment #8)
> > Created an attachment (id=236299) [details] [details]
> > Patch
> 
> Commit message (first line) still incorrectly mentions “octals”.

Generally the first line of the commit message matches the title of the bug in the bugzilla. I think is better not to modify the commit message unless the title of the bug is modified too.

[Oliver Hunt]

(In reply to comment #17)
> Apparently there's a test for octal numbers in strict mode in js/basic-strict-mode. Since I have to resend the patch, should I move the tests I added to js/parser-syntax-check-expected to js/basic-strict-mode?

fix the basic-strict-mode test, and leave your new tests in the parser-syntax-check test

[Diego Pino]

Created attachment 236333 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 236333 [details]
Patch

Clearing flags on attachment: 236333

Committed r172380: <http://trac.webkit.org/changeset/172380>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Oliver Hunt]

Thanks!