Skip to content

Commit 60dd1e5

Browse files
woesswoess
committed
Explicitly check for revoked proxy in order to provide a better error message.
1 parent a27209b commit 60dd1e5

File tree

5 files changed

+45
-42
lines changed

5 files changed

+45
-42
lines changed

‎graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/access/JSProxyCallNode.java‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import com.oracle.truffle.api.nodes.NodeCost;
4646
import com.oracle.truffle.api.nodes.NodeInfo;
4747
import com.oracle.truffle.api.object.DynamicObject;
48+
import com.oracle.truffle.api.profiles.BranchProfile;
4849
import com.oracle.truffle.api.profiles.ConditionProfile;
4950
import com.oracle.truffle.js.nodes.JavaScriptBaseNode;
5051
import com.oracle.truffle.js.nodes.function.JSFunctionCallNode;
@@ -69,8 +70,8 @@ public abstract class JSProxyCallNode extends JavaScriptBaseNode {
6970
@Child private JSFunctionCallNode callTrapNode;
7071
protected final boolean isNew;
7172
protected final boolean isNewTarget;
72-
private final ConditionProfile callableProfile = ConditionProfile.createBinaryProfile();
7373
private final ConditionProfile pxTrapFunProfile = ConditionProfile.createBinaryProfile();
74+
private final BranchProfile errorBranch = BranchProfile.create();
7475

7576
protected JSProxyCallNode(JSContext context, boolean isNew, boolean isNewTarget) {
7677
this.callNode = isNewTarget ? JSFunctionCallNode.createNewTarget() : isNew ? JSFunctionCallNode.createNew() : JSFunctionCallNode.createCall();
@@ -97,10 +98,11 @@ protected Object doCall(Object[] arguments) {
9798
assert JSProxy.isProxy(function);
9899
DynamicObject proxy = (DynamicObject) function;
99100

100-
if (!callableProfile.profile(JSRuntime.isCallableProxy(proxy))) {
101+
if (!JSRuntime.isCallableProxy(proxy)) {
102+
errorBranch.enter();
101103
throw Errors.createTypeErrorNotAFunction(function, this);
102104
} else {
103-
DynamicObject pxHandler = JSProxy.getHandlerChecked(proxy);
105+
DynamicObject pxHandler = JSProxy.getHandlerChecked(proxy, errorBranch);
104106
Object pxTarget = JSProxy.getTarget(proxy);
105107
Object pxTrapFun = trapGetter.executeWithTarget(pxHandler);
106108
Object[] proxyArguments = JSArguments.extractUserArguments(arguments);
@@ -121,10 +123,11 @@ protected Object doConstruct(Object[] arguments) {
121123
assert JSProxy.isProxy(function);
122124
DynamicObject proxy = (DynamicObject) function;
123125

124-
if (!callableProfile.profile(JSRuntime.isConstructorProxy(proxy))) {
126+
if (!JSRuntime.isConstructorProxy(proxy)) {
127+
errorBranch.enter();
125128
throw Errors.createTypeErrorNotAFunction(function, this);
126129
} else {
127-
DynamicObject pxHandler = JSProxy.getHandlerChecked(proxy);
130+
DynamicObject pxHandler = JSProxy.getHandlerChecked(proxy, errorBranch);
128131
Object pxTarget = JSProxy.getTarget(proxy);
129132
Object pxTrapFun = trapGetter.executeWithTarget(pxHandler);
130133
Object newTarget = isNewTarget ? JSArguments.getNewTarget(arguments) : proxy;
@@ -133,12 +136,14 @@ protected Object doConstruct(Object[] arguments) {
133136
if (!JSObject.isJSObject(pxTarget)) {
134137
return JSInteropUtil.construct(pxTarget, constructorArguments);
135138
}
136-
return callNode.executeCall(isNewTarget ? JSArguments.createWithNewTarget(JSFunction.CONSTRUCT, pxTarget, newTarget, constructorArguments)
139+
return callNode.executeCall(isNewTarget
140+
? JSArguments.createWithNewTarget(JSFunction.CONSTRUCT, pxTarget, newTarget, constructorArguments)
137141
: JSArguments.create(JSFunction.CONSTRUCT, pxTarget, constructorArguments));
138142
}
139143
Object[] trapArgs = new Object[]{pxTarget, JSArray.createConstant(context, constructorArguments), newTarget};
140144
Object result = callTrapNode.executeCall(JSArguments.create(pxHandler, pxTrapFun, trapArgs));
141145
if (!JSRuntime.isObject(result)) {
146+
errorBranch.enter();
142147
throw Errors.createTypeErrorNotAnObject(result, this);
143148
}
144149
return result;

‎graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/access/JSProxyHasPropertyNode.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import com.oracle.truffle.api.nodes.NodeCost;
4747
import com.oracle.truffle.api.nodes.NodeInfo;
4848
import com.oracle.truffle.api.object.DynamicObject;
49+
import com.oracle.truffle.api.profiles.BranchProfile;
4950
import com.oracle.truffle.api.profiles.ConditionProfile;
5051
import com.oracle.truffle.js.nodes.JavaScriptBaseNode;
5152
import com.oracle.truffle.js.nodes.cast.JSToBooleanNode;
@@ -67,6 +68,7 @@ public abstract class JSProxyHasPropertyNode extends JavaScriptBaseNode {
6768
@Child private JSFunctionCallNode callNode;
6869
@Child private JSToBooleanNode toBooleanNode;
6970
@Child private JSToPropertyKeyNode toPropertyKeyNode;
71+
private final BranchProfile errorBranch = BranchProfile.create();
7072

7173
public JSProxyHasPropertyNode(JSContext context) {
7274
this.callNode = JSFunctionCallNode.createCall();
@@ -81,19 +83,13 @@ public static JSProxyHasPropertyNode create(JSContext context) {
8183

8284
public abstract boolean executeWithTargetAndKeyBoolean(Object shared, Object key);
8385

84-
private void checkTrapResult(boolean accessible, boolean trapResult) {
85-
if (!accessible && !trapResult) {
86-
throw Errors.createTypeError("Proxy can't successfully access a non-writable, non-configurable property", this);
87-
}
88-
}
89-
9086
@Specialization
9187
protected boolean doGeneric(DynamicObject proxy, Object key,
9288
@Cached("createBinaryProfile()") ConditionProfile trapFunProfile) {
9389
assert JSProxy.isProxy(proxy);
9490
Object propertyKey = toPropertyKeyNode.execute(key);
91+
DynamicObject handler = JSProxy.getHandlerChecked(proxy, errorBranch);
9592
Object target = JSProxy.getTarget(proxy);
96-
DynamicObject handler = JSProxy.getHandler(proxy);
9793
Object trapFun = trapGetter.executeWithTarget(handler);
9894
if (trapFunProfile.profile(trapFun == Undefined.instance)) {
9995
if (JSObject.isJSObject(target)) {
@@ -104,8 +100,12 @@ protected boolean doGeneric(DynamicObject proxy, Object key,
104100
} else {
105101
Object callResult = callNode.executeCall(JSArguments.create(handler, trapFun, target, propertyKey));
106102
boolean trapResult = toBooleanNode.executeBoolean(callResult);
107-
boolean accessible = JSProxy.checkPropertyIsSettable(target, propertyKey);
108-
checkTrapResult(accessible, trapResult);
103+
if (!trapResult) {
104+
errorBranch.enter();
105+
if (!JSProxy.checkPropertyIsSettable(target, propertyKey)) {
106+
throw Errors.createTypeError("Proxy can't successfully access a non-writable, non-configurable property", this);
107+
}
108+
}
109109
return trapResult;
110110
}
111111
}

‎graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/access/JSProxyPropertyGetNode.java‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ public abstract class JSProxyPropertyGetNode extends JavaScriptBaseNode {
6969

7070
@Child protected GetMethodNode trapGet;
7171
@Child private JSFunctionCallNode callNode;
72-
@Child private JSToPropertyKeyNode toPropertyKeyNode;
7372
@Child private JSGetOwnPropertyNode getOwnPropertyNode;
7473
@Child private JSIdenticalNode sameValueNode;
7574
private final BranchProfile errorBranch = BranchProfile.create();
@@ -87,12 +86,13 @@ public static JSProxyPropertyGetNode create(JSContext context) {
8786

8887
@Specialization
8988
protected Object doGeneric(DynamicObject proxy, Object receiver, Object key,
89+
@Cached JSToPropertyKeyNode toPropertyKeyNode,
9090
@Cached("createBinaryProfile()") ConditionProfile hasTrap,
9191
@Cached JSClassProfile targetClassProfile) {
9292
assert JSProxy.isProxy(proxy);
9393
assert !(key instanceof HiddenKey);
94-
Object propertyKey = toPropertyKey(key);
95-
DynamicObject handler = JSProxy.getHandler(proxy);
94+
Object propertyKey = toPropertyKeyNode.execute(key);
95+
DynamicObject handler = JSProxy.getHandlerChecked(proxy, errorBranch);
9696
Object target = JSProxy.getTarget(proxy);
9797
Object trapFun = trapGet.executeWithTarget(handler);
9898
if (hasTrap.profile(trapFun == Undefined.instance)) {
@@ -145,12 +145,4 @@ private PropertyDescriptor getOwnProperty(DynamicObject target, Object propertyK
145145
}
146146
return getOwnPropertyNode.execute(target, propertyKey);
147147
}
148-
149-
private Object toPropertyKey(Object key) {
150-
if (toPropertyKeyNode == null) {
151-
CompilerDirectives.transferToInterpreterAndInvalidate();
152-
toPropertyKeyNode = insert(JSToPropertyKeyNode.create());
153-
}
154-
return toPropertyKeyNode.execute(key);
155-
}
156148
}

‎graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/access/JSProxyPropertySetNode.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import com.oracle.truffle.api.nodes.NodeInfo;
4949
import com.oracle.truffle.api.object.DynamicObject;
5050
import com.oracle.truffle.api.object.HiddenKey;
51+
import com.oracle.truffle.api.profiles.BranchProfile;
5152
import com.oracle.truffle.api.profiles.ConditionProfile;
5253
import com.oracle.truffle.js.nodes.JavaScriptBaseNode;
5354
import com.oracle.truffle.js.nodes.cast.JSToBooleanNode;
@@ -74,6 +75,7 @@ public abstract class JSProxyPropertySetNode extends JavaScriptBaseNode {
7475
@Child private JSToPropertyKeyNode toPropertyKeyNode;
7576
@Child private InteropLibrary interopNode;
7677
@Child private ExportValueNode exportValueNode;
78+
private final BranchProfile errorBranch = BranchProfile.create();
7779

7880
protected JSProxyPropertySetNode(JSContext context, boolean isStrict) {
7981
this.call = JSFunctionCallNode.createCall();
@@ -97,7 +99,7 @@ protected boolean doGeneric(DynamicObject proxy, Object receiver, Object value,
9799
assert JSProxy.isProxy(proxy);
98100
assert !(key instanceof HiddenKey);
99101
Object propertyKey = toPropertyKey(key);
100-
DynamicObject handler = JSProxy.getHandler(proxy);
102+
DynamicObject handler = JSProxy.getHandlerChecked(proxy, errorBranch);
101103
Object target = JSProxy.getTarget(proxy);
102104
Object trapFun = trapGet.executeWithTarget(handler);
103105
if (hasTrap.profile(trapFun == Undefined.instance)) {
@@ -111,6 +113,7 @@ protected boolean doGeneric(DynamicObject proxy, Object receiver, Object value,
111113
Object trapResult = call.executeCall(JSArguments.create(handler, trapFun, target, propertyKey, value, receiver));
112114
boolean booleanTrapResult = toBoolean.executeBoolean(trapResult);
113115
if (!booleanTrapResult) {
116+
errorBranch.enter();
114117
if (isStrict) {
115118
throw Errors.createTypeErrorTrapReturnedFalsish(JSProxy.SET, propertyKey);
116119
} else {

‎graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/builtins/JSProxy.java‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import com.oracle.truffle.api.object.HiddenKey;
5151
import com.oracle.truffle.api.object.Property;
5252
import com.oracle.truffle.api.object.Shape;
53+
import com.oracle.truffle.api.profiles.BranchProfile;
5354
import com.oracle.truffle.js.builtins.ConstructorBuiltins;
5455
import com.oracle.truffle.js.builtins.ProxyFunctionBuiltins;
5556
import com.oracle.truffle.js.runtime.Boundaries;
@@ -104,15 +105,6 @@ public final class JSProxy extends AbstractJSClass implements PrototypeSupplier
104105
PROXY_HANDLER_PROPERTY = JSObjectUtil.makeHiddenProperty(PROXY_HANDLER, allocator.locationForType(DynamicObject.class));
105106
}
106107

107-
public static boolean isAccessibleProperty(DynamicObject proxy, Object key) {
108-
Object target = JSProxy.getTarget(proxy);
109-
if (JSObject.isJSObject(target)) {
110-
return checkPropertyIsSettable(target, key);
111-
} else {
112-
return true; // best guess
113-
}
114-
}
115-
116108
public static boolean checkPropertyIsSettable(Object truffleTarget, Object key) {
117109
assert JSRuntime.isPropertyKey(key);
118110
if (!JSObject.isJSObject(truffleTarget)) {
@@ -176,7 +168,16 @@ public static DynamicObject getHandler(DynamicObject obj) {
176168
public static DynamicObject getHandlerChecked(DynamicObject obj) {
177169
DynamicObject handler = getHandler(obj);
178170
if (handler == Null.instance) {
179-
throw Errors.createTypeError("proxy handler must not be null");
171+
throw Errors.createTypeErrorProxyRevoked();
172+
}
173+
return handler;
174+
}
175+
176+
public static DynamicObject getHandlerChecked(DynamicObject obj, BranchProfile errorBranch) {
177+
DynamicObject handler = getHandler(obj);
178+
if (handler == Null.instance) {
179+
errorBranch.enter();
180+
throw Errors.createTypeErrorProxyRevoked();
180181
}
181182
return handler;
182183
}
@@ -222,7 +223,7 @@ public Object getOwnHelper(DynamicObject store, Object receiver, long index) {
222223
@TruffleBoundary
223224
private static Object proxyGetHelper(DynamicObject proxy, Object key, Object receiver) {
224225
assert JSRuntime.isPropertyKey(key);
225-
DynamicObject handler = getHandler(proxy);
226+
DynamicObject handler = getHandlerChecked(proxy);
226227
Object target = getTarget(proxy);
227228
Object trap = getTrapFromObject(handler, GET);
228229
if (trap == Undefined.instance) {
@@ -276,7 +277,7 @@ public boolean set(DynamicObject thisObj, long index, Object value, Object recei
276277
@TruffleBoundary
277278
private static boolean proxySet(DynamicObject thisObj, Object key, Object value, Object receiver, boolean isStrict) {
278279
assert JSRuntime.isPropertyKey(key);
279-
DynamicObject handler = getHandler(thisObj);
280+
DynamicObject handler = getHandlerChecked(thisObj);
280281
Object target = getTarget(thisObj);
281282
Object trap = getTrapFromObject(handler, SET);
282283
if (trap == Undefined.instance) {
@@ -348,7 +349,7 @@ public boolean hasProperty(DynamicObject thisObj, long index) {
348349
@Override
349350
public boolean hasProperty(DynamicObject thisObj, Object key) {
350351
assert JSRuntime.isPropertyKey(key);
351-
DynamicObject handler = getHandler(thisObj);
352+
DynamicObject handler = getHandlerChecked(thisObj);
352353
Object target = getTarget(thisObj);
353354
Object trap = getTrapFromObject(handler, HAS);
354355
if (trap == Undefined.instance) {
@@ -360,8 +361,10 @@ public boolean hasProperty(DynamicObject thisObj, Object key) {
360361
}
361362

362363
boolean trapResult = JSRuntime.toBoolean(JSRuntime.call(trap, handler, new Object[]{target, key}));
363-
if (!trapResult && !isAccessibleProperty(thisObj, key)) {
364-
throw Errors.createTypeErrorConfigurableExpected();
364+
if (!trapResult) {
365+
if (!JSProxy.checkPropertyIsSettable(target, key)) {
366+
throw Errors.createTypeErrorConfigurableExpected();
367+
}
365368
}
366369
return trapResult;
367370
}

0 commit comments

Comments
 (0)