diff --git a/moli-renderer-v8/src/script_vm/tests/webidl_fetch.rs b/moli-renderer-v8/src/script_vm/tests/webidl_fetch.rs index e97e0ac1f4..3541a33d56 100644 --- a/moli-renderer-v8/src/script_vm/tests/webidl_fetch.rs +++ b/moli-renderer-v8/src/script_vm/tests/webidl_fetch.rs @@ -6114,10 +6114,124 @@ fn webidl_sequence_conversion_uses_iterator_without_mutable_array_from() { assert_eq!( result, - "arrayFrom:0:0.25,0.75|arrayIterator:0:throw:RangeError:array-iterator-used:1|order:iterator,next:0,done:0,value:0,toString:0,next:1,done:1,value:1,toString:1,next:2,done:2|nextThrow:throw:RangeError:next-boom|doneThrow:throw:TypeError:done-boom|valueThrow:throw:SyntaxError:value-boom|elementThrow:throw:URIError:string-boom:true" + "arrayFrom:0:0.25,0.75|arrayIterator:0:throw:RangeError:array-iterator-used:1|order:iterator,next:0,done:0,value:0,toString:0,next:1,done:1,value:1,toString:1,next:2,done:2|nextThrow:throw:RangeError:next-boom|doneThrow:throw:TypeError:done-boom|valueThrow:throw:SyntaxError:value-boom|elementThrow:throw:URIError:string-boom:false" ); } +#[test] +fn webidl_sequences_propagate_abrupt_completion_without_closing_iterators() { + let mut vm = new_storage_test_vm("https://sequence-abrupt.test/"); + let result = vm.eval(r#" +(() => { + const check = (condition, label) => { if (!condition) throw new Error(label); }; + const consumers = [ + ['USVString', input => new URLSearchParams([input])], + ['DOMString', input => new PerformanceObserver(() => {}).observe({entryTypes: input})] + ]; + if (typeof IntersectionObserver === 'function') consumers.push( + ['double', input => new IntersectionObserver(() => {}, {threshold: input})]); + for (const [name, consume] of consumers) { + for (const stage of ['next', 'done', 'value', 'convert', 'symbol']) { + const marker = {}; + const log = []; + const fail = () => { log.push(stage); throw marker; }; + const input = {[Symbol.iterator]() { + let finished = false; + return { + next() { + if (finished) return {done: true}; + finished = true; + if (stage === 'next') fail(); + return { + get done() { if (stage === 'done') fail(); return false; }, + get value() { + if (stage === 'value') fail(); + return stage === 'symbol' ? Symbol() : {[Symbol.toPrimitive]: fail}; + } + }; + }, + get return() { log.push('get:return'); throw new Error('return must not be read'); } + }; + }}; + let caught; + try { consume(input); } catch (error) { caught = error; } + check(stage === 'symbol' ? caught instanceof TypeError : caught === marker, name + ' ' + stage + ' exception'); + const expected = stage === 'symbol' ? [] : [stage]; + check(JSON.stringify(log) === JSON.stringify(expected), name + ' ' + stage + ': ' + JSON.stringify(log)); + } + } + return 'ok'; +})() +"#).unwrap(); + assert_eq!(result, "ok"); +} + +#[test] +fn webidl_nested_and_interface_sequences_do_not_read_iterator_return_on_errors() { + let mut vm = new_storage_test_vm("https://nested-sequence-abrupt.test/"); + let result = vm.eval(r#" +(() => { + const check = (condition, label) => { if (!condition) throw new Error(label); }; + for (const stage of ['iterator', 'next', 'value', 'convert']) { + const marker = {}; + let returnReads = 0; + const fail = () => { throw marker; }; + const wrap = value => ({[Symbol.iterator]() { + let finished = false; + return { + next() { + if (finished) return {done: true}; + finished = true; + return {done: false, value}; + }, + get return() { returnReads++; throw new Error('outer return'); } + }; + }}); + const pair = {get [Symbol.iterator]() { + if (stage === 'iterator') fail(); + return function() { + let finished = false; + return { + next() { + if (finished) return {done: true}; + finished = true; + if (stage === 'next') fail(); + return {done: false, get value() { + if (stage === 'value') fail(); + return {[Symbol.toPrimitive]: fail}; + }}; + }, + get return() { returnReads++; throw new Error('inner return'); } + }; + }; + }}; + let caught; + try { new URLSearchParams(wrap(pair)); } catch (error) { caught = error; } + check(caught === marker && returnReads === 0, stage + ' must propagate without closing either iterator'); + } + for (const member of ['coalescedEvents', 'predictedEvents']) { + let returnReads = 0; + const events = {[Symbol.iterator]() { + let finished = false; + return { + next() { + if (finished) return {done: true}; + finished = true; + return {done: false, value: new Event('invalid')}; + }, + get return() { returnReads++; throw new Error('interface sequence return'); } + }; + }}; + let caught; + try { new PointerEvent('pointermove', {[member]: events}); } catch (error) { caught = error; } + check(caught instanceof TypeError && returnReads === 0, member + ' must reject the interface without closing'); + } + return 'ok'; +})() +"#).unwrap(); + assert_eq!(result, "ok"); +} + #[test] fn url_search_params_sequence_discrimination_reads_iterator_once() { let mut vm = new_storage_test_vm("https://url-search-params-sequence.test/"); @@ -6158,8 +6272,13 @@ fn url_search_params_sequence_discrimination_reads_iterator_once() { try { new URLSearchParams({ [Symbol.iterator]() { + let done = false; return { - next() { return { done: false, value: ['short'] }; }, + next() { + if (done) return { done: true }; + done = true; + return { done: false, value: ['short'] }; + }, return() { outerClosed = true; throw new SyntaxError('close error'); @@ -6210,7 +6329,10 @@ fn url_search_params_sequence_discrimination_reads_iterator_once() { ) .expect("URLSearchParams sequence discrimination probe should evaluate"); - assert_eq!(result, "1|one|two|TypeError|TypeError:true|RangeError:true"); + assert_eq!( + result, + "1|one|two|TypeError|TypeError:false|RangeError:false" + ); } #[test] diff --git a/moli-webidl/src/convert.rs b/moli-webidl/src/convert.rs index 0736515be7..a1338ea10a 100644 --- a/moli-webidl/src/convert.rs +++ b/moli-webidl/src/convert.rs @@ -352,28 +352,8 @@ where sequence_iterator_from_method(scope, value, iterator_method, context)?; let mut values = Vec::new(); while let Some(item) = sequence_iterator_next(scope, iterator, next_method, context)? { - let (converted, caught_exception) = { - let try_catch = std::pin::pin!(v8::TryCatch::new(scope)); - let mut conversion_scope = try_catch.init(); - let converted = T::convert(&mut conversion_scope, item, context, options); - let caught_exception = conversion_scope - .has_caught() - .then(|| conversion_scope.exception()) - .flatten() - .map(|exception| v8::Global::new(&conversion_scope, exception)); - (converted, caught_exception) - }; - match converted { - Ok(value) => values.push(value), - Err(error) => { - sequence_iterator_close_ignoring_errors(scope, iterator); - if let Some(exception) = caught_exception { - let exception = v8::Local::new(scope, &exception); - scope.throw_exception(exception); - } - return Err(error); - } - } + // WebIDL sequence conversion propagates errors without IteratorClose. + values.push(T::convert(scope, item, context, options)?); } Ok(Some(Sequence(values))) } @@ -1163,27 +1143,6 @@ fn sequence_iterator_next<'s>( Ok(Some(value)) } -fn sequence_iterator_close_ignoring_errors<'s>( - scope: &mut v8::PinScope<'s, '_>, - iterator: v8::Local<'s, v8::Object>, -) { - let try_catch = std::pin::pin!(v8::TryCatch::new(scope)); - let scope = try_catch.init(); - let Some(return_key) = v8::String::new(&scope, "return") else { - return; - }; - let Some(return_method) = iterator.get(&scope, return_key.into()) else { - return; - }; - if return_method.is_null_or_undefined() { - return; - } - let Ok(return_method) = v8::Local::::try_from(return_method) else { - return; - }; - let _ = return_method.call(&scope, iterator.into(), &[]); -} - fn call_sequence_function<'s>( scope: &mut v8::PinScope<'s, '_>, function: v8::Local<'s, v8::Function>, diff --git a/moli-wpt-compat/fixtures/wpt/ported/url/urlsearchparams-basic.html b/moli-wpt-compat/fixtures/wpt/ported/url/urlsearchparams-basic.html index 5692f50999..d478694b96 100644 --- a/moli-wpt-compat/fixtures/wpt/ported/url/urlsearchparams-basic.html +++ b/moli-wpt-compat/fixtures/wpt/ported/url/urlsearchparams-basic.html @@ -431,9 +431,9 @@ test(function () { }, }); }, "top-level iterator should propagate pair conversion errors"); - assert_true( + assert_false( closedAfterStringification, - "pair conversion errors should close the top-level iterator", + "pair conversion errors must not close the top-level iterator", ); let closedAfterInvalidPair = false; @@ -444,10 +444,8 @@ test(function () { return { next: function () { count += 1; - return { - done: false, - value: count === 1 ? ["short"] : ["unreached", "value"], - }; + if (count === 1) return { done: false, value: ["short"] }; + return { done: true }; }, return: function () { closedAfterInvalidPair = true; @@ -457,11 +455,11 @@ test(function () { }, }); }, "top-level iterator should propagate invalid pair errors"); - assert_true( + assert_false( closedAfterInvalidPair, - "invalid pair errors should close the top-level iterator", + "invalid pair errors must not close the top-level iterator", ); -}, "URLSearchParams constructor closes top-level iterators on pair conversion errors"); +}, "URLSearchParams constructor does not close top-level iterators on pair errors"); test(function () { let innerClosedAfterValueConversion = false; @@ -497,11 +495,11 @@ test(function () { assert_throws_name("RangeError", function () { new URLSearchParams([pair]); }, "inner pair iterator should propagate element conversion errors"); - assert_true( + assert_false( innerClosedAfterValueConversion, - "pair element conversion errors should close the inner pair iterator", + "pair element conversion errors must not close the inner pair iterator", ); -}, "URLSearchParams constructor closes inner pair iterators on element conversion errors"); +}, "URLSearchParams constructor does not close inner pair iterators on conversion errors"); test(function () { let topLevelReturnCalled = false; @@ -527,7 +525,7 @@ test(function () { }, }); }, "top-level iterator return errors should not replace the original conversion error"); - assert_true(topLevelReturnCalled, "top-level iterator return should be called"); + assert_false(topLevelReturnCalled, "top-level iterator return must not be called"); let innerReturnCalled = false; const pair = { @@ -558,16 +556,19 @@ test(function () { assert_throws_name("RangeError", function () { new URLSearchParams([pair]); }, "inner pair iterator return errors should not replace the original conversion error"); - assert_true(innerReturnCalled, "inner pair iterator return should be called"); -}, "URLSearchParams constructor preserves original errors when iterator return throws"); + assert_false(innerReturnCalled, "inner pair iterator return must not be called"); +}, "URLSearchParams constructor does not call throwing return methods after conversion errors"); test(function () { let returnCalled = false; assert_throws_name("TypeError", function () { new URLSearchParams({ [Symbol.iterator]: function () { + let done = false; return { next: function () { + if (done) return { done: true }; + done = true; return { done: false, value: ["short"] }; }, return: function () { @@ -578,8 +579,8 @@ test(function () { }, }); }, "top-level iterator return errors should not replace invalid pair length errors"); - assert_true(returnCalled, "top-level iterator return should be called"); -}, "URLSearchParams constructor preserves invalid pair length errors when iterator return throws"); + assert_false(returnCalled, "top-level iterator return must not be called"); +}, "URLSearchParams constructor does not call throwing return methods for invalid pairs"); test(function () { let topLevelReturnCalled = false;