[[HasProperty]] result of Proxy in prototype chain is ignored
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Nov 2019 18:31:31 +0000 (18:31 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Nov 2019 18:31:31 +0000 (18:31 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@252191 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/proxy-get-prototype-of.js
JSTests/test262/expectations.yaml
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSObjectInlines.h
Source/JavaScriptCore/runtime/ProxyObject.cpp

index b4dab01..2540353 100644 (file)
@@ -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.
index de01de4..f6909ed 100644 (file)
@@ -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);
     }
 }
 
index 7f45f19..55fa62b 100644 (file)
@@ -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'
index 88b6b20..1a5aac6 100644 (file)
@@ -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.
index f3f5ab5..6027f19 100644 (file)
@@ -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))
index f83b913..cc72ba7 100644 (file)
@@ -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())