From 9bae24cc4a9793a3e85a5265601b8c93d444cd1c Mon Sep 17 00:00:00 2001 From: Andrew Kaster Date: Wed, 23 Apr 2025 01:30:43 -0600 Subject: [PATCH] LibJS: Add and use ValidateNonRevokedProxy AO This refactor is from two editorial changes to the spec from a while back. https://github.com/tc39/ecma262/commit/44d1cae2b28f14b24f992b1db7c2fd3e9fc90c4f https://github.com/tc39/ecma262/commit/21ffeee8697e5d7c30871322608e57fe7bac509e --- .../LibJS/Runtime/AbstractOperations.cpp | 27 ++- Libraries/LibJS/Runtime/ProxyObject.cpp | 185 ++++++++---------- Libraries/LibJS/Runtime/ProxyObject.h | 1 + Libraries/LibJS/Runtime/Value.cpp | 16 +- 4 files changed, 107 insertions(+), 122 deletions(-) diff --git a/Libraries/LibJS/Runtime/AbstractOperations.cpp b/Libraries/LibJS/Runtime/AbstractOperations.cpp index d8fdec1a940..82965d1317f 100644 --- a/Libraries/LibJS/Runtime/AbstractOperations.cpp +++ b/Libraries/LibJS/Runtime/AbstractOperations.cpp @@ -175,29 +175,26 @@ ThrowCompletionOr get_function_realm(VM& vm, FunctionObject const& funct } // 2. If obj is a bound function exotic object, then - if (is(function)) { - auto& bound_function = static_cast(function); + if (auto const* bound_function = as_if(function)) { + // a. Let boundTargetFunction be obj.[[BoundTargetFunction]]. + auto& bound_target_function = bound_function->bound_target_function(); - // a. Let target be obj.[[BoundTargetFunction]]. - auto& target = bound_function.bound_target_function(); - - // b. Return ? GetFunctionRealm(target). - return get_function_realm(vm, target); + // b. Return ? GetFunctionRealm(boundTargetFunction). + return get_function_realm(vm, bound_target_function); } // 3. If obj is a Proxy exotic object, then - if (is(function)) { - auto& proxy = static_cast(function); - - // a. If obj.[[ProxyHandler]] is null, throw a TypeError exception. - if (proxy.is_revoked()) - return vm.throw_completion(ErrorType::ProxyRevoked); + if (auto const* proxy = as_if(function)) { + // a. a. Perform ? ValidateNonRevokedProxy(obj). + TRY(proxy->validate_non_revoked_proxy()); // b. Let proxyTarget be obj.[[ProxyTarget]]. - auto& proxy_target = proxy.target(); + auto& proxy_target = proxy->target(); - // c. Return ? GetFunctionRealm(proxyTarget). + // c. Assert: proxyTarget is a function object. VERIFY(proxy_target.is_function()); + + // d. Return ? GetFunctionRealm(proxyTarget). return get_function_realm(vm, static_cast(proxy_target)); } diff --git a/Libraries/LibJS/Runtime/ProxyObject.cpp b/Libraries/LibJS/Runtime/ProxyObject.cpp index 318742043ed..abc2b9714e1 100644 --- a/Libraries/LibJS/Runtime/ProxyObject.cpp +++ b/Libraries/LibJS/Runtime/ProxyObject.cpp @@ -67,14 +67,12 @@ ThrowCompletionOr ProxyObject::internal_get_prototype_of() const auto& vm = this->vm(); - // 1. Let handler be O.[[ProxyHandler]]. + // 1. Perform ? ValidateNonRevokedProxy(O). + TRY(validate_non_revoked_proxy()); - // 2. If handler is null, throw a TypeError exception. - if (m_is_revoked) - return vm.throw_completion(ErrorType::ProxyRevoked); - - // 3. Assert: Type(handler) is Object. - // 4. Let target be O.[[ProxyTarget]]. + // 2. Let target be O.[[ProxyTarget]]. + // 3. Let handler be O.[[ProxyHandler]]. + // 4. Assert: handler is an Object. // 5. Let trap be ? GetMethod(handler, "getPrototypeOf"). auto trap = TRY(Value(m_handler).get_method(vm, vm.names.getPrototypeOf)); @@ -117,14 +115,12 @@ ThrowCompletionOr ProxyObject::internal_set_prototype_of(Object* prototype auto& vm = this->vm(); - // 1. Let handler be O.[[ProxyHandler]]. + // 1. Perform ? ValidateNonRevokedProxy(O). + TRY(validate_non_revoked_proxy()); - // 2. If handler is null, throw a TypeError exception. - if (m_is_revoked) - return vm.throw_completion(ErrorType::ProxyRevoked); - - // 3. Assert: Type(handler) is Object. - // 4. Let target be O.[[ProxyTarget]]. + // 2. Let target be O.[[ProxyTarget]]. + // 3. Let handler be O.[[ProxyHandler]]. + // 4. Assert: handler is an Object. // 5. Let trap be ? GetMethod(handler, "setPrototypeOf"). auto trap = TRY(Value(m_handler).get_method(vm, vm.names.setPrototypeOf)); @@ -167,14 +163,12 @@ ThrowCompletionOr ProxyObject::internal_is_extensible() const auto& vm = this->vm(); - // 1. Let handler be O.[[ProxyHandler]]. + // 1. Perform ? ValidateNonRevokedProxy(O). + TRY(validate_non_revoked_proxy()); - // 2. If handler is null, throw a TypeError exception. - if (m_is_revoked) - return vm.throw_completion(ErrorType::ProxyRevoked); - - // 3. Assert: Type(handler) is Object. - // 4. Let target be O.[[ProxyTarget]]. + // 2. Let target be O.[[ProxyTarget]]. + // 3. Let handler be O.[[ProxyHandler]]. + // 4. Assert: handler is an Object. // 5. Let trap be ? GetMethod(handler, "isExtensible"). auto trap = TRY(Value(m_handler).get_method(vm, vm.names.isExtensible)); @@ -206,14 +200,12 @@ ThrowCompletionOr ProxyObject::internal_prevent_extensions() auto& vm = this->vm(); - // 1. Let handler be O.[[ProxyHandler]]. + // 1. Perform ? ValidateNonRevokedProxy(O). + TRY(validate_non_revoked_proxy()); - // 2. If handler is null, throw a TypeError exception. - if (m_is_revoked) - return vm.throw_completion(ErrorType::ProxyRevoked); - - // 3. Assert: Type(handler) is Object. - // 4. Let target be O.[[ProxyTarget]]. + // 2. Let target be O.[[ProxyTarget]]. + // 3. Let handler be O.[[ProxyHandler]]. + // 4. Assert: handler is an Object. // 5. Let trap be ? GetMethod(handler, "preventExtensions"). auto trap = TRY(Value(m_handler).get_method(vm, vm.names.preventExtensions)); @@ -248,14 +240,12 @@ ThrowCompletionOr> ProxyObject::internal_get_own_pr auto& vm = this->vm(); - // 1. Let handler be O.[[ProxyHandler]]. + // 1. Perform ? ValidateNonRevokedProxy(O). + TRY(validate_non_revoked_proxy()); - // 2. If handler is null, throw a TypeError exception. - if (m_is_revoked) - return vm.throw_completion(ErrorType::ProxyRevoked); - - // 3. Assert: Type(handler) is Object. - // 4. Let target be O.[[ProxyTarget]]. + // 2. Let target be O.[[ProxyTarget]]. + // 3. Let handler be O.[[ProxyHandler]]. + // 4. Assert: handler is an Object. // 5. Let trap be ? GetMethod(handler, "getOwnPropertyDescriptor"). auto trap = TRY(Value(m_handler).get_method(vm, vm.names.getOwnPropertyDescriptor)); @@ -339,14 +329,12 @@ ThrowCompletionOr ProxyObject::internal_define_own_property(PropertyKey co auto& vm = this->vm(); - // 1. Let handler be O.[[ProxyHandler]]. + // 1. Perform ? ValidateNonRevokedProxy(O). + TRY(validate_non_revoked_proxy()); - // 2. If handler is null, throw a TypeError exception. - if (m_is_revoked) - return vm.throw_completion(ErrorType::ProxyRevoked); - - // 3. Assert: Type(handler) is Object. - // 4. Let target be O.[[ProxyTarget]]. + // 2. Let target be O.[[ProxyTarget]]. + // 3. Let handler be O.[[ProxyHandler]]. + // 4. Assert: handler is an Object. // 5. Let trap be ? GetMethod(handler, "defineProperty"). auto trap = TRY(Value(m_handler).get_method(vm, vm.names.defineProperty)); @@ -421,14 +409,12 @@ ThrowCompletionOr ProxyObject::internal_has_property(PropertyKey const& pr auto& vm = this->vm(); - // 1. Let handler be O.[[ProxyHandler]]. + // 1. Perform ? ValidateNonRevokedProxy(O). + TRY(validate_non_revoked_proxy()); - // 2. If handler is null, throw a TypeError exception. - if (m_is_revoked) - return vm.throw_completion(ErrorType::ProxyRevoked); - - // 3. Assert: Type(handler) is Object. - // 4. Let target be O.[[ProxyTarget]]. + // 2. Let target be O.[[ProxyTarget]]. + // 3. Let handler be O.[[ProxyHandler]]. + // 4. Assert: handler is an Object. // NOTE: We need to protect ourselves from a Proxy with the handler's prototype set to the // Proxy itself, which would by default bounce between these functions indefinitely and lead to @@ -490,14 +476,12 @@ ThrowCompletionOr ProxyObject::internal_get(PropertyKey const& property_k auto& vm = this->vm(); - // 1. Let handler be O.[[ProxyHandler]]. + // 1. Perform ? ValidateNonRevokedProxy(O). + TRY(validate_non_revoked_proxy()); - // 2. If handler is null, throw a TypeError exception. - if (m_is_revoked) - return vm.throw_completion(ErrorType::ProxyRevoked); - - // 3. Assert: Type(handler) is Object. - // 4. Let target be O.[[ProxyTarget]]. + // 2. Let target be O.[[ProxyTarget]]. + // 3. Let handler be O.[[ProxyHandler]]. + // 4. Assert: handler is an Object. // NOTE: We need to protect ourselves from a Proxy with its (or handler's) prototype set to the // Proxy itself, which would by default bounce between these functions indefinitely and lead to @@ -560,14 +544,12 @@ ThrowCompletionOr ProxyObject::internal_set(PropertyKey const& property_ke VERIFY(!value.is_special_empty_value()); VERIFY(!receiver.is_special_empty_value()); - // 1. Let handler be O.[[ProxyHandler]]. + // 1. Perform ? ValidateNonRevokedProxy(O). + TRY(validate_non_revoked_proxy()); - // 2. If handler is null, throw a TypeError exception. - if (m_is_revoked) - return vm.throw_completion(ErrorType::ProxyRevoked); - - // 3. Assert: Type(handler) is Object. - // 4. Let target be O.[[ProxyTarget]]. + // 2. Let target be O.[[ProxyTarget]]. + // 3. Let handler be O.[[ProxyHandler]]. + // 4. Assert: handler is an Object. // NOTE: We need to protect ourselves from a Proxy with its prototype set to the // Proxy itself, which would by default bounce between these functions indefinitely and lead to @@ -631,14 +613,12 @@ ThrowCompletionOr ProxyObject::internal_delete(PropertyKey const& property auto& vm = this->vm(); - // 1. Let handler be O.[[ProxyHandler]]. + // 1. Perform ? ValidateNonRevokedProxy(O). + TRY(validate_non_revoked_proxy()); - // 2. If handler is null, throw a TypeError exception. - if (m_is_revoked) - return vm.throw_completion(ErrorType::ProxyRevoked); - - // 3. Assert: Type(handler) is Object. - // 4. Let target be O.[[ProxyTarget]]. + // 2. Let target be O.[[ProxyTarget]]. + // 3. Let handler be O.[[ProxyHandler]]. + // 4. Assert: handler is an Object. // 5. Let trap be ? GetMethod(handler, "deleteProperty"). auto trap = TRY(Value(m_handler).get_method(vm, vm.names.deleteProperty)); @@ -685,14 +665,12 @@ ThrowCompletionOr> ProxyObject::internal_own_property_keys auto& vm = this->vm(); - // 1. Let handler be O.[[ProxyHandler]]. + // 1. Perform ? ValidateNonRevokedProxy(O). + TRY(validate_non_revoked_proxy()); - // 2. If handler is null, throw a TypeError exception. - if (m_is_revoked) - return vm.throw_completion(ErrorType::ProxyRevoked); - - // 3. Assert: Type(handler) is Object. - // 4. Let target be O.[[ProxyTarget]]. + // 2. Let target be O.[[ProxyTarget]]. + // 3. Let handler be O.[[ProxyHandler]]. + // 4. Assert: handler is an Object. // 5. Let trap be ? GetMethod(handler, "ownKeys"). auto trap = TRY(Value(m_handler).get_method(vm, vm.names.ownKeys)); @@ -808,18 +786,16 @@ ThrowCompletionOr ProxyObject::internal_call(Value this_argument, Readonl auto& vm = this->vm(); auto& realm = *vm.current_realm(); - // A Proxy exotic object only has a [[Call]] internal method if the initial value of its [[ProxyTarget]] internal slot is an object that has a [[Call]] internal method. + // 1. Perform ? ValidateNonRevokedProxy(O). + TRY(validate_non_revoked_proxy()); + + // 2. Let target be O.[[ProxyTarget]]. + // 3. Let handler be O.[[ProxyHandler]]. + // 4. Assert: handler is an Object. + + // NOTE: A Proxy exotic object only has a [[Call]] internal method if the initial value of its [[ProxyTarget]] internal slot is an object that has a [[Call]] internal method. VERIFY(is_function()); - // 1. Let handler be O.[[ProxyHandler]]. - - // 2. If handler is null, throw a TypeError exception. - if (m_is_revoked) - return vm.throw_completion(ErrorType::ProxyRevoked); - - // 3. Assert: Type(handler) is Object. - // 4. Let target be O.[[ProxyTarget]]. - // 5. Let trap be ? GetMethod(handler, "apply"). auto trap = TRY(Value(m_handler).get_method(vm, vm.names.apply)); @@ -854,19 +830,16 @@ ThrowCompletionOr> ProxyObject::internal_construct(ReadonlySpan< auto& vm = this->vm(); auto& realm = *vm.current_realm(); - // A Proxy exotic object only has a [[Construct]] internal method if the initial value of its [[ProxyTarget]] internal slot is an object that has a [[Construct]] internal method. + // 1. Perform ? ValidateNonRevokedProxy(O). + TRY(validate_non_revoked_proxy()); + + // 2. Let target be O.[[ProxyTarget]]. + // 3. Let handler be O.[[ProxyHandler]]. + // 4. Assert: handler is an Object. + + // NOTE: A Proxy exotic object only has a [[Construct]] internal method if the initial value of its [[ProxyTarget]] internal slot is an object that has a [[Construct]] internal method. VERIFY(is_function()); - // 1. Let handler be O.[[ProxyHandler]]. - - // 2. If handler is null, throw a TypeError exception. - if (m_is_revoked) - return vm.throw_completion(ErrorType::ProxyRevoked); - - // 3. Assert: Type(handler) is Object. - // 4. Let target be O.[[ProxyTarget]]. - // 5. Assert: IsConstructor(target) is true. - // 6. Let trap be ? GetMethod(handler, "construct"). auto trap = TRY(Value(m_handler).get_method(vm, vm.names.construct)); @@ -890,6 +863,22 @@ ThrowCompletionOr> ProxyObject::internal_construct(ReadonlySpan< return new_object.as_object(); } +// 10.5.14 ValidateNonRevokedProxy ( proxy ) +ThrowCompletionOr ProxyObject::validate_non_revoked_proxy() const +{ + auto& vm = this->vm(); + + // FIXME: The spec expects us to model a revoked proxy by having ProxyTarget and ProxyHandler be nullable. + + // 1. If proxy.[[ProxyTarget]] is null, throw a TypeError exception. + // 2. Assert: proxy.[[ProxyHandler]] is not null. + if (m_is_revoked) + return vm.throw_completion(ErrorType::ProxyRevoked); + + // 3. Return unused. + return {}; +} + void ProxyObject::visit_edges(Cell::Visitor& visitor) { Base::visit_edges(visitor); diff --git a/Libraries/LibJS/Runtime/ProxyObject.h b/Libraries/LibJS/Runtime/ProxyObject.h index e52cda49db1..8f77eea9738 100644 --- a/Libraries/LibJS/Runtime/ProxyObject.h +++ b/Libraries/LibJS/Runtime/ProxyObject.h @@ -44,6 +44,7 @@ public: virtual ThrowCompletionOr> internal_own_property_keys() const override; virtual ThrowCompletionOr internal_call(Value this_argument, ReadonlySpan arguments_list) override; virtual ThrowCompletionOr> internal_construct(ReadonlySpan arguments_list, FunctionObject& new_target) override; + ThrowCompletionOr validate_non_revoked_proxy() const; private: ProxyObject(Object& target, Object& handler, Object& prototype); diff --git a/Libraries/LibJS/Runtime/Value.cpp b/Libraries/LibJS/Runtime/Value.cpp index 76045464b81..9745cb039c0 100644 --- a/Libraries/LibJS/Runtime/Value.cpp +++ b/Libraries/LibJS/Runtime/Value.cpp @@ -233,18 +233,16 @@ ThrowCompletionOr Value::is_array(VM& vm) const return true; // 3. If argument is a Proxy exotic object, then - if (is(object)) { - auto const& proxy = static_cast(object); + if (auto const* proxy = as_if(object)) { - // a. If argument.[[ProxyHandler]] is null, throw a TypeError exception. - if (proxy.is_revoked()) - return vm.throw_completion(ErrorType::ProxyRevoked); + // a. Perform ? ValidateNonRevokedProxy(argument). + TRY(proxy->validate_non_revoked_proxy()); - // b. Let target be argument.[[ProxyTarget]]. - auto const& target = proxy.target(); + // b. Let proxyTarget be argument.[[ProxyTarget]]. + auto& proxy_target = proxy->target(); - // c. Return ? IsArray(target). - return Value(&target).is_array(vm); + // c. Return ? IsArray(proxyTarget). + return Value(&proxy_target).is_array(vm); } // 4. Return false.