Skip to content

Commit

Permalink
[JSC] Close Iterator Helpers underlying iterator when arguments are i…
Browse files Browse the repository at this point in the history
…nvalid

https://bugs.webkit.org/show_bug.cgi?id=284709

Reviewed by Yusuke Suzuki.

The normative change[1] that requires closing underlying iterators when
argument validation fails for iterator helpers reached consensus at the
TC39 meeting in December 2024[2].

This patch implements it.

[1]: tc39/ecma262#3467
[2]: https://github.com/tc39/agendas/blob/main/2024/12.md

* JSTests/stress/iterator-helpers-close-for-invalid-argument.js: Added.
(shouldThrow):
(shouldBe):
(throw.new.Error.let.closable.get next):
(throw.new.Error):
(shouldBe.let.closable.get next):
(shouldBe.OurError):
(let.closable.get next):
(shouldBe.get shouldBe):
(OurError):
(get shouldBe):
* Source/JavaScriptCore/builtins/JSIteratorPrototype.js:
(some.wrapper.iterator):
(some):
(every.wrapper.iterator):
(every):
(find.wrapper.iterator):
(find):
(reduce):

Canonical link: https://commits.webkit.org/290907@main
  • Loading branch information
sosukesuzuki committed Feb 23, 2025
1 parent febcbad commit 6e8fef4
Show file tree
Hide file tree
Showing 4 changed files with 344 additions and 51 deletions.
296 changes: 296 additions & 0 deletions JSTests/stress/iterator-helpers-close-for-invalid-argument.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,296 @@
function shouldThrow(errorType, func) {
let error;
try {
func();
} catch (e) {
error = e;
}
if (!(error instanceof errorType)) {
print(error.message);
throw new Error(`Expected ${errorType.name}! got ${error.name}`);
}
}

function shouldBe(a, b) {
if (a !== b)
throw new Error(`Expected ${b} but got ${a}`);
}

{
// Iterator.prototype.map
let closed = false;
let closable = {
__proto__: Iterator.prototype,
get next() {
throw new Error('next should not be read');
},
return() {
closed = true;
return {};
},
};
shouldThrow(TypeError, function() {
closable.map();
});
shouldBe(closed, true);

closed = false;
shouldThrow(TypeError, function() {
closable.map({});
});
shouldBe(closed, true);
}

{
// Iterator.prototype.filter
let closed = false;
let closable = {
__proto__: Iterator.prototype,
get next() {
throw new Error('next should not be read');
},
return() {
closed = true;
return {};
},
};
shouldThrow(TypeError, function() {
closable.filter();
});
shouldBe(closed, true);

closed = false;
shouldThrow(TypeError, function() {
closable.filter({});
});
shouldBe(closed, true);
}

{
// Iterator.prototype.take
let closed = false;
let closable = {
__proto__: Iterator.prototype,
get next() {
throw new Error('next should not be read');
},
return() {
closed = true;
return {};
},
};

shouldThrow(RangeError, function() {
closable.take();
});
shouldBe(closed, true);

closed = false;
shouldThrow(RangeError, function() {
closable.take(NaN);
});
shouldBe(closed, true);

closed = false;
shouldThrow(RangeError, function() {
closable.take(-1);
});
shouldBe(closed, true);

closed = false;
function OurError() {}
shouldThrow(OurError, function() {
closable.take({ get valueOf() { throw new OurError(); }});
});
shouldBe(closed, true);
}

{
// Iterator.prototype.drop
let closed = false;
let closable = {
__proto__: Iterator.prototype,
get next() {
throw new Error('next should not be read');
},
return() {
closed = true;
return {};
},
};

shouldThrow(RangeError, function() {
closable.drop();
});
shouldBe(closed, true);

closed = false;
shouldThrow(RangeError, function() {
closable.drop(NaN);
});
shouldBe(closed, true);

closed = false;
shouldThrow(RangeError, function() {
closable.drop(-1);
});
shouldBe(closed, true);

closed = false;
function OurError() {}
shouldThrow(OurError, function() {
closable.drop({ get valueOf() { throw new OurError(); }});
});
shouldBe(closed, true);
}

{
// Iterator.prototype.flatMap
let closed = false;
let closable = {
__proto__: Iterator.prototype,
get next() {
throw new Error('next should not be read');
},
return() {
closed = true;
return {};
},
};
shouldThrow(TypeError, function() {
closable.flatMap();
});
shouldBe(closed, true);

closed = false;
shouldThrow(TypeError, function() {
closable.flatMap({});
});
shouldBe(closed, true);
}

{
// Iterator.prototype.some
let closed = false;
let closable = {
__proto__: Iterator.prototype,
get next() {
throw new Error('next should not be read');
},
return() {
closed = true;
return {};
},
};
shouldThrow(TypeError, function() {
closable.some();
});
shouldBe(closed, true);

closed = false;
shouldThrow(TypeError, function() {
closable.some({});
});
shouldBe(closed, true);
}

{
// Iterator.prototype.every
let closed = false;
let closable = {
__proto__: Iterator.prototype,
get next() {
throw new Error('next should not be read');
},
return() {
closed = true;
return {};
},
};
shouldThrow(TypeError, function() {
closable.every();
});
shouldBe(closed, true);

closed = false;
shouldThrow(TypeError, function() {
closable.every({});
});
shouldBe(closed, true);
}

{
// Iterator.prototype.find
let closed = false;
let closable = {
__proto__: Iterator.prototype,
get next() {
throw new Error('next should not be read');
},
return() {
closed = true;
return {};
},
};
shouldThrow(TypeError, function() {
closable.find();
});
shouldBe(closed, true);

closed = false;
shouldThrow(TypeError, function() {
closable.find({});
});
shouldBe(closed, true);
}

{
// Iterator.prototype.reduce
let closed = false;
let closable = {
__proto__: Iterator.prototype,
get next() {
throw new Error('next should not be read');
},
return() {
closed = true;
return {};
},
};
shouldThrow(TypeError, function() {
closable.reduce();
});
shouldBe(closed, true);

closed = false;
shouldThrow(TypeError, function() {
closable.reduce({});
});
shouldBe(closed, true);
}

{
// Iterator.prototype.forEach
let closed = false;
let closable = {
__proto__: Iterator.prototype,
get next() {
throw new Error('next should not be read');
},
return() {
closed = true;
return {};
},
};
shouldThrow(TypeError, function() {
closable.forEach();
});
shouldBe(closed, true);

closed = false;
shouldThrow(TypeError, function() {
closable.forEach({});
});
shouldBe(closed, true);
}

30 changes: 0 additions & 30 deletions JSTests/test262/expectations.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,36 +25,6 @@ test/built-ins/Iterator/concat/fresh-iterator-result.js:
test/built-ins/Iterator/concat/next-method-returns-throwing-value.js:
default: 'Test262Error: '
strict mode: 'Test262Error: '
test/built-ins/Iterator/prototype/drop/argument-validation-failure-closes-underlying.js:
default: 'Test262Error: Expected SameValue(«false», «true») to be true'
strict mode: 'Test262Error: Expected SameValue(«false», «true») to be true'
test/built-ins/Iterator/prototype/every/argument-validation-failure-closes-underlying.js:
default: 'Test262Error: Expected SameValue(«false», «true») to be true'
strict mode: 'Test262Error: Expected SameValue(«false», «true») to be true'
test/built-ins/Iterator/prototype/filter/argument-validation-failure-closes-underlying.js:
default: 'Test262Error: Expected SameValue(«false», «true») to be true'
strict mode: 'Test262Error: Expected SameValue(«false», «true») to be true'
test/built-ins/Iterator/prototype/find/argument-validation-failure-closes-underlying.js:
default: 'Test262Error: Expected SameValue(«false», «true») to be true'
strict mode: 'Test262Error: Expected SameValue(«false», «true») to be true'
test/built-ins/Iterator/prototype/flatMap/argument-validation-failure-closes-underlying.js:
default: 'Test262Error: Expected SameValue(«false», «true») to be true'
strict mode: 'Test262Error: Expected SameValue(«false», «true») to be true'
test/built-ins/Iterator/prototype/forEach/argument-validation-failure-closes-underlying.js:
default: 'Test262Error: Expected SameValue(«false», «true») to be true'
strict mode: 'Test262Error: Expected SameValue(«false», «true») to be true'
test/built-ins/Iterator/prototype/map/argument-validation-failure-closes-underlying.js:
default: 'Test262Error: Expected SameValue(«false», «true») to be true'
strict mode: 'Test262Error: Expected SameValue(«false», «true») to be true'
test/built-ins/Iterator/prototype/reduce/argument-validation-failure-closes-underlying.js:
default: 'Test262Error: Expected SameValue(«false», «true») to be true'
strict mode: 'Test262Error: Expected SameValue(«false», «true») to be true'
test/built-ins/Iterator/prototype/some/argument-validation-failure-closes-underlying.js:
default: 'Test262Error: Expected SameValue(«false», «true») to be true'
strict mode: 'Test262Error: Expected SameValue(«false», «true») to be true'
test/built-ins/Iterator/prototype/take/argument-validation-failure-closes-underlying.js:
default: 'Test262Error: Expected SameValue(«false», «true») to be true'
strict mode: 'Test262Error: Expected SameValue(«false», «true») to be true'
test/built-ins/Object/entries/order-after-define-property-with-function.js:
default: 'Test262Error: Actual [a, name] and expected [name, a] should have the same contents. '
strict mode: 'Test262Error: Actual [a, name] and expected [name, a] should have the same contents. '
Expand Down
Loading

0 comments on commit 6e8fef4

Please # to comment.