Bug 205983 – Object.keys should throw if called on module namespace object with uninitialized binding

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Alexey Shvayka]

ECMA262: call stack is quite deep, please see "info" meta of the test ↓
Test262: https://test262.report/browse/language/module-code/namespace/internals/object-keys-binding-uninit.js

[Alexey Shvayka]

Created attachment 387186 [details]
Patch

[Alexey Shvayka]

(In reply to Alexey Shvayka from comment #1)
> Created attachment 387186 [details]
> Patch

All credit goes to Caitlin Potter: this patch was nicely extracted from her solution to https://bugs.webkit.org/show_bug.cgi?id=189034.

[Ross Kirsling]

Comment on attachment 387186 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:1
> [phone]  Caitlin Potter  <[e-mail]>

I recently learned (from r253987) that it's valid to insert two authors separated by "and" here -- might make sense in a case like this?

> Source/JavaScriptCore/ChangeLog:10
> +        If [[OwnPropertyKeys]] method of module namespace object is called by
> +        Object.keys or for/in loop, it should invoke [[GetOwnProperty]] on
> +        every binding so a ReferenceError is thrown if the binding is uninitialized.

Wait, is [[OwnPropertyKeys]] directly relevant? It doesn't throw.

Seems like [[GetOwnProperty]] is called from EnumerableOwnPropertyNames *after* [[OwnPropertyKeys]], no?
https://tc39.es/ecma262/#sec-enumerableownpropertynames

[Alexey Shvayka]

Created attachment 387383 [details]
Patch

Add myself as co-author and fix method name in ChangeLog.

[Alexey Shvayka]

(In reply to Ross Kirsling from comment #3)
>
> I recently learned (from r253987) that it's valid to insert two authors
> separated by "and" here -- might make sense in a case like this?

Thank you for the tip.
I think it makes sense to add myself there for bookkeeping purposes or so I could be contacted regarding the change.

> Seems like [[GetOwnProperty]] is called from EnumerableOwnPropertyNames
> *after* [[OwnPropertyKeys]], no?
> https://tc39.es/ecma262/#sec-enumerableownpropertynames

Nice catch! Indeed, throwing occurs after [[OwnPropertyKeys]].
I've replaced [[OwnPropertyKeys]] with JSModuleNamespaceObject::getOwnPropertyNames.

[Yusuke Suzuki]

Comment on attachment 387383 [details]
Patch

r=me

[Yusuke Suzuki]

(In reply to Yusuke Suzuki from comment #6)
> Comment on attachment 387383 [details]
> Patch
> 
> r=me

Land it after resolving Ross's comment :)

[Ross Kirsling]

Comment on attachment 387383 [details]
Patch

Alexey's comment came after his new patch, so it should be okay :D

[WebKit Commit Bot]

Comment on attachment 387383 [details]
Patch

Clearing flags on attachment: 387383

Committed r254390: <https://trac.webkit.org/changeset/254390>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/58499079>