From 83be39c91aeff53c5dfd158844e8ad73f10ae9d2 Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Tue, 8 Jun 2021 21:53:36 +0100 Subject: [PATCH] LibJS: Handle Proxy with Array target in IsArray() abstract operation This was missing from Value::is_array(), which is equivalent to the spec's IsArray() abstract operation - it treats a Proxy value with an Array target object as being an Array. It can throw, so needs both the global object and an exception check now. --- Userland/Libraries/LibJS/AST.cpp | 3 +-- Userland/Libraries/LibJS/MarkupGenerator.cpp | 5 ++--- .../LibJS/Runtime/ArrayConstructor.cpp | 2 +- .../LibJS/Runtime/ArrayPrototype.cpp | 14 +++++++----- .../Libraries/LibJS/Runtime/JSONObject.cpp | 8 +++++-- .../Libraries/LibJS/Runtime/ProxyObject.h | 1 - .../LibJS/Runtime/StringConstructor.cpp | 3 ++- Userland/Libraries/LibJS/Runtime/Value.cpp | 22 ++++++++++++++++--- Userland/Libraries/LibJS/Runtime/Value.h | 4 ++-- .../Tests/builtins/Array/Array.isArray.js | 9 ++++++++ Userland/Utilities/js.cpp | 5 ++--- 11 files changed, 52 insertions(+), 24 deletions(-) diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index 35eaada90d..b01ea92b40 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -1669,7 +1669,7 @@ Value ObjectExpression::execute(Interpreter& interpreter, GlobalObject& global_o return {}; if (property.type() == ObjectProperty::Type::Spread) { - if (key.is_array()) { + if (key.is_object() && key.as_object().is_array()) { auto& array_to_spread = static_cast(key.as_object()); for (auto& entry : array_to_spread.indexed_properties()) { object->indexed_properties().put(object, entry.index(), entry.value_and_attributes(&array_to_spread).value); @@ -1695,7 +1695,6 @@ Value ObjectExpression::execute(Interpreter& interpreter, GlobalObject& global_o return {}; } } - continue; } diff --git a/Userland/Libraries/LibJS/MarkupGenerator.cpp b/Userland/Libraries/LibJS/MarkupGenerator.cpp index 1b9cd408d1..e4af4f7021 100644 --- a/Userland/Libraries/LibJS/MarkupGenerator.cpp +++ b/Userland/Libraries/LibJS/MarkupGenerator.cpp @@ -58,11 +58,10 @@ void MarkupGenerator::value_to_html(Value value, StringBuilder& output_html, Has seen_objects.set(&value.as_object()); } - if (value.is_array()) - return array_to_html(static_cast(value.as_object()), output_html, seen_objects); - if (value.is_object()) { auto& object = value.as_object(); + if (object.is_array()) + return array_to_html(static_cast(object), output_html, seen_objects); output_html.append(wrap_string_in_style(object.class_name(), StyleType::ObjectType)); if (object.is_function()) return function_to_html(object, output_html, seen_objects); diff --git a/Userland/Libraries/LibJS/Runtime/ArrayConstructor.cpp b/Userland/Libraries/LibJS/Runtime/ArrayConstructor.cpp index da435651e6..59a28eccda 100644 --- a/Userland/Libraries/LibJS/Runtime/ArrayConstructor.cpp +++ b/Userland/Libraries/LibJS/Runtime/ArrayConstructor.cpp @@ -142,7 +142,7 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayConstructor::from) JS_DEFINE_NATIVE_FUNCTION(ArrayConstructor::is_array) { auto value = vm.argument(0); - return Value(value.is_array()); + return Value(value.is_array(global_object)); } JS_DEFINE_NATIVE_FUNCTION(ArrayConstructor::of) diff --git a/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp b/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp index 4a0e4a190c..94f9001c54 100644 --- a/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp @@ -358,14 +358,14 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayPrototype::concat) for (size_t i = 0; i < vm.argument_count(); ++i) { auto argument = vm.argument(i); - if (argument.is_array()) { + if (argument.is_array(global_object)) { auto& argument_object = argument.as_object(); new_array->indexed_properties().append_all(&argument_object, argument_object.indexed_properties()); - if (vm.exception()) - return {}; - } else { - new_array->indexed_properties().append(argument); + continue; } + if (vm.exception()) + return {}; + new_array->indexed_properties().append(argument); } return Value(new_array); @@ -1027,10 +1027,12 @@ static void recursive_array_flat(VM& vm, GlobalObject& global_object, Array& new if (vm.exception()) return; - if (depth > 0 && value.is_array()) { + if (depth > 0 && value.is_array(global_object)) { recursive_array_flat(vm, global_object, new_array, value.as_array(), depth - 1); continue; } + if (vm.exception()) + return; if (!value.is_empty()) { new_array.indexed_properties().append(value); if (vm.exception()) diff --git a/Userland/Libraries/LibJS/Runtime/JSONObject.cpp b/Userland/Libraries/LibJS/Runtime/JSONObject.cpp index 7b2a6ce1d3..e10cb9c87e 100644 --- a/Userland/Libraries/LibJS/Runtime/JSONObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/JSONObject.cpp @@ -48,7 +48,7 @@ String JSONObject::stringify_impl(GlobalObject& global_object, Value value, Valu if (replacer.is_object()) { if (replacer.as_object().is_function()) { state.replacer_function = &replacer.as_function(); - } else if (replacer.is_array()) { + } else if (replacer.is_array(global_object)) { auto& replacer_object = replacer.as_object(); auto replacer_length = length_of_array_like(global_object, replacer_object); if (vm.exception()) @@ -77,6 +77,8 @@ String JSONObject::stringify_impl(GlobalObject& global_object, Value value, Valu } state.property_list = list; } + if (vm.exception()) + return {}; } if (space.is_object()) { @@ -172,8 +174,10 @@ String JSONObject::serialize_json_property(GlobalObject& global_object, Stringif return "null"; } if (value.is_object() && !value.is_function()) { - if (value.is_array()) + if (value.is_array(global_object)) return serialize_json_array(global_object, state, static_cast(value.as_object())); + if (vm.exception()) + return {}; return serialize_json_object(global_object, state, value.as_object()); } if (value.is_bigint()) diff --git a/Userland/Libraries/LibJS/Runtime/ProxyObject.h b/Userland/Libraries/LibJS/Runtime/ProxyObject.h index fd9cad948d..09cec70756 100644 --- a/Userland/Libraries/LibJS/Runtime/ProxyObject.h +++ b/Userland/Libraries/LibJS/Runtime/ProxyObject.h @@ -46,7 +46,6 @@ private: virtual void visit_edges(Visitor&) override; virtual bool is_function() const override { return m_target.is_function(); } - virtual bool is_array() const override { return m_target.is_array(); }; Object& m_target; Object& m_handler; diff --git a/Userland/Libraries/LibJS/Runtime/StringConstructor.cpp b/Userland/Libraries/LibJS/Runtime/StringConstructor.cpp index 30746e16e4..f27a47be6f 100644 --- a/Userland/Libraries/LibJS/Runtime/StringConstructor.cpp +++ b/Userland/Libraries/LibJS/Runtime/StringConstructor.cpp @@ -72,7 +72,8 @@ JS_DEFINE_NATIVE_FUNCTION(StringConstructor::raw) vm.throw_exception(global_object, ErrorType::StringRawCannotConvert, raw.is_null() ? "null" : "undefined"); return {}; } - if (!raw.is_array()) + // FIXME: This should use length_of_array_like() and work with any object + if (!raw.is_object() || !raw.as_object().is_array()) return js_string(vm, ""); auto* array = static_cast(raw.to_object(global_object)); diff --git a/Userland/Libraries/LibJS/Runtime/Value.cpp b/Userland/Libraries/LibJS/Runtime/Value.cpp index 69e8c3272c..5120b700ad 100644 --- a/Userland/Libraries/LibJS/Runtime/Value.cpp +++ b/Userland/Libraries/LibJS/Runtime/Value.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -196,14 +197,29 @@ static String double_to_string(double d) return builder.to_string(); } -bool Value::is_array() const +// 7.2.2 IsArray, https://tc39.es/ecma262/#sec-isarray +bool Value::is_array(GlobalObject& global_object) const { - return is_object() && as_object().is_array(); + if (!is_object()) + return false; + auto& object = as_object(); + if (object.is_array()) + return true; + if (is(object)) { + auto& proxy = static_cast(object); + if (proxy.is_revoked()) { + auto& vm = global_object.vm(); + vm.throw_exception(global_object, ErrorType::ProxyRevoked); + return false; + } + return Value(&proxy.target()).is_array(global_object); + } + return false; } Array& Value::as_array() { - VERIFY(is_array()); + VERIFY(is_object() && as_object().is_array()); return static_cast(*m_value.as_object); } diff --git a/Userland/Libraries/LibJS/Runtime/Value.h b/Userland/Libraries/LibJS/Runtime/Value.h index 40122ee9a6..2f30615489 100644 --- a/Userland/Libraries/LibJS/Runtime/Value.h +++ b/Userland/Libraries/LibJS/Runtime/Value.h @@ -59,10 +59,10 @@ public: bool is_native_property() const { return m_type == Type::NativeProperty; } bool is_nullish() const { return is_null() || is_undefined(); } bool is_cell() const { return is_string() || is_accessor() || is_object() || is_bigint() || is_symbol() || is_native_property(); } - bool is_array() const; + bool is_array(GlobalObject&) const; bool is_function() const; bool is_constructor() const; - bool is_regexp(GlobalObject& global_object) const; + bool is_regexp(GlobalObject&) const; bool is_nan() const { return is_number() && __builtin_isnan(as_double()); } bool is_infinity() const { return is_number() && __builtin_isinf(as_double()); } diff --git a/Userland/Libraries/LibJS/Tests/builtins/Array/Array.isArray.js b/Userland/Libraries/LibJS/Tests/builtins/Array/Array.isArray.js index e0577a70fa..a340e97b8c 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/Array/Array.isArray.js +++ b/Userland/Libraries/LibJS/Tests/builtins/Array/Array.isArray.js @@ -22,4 +22,13 @@ test("arguments that evaluate to true", () => { expect(Array.isArray(new Array(10))).toBeTrue(); expect(Array.isArray(new Array("a", "b", "c"))).toBeTrue(); expect(Array.isArray(Array.prototype)).toBeTrue(); + expect(Array.isArray(new Proxy([], {}))).toBeTrue(); +}); + +test("Revoked Proxy as argument throws", () => { + const revocable = Proxy.revocable([], {}); + revocable.revoke(); + expect(() => { + Array.isArray(revocable.proxy); + }).toThrowWithMessage(TypeError, "An operation was performed on a revoked Proxy object"); }); diff --git a/Userland/Utilities/js.cpp b/Userland/Utilities/js.cpp index d4d4c091eb..c3c15ebba2 100644 --- a/Userland/Utilities/js.cpp +++ b/Userland/Utilities/js.cpp @@ -386,11 +386,10 @@ static void print_value(JS::Value value, HashTable& seen_objects) seen_objects.set(&value.as_object()); } - if (value.is_array()) - return print_array(static_cast(value.as_object()), seen_objects); - if (value.is_object()) { auto& object = value.as_object(); + if (object.is_array()) + return print_array(static_cast(object), seen_objects); if (object.is_function()) return print_function(object, seen_objects); if (is(object))