Closed Bug 1021835 Opened 5 years ago Closed 3 years ago

for-of should throw TypeError if the @@iterator method returns a non-object value

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: jorendorff, Assigned: arai)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

Spotted by André Bargull. See bug 904723 comment 28.

// Primitive values can't be iterators.
var a = {};
a[std_iterator] = () => 0;
Number.prototype.next = () => ({value: undefined, done: true});
assertThrowsInstanceOf(() => Array.from(a), TypeError);
Summary: Array.from should throw TypeError if the @@iterator method returns a non-object value → for-of, Array.from, etc. should throw TypeError if the @@iterator method returns a non-object value
Blocks: es6
See Also: → 1016936
It seems like most code now uses either GetIterator (self-hosted) or JS::ForOfIterator (C++), both check for primitive return values.

So we really just need to emit some extra byte-code to disallow primitives in for-of.

Number.prototype.next = () => ({value: undefined, done: true});
for (let x of {[Symbol.iterator]() { return 0 }}) {}
Summary: for-of, Array.from, etc. should throw TypeError if the @@iterator method returns a non-object value → for-of should throw TypeError if the @@iterator method returns a non-object value
will post patch after test
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
There are several places that does GetIterator, and separated patch for each

with Bytecode (Part 1)
  * array destructuring
  * spread
  * for-of
  * yield*

ForOfIterator::init (Part 2)
  * Promise.all
  * Promise.race

GetIterator self-hosted function (Part 3, Part 4)
  * Array.from
  * %TypedArray%.from
  * %TypedArray% ctor

Inlined for better error message with decompiling (Part 4)
  * Map ctor
  * Set ctor
  * WeakMap ctor
  * WeakSet ctor


in Part 1, added JSOP_CHECKISOBJ after emitIterator.
Attachment #8810010 - Flags: review?(evilpies)
in ForOfIterator::init, replaced ToObject with isObject+toObject, to avoid converting primitive to object.

then, to test Promise.all/race, moved helper functions from js/src/tests/ecma_7/AsyncFunctions/shell.js to js/src/tests/shell.js.
Attachment #8810011 - Flags: review?(till)
in GetIterator, JSMSG_NOT_ITERABLE is used to report invalid iterator, that is wrong.
replaced it with JSMSG_NOT_ITERATOR, that says "iterator" instead of "iterable".
Attachment #8810012 - Flags: review?(evilpies)
just added testcases.
Attachment #8810013 - Flags: review?(evilpies)
Comment on attachment 8810011 [details] [diff] [review]
Part 2: Do not convert iterable[@@iterator]() value to object in ForOfIterator::init.

Review of attachment 8810011 [details] [diff] [review]:
-----------------------------------------------------------------

Nice, r=me
Attachment #8810011 - Flags: review?(till) → review+
Attachment #8810010 - Flags: review?(evilpies) → review+
Attachment #8810012 - Flags: review?(evilpies) → review+
Attachment #8810013 - Flags: review?(evilpies) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/d27d2fec192e62ec05267147c60769b6379bf2e4
Bug 1021835 - Part 1: Emit JSOP_CHECKISOBJ after GetIterator in byte code. r=evilpie

https://hg.mozilla.org/integration/mozilla-inbound/rev/9b186ff95a5ef9ff00237fa72b19a0aa2d620a6d
Bug 1021835 - Part 2: Do not convert iterable[@@iterator]() value to object in ForOfIterator::init. r=till

https://hg.mozilla.org/integration/mozilla-inbound/rev/953d2b8f17036a81990d82eac213622191019b8e
Bug 1021835 - Part 3: Use "iterator" instead of "iterable" in the error message for iterator. r=evilpie

https://hg.mozilla.org/integration/mozilla-inbound/rev/7b4af0b1626b70d331c0769a91faa3f72a876dc2
Bug 1021835 - Part 4: Add testcases for primitive iterator. r=evilpie
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e366a3e90d2f87c8466dd9edf19581029023659
Bug 1021835 - followup: Skip test that needs drainJobQueue in jsreftest. r=bustage
You need to log in before you can comment on or make changes to this bug.