From 75c9377766326589faa844a95d5997a156f6aed0 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Thu, 15 Mar 2018 21:11:35 +0100 Subject: Close iterator after error in {Map,Set,WeakMap,WeakSet} constructors Issue #17 --- js/src/builtin/Map.js | 11 +- js/src/builtin/Set.js | 7 +- js/src/builtin/Utilities.js | 23 ++ js/src/builtin/WeakMap.js | 11 +- js/src/builtin/WeakSet.js | 7 +- .../tests/ecma_6/Map/constructor-iterator-close.js | 253 +++++++++++++++++++++ 6 files changed, 306 insertions(+), 6 deletions(-) create mode 100644 js/src/tests/ecma_6/Map/constructor-iterator-close.js diff --git a/js/src/builtin/Map.js b/js/src/builtin/Map.js index 432364614..27a12bfff 100644 --- a/js/src/builtin/Map.js +++ b/js/src/builtin/Map.js @@ -40,11 +40,18 @@ function MapConstructorInit(iterable) { var nextItem = next.value; // Step 8.d. - if (!IsObject(nextItem)) + if (!IsObject(nextItem)) { + IteratorCloseThrow(iter); ThrowTypeError(JSMSG_INVALID_MAP_ITERABLE, "Map"); + } // Steps 8.e-j. - callContentFunction(adder, map, nextItem[0], nextItem[1]); + try { + callContentFunction(adder, map, nextItem[0], nextItem[1]); + } catch (e) { + IteratorCloseThrow(iter); + throw e; + } } } diff --git a/js/src/builtin/Set.js b/js/src/builtin/Set.js index accc70120..c61a49ef8 100644 --- a/js/src/builtin/Set.js +++ b/js/src/builtin/Set.js @@ -40,7 +40,12 @@ function SetConstructorInit(iterable) { var nextValue = next.value; // Steps 8.d-e. - callContentFunction(adder, set, nextValue); + try { + callContentFunction(adder, set, nextValue); + } catch (e) { + IteratorCloseThrow(iter); + throw e; + } } } diff --git a/js/src/builtin/Utilities.js b/js/src/builtin/Utilities.js index bfb1fe7f4..2dece3801 100644 --- a/js/src/builtin/Utilities.js +++ b/js/src/builtin/Utilities.js @@ -154,6 +154,29 @@ function GetIterator(obj, method) { return iterator; } +// ES2017 draft rev 7.4.6. +// When completion.[[Type]] is throw. +function IteratorCloseThrow(iter) { + // Steps 1-2 (implicit) + + // Step 3. + var returnMethod = GetMethod(iter, "return"); + + // Step 4 (done in caller). + if (returnMethod === undefined) + return; + + try { + // Step 5. + callContentFunction(returnMethod, iter); + } catch (e) { + } + + // Step 6 (done in caller). + + // Steps 7-9 (skipped). +} + var _builtinCtorsCache = {__proto__: null}; function GetBuiltinConstructor(builtinName) { diff --git a/js/src/builtin/WeakMap.js b/js/src/builtin/WeakMap.js index 066a72bfe..708be8424 100644 --- a/js/src/builtin/WeakMap.js +++ b/js/src/builtin/WeakMap.js @@ -40,10 +40,17 @@ function WeakMapConstructorInit(iterable) { var nextItem = next.value; // Step 8.d. - if (!IsObject(nextItem)) + if (!IsObject(nextItem)) { + IteratorCloseThrow(iter); ThrowTypeError(JSMSG_INVALID_MAP_ITERABLE, "WeakMap"); + } // Steps 8.e-j. - callContentFunction(adder, map, nextItem[0], nextItem[1]); + try { + callContentFunction(adder, map, nextItem[0], nextItem[1]); + } catch (e) { + IteratorCloseThrow(iter); + throw e; + } } } diff --git a/js/src/builtin/WeakSet.js b/js/src/builtin/WeakSet.js index eb7c2378f..8589f9dc6 100644 --- a/js/src/builtin/WeakSet.js +++ b/js/src/builtin/WeakSet.js @@ -40,7 +40,12 @@ function WeakSetConstructorInit(iterable) { var nextValue = next.value; // Steps 8.d-e. - callContentFunction(adder, set, nextValue); + try { + callContentFunction(adder, set, nextValue); + } catch (e) { + IteratorCloseThrow(iter); + throw e; + } } } diff --git a/js/src/tests/ecma_6/Map/constructor-iterator-close.js b/js/src/tests/ecma_6/Map/constructor-iterator-close.js new file mode 100644 index 000000000..ba4bee4a7 --- /dev/null +++ b/js/src/tests/ecma_6/Map/constructor-iterator-close.js @@ -0,0 +1,253 @@ +var BUGNUMBER = 1180306; +var summary = 'Map/Set/WeakMap/WeakSet constructor should close iterator on error'; + +print(BUGNUMBER + ": " + summary); + +function test(ctors, { nextVal=undefined, + nextThrowVal=undefined, + modifier=undefined, + exceptionVal=undefined, + exceptionType=undefined, + closed=true }) { + function getIterable() { + let iterable = { + closed: false, + [Symbol.iterator]() { + let iterator = { + first: true, + next() { + if (this.first) { + this.first = false; + if (nextThrowVal) + throw nextThrowVal; + return nextVal; + } + return { value: undefined, done: true }; + }, + return() { + iterable.closed = true; + return {}; + } + }; + if (modifier) + modifier(iterator, iterable); + + return iterator; + } + }; + return iterable; + } + + for (let ctor of ctors) { + let iterable = getIterable(); + if (exceptionVal) { + let caught = false; + try { + new ctor(iterable); + } catch (e) { + assertEq(e, exceptionVal); + caught = true; + } + assertEq(caught, true); + } else if (exceptionType) { + assertThrowsInstanceOf(() => new ctor(iterable), exceptionType); + } else { + new ctor(iterable); + } + assertEq(iterable.closed, closed); + } +} + +// == Error cases with close == + +// ES 2017 draft 23.1.1.1 Map step 8.d.ii. +// ES 2017 draft 23.3.1.1 WeakMap step 8.d.ii. +test([Map, WeakMap], { + nextVal: { value: "non object", done: false }, + exceptionType: TypeError, + closed: true, +}); + +// ES 2017 draft 23.1.1.1 Map step 8.f. +// ES 2017 draft 23.3.1.1 WeakMap step 8.f. +test([Map, WeakMap], { + nextVal: { value: { get 0() { throw "0 getter throws"; } }, done: false }, + exceptionVal: "0 getter throws", + closed: true, +}); + +// ES 2017 draft 23.1.1.1 Map step 8.h. +// ES 2017 draft 23.3.1.1 WeakMap step 8.h. +test([Map, WeakMap], { + nextVal: { value: { 0: {}, get 1() { throw "1 getter throws"; } }, done: false }, + exceptionVal: "1 getter throws", + closed: true, +}); + +// ES 2017 draft 23.1.1.1 Map step 8.j. +// ES 2017 draft 23.3.1.1 WeakMap step 8.j. +class MyMap extends Map { + set(k, v) { + throw "setter throws"; + } +} +class MyWeakMap extends WeakMap { + set(k, v) { + throw "setter throws"; + } +} +test([MyMap, MyWeakMap], { + nextVal: { value: [{}, {}], done: false }, + exceptionVal: "setter throws", + closed: true, +}); + +// ES 2017 draft 23.2.1.1 Set step 8.e. +// ES 2017 draft 23.4.1.1 WeakSet step 8.e. +class MySet extends Set { + add(v) { + throw "adder throws"; + } +} +class MyWeakSet extends WeakSet { + add(v) { + throw "adder throws"; + } +} +test([MySet, MyWeakSet], { + nextVal: { value: {}, done: false }, + exceptionVal: "adder throws", + closed: true, +}); + +// ES 2017 draft 7.4.6 step 3. +// if GetMethod fails, the thrown value should be used. +test([MyMap, MySet, MyWeakMap, MyWeakSet], { + nextVal: { value: [{}, {}], done: false }, + modifier: (iterator, iterable) => { + Object.defineProperty(iterator, "return", { + get: function() { + iterable.closed = true; + throw "return getter throws"; + } + }); + }, + exceptionVal: "return getter throws", + closed: true, +}); +test([MyMap, MySet, MyWeakMap, MyWeakSet], { + nextVal: { value: [{}, {}], done: false }, + modifier: (iterator, iterable) => { + Object.defineProperty(iterator, "return", { + get: function() { + iterable.closed = true; + return "non object"; + } + }); + }, + exceptionType: TypeError, + closed: true, +}); +test([MyMap, MySet, MyWeakMap, MyWeakSet], { + nextVal: { value: [{}, {}], done: false }, + modifier: (iterator, iterable) => { + Object.defineProperty(iterator, "return", { + get: function() { + iterable.closed = true; + // Non callable. + return {}; + } + }); + }, + exceptionType: TypeError, + closed: true, +}); + +// ES 2017 draft 7.4.6 steps 6. +// if return method throws, the thrown value should be ignored. +test([MyMap, MyWeakMap], { + nextVal: { value: [{}, {}], done: false }, + modifier: (iterator, iterable) => { + iterator.return = function() { + iterable.closed = true; + throw "return throws"; + }; + }, + exceptionVal: "setter throws", + closed: true, +}); +test([MySet, MyWeakSet], { + nextVal: { value: [{}, {}], done: false }, + modifier: (iterator, iterable) => { + iterator.return = function() { + iterable.closed = true; + throw "return throws"; + }; + }, + exceptionVal: "adder throws", + closed: true, +}); + +test([MyMap, MyWeakMap], { + nextVal: { value: [{}, {}], done: false }, + modifier: (iterator, iterable) => { + iterator.return = function() { + iterable.closed = true; + return "non object"; + }; + }, + exceptionVal: "setter throws", + closed: true, +}); +test([MySet, MyWeakSet], { + nextVal: { value: [{}, {}], done: false }, + modifier: (iterator, iterable) => { + iterator.return = function() { + iterable.closed = true; + return "non object"; + }; + }, + exceptionVal: "adder throws", + closed: true, +}); + +// == Error cases without close == + +// ES 2017 draft 23.1.1.1 Map step 8.a. +// ES 2017 draft 23.3.1.1 WeakMap step 8.a. +// ES 2017 draft 23.2.1.1 Set step 8.a. +// ES 2017 draft 23.4.1.1 WeakSet step 8.a. +test([Map, WeakMap, Set, WeakSet], { + nextThrowVal: "next throws", + exceptionVal: "next throws", + closed: false, +}); +test([Map, WeakMap, Set, WeakSet], { + nextVal: { value: {}, get done() { throw "done getter throws"; } }, + exceptionVal: "done getter throws", + closed: false, +}); + +// ES 2017 draft 23.1.1.1 Map step 8.c. +// ES 2017 draft 23.3.1.1 WeakMap step 8.c. +// ES 2017 draft 23.2.1.1 Set step 8.c. +// ES 2017 draft 23.4.1.1 WeakSet step 8.c. +test([Map, WeakMap, Set, WeakSet], { + nextVal: { get value() { throw "value getter throws"; }, done: false }, + exceptionVal: "value getter throws", + closed: false, +}); + +// == Successful cases == + +test([Map, WeakMap], { + nextVal: { value: [{}, {}], done: false }, + closed: false, +}); +test([Set, WeakSet], { + nextVal: { value: {}, done: false }, + closed: false, +}); + +if (typeof reportCompare === 'function') + reportCompare(true, true); -- cgit v1.2.3