Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normative: Remove steps 2 and 4 from ProxyCreate #1814

Merged

Conversation

bathos
Copy link
Contributor

@bathos bathos commented Dec 15, 2019

Currently, ProxyCreate is sensitive to whether its arguments are revoked proxies. If either is, it throws a TypeError. This PR alters the algorithm to remove that sensitivity. Its consequence is that a Proxy could be created successfully where target or handlers are revoked Proxy exotic objects. (It was already possible for such Proxies to exist — just not for them to be created while already in that state.)

The motivation for this change is discussed in issue #1798. Quoting @erights:

It is a mistake because it makes the revocable-ness state of a proxy observable. When we first introduced revocable proxies, we intended observational equivalence to a handler that only throws. We were not trying to introduce behavior that could not otherwise be expressed. We were trying to enable gc. [...] Step 2 breaks that illusion and makes detectable a state change that should have been encapsulated in the proxy.

Per @caridy the intention for this PR is to

gather feedback from now until the next meeting (early next year).

Note that while part of the motivation for this change is to reduce the observability of an object’s status as a (revoked) Proxy exotic object, it would not in itself eliminate it. It will remain possible to detect if any object is a revoked Proxy because of step 3.a. of IsArray and, for a subset of objects, through more complex maneuvers, GetFunctionRealm. It’s possible that both of these algorithms could be altered to cease revealing this information web-safely, but I’m hardly certain of that.

Fixes #1798.

@ljharb ljharb added needs consensus This needs committee consensus before it can be eligible to be merged. needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 normative change Affects behavior required to correctly evaluate some ECMAScript source text labels Dec 15, 2019
Copy link

@erights erights left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ljharb ljharb added has consensus This has committee consensus. and removed needs consensus This needs committee consensus before it can be eligible to be merged. labels Feb 5, 2020
@rwaldron rwaldron added has test262 tests and removed needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels Mar 30, 2020
graalvmbot pushed a commit to oracle/graaljs that referenced this pull request Apr 2, 2020
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Apr 15, 2020
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 15, 2020
Implements the changes from <tc39/ecma262#1814>.

Depends on D70814

Differential Revision: https://phabricator.services.mozilla.com/D70815

--HG--
extra : moz-landing-system : lando
@ljharb ljharb self-assigned this Apr 23, 2020
bertogg pushed a commit to Igalia/webkit that referenced this pull request Apr 24, 2020
https://bugs.webkit.org/show_bug.cgi?id=210862

Reviewed by Ross Kirsling.

JSTests:

Removes expectations for 2 invalid ChakraCore tests.

* ChakraCore.yaml: Mark 2 test cases as passing.
* ChakraCore/test/es6/arraywithproxy.baseline: Removed.
* ChakraCore/test/es6/proxytest9.baseline: Removed.
* stress/proxy-revoke.js: Adjust test.
* test262/expectations.yaml: Mark 12 test cases as passing.

Source/JavaScriptCore:

This change removes revoked Proxy checks from ProxyCreate [1], implementing
tc39/ecma262#1814 and aligning JSC with SpiderMonkey.
Also cleans up ProxyObject creation by using isFunction() instead of
isCallable(), which are identical.

[1]: https://tc39.es/ecma262/#sec-proxycreate (steps 2, 4)

* runtime/ProxyObject.cpp:
(JSC::ProxyObject::structureForTarget):
(JSC::ProxyObject::finishCreation):


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@260621 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@ljharb ljharb force-pushed the remove-sensitivity-to-revocation-from-proxy-create branch from 7c80f1d to ebe4001 Compare April 29, 2020 20:57
@ljharb ljharb merged commit ebe4001 into tc39:master Apr 29, 2020
galpeter added a commit to galpeter/jerryscript that referenced this pull request Oct 1, 2020
In the newer ecma262 standard (post ES11) the ProxyCreate was
changed and the revoked Proxy handler/target is not checked.

Related standard PR: tc39/ecma262#1814

JerryScript-DCO-1.0-Signed-off-by: Peter Gal pgal.usz@partner.samsung.com
ryanhaddad pushed a commit to WebKit/WebKit that referenced this pull request Dec 22, 2020
https://bugs.webkit.org/show_bug.cgi?id=210862

Reviewed by Ross Kirsling.

JSTests:

Removes expectations for 2 invalid ChakraCore tests.

* ChakraCore.yaml: Mark 2 test cases as passing.
* ChakraCore/test/es6/arraywithproxy.baseline: Removed.
* ChakraCore/test/es6/proxytest9.baseline: Removed.
* stress/proxy-revoke.js: Adjust test.
* test262/expectations.yaml: Mark 12 test cases as passing.

Source/JavaScriptCore:

This change removes revoked Proxy checks from ProxyCreate [1], implementing
tc39/ecma262#1814 and aligning JSC with SpiderMonkey.
Also cleans up ProxyObject creation by using isFunction() instead of
isCallable(), which are identical.

[1]: https://tc39.es/ecma262/#sec-proxycreate (steps 2, 4)

* runtime/ProxyObject.cpp:
(JSC::ProxyObject::structureForTarget):
(JSC::ProxyObject::finishCreation):


Canonical link: https://commits.webkit.org/223843@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@260621 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has consensus This has committee consensus. has test262 tests normative change Affects behavior required to correctly evaluate some ECMAScript source text
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ProxyCreate’s sensitivity to revocation of arguments
7 participants