Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-11 16:35:33 +01:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #24866 - Manishearth:iterator-invalidation, r=jdm

Browse source 
Fix iterator invalidation in our forEach implementation.

Currently we iterate over iterables in forEach based on the length they report when we start iterating, however the inner callback is able to change this.

```js
let params = new URLSearchParams("foo=bar&baz=qux");
params.forEach((p) => {
    console.log(p);
    params.delete("baz");
})
```

This causes us to panic [here](f1aa5d8dbd/components/script/dom/bindings/codegen/CodegenRust.py (L7412)) over an attempt to access out of bounds.

Relevant spec: https://heycam.github.io/webidl/#es-forEach

r? @jdm
This commit is contained in:
bors-servo 2019-11-25 23:27:33 -05:00 • committed by GitHub
commit 74bf0ce5c2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 62 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

13
components/script/dom/bindings/codegen/CodegenRust.py
View file

@ -7408,7 +7408,16 @@ class CGIterableMethodGenerator(CGGeneric):
rooted!(in(*cx) let mut call_arg2 = UndefinedValue()); rooted!(in(*cx) let mut call_arg2 = UndefinedValue());
let mut call_args = vec![UndefinedValue(), UndefinedValue(), ObjectValue(*_obj)]; let mut call_args = vec![UndefinedValue(), UndefinedValue(), ObjectValue(*_obj)];
rooted!(in(*cx) let mut ignoredReturnVal = UndefinedValue()); rooted!(in(*cx) let mut ignoredReturnVal = UndefinedValue());
for i in 0..(*this).get_iterable_length() {
// This has to be a while loop since get_iterable_length() may change during
// the callback, and we need to avoid iterator invalidation.
//
// It is possible for this to loop infinitely, but that matches the spec
// and other browsers.
//
// https://heycam.github.io/webidl/#es-forEach
let mut i = 0;
while i < (*this).get_iterable_length() {
(*this).get_value_at_index(i).to_jsval(*cx, call_arg1.handle_mut()); (*this).get_value_at_index(i).to_jsval(*cx, call_arg1.handle_mut());
(*this).get_key_at_index(i).to_jsval(*cx, call_arg2.handle_mut()); (*this).get_key_at_index(i).to_jsval(*cx, call_arg2.handle_mut());
call_args[0] = call_arg1.handle().get(); call_args[0] = call_arg1.handle().get();
@ -7418,6 +7427,8 @@ class CGIterableMethodGenerator(CGGeneric):
ignoredReturnVal.handle_mut()) { ignoredReturnVal.handle_mut()) {
return false; return false;
} }
i += 1;
} }
let result = (); let result = ();

10
tests/wpt/metadata/MANIFEST.json
View file

@ -308306,6 +308306,12 @@
{} {}
] ]
], ],
"WebIDL/ecmascript-binding/iterator-invalidation-foreach.html": [
[
"WebIDL/ecmascript-binding/iterator-invalidation-foreach.html",
{}
]
],
"WebIDL/ecmascript-binding/iterator-prototype-object.html": [ "WebIDL/ecmascript-binding/iterator-prototype-object.html": [
[ [
"WebIDL/ecmascript-binding/iterator-prototype-object.html", "WebIDL/ecmascript-binding/iterator-prototype-object.html",
@ -466654,6 +466660,10 @@
"03ada7aa0d4d43811652fc679a00a41b9653013d", "03ada7aa0d4d43811652fc679a00a41b9653013d",
"testharness" "testharness"
], ],
"WebIDL/ecmascript-binding/iterator-invalidation-foreach.html": [
"d6498c3e388e0c637830fa080cca78b0ab0e5305",
"testharness"
],
"WebIDL/ecmascript-binding/iterator-prototype-object.html": [ "WebIDL/ecmascript-binding/iterator-prototype-object.html": [
"5a935fa20135d88a7268b872b68ab7fe548ab5c7", "5a935fa20135d88a7268b872b68ab7fe548ab5c7",
"testharness" "testharness"

40
tests/wpt/web-platform-tests/WebIDL/ecmascript-binding/iterator-invalidation-foreach.html Normal file
View file

@ -0,0 +1,40 @@
<!doctype html>
<meta charset="utf-8">
<title>Behavior of iterators when modified within foreach</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<link rel="help" href="https://heycam.github.io/webidl/#es-forEach">
<link rel="author" title="Manish Goregaokar" href="mailto:manishsmail@gmail.com">
<script>
test(function() {
let params = new URLSearchParams("a=1&b=2&c=3");
let arr = [];
params.forEach((p) => {
arr.push(p);
params.delete("b");
})
assert_array_equals(arr, ["1", "3"]);
}, "forEach will not iterate over elements removed during iteration");
test(function() {
let params = new URLSearchParams("a=1&b=2&c=3&d=4");
let arr = [];
params.forEach((p) => {
arr.push(p);
if (p == "2") {
params.delete("a");
}
})
assert_array_equals(arr, ["1", "2", "4"]);
}, "Removing elements already iterated over during forEach will cause iterator to skip an element");
test(function() {
let params = new URLSearchParams("a=1&b=2&c=3&d=4");
let arr = [];
params.forEach((p) => {
arr.push(p);
if (p == "2") {
params.append("e", "5");
}
})
assert_array_equals(arr, ["1", "2", "3", "4", "5"]);
}, "Elements added during iteration with forEach will be reached");
</script>