⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 259822 in webkit


Ignore:
Timestamp:
Apr 9, 2020, 1:40:38 PM (6 years ago)
Author:
Alexey Shvayka
Message:

ProxyObject::defineOwnProperty() should conditionally throw on falsy trap result
​https://bugs.webkit.org/show_bug.cgi?id=210267

Reviewed by Ross Kirsling.

JSTests:

  • test262/expectations.yaml: Mark 2 test cases as passing.

Source/JavaScriptCore:

This change adds conditional TypeError for falsy trap result [1], like there is in
ProxyObject::performPut(), aligning JSC with V8 and SpiderMonkey. Also replaces
throwVMTypeError() calls which results are unused with throwTypeError().

[1]: ​https://tc39.es/ecma262/#sec-definepropertyorthrow (step 4)

  • runtime/ProxyObject.cpp:

(JSC::ProxyObject::performDefineOwnProperty)

Location:
trunk
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r259807 r259822  
     12020-04-09  Alexey Shvayka  <shvaikalesh@gmail.com>
     2
     3        ProxyObject::defineOwnProperty() should conditionally throw on falsy trap result
     4        https://bugs.webkit.org/show_bug.cgi?id=210267
     5
     6        Reviewed by Ross Kirsling.
     7
     8        * test262/expectations.yaml: Mark 2 test cases as passing.
     9
    1102020-04-09  Saam Barati  <sbarati@apple.com>
    211

  • trunk/JSTests/test262/expectations.yaml

    r259800 r259822  
    12501250  default: "TypeError: A Proxy's 'target' shouldn't be a revoked Proxy"
    12511251  strict mode: "TypeError: A Proxy's 'target' shouldn't be a revoked Proxy"
    1252 test/built-ins/Proxy/defineProperty/trap-is-undefined-target-is-proxy.js:
    1253   default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
    1254   strict mode: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
    12551252test/built-ins/Proxy/ownKeys/trap-is-undefined-target-is-proxy.js:
    12561253  default: 'Test262Error: Expected [length, foo, 0, Symbol()] and [Symbol(), length, foo, 0] to have the same contents. '

  • trunk/Source/JavaScriptCore/ChangeLog

    r259810 r259822  
     12020-04-09  Alexey Shvayka  <shvaikalesh@gmail.com>
     2
     3        ProxyObject::defineOwnProperty() should conditionally throw on falsy trap result
     4        https://bugs.webkit.org/show_bug.cgi?id=210267
     5
     6        Reviewed by Ross Kirsling.
     7
     8        This change adds conditional TypeError for falsy trap result [1], like there is in
     9        ProxyObject::performPut(), aligning JSC with V8 and SpiderMonkey. Also replaces
     10        throwVMTypeError() calls which results are unused with throwTypeError().
     11
     12        [1]: https://tc39.es/ecma262/#sec-definepropertyorthrow (step 4)
     13
     14        * runtime/ProxyObject.cpp:
     15        (JSC::ProxyObject::performDefineOwnProperty)
     16
    1172020-04-09  Devin Rousso  <drousso@apple.com>
    218

  • trunk/Source/JavaScriptCore/runtime/ProxyObject.cpp

    r259676 r259822  
    227227    JSValue handlerValue = this->handler();
    228228    if (handlerValue.isNull()) {
    229         throwVMTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
     229        throwTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
    230230        return false;
    231231    }
    … …  
    247247
    248248    if (!trapResult.isUndefined() && !trapResult.isObject()) {
    249         throwVMTypeError(globalObject, scope, "result of 'getOwnPropertyDescriptor' call should either be an Object or undefined"_s);
     249        throwTypeError(globalObject, scope, "result of 'getOwnPropertyDescriptor' call should either be an Object or undefined"_s);
    250250        return false;
    251251    }
    … …  
    259259            return false;
    260260        if (!targetPropertyDescriptor.configurable()) {
    261             throwVMTypeError(globalObject, scope, "When the result of 'getOwnPropertyDescriptor' is undefined the target must be configurable"_s);
     261            throwTypeError(globalObject, scope, "When the result of 'getOwnPropertyDescriptor' is undefined the target must be configurable"_s);
    262262            return false;
    263263        }
    … …  
    265265        RETURN_IF_EXCEPTION(scope, false);
    266266        if (!isExtensible) {
    267             throwVMTypeError(globalObject, scope, "When 'getOwnPropertyDescriptor' returns undefined, the 'target' of a Proxy should be extensible"_s);
     267            throwTypeError(globalObject, scope, "When 'getOwnPropertyDescriptor' returns undefined, the 'target' of a Proxy should be extensible"_s);
    268268            return false;
    269269        }
    … …  
    282282    RETURN_IF_EXCEPTION(scope, false);
    283283    if (!valid) {
    284         throwVMTypeError(globalObject, scope, "Result from 'getOwnPropertyDescriptor' fails the IsCompatiblePropertyDescriptor test"_s);
     284        throwTypeError(globalObject, scope, "Result from 'getOwnPropertyDescriptor' fails the IsCompatiblePropertyDescriptor test"_s);
    285285        return false;
    286286    }
    … …  
    288288    if (!trapResultAsDescriptor.configurable()) {
    289289        if (!isTargetPropertyDescriptorDefined || targetPropertyDescriptor.configurable()) {
    290             throwVMTypeError(globalObject, scope, "Result from 'getOwnPropertyDescriptor' can't be non-configurable when the 'target' doesn't have it as an own property or if it is a configurable own property on 'target'"_s);
     290            throwTypeError(globalObject, scope, "Result from 'getOwnPropertyDescriptor' can't be non-configurable when the 'target' doesn't have it as an own property or if it is a configurable own property on 'target'"_s);
    291291            return false;
    292292        }
    293293        if (trapResultAsDescriptor.writablePresent() && !trapResultAsDescriptor.writable() && targetPropertyDescriptor.writable()) {
    294             throwVMTypeError(globalObject, scope, "Result from 'getOwnPropertyDescriptor' can't be non-configurable and non-writable when the target's property is writable"_s);
     294            throwTypeError(globalObject, scope, "Result from 'getOwnPropertyDescriptor' can't be non-configurable and non-writable when the target's property is writable"_s);
    295295            return false;
    296296        }
    … …  
    331331    JSValue handlerValue = this->handler();
    332332    if (handlerValue.isNull()) {
    333         throwVMTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
     333        throwTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
    334334        return false;
    335335    }
    … …  
    359359        if (isPropertyDescriptorDefined) {
    360360            if (!descriptor.configurable()) {
    361                 throwVMTypeError(globalObject, scope, "Proxy 'has' must return 'true' for non-configurable properties"_s);
     361                throwTypeError(globalObject, scope, "Proxy 'has' must return 'true' for non-configurable properties"_s);
    362362                return false;
    363363            }
    … …  
    365365            RETURN_IF_EXCEPTION(scope, false);
    366366            if (!isExtensible) {
    367                 throwVMTypeError(globalObject, scope, "Proxy 'has' must return 'true' for a non-extensible 'target' object with a configurable property"_s);
     367                throwTypeError(globalObject, scope, "Proxy 'has' must return 'true' for a non-extensible 'target' object with a configurable property"_s);
    368368                return false;
    369369            }
    … …  
    434434    JSValue handlerValue = this->handler();
    435435    if (handlerValue.isNull()) {
    436         throwVMTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
     436        throwTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
    437437        return false;
    438438    }
    … …  
    459459    if (!trapResultAsBool) {
    460460        if (shouldThrow)
    461             throwVMTypeError(globalObject, scope, makeString("Proxy object's 'set' trap returned falsy value for property '", String(propertyName.uid()), "'"));
     461            throwTypeError(globalObject, scope, makeString("Proxy object's 'set' trap returned falsy value for property '", String(propertyName.uid()), "'"));
    462462        return false;
    463463    }
    … …  
    471471            RETURN_IF_EXCEPTION(scope, false);
    472472            if (!isSame) {
    473                 throwVMTypeError(globalObject, scope, "Proxy handler's 'set' on a non-configurable and non-writable property on 'target' should either return false or be the same value already on the 'target'"_s);
     473                throwTypeError(globalObject, scope, "Proxy handler's 'set' on a non-configurable and non-writable property on 'target' should either return false or be the same value already on the 'target'"_s);
    474474                return false;
    475475            }
    476476        } else if (descriptor.isAccessorDescriptor() && !descriptor.configurable() && descriptor.setter().isUndefined()) {
    477             throwVMTypeError(globalObject, scope, "Proxy handler's 'set' method on a non-configurable accessor property without a setter should return false"_s);
     477            throwTypeError(globalObject, scope, "Proxy handler's 'set' method on a non-configurable accessor property without a setter should return false"_s);
    478478            return false;
    479479        }
    … …  
    639639    JSValue handlerValue = this->handler();
    640640    if (handlerValue.isNull()) {
    641         throwVMTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
     641        throwTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
    642642        return false;
    643643    }
    … …  
    670670    if (result) {
    671671        if (!descriptor.configurable()) {
    672             throwVMTypeError(globalObject, scope, "Proxy handler's 'deleteProperty' method should return false when the target's property is not configurable"_s);
     672            throwTypeError(globalObject, scope, "Proxy handler's 'deleteProperty' method should return false when the target's property is not configurable"_s);
    673673            return false;
    674674        }
    … …  
    676676        RETURN_IF_EXCEPTION(scope, false);
    677677        if (!targetIsExtensible) {
    678             throwVMTypeError(globalObject, scope, "Proxy handler's 'deleteProperty' method should return false when the target has property and is not extensible"_s);
     678            throwTypeError(globalObject, scope, "Proxy handler's 'deleteProperty' method should return false when the target has property and is not extensible"_s);
    679679            return false;
    680680        }
    … …  
    721721    JSValue handlerValue = this->handler();
    722722    if (handlerValue.isNull()) {
    723         throwVMTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
     723        throwTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
    724724        return false;
    725725    }
    … …  
    747747        RETURN_IF_EXCEPTION(scope, false);
    748748        if (targetIsExtensible) {
    749             throwVMTypeError(globalObject, scope, "Proxy's 'preventExtensions' trap returned true even though its target is extensible. It should have returned false"_s);
     749            throwTypeError(globalObject, scope, "Proxy's 'preventExtensions' trap returned true even though its target is extensible. It should have returned false"_s);
    750750            return false;
    751751        }
    … …  
    773773    JSValue handlerValue = this->handler();
    774774    if (handlerValue.isNull()) {
    775         throwVMTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
     775        throwTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
    776776        return false;
    777777    }
    … …  
    802802        if (isTargetExtensible) {
    803803            ASSERT(!trapResultAsBool);
    804             throwVMTypeError(globalObject, scope, "Proxy object's 'isExtensible' trap returned false when the target is extensible. It should have returned true"_s);
     804            throwTypeError(globalObject, scope, "Proxy object's 'isExtensible' trap returned false when the target is extensible. It should have returned true"_s);
    805805        } else {
    806806            ASSERT(!isTargetExtensible);
    807807            ASSERT(trapResultAsBool);
    808             throwVMTypeError(globalObject, scope, "Proxy object's 'isExtensible' trap returned true when the target is non-extensible. It should have returned false"_s);
     808            throwTypeError(globalObject, scope, "Proxy object's 'isExtensible' trap returned true when the target is non-extensible. It should have returned false"_s);
    809809        }
    810810    }
    … …  
    839839    JSValue handlerValue = this->handler();
    840840    if (handlerValue.isNull()) {
    841         throwVMTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
     841        throwTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
    842842        return false;
    843843    }
    … …  
    866866    RETURN_IF_EXCEPTION(scope, false);
    867867
    868     if (!trapResultAsBool)
    869         return false;
     868    if (!trapResultAsBool) {
     869        if (shouldThrow)
     870            throwTypeError(globalObject, scope, makeString("Proxy's 'defineProperty' trap returned falsy value for property '", String(propertyName.uid()), "'"));
     871        return false;
     872    }
    870873
    871874    PropertyDescriptor targetDescriptor;
    … …  
    879882    if (!isTargetDescriptorDefined) {
    880883        if (!targetIsExtensible) {
    881             throwVMTypeError(globalObject, scope, "Proxy's 'defineProperty' trap returned true even though getOwnPropertyDescriptor of the Proxy's target returned undefined and the target is non-extensible"_s);
     884            throwTypeError(globalObject, scope, "Proxy's 'defineProperty' trap returned true even though getOwnPropertyDescriptor of the Proxy's target returned undefined and the target is non-extensible"_s);
    882885            return false;
    883886        }
    884887        if (settingConfigurableToFalse) {
    885             throwVMTypeError(globalObject, scope, "Proxy's 'defineProperty' trap returned true for a non-configurable field even though getOwnPropertyDescriptor of the Proxy's target returned undefined"_s);
     888            throwTypeError(globalObject, scope, "Proxy's 'defineProperty' trap returned true for a non-configurable field even though getOwnPropertyDescriptor of the Proxy's target returned undefined"_s);
    886889            return false;
    887890        }
    … …  
    897900    RETURN_IF_EXCEPTION(scope, false);   
    898901    if (!isCompatibleDescriptor) {
    899         throwVMTypeError(globalObject, scope, "Proxy's 'defineProperty' trap did not define a property on its target that is compatible with the trap's input descriptor"_s);
     902        throwTypeError(globalObject, scope, "Proxy's 'defineProperty' trap did not define a property on its target that is compatible with the trap's input descriptor"_s);
    900903        return false;
    901904    }
    902905    if (settingConfigurableToFalse && targetDescriptor.configurable()) {
    903         throwVMTypeError(globalObject, scope, "Proxy's 'defineProperty' trap did not define a non-configurable property on its target even though the input descriptor to the trap said it must do so"_s);
     906        throwTypeError(globalObject, scope, "Proxy's 'defineProperty' trap did not define a non-configurable property on its target even though the input descriptor to the trap said it must do so"_s);
    904907        return false;
    905908    }
    … …  
    932935    JSValue handlerValue = this->handler();
    933936    if (handlerValue.isNull()) {
    934         throwVMTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
     937        throwTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
    935938        return;
    936939    }
    … …  
    10241027    for (UniquedStringImpl* impl : targetNonConfigurableKeys) {
    10251028        if (removeIfContainedInUncheckedResultKeys(impl) == IsNotContainedIn) {
    1026             throwVMTypeError(globalObject, scope, makeString("Proxy object's 'target' has the non-configurable property '", String(impl), "' that was not in the result from the 'ownKeys' trap"));
     1029            throwTypeError(globalObject, scope, makeString("Proxy object's 'target' has the non-configurable property '", String(impl), "' that was not in the result from the 'ownKeys' trap"));
    10271030            return;
    10281031        }
    … …  
    10321035        for (UniquedStringImpl* impl : targetConfigurableKeys) {
    10331036            if (removeIfContainedInUncheckedResultKeys(impl) == IsNotContainedIn) {
    1034                 throwVMTypeError(globalObject, scope, makeString("Proxy object's non-extensible 'target' has configurable property '", String(impl), "' that was not in the result from the 'ownKeys' trap"));
     1037                throwTypeError(globalObject, scope, makeString("Proxy object's non-extensible 'target' has configurable property '", String(impl), "' that was not in the result from the 'ownKeys' trap"));
    10351038                return;
    10361039            }
    … …  
    10381041
    10391042        if (uncheckedResultKeys.size()) {
    1040             throwVMTypeError(globalObject, scope, "Proxy handler's 'ownKeys' method returned a key that was not present in its non-extensible target"_s);
     1043            throwTypeError(globalObject, scope, "Proxy handler's 'ownKeys' method returned a key that was not present in its non-extensible target"_s);
    10411044            return;
    10421045        }
    … …  
    11111114    JSValue handlerValue = this->handler();
    11121115    if (handlerValue.isNull()) {
    1113         throwVMTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
     1116        throwTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
    11141117        return false;
    11151118    }
    … …  
    11371140    if (!trapResultAsBool) {
    11381141        if (shouldThrowIfCantSet)
    1139             throwVMTypeError(globalObject, scope, "Proxy 'setPrototypeOf' returned false indicating it could not set the prototype value. The operation was expected to succeed"_s);
     1142            throwTypeError(globalObject, scope, "Proxy 'setPrototypeOf' returned false indicating it could not set the prototype value. The operation was expected to succeed"_s);
    11401143        return false;
    11411144    }
    … …  
    11511154    RETURN_IF_EXCEPTION(scope, false);
    11521155    if (!isSame) {
    1153         throwVMTypeError(globalObject, scope, "Proxy 'setPrototypeOf' trap returned true when its target is non-extensible and the new prototype value is not the same as the current prototype value. It should have returned false"_s);
     1156        throwTypeError(globalObject, scope, "Proxy 'setPrototypeOf' trap returned true when its target is non-extensible and the new prototype value is not the same as the current prototype value. It should have returned false"_s);
    11541157        return false;
    11551158    }
    … …  
    11761179    JSValue handlerValue = this->handler();
    11771180    if (handlerValue.isNull()) {
    1178         throwVMTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
     1181        throwTypeError(globalObject, scope, s_proxyAlreadyRevokedErrorMessage);
    11791182        return { };
    11801183    }
    … …  
    11971200
    11981201    if (!trapResult.isObject() && !trapResult.isNull()) {
    1199         throwVMTypeError(globalObject, scope, "Proxy handler's 'getPrototypeOf' trap should either return an object or null"_s);
     1202        throwTypeError(globalObject, scope, "Proxy handler's 'getPrototypeOf' trap should either return an object or null"_s);
    12001203        return { };
    12011204    }
    … …  
    12111214    RETURN_IF_EXCEPTION(scope, { });
    12121215    if (!isSame) {
    1213         throwVMTypeError(globalObject, scope, "Proxy's 'getPrototypeOf' trap for a non-extensible target should return the same value as the target's prototype"_s);
     1216        throwTypeError(globalObject, scope, "Proxy's 'getPrototypeOf' trap for a non-extensible target should return the same value as the target's prototype"_s);
    12141217        return { };
    12151218    }
Note: See TracChangeset for help on using the changeset viewer.