From 00b8ce4a6d2d166f16c3f5f5ff1280350221243e Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Mon, 11 Apr 2022 22:47:50 +0100 Subject: [PATCH] LibJS: Pass this value to fallback func in Array.prototype.toString() The existing code looks innocently correct, implementing the following step: 3. If IsCallable(func) is false, set func to the intrinsic function %Object.prototype.toString%. as return ObjectPrototype::to_string(vm, global_object); However, this misses the fact that the next step calls the function with the previously ToObject()'d this value (`array`): 4. Return ? Call(func, array). This doesn't happen in the current implementation, which will use the unaltered this value from the Array.prototype.toString() call, and make another, unequal object in %Object.prototype.toString%. Since both that and Array.prototype.toString() do a Get() call on said object, this behavior is observable (see newly added test). Fix this by actually doing what the spec says and calling the fallback function the regular way. --- .../LibJS/Runtime/ArrayPrototype.cpp | 19 +++++++---- .../Libraries/LibJS/Runtime/GlobalObject.cpp | 6 ++-- .../Libraries/LibJS/Runtime/GlobalObject.h | 6 ++-- .../Array/Array.prototype.toString.js | 32 +++++++++++++++++++ 4 files changed, 53 insertions(+), 10 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp b/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp index 8dc7682d1a4..de186530121 100644 --- a/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp @@ -1,6 +1,6 @@ /* * Copyright (c) 2020, Andreas Kling - * Copyright (c) 2020-2021, Linus Groh + * Copyright (c) 2020-2022, Linus Groh * Copyright (c) 2020, Marcin Gasperowicz * Copyright (c) 2021, David Tuin * @@ -381,11 +381,18 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayPrototype::shift) // 23.1.3.31 Array.prototype.toString ( ), https://tc39.es/ecma262/#sec-array.prototype.tostring JS_DEFINE_NATIVE_FUNCTION(ArrayPrototype::to_string) { - auto* this_object = TRY(vm.this_value(global_object).to_object(global_object)); - auto join_function = TRY(this_object->get(vm.names.join)); - if (!join_function.is_function()) - return ObjectPrototype::to_string(vm, global_object); - return TRY(call(global_object, join_function.as_function(), this_object)); + // 1. Let array be ? ToObject(this value). + auto* array = TRY(vm.this_value(global_object).to_object(global_object)); + + // 2. Let func be ? Get(array, "join"). + auto func = TRY(array->get(vm.names.join)); + + // 3. If IsCallable(func) is false, set func to the intrinsic function %Object.prototype.toString%. + if (!func.is_function()) + func = global_object.object_prototype_to_string_function(); + + // 4. Return ? Call(func, array). + return TRY(call(global_object, func.as_function(), array)); } // 19.5.1 Array.prototype.toLocaleString ( [ locales [ , options ] ] ), https://tc39.es/ecma402/#sup-array.prototype.tolocalestring diff --git a/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp b/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp index 163107937e1..1ce4a036e7a 100644 --- a/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp @@ -1,6 +1,6 @@ /* * Copyright (c) 2020, Andreas Kling - * Copyright (c) 2020-2021, Linus Groh + * Copyright (c) 2020-2022, Linus Groh * * SPDX-License-Identifier: BSD-2-Clause */ @@ -302,6 +302,7 @@ void GlobalObject::initialize_global_object() m_date_constructor_now_function = &m_date_constructor->get_without_side_effects(vm.names.now).as_function(); m_eval_function = &get_without_side_effects(vm.names.eval).as_function(); m_json_parse_function = &get_without_side_effects(vm.names.JSON).as_object().get_without_side_effects(vm.names.parse).as_function(); + m_object_prototype_to_string_function = &m_object_prototype->get_without_side_effects(vm.names.toString).as_function(); } GlobalObject::~GlobalObject() = default; @@ -320,8 +321,9 @@ void GlobalObject::visit_edges(Visitor& visitor) visitor.visit(m_array_prototype_values_function); visitor.visit(m_date_constructor_now_function); visitor.visit(m_eval_function); - visitor.visit(m_throw_type_error_function); visitor.visit(m_json_parse_function); + visitor.visit(m_object_prototype_to_string_function); + visitor.visit(m_throw_type_error_function); #define __JS_ENUMERATE(ClassName, snake_name, PrototypeName, ConstructorName, ArrayType) \ visitor.visit(m_##snake_name##_constructor); \ diff --git a/Userland/Libraries/LibJS/Runtime/GlobalObject.h b/Userland/Libraries/LibJS/Runtime/GlobalObject.h index 517bbb694d1..8b5b82d76c8 100644 --- a/Userland/Libraries/LibJS/Runtime/GlobalObject.h +++ b/Userland/Libraries/LibJS/Runtime/GlobalObject.h @@ -44,8 +44,9 @@ public: FunctionObject* array_prototype_values_function() const { return m_array_prototype_values_function; } FunctionObject* date_constructor_now_function() const { return m_date_constructor_now_function; } FunctionObject* eval_function() const { return m_eval_function; } - FunctionObject* throw_type_error_function() const { return m_throw_type_error_function; } FunctionObject* json_parse_function() const { return m_json_parse_function; } + FunctionObject* object_prototype_to_string_function() const { return m_object_prototype_to_string_function; } + FunctionObject* throw_type_error_function() const { return m_throw_type_error_function; } #define __JS_ENUMERATE(ClassName, snake_name, PrototypeName, ConstructorName, ArrayType) \ ConstructorName* snake_name##_constructor() { return m_##snake_name##_constructor; } \ @@ -115,8 +116,9 @@ private: FunctionObject* m_array_prototype_values_function { nullptr }; FunctionObject* m_date_constructor_now_function { nullptr }; FunctionObject* m_eval_function { nullptr }; - FunctionObject* m_throw_type_error_function { nullptr }; FunctionObject* m_json_parse_function { nullptr }; + FunctionObject* m_object_prototype_to_string_function { nullptr }; + FunctionObject* m_throw_type_error_function { nullptr }; #define __JS_ENUMERATE(ClassName, snake_name, PrototypeName, ConstructorName, ArrayType) \ ConstructorName* m_##snake_name##_constructor { nullptr }; \ diff --git a/Userland/Libraries/LibJS/Tests/builtins/Array/Array.prototype.toString.js b/Userland/Libraries/LibJS/Tests/builtins/Array/Array.prototype.toString.js index 1c18fb54ed6..68cb012173b 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/Array/Array.prototype.toString.js +++ b/Userland/Libraries/LibJS/Tests/builtins/Array/Array.prototype.toString.js @@ -63,4 +63,36 @@ describe("normal behavior", () => { // [ "foo", , [ 1, 2, ], [ "bar" ] ] expect(a.toString()).toBe("foo,,1,2,,bar"); }); + + test("this value object remains the same in the %Object.prototype.toString% fallback", () => { + let arrayPrototypeToStringThis; + let objectPrototypeToStringThis; + + // Inject a Proxy into the Number prototype chain, so we can + // observe Get() operations on the different object created + // from the primitive number value. + Number.prototype.__proto__ = new Proxy( + {}, + { + get(target, prop, receiver) { + // In Array.prototype.toString(): + // 2. Let func be ? Get(array, "join"). + if (prop === "join") { + arrayPrototypeToStringThis = receiver; + } + + // In Object.prototype.toString(): + // 15. Let tag be ? Get(O, @@toStringTag). + if (prop === Symbol.toStringTag) { + objectPrototypeToStringThis = receiver; + } + }, + } + ); + + Array.prototype.toString.call(123); + expect(typeof arrayPrototypeToStringThis).toBe("object"); + expect(typeof objectPrototypeToStringThis).toBe("object"); + expect(arrayPrototypeToStringThis).toBe(objectPrototypeToStringThis); + }); });