Proxy() constructor should throw when either target or handler are revoked proxies

RESOLVED FIXED in Firefox 40

Status

()

Core
JavaScript: Standard Library
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: caitp, Assigned: arai)

Tracking

(Blocks: 1 bug)

Trunk
mozilla40
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2356.0 Safari/537.36

Steps to reproduce:

var r = Proxy.revocable({}, {});
var p = r.proxy;
r.revoke();
new Proxy(p, {});
new Proxy({}, p);

$log("We didn't throw!");

(Runnable example: http://jsfiddle.net/w6xyoy3j/)


Actual results:

We logged "We didn't throw!", indicating that these operations completed successfully.

When run in the dev console, [object Object] is printed --- indicating that touching the resulting object for printing threw, but actually constructing the object did not throw.


Expected results:

https://people.mozilla.org/~jorendorff/es6-draft.html#sec-proxycreate says to throw if either the target or handler is a revoked proxy
(Reporter)

Updated

3 years ago
Blocks: 694100
Component: General → JavaScript: Standard Library
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Comment 1

3 years ago
Created attachment 8588447 [details] [diff] [review]
Throw if either the target or handler is a revoked proxy in Proxy constructor.

Implemented simple cases.

Do we need to handle following case too? (it's not caught by step 2 in this patch)
  var p = new Proxy(newGlobal().eval("var r = Proxy.revocable({}, {}); var p = r.proxy; r.revoke(); p;"), {});

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a4f6edb9cb61
Assignee: nobody → arai.unmht
Attachment #8588447 - Flags: review?(efaustbmo)
(Reporter)

Comment 2

3 years ago
It seems to me that a promise revoked in realm A should still be revoked in realm B, so on those grounds I think it does matter. Might be mistaken about that, though
(Assignee)

Comment 3

3 years ago
Comment on attachment 8588447 [details] [diff] [review]
Throw if either the target or handler is a revoked proxy in Proxy constructor.

Thanks, I'll post fixed patch after it passed try run
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6008a7cc90bf
Attachment #8588447 - Flags: review?(efaustbmo)
(Assignee)

Comment 4

3 years ago
Created attachment 8588553 [details] [diff] [review]
Throw if either the target or handler is a revoked proxy in Proxy constructor.

Last try fails with DOM proxy as target argument, so I used IsScriptedProxy to test the existence of target object only on scripted proxy.

Now green on try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bbea4c67210a
Attachment #8588447 - Attachment is obsolete: true
Attachment #8588553 - Flags: review?(efaustbmo)

Comment 5

3 years ago
Comment on attachment 8588553 [details] [diff] [review]
Throw if either the target or handler is a revoked proxy in Proxy constructor.

Review of attachment 8588553 [details] [diff] [review]:
-----------------------------------------------------------------

Looks right to me. Security concerns are also a valid reason to throw, as noted. Thanks for updating the step comments as well.
Attachment #8588553 - Flags: review?(efaustbmo) → review+
https://hg.mozilla.org/mozilla-central/rev/17a40c12a007
Status: UNCONFIRMED → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.