Changeset 205535 in webkit


Ignore:
Timestamp:
Sep 6, 2016 7:48:35 PM (8 years ago)
Author:
sbarati@apple.com
Message:

ProxyObject's structure should not have ObjectPrototype as its prototype and it should not have special behavior for intercepting "proto"
https://bugs.webkit.org/show_bug.cgi?id=161558

Reviewed by Benjamin Poulain.

JSTests:

  • stress/proxy-get-prototype-of.js:
  • stress/proxy-set-prototype-of.js:

(let.handler.setPrototypeOf): Deleted.

  • stress/proxy-underscore-proto.js: Added.

(assert):

Source/JavaScriptCore:

ProxyObject had ObjectPrototype as its direct prototype.
This could lead to infinite loops when doing a getDirectPrototype()
loop.

Fixing this bug revealed another bug, which I made when implementing Proxy.
We should not special case "proto" in get and set for Proxy Object's
hooks. "proto" should just go through the normal set and get path.

  • runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::init):

  • runtime/ProxyObject.cpp:

(JSC::performProxyGet):
(JSC::ProxyObject::put):

Location:
trunk
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed
  • trunk/JSTests/ChangeLog

    r205520 r205535  
     12016-09-06  Saam Barati  <sbarati@apple.com>
     2
     3        ProxyObject's structure should not have ObjectPrototype as its prototype and it should not have special behavior for intercepting "__proto__"
     4        https://bugs.webkit.org/show_bug.cgi?id=161558
     5
     6        Reviewed by Benjamin Poulain.
     7
     8        * stress/proxy-get-prototype-of.js:
     9        * stress/proxy-set-prototype-of.js:
     10        (let.handler.setPrototypeOf): Deleted.
     11        * stress/proxy-underscore-proto.js: Added.
     12        (assert):
     13
    1142016-09-06  Saam Barati  <sbarati@apple.com>
    215
  • trunk/JSTests/stress/proxy-get-prototype-of.js

    r198813 r205535  
    1919            () => Reflect.getPrototypeOf(proxy),
    2020            () => Object.getPrototypeOf(proxy),
    21             () => proxy.__proto__,
    2221        ];
    2322        for (let get of getters) {
     
    4948            () => Reflect.getPrototypeOf(proxy),
    5049            () => Object.getPrototypeOf(proxy),
    51             () => proxy.__proto__,
    5250        ];
    5351        for (let get of getters) {
     
    8482            () => Reflect.getPrototypeOf(proxy),
    8583            () => Object.getPrototypeOf(proxy),
    86             () => proxy.__proto__,
    8784        ];
    8885        for (let get of getters) {
     
    110107            () => Reflect.getPrototypeOf(proxy),
    111108            () => Object.getPrototypeOf(proxy),
    112             () => proxy.__proto__,
    113109        ];
    114110        for (let get of getters) {
     
    138134            () => Reflect.getPrototypeOf(proxy),
    139135            () => Object.getPrototypeOf(proxy),
    140             () => proxy.__proto__,
    141136        ];
    142137        for (let get of getters) {
     
    166161            () => Reflect.getPrototypeOf(proxy),
    167162            () => Object.getPrototypeOf(proxy),
    168             () => proxy.__proto__,
    169163        ];
    170164        for (let get of getters) {
     
    195189            () => Reflect.getPrototypeOf(proxy),
    196190            () => Object.getPrototypeOf(proxy),
    197             () => proxy.__proto__,
    198191        ];
    199192        for (let get of getters) {
     
    225218            () => Reflect.getPrototypeOf(proxy),
    226219            () => Object.getPrototypeOf(proxy),
    227             () => proxy.__proto__,
    228220        ];
    229221        for (let get of getters) {
     
    256248            () => Reflect.getPrototypeOf(proxy),
    257249            () => Object.getPrototypeOf(proxy),
    258             () => proxy.__proto__,
    259250        ];
    260251        for (let get of getters) {
     
    286277            () => Reflect.getPrototypeOf(proxy),
    287278            () => Object.getPrototypeOf(proxy),
    288             () => proxy.__proto__,
    289279        ];
    290280        for (let get of getters) {
     
    309299            () => Reflect.getPrototypeOf(proxy),
    310300            () => Object.getPrototypeOf(proxy),
    311             () => proxy.__proto__,
    312301        ];
    313302        for (let get of getters) {
     
    334323            () => Reflect.getPrototypeOf(proxy),
    335324            () => Object.getPrototypeOf(proxy),
    336             () => proxy.__proto__,
    337325        ];
    338326        for (let get of getters) {
     
    361349            () => Reflect.getPrototypeOf(proxy),
    362350            () => Object.getPrototypeOf(proxy),
    363             () => proxy.__proto__,
    364351        ];
    365352        for (let get of getters) {
     
    388375            () => Reflect.getPrototypeOf(proxy),
    389376            () => Object.getPrototypeOf(proxy),
    390             () => proxy.__proto__,
    391377        ];
    392378        for (let get of getters) {
  • trunk/JSTests/stress/proxy-set-prototype-of.js

    r198813 r205535  
    1919            () => Reflect.setPrototypeOf(proxy, {}),
    2020            () => Object.setPrototypeOf(proxy, {}),
    21             () => proxy.__proto__ = {},
    2221        ];
    2322        for (let set of setters) {
     
    4948            () => Reflect.setPrototypeOf(proxy, {}),
    5049            () => Object.setPrototypeOf(proxy, {}),
    51             () => proxy.__proto__ = {},
    5250        ];
    5351        for (let set of setters) {
     
    7674            () => Reflect.setPrototypeOf(proxy, {}),
    7775            () => Object.setPrototypeOf(proxy, {}),
    78             () => proxy.__proto__ = {},
    7976        ];
    8077        for (let set of setters) {
     
    105102            () => Reflect.setPrototypeOf(proxy, {}),
    106103            () => Object.setPrototypeOf(proxy, {}),
    107             () => proxy.__proto__ = {},
    108104        ];
    109105        for (let set of setters) {
     
    111107            assert(result);
    112108            assert(Reflect.getPrototypeOf(target) === null);
    113             // FIXME: when we implement Proxy.[[GetPrototypeOf]] this should work.
    114             //assert(Reflect.getPrototypeOf(proxy) === null);
    115             //assert(proxy.__proto__ === null);
     109            assert(Reflect.getPrototypeOf(proxy) === null);
     110            assert(proxy.__proto__ === undefined);
    116111        }
    117112    }
     
    134129            () => Reflect.setPrototypeOf(proxy, obj),
    135130            () => Object.setPrototypeOf(proxy, obj),
    136             () => proxy.__proto__ = obj,
    137131        ];
    138132        for (let set of setters) {
     
    140134            assert(result);
    141135            assert(Reflect.getPrototypeOf(target) === obj);
    142             // FIXME: when we implement Proxy.[[GetPrototypeOf]] this should work.
    143             //assert(Reflect.getPrototypeOf(proxy) === obj);
    144             //assert(proxy.__proto__ === obj);
     136            assert(Reflect.getPrototypeOf(proxy) === obj);
     137            assert(proxy.__proto__ === obj);
    145138        }
    146139    }
     
    168161            () => Reflect.setPrototypeOf(proxy, obj),
    169162            () => Object.setPrototypeOf(proxy, obj),
    170             () => proxy.__proto__ = obj,
    171163        ];
    172164        for (let set of setters) {
     
    183175            assert(threw);
    184176            assert(Reflect.getPrototypeOf(target) === null);
    185             // FIXME: when we implement Proxy.[[GetPrototypeOf]] this should work.
    186             //assert(Reflect.getPrototypeOf(proxy) === null);
    187             //assert(proxy.__proto__ === null);
     177            assert(Reflect.getPrototypeOf(proxy) === null);
     178            assert(proxy.__proto__ === undefined);
    188179        }
    189180    }
     
    210201            [() => Reflect.setPrototypeOf(proxy, null), true],
    211202            [() => Object.setPrototypeOf(proxy, null), proxy],
    212             [() => proxy.__proto__ = null, null]
    213203        ];
    214204        for (let [set, expectedResult] of setters) {
     
    218208            called = false;
    219209            assert(Reflect.getPrototypeOf(target) === null);
    220             // FIXME: when we implement Proxy.[[GetPrototypeOf]] this should work.
    221             //assert(Reflect.getPrototypeOf(proxy) === null);
    222             //assert(proxy.__proto__ === null);
     210            assert(Reflect.getPrototypeOf(proxy) === null);
     211            assert(proxy.__proto__ === undefined);
    223212        }
    224213    }
     
    246235            [() => Reflect.setPrototypeOf(proxy, obj), true],
    247236            [() => Object.setPrototypeOf(proxy, obj), proxy],
    248             [() => proxy.__proto__ = obj, obj]
    249237        ];
    250238        for (let [set, expectedResult] of setters) {
     
    254242            called = false;
    255243            assert(Reflect.getPrototypeOf(target) === obj);
    256             // FIXME: when we implement Proxy.[[GetPrototypeOf]] this should work.
    257             //assert(Reflect.getPrototypeOf(proxy) === null);
    258             //assert(proxy.__proto__ === null);
     244            assert(Reflect.getPrototypeOf(proxy) === obj);
     245            assert(proxy.__proto__ === obj);
    259246        }
    260247    }
     
    284271        assert(threw);
    285272        assert(Reflect.getPrototypeOf(target) === null);
    286         // FIXME: when we implement Proxy.[[GetPrototypeOf]] this should work.
    287         //assert(Reflect.getPrototypeOf(proxy) === null);
    288         //assert(proxy.__proto__ === null);
    289     }
    290 }
    291 
    292 {
    293     let target = {};
    294     target.__proto__ = null;
    295     Reflect.preventExtensions(target);
    296     let handler = {
    297         setPrototypeOf: function(theTarget, value) {
    298             return Reflect.setPrototypeOf(theTarget, value);
    299         }
    300     };
    301    
    302     let proxy = new Proxy(target, handler);
    303     for (let i = 0; i < 500; i++) {
    304         let obj = {};
    305         let threw = false;
    306         try {
    307             proxy.__proto__ = obj;
    308         } catch(e) {
    309             threw = true;
    310             assert(e.toString() === "TypeError: Proxy 'setPrototypeOf' returned false indicating it could not set the prototype value. The operation was expected to succeed");
    311         }
    312 
    313         assert(threw);
    314         assert(Reflect.getPrototypeOf(target) === null);
    315         // FIXME: when we implement Proxy.[[GetPrototypeOf]] this should work.
    316         //assert(Reflect.getPrototypeOf(proxy) === null);
    317         //assert(proxy.__proto__ === null);
     273        assert(Reflect.getPrototypeOf(proxy) === null);
     274        assert(proxy.__proto__ === undefined);
    318275    }
    319276}
     
    340297        assert(called);
    341298        called = false;
    342         // FIXME: when we implement Proxy.[[GetPrototypeOf]] this should work.
    343         //assert(Reflect.getPrototypeOf(proxy) === null);
    344         //assert(proxy.__proto__ === null);
     299        assert(Reflect.getPrototypeOf(proxy) === null);
     300        assert(proxy.__proto__ === undefined);
    345301    }
    346302}
     
    367323        called = false;
    368324
    369         // FIXME: when we implement Proxy.[[GetPrototypeOf]] this should work.
    370         //assert(Reflect.getPrototypeOf(proxy) === null);
    371         //assert(proxy.__proto__ === null);
     325        assert(Reflect.getPrototypeOf(proxy) === null);
     326        assert(proxy.__proto__ === undefined);
    372327    }
    373328}
  • trunk/Source/JavaScriptCore/ChangeLog

    r205534 r205535  
     12016-09-06  Saam Barati  <sbarati@apple.com>
     2
     3        ProxyObject's structure should not have ObjectPrototype as its prototype and it should not have special behavior for intercepting "__proto__"
     4        https://bugs.webkit.org/show_bug.cgi?id=161558
     5
     6        Reviewed by Benjamin Poulain.
     7
     8        ProxyObject had ObjectPrototype as its direct prototype.
     9        This could lead to infinite loops when doing a getDirectPrototype()
     10        loop.
     11
     12        Fixing this bug revealed another bug, which I made when implementing Proxy.
     13        We should not special case "__proto__" in get and set for Proxy Object's
     14        hooks. "__proto__" should just go through the normal set and get path.
     15
     16        * runtime/JSGlobalObject.cpp:
     17        (JSC::JSGlobalObject::init):
     18        * runtime/ProxyObject.cpp:
     19        (JSC::performProxyGet):
     20        (JSC::ProxyObject::put):
     21
    1222016-09-06  Yusuke Suzuki  <utatane.tea@gmail.com>
    223
  • trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp

    r205520 r205535  
    509509    {
    510510        bool isCallable = false;
    511         m_proxyObjectStructure.set(vm, this, ProxyObject::createStructure(vm, this, m_objectPrototype.get(), isCallable));
     511        m_proxyObjectStructure.set(vm, this, ProxyObject::createStructure(vm, this, jsNull(), isCallable));
    512512        isCallable = true;
    513         m_callableProxyObjectStructure.set(vm, this, ProxyObject::createStructure(vm, this, m_objectPrototype.get(), isCallable));
     513        m_callableProxyObjectStructure.set(vm, this, ProxyObject::createStructure(vm, this, jsNull(), isCallable));
    514514    }
    515515    m_proxyRevokeStructure.set(vm, this, ProxyRevoke::createStructure(vm, this, m_functionPrototype.get()));
  • trunk/Source/JavaScriptCore/runtime/ProxyObject.cpp

    r205462 r205535  
    126126    JSObject* target = proxyObject->target();
    127127
    128     if (propertyName == vm.propertyNames->underscoreProto)
    129         return proxyObject->performGetPrototype(exec);
    130 
    131128    auto performDefaultGet = [&] {
    132129        return target->get(exec, propertyName);
     
    459456    VM& vm = exec->vm();
    460457    slot.disableCaching();
    461     if (propertyName == vm.propertyNames->underscoreProto)
    462         return Base::put(cell, exec, propertyName, value, slot);
    463458
    464459    ProxyObject* thisObject = jsCast<ProxyObject*>(cell);
Note: See TracChangeset for help on using the changeset viewer.