Audit valuesArray code in PerformPromiseAll
Categories
(Core :: JavaScript Engine, enhancement, P3)
Tracking
()
People
(Reporter: jandem, Unassigned)
References
Details
Boris noticed this code might need an AutoRealm in the !IsWrapper case: https://searchfox.org/mozilla-central/rev/00c0d068ece99717bea7475f7dc07e61f7f35984/js/src/builtin/Promise.cpp#2815-2838
We should investigate where/how this array is used.
Reporter | ||
Updated•6 years ago
|
Comment 1•6 years ago
|
||
The array is exposed to user code as the value of the resolve function:
js> function C(exec) {exec(v => { dumpObject(v); }, e => {})}
js> C.resolve = v => v;
v => v
js> Promise.all.call(C, [Promise.resolve(0), Promise.resolve(1)])
({})
object 385611b00788
global 9af3e98b060 [global]
class 558c65c0d2b0 Array
group 9af3e9887c0
flags:
proto <Array object at 9af3e9ad040>
properties:
"length" (shape 9af3e9b0078 permanent getterOp 558c641ddcc0 setterOp 558c641ddd20)
elements:
0: 0
1: 1
Comment 2•6 years ago
|
||
Right, so a simple testcase:
<iframe></iframe>
<script>
frames[0].Promise.all
.call(Promise, [Promise.resolve(0), Promise.resolve(1)])
.then((arg) => {
console.log(arg instanceof Array);
console.log(arg instanceof frames[0].Array);
})
</script>
This logs "true, false" in current nightly. It logs "false, true" with my changes to make the two globals share the same compartment, and logs "false, true" in Chrome and Safari. Pretty sure it should log "false, true" per spec.
Reporter | ||
Comment 3•6 years ago
|
||
(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #2)
This logs "true, false" in current nightly. It logs "false, true" with my changes to make the two globals share the same compartment, and logs "false, true" in Chrome and Safari. Pretty sure it should log "false, true" per spec.
I could add a shell test based on the testcase in comment 2 (with a same-compartment global)? Then we can close this or keep it open for the pre-existing cross-compartment issue..
Comment 4•6 years ago
|
||
Seems reasonable. Not sure how we fix the cross-compartment case, but ideally we would...
Comment 5•6 years ago
|
||
Adding a test-case sounds fine.
I wonder if it is possible to create the values array with the correct prototype in the cross-compartment case, because that should fix the instanceof
test. I'm just not sure (because I simply don't know enough about either topic) if that'll work when one of the compartments is less-privileged resp. if it has any unintended side-effects when Xrays are involved.
Updated•6 years ago
|
Updated•2 years ago
|
Description
•