Skip to content
Permalink
Browse files
[[HasProperty]] result of Proxy in prototype chain is ignored
https://bugs.webkit.org/show_bug.cgi?id=203560

Patch by Alexey Shvayka <shvaikalesh@gmail.com> on 2019-11-07
Reviewed by Ross Kirsling.

JSTests:

* stress/proxy-get-prototype-of.js: Correct Proxy "has" trap test.
* test262/expectations.yaml: Mark 6 test cases as passing.

Source/JavaScriptCore:

Before this change, when [[HasProperty]] was called on ordinary object with Proxy in prototype chain,
falsy result of Proxy's "has" trap was ignored and prototype chain was inspected further.

According to spec, OrdinaryHasProperty unconditionally returns result of parent's [[HasProperty]] call.
(step 5.a of https://tc39.es/ecma262/#sec-ordinaryhasproperty)

* runtime/JSObjectInlines.h:
(JSC::JSObject::getPropertySlot):
(JSC::JSObject::getNonIndexPropertySlot):
* runtime/ProxyObject.cpp:
(JSC::ProxyObject::performHasProperty): Walk the prototype chain in performDefaultHasProperty.

Canonical link: https://commits.webkit.org/217292@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@252191 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
shvaikalesh authored and webkit-commit-queue committed Nov 7, 2019
1 parent 673e51c commit ae717be6e885a913af1676bcfb72d34ff80e8f72
@@ -1,3 +1,13 @@
2019-11-07 Alexey Shvayka <shvaikalesh@gmail.com>

[[HasProperty]] result of Proxy in prototype chain is ignored
https://bugs.webkit.org/show_bug.cgi?id=203560

Reviewed by Ross Kirsling.

* stress/proxy-get-prototype-of.js: Correct Proxy "has" trap test.
* test262/expectations.yaml: Mark 6 test cases as passing.

2019-11-06 Mark Lam <mark.lam@apple.com>

JSGlobalObject::fireWatchpointAndMakeAllArrayStructuresSlowPut() should fire its watchpoint as the last step.
@@ -389,7 +389,6 @@ function assert(b) {
let called = false;
let handler = {
getPrototypeOf: function(theTarget) {
assert(theTarget === target);
called = true;
return proto;
},
@@ -400,9 +399,8 @@ function assert(b) {

let proxy = new Proxy(target, handler);
for (let i = 0; i < 500; i++) {
let result = "x" in proxy;
assert(called);
called = false;
let result = 1 in proxy;
assert(!called);
}
}

@@ -633,12 +633,6 @@ test/built-ins/Array/proto-from-ctor-realm-zero.js:
test/built-ins/Array/prototype/filter/target-array-with-non-writable-property.js:
default: 'TypeError: Attempted to assign to readonly property.'
strict mode: 'TypeError: Attempted to assign to readonly property.'
test/built-ins/Array/prototype/indexOf/calls-only-has-on-prototype-after-length-zeroed.js:
default: 'Test262Error: [[GetPrototypeOf]] trap called'
strict mode: 'Test262Error: [[GetPrototypeOf]] trap called'
test/built-ins/Array/prototype/lastIndexOf/calls-only-has-on-prototype-after-length-zeroed.js:
default: 'Test262Error: [[GetPrototypeOf]] trap called'
strict mode: 'Test262Error: [[GetPrototypeOf]] trap called'
test/built-ins/Array/prototype/map/target-array-with-non-writable-property.js:
default: 'TypeError: Attempted to assign to readonly property.'
strict mode: 'TypeError: Attempted to assign to readonly property.'
@@ -664,8 +658,8 @@ test/built-ins/Array/prototype/push/throws-if-integer-limit-exceeded.js:
default: 'Test262Error: Length is 2**53 - 1 Expected a TypeError to be thrown but no exception was thrown at all'
strict mode: 'Test262Error: Length is 2**53 - 1 Expected a TypeError to be thrown but no exception was thrown at all'
test/built-ins/Array/prototype/reverse/length-exceeding-integer-limit-with-proxy.js:
default: 'Test262Error: Expected a StopReverse but got a Test262Error'
strict mode: 'Test262Error: Expected a StopReverse but got a Test262Error'
default: 'Test262Error: Expected [Get:length, Has:0, Get:0, Has:4294967294, Delete:0, Set:4294967294, GetOwnPropertyDescriptor:4294967294, DefineProperty:4294967294, Has:1, Has:4294967293, Delete:1, Delete:4294967293, Has:2, Get:2, Has:4294967292, Delete:2, Set:4294967292, GetOwnPropertyDescriptor:4294967292, DefineProperty:4294967292, Has:3, Has:4294967291, Delete:3, Delete:4294967291, Has:4, Get:4] and [Get:length, Has:0, Get:0, Has:9007199254740990, Get:9007199254740990, Set:0, GetOwnPropertyDescriptor:0, DefineProperty:0, Set:9007199254740990, GetOwnPropertyDescriptor:9007199254740990, DefineProperty:9007199254740990, Has:1, Has:9007199254740989, Has:2, Get:2, Has:9007199254740988, Delete:2, Set:9007199254740988, GetOwnPropertyDescriptor:9007199254740988, DefineProperty:9007199254740988, Has:3, Has:9007199254740987, Get:9007199254740987, Set:3, GetOwnPropertyDescriptor:3, DefineProperty:3, Delete:9007199254740987, Has:4, Get:4] to have the same contents. '
strict mode: 'Test262Error: Expected [Get:length, Has:0, Get:0, Has:4294967294, Delete:0, Set:4294967294, GetOwnPropertyDescriptor:4294967294, DefineProperty:4294967294, Has:1, Has:4294967293, Delete:1, Delete:4294967293, Has:2, Get:2, Has:4294967292, Delete:2, Set:4294967292, GetOwnPropertyDescriptor:4294967292, DefineProperty:4294967292, Has:3, Has:4294967291, Delete:3, Delete:4294967291, Has:4, Get:4] and [Get:length, Has:0, Get:0, Has:9007199254740990, Get:9007199254740990, Set:0, GetOwnPropertyDescriptor:0, DefineProperty:0, Set:9007199254740990, GetOwnPropertyDescriptor:9007199254740990, DefineProperty:9007199254740990, Has:1, Has:9007199254740989, Has:2, Get:2, Has:9007199254740988, Delete:2, Set:9007199254740988, GetOwnPropertyDescriptor:9007199254740988, DefineProperty:9007199254740988, Has:3, Has:9007199254740987, Get:9007199254740987, Set:3, GetOwnPropertyDescriptor:3, DefineProperty:3, Delete:9007199254740987, Has:4, Get:4] to have the same contents. '
test/built-ins/Array/prototype/slice/length-exceeding-integer-limit-proxied-array.js:
default: 'Test262Error: Expected [] and [9007199254740989, 9007199254740990] to have the same contents. slice(9007199254740989)'
strict mode: 'Test262Error: Expected [] and [9007199254740989, 9007199254740990] to have the same contents. slice(9007199254740989)'
@@ -685,8 +679,8 @@ test/built-ins/Array/prototype/splice/create-proxy.js:
default: 'TypeError: Attempted to assign to readonly property.'
strict mode: 'TypeError: Attempted to assign to readonly property.'
test/built-ins/Array/prototype/splice/create-species-length-exceeding-integer-limit.js:
default: 'Test262Error: Expected a StopSplice but got a Test262Error'
strict mode: 'Test262Error: Expected a StopSplice but got a Test262Error'
default: 'Test262Error: length and deleteCount were correctly clamped to 2^53-1 Expected SameValue(«4294967295», «9007199254740991») to be true'
strict mode: 'Test262Error: length and deleteCount were correctly clamped to 2^53-1 Expected SameValue(«4294967295», «9007199254740991») to be true'
test/built-ins/Array/prototype/splice/length-and-deleteCount-exceeding-integer-limit.js:
default: "Test262Error: Expected [] and [9007199254740989, 9007199254740990] to have the same contents. arrayLike['9007199254740989'] and arrayLike['9007199254740990'] are removed"
strict mode: "Test262Error: Expected [] and [9007199254740989, 9007199254740990] to have the same contents. arrayLike['9007199254740989'] and arrayLike['9007199254740990'] are removed"
@@ -1153,9 +1147,6 @@ test/built-ins/Proxy/construct/return-not-object-throws-undefined-realm.js:
test/built-ins/Proxy/construct/trap-is-not-callable-realm.js:
default: 'Test262Error: Expected a TypeError but got a TypeError'
strict mode: 'Test262Error: Expected a TypeError but got a TypeError'
test/built-ins/Proxy/has/call-in-prototype.js:
default: 'Test262Error: [[GetPrototypeOf]] trap called'
strict mode: 'Test262Error: [[GetPrototypeOf]] trap called'
test/built-ins/Proxy/revocable/revocation-function-name.js:
default: 'Test262Error: obj should have an own property name'
strict mode: 'Test262Error: obj should have an own property name'
@@ -1,3 +1,22 @@
2019-11-07 Alexey Shvayka <shvaikalesh@gmail.com>

[[HasProperty]] result of Proxy in prototype chain is ignored
https://bugs.webkit.org/show_bug.cgi?id=203560

Reviewed by Ross Kirsling.

Before this change, when [[HasProperty]] was called on ordinary object with Proxy in prototype chain,
falsy result of Proxy's "has" trap was ignored and prototype chain was inspected further.

According to spec, OrdinaryHasProperty unconditionally returns result of parent's [[HasProperty]] call.
(step 5.a of https://tc39.es/ecma262/#sec-ordinaryhasproperty)

* runtime/JSObjectInlines.h:
(JSC::JSObject::getPropertySlot):
(JSC::JSObject::getNonIndexPropertySlot):
* runtime/ProxyObject.cpp:
(JSC::ProxyObject::performHasProperty): Walk the prototype chain in performDefaultHasProperty.

2019-11-06 Mark Lam <mark.lam@apple.com>

Remove remnants of support code for an upwards growing stack.
@@ -126,6 +126,8 @@ ALWAYS_INLINE bool JSObject::getPropertySlot(JSGlobalObject* globalObject, unsig
RETURN_IF_EXCEPTION(scope, false);
if (hasSlot)
return true;
if (object->type() == ProxyObjectType && slot.internalMethodType() == PropertySlot::InternalMethodType::HasProperty)
return false;
JSValue prototype;
if (LIKELY(structure->classInfo()->methodTable.getPrototype == defaultGetPrototype || slot.internalMethodType() == PropertySlot::InternalMethodType::VMInquiry))
prototype = object->getPrototypeDirect(vm);
@@ -159,6 +161,8 @@ ALWAYS_INLINE bool JSObject::getNonIndexPropertySlot(JSGlobalObject* globalObjec
RETURN_IF_EXCEPTION(scope, false);
if (hasSlot)
return true;
if (object->type() == ProxyObjectType && slot.internalMethodType() == PropertySlot::InternalMethodType::HasProperty)
return false;
}
JSValue prototype;
if (LIKELY(structure->classInfo()->methodTable.getPrototype == defaultGetPrototype || slot.internalMethodType() == PropertySlot::InternalMethodType::VMInquiry))
@@ -322,7 +322,7 @@ bool ProxyObject::performHasProperty(JSGlobalObject* globalObject, PropertyName
slot.setValue(this, static_cast<unsigned>(PropertyAttribute::None), jsUndefined()); // Nobody should rely on our value, but be safe and protect against any bad actors reading our value.

auto performDefaultHasProperty = [&] {
return target->methodTable(vm)->getOwnPropertySlot(target, globalObject, propertyName, slot);
return target->getPropertySlot(globalObject, propertyName, slot);
};

if (propertyName.isPrivateName())

0 comments on commit ae717be

Please sign in to comment.