From a97b5393d0b54323123087b8ea255650e4a6c010 Mon Sep 17 00:00:00 2001 From: Anonymous Date: Thu, 17 Feb 2022 15:09:39 -0800 Subject: [PATCH] LibJS: Ensure we only call toString on computed properties once --- Userland/Libraries/LibJS/AST.cpp | 19 ++- .../Tests/computed-property-sideeffects.js | 145 ++++++++++++++++++ 2 files changed, 154 insertions(+), 10 deletions(-) create mode 100644 Userland/Libraries/LibJS/Tests/computed-property-sideeffects.js diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index 6bdada2f54..3120ab1677 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -81,13 +81,11 @@ static void update_function_name(Value value, FlyString const& name) static_cast(function).set_name(name); } -static ThrowCompletionOr get_function_name(GlobalObject& global_object, Value value) +static ThrowCompletionOr get_function_property_name(PropertyKey key) { - if (value.is_symbol()) - return String::formatted("[{}]", value.as_symbol().description()); - if (value.is_string()) - return value.as_string().string(); - return value.to_string(global_object); + if (key.is_symbol()) + return String::formatted("[{}]", key.as_symbol()->description()); + return key.to_string(); } // 14.2.2 Runtime Semantics: Evaluation, https://tc39.es/ecma262/#sec-block-runtime-semantics-evaluation @@ -2999,7 +2997,8 @@ Completion ObjectExpression::execute(Interpreter& interpreter, GlobalObject& glo if (value.is_function() && property.is_method()) static_cast(value.as_function()).set_home_object(object); - auto name = TRY(get_function_name(global_object, key)); + auto property_key = TRY(PropertyKey::from_value(global_object, key)); + auto name = TRY(get_function_property_name(property_key)); if (property.type() == ObjectProperty::Type::Getter) { name = String::formatted("get {}", name); } else if (property.type() == ObjectProperty::Type::Setter) { @@ -3011,14 +3010,14 @@ Completion ObjectExpression::execute(Interpreter& interpreter, GlobalObject& glo switch (property.type()) { case ObjectProperty::Type::Getter: VERIFY(value.is_function()); - object->define_direct_accessor(TRY(PropertyKey::from_value(global_object, key)), &value.as_function(), nullptr, Attribute::Configurable | Attribute::Enumerable); + object->define_direct_accessor(property_key, &value.as_function(), nullptr, Attribute::Configurable | Attribute::Enumerable); break; case ObjectProperty::Type::Setter: VERIFY(value.is_function()); - object->define_direct_accessor(TRY(PropertyKey::from_value(global_object, key)), nullptr, &value.as_function(), Attribute::Configurable | Attribute::Enumerable); + object->define_direct_accessor(property_key, nullptr, &value.as_function(), Attribute::Configurable | Attribute::Enumerable); break; case ObjectProperty::Type::KeyValue: - object->define_direct_property(TRY(PropertyKey::from_value(global_object, key)), value, JS::default_attributes); + object->define_direct_property(property_key, value, JS::default_attributes); break; case ObjectProperty::Type::Spread: default: diff --git a/Userland/Libraries/LibJS/Tests/computed-property-sideeffects.js b/Userland/Libraries/LibJS/Tests/computed-property-sideeffects.js new file mode 100644 index 0000000000..150d567d6d --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/computed-property-sideeffects.js @@ -0,0 +1,145 @@ +let calledToStringError = {}; +let throwingToString = { + toString: () => { + throw calledToStringError; + }, +}; +let calledValueOfError = {}; +let throwingValueOf = { + toString: undefined, + valueOf: () => { + throw calledValueOfError; + }, +}; +let calledToStringAccessorError = {}; +let throwingToStringAccessor = { + get toString() { + throw calledToStringAccessorError; + }, +}; +let calledValueOfAccessorError = {}; +let throwingValueOfAccessor = { + toString: undefined, + get valueOf() { + throw calledValueOfAccessorError; + }, +}; + +test("Exceptions thrown by computed properties are caught", () => { + var i = 0; + var j = 0; + var k = 0; + expect(() => { + return { first: k++, [throwingToString]: i++, second: j++ }; + }).toThrow(calledToStringError); + expect(i).toBe(1); + expect(j).toBe(0); + expect(k).toBe(1); + expect(() => { + return { first: k++, [throwingValueOf]: i++, second: j++ }; + }).toThrow(calledValueOfError); + expect(i).toBe(2); + expect(j).toBe(0); + expect(k).toBe(2); + expect(() => { + return { first: k++, [throwingToStringAccessor]: i++, second: j++ }; + }).toThrow(calledToStringAccessorError); + expect(i).toBe(3); + expect(j).toBe(0); + expect(k).toBe(3); + expect(() => { + return { first: k++, [throwingValueOfAccessor]: i++, second: j++ }; + }).toThrow(calledValueOfAccessorError); + expect(i).toBe(4); + expect(j).toBe(0); + expect(k).toBe(4); +}); + +test("Test toString and valueOf are only called once", () => { + var counter = 0; + var key1 = { + valueOf: function () { + expect(counter++).toBe(0); + return "a"; + }, + toString: null, + }; + var key2 = { + valueOf: function () { + expect(counter++).toBe(1); + return "b"; + }, + toString: null, + }; + var key3 = { + get toString() { + expect(counter++).toBe(2); + return function () { + return "c"; + }; + }, + }; + var key4 = { + get valueOf() { + expect(counter++).toBe(3); + return function () { + return "d"; + }; + }, + toString: null, + }; + var key5 = { + valueOf: function () { + expect(counter++).toBe(4); + return 0; + }, + toString: null, + }; + var key6 = { + valueOf: function () { + expect(counter++).toBe(5); + return 2; + }, + toString: null, + }; + var key7 = { + get toString() { + expect(counter++).toBe(6); + return function () { + return 4; + }; + }, + }; + var key8 = { + get valueOf() { + expect(counter++).toBe(7); + return function () { + return 8; + }; + }, + toString: null, + }; + + var object = { + [key1]: "a", + [key2]: "b", + [key3]: "c", + [key4]: "d", + [key5]: "e", + [key6]: "f", + [key7]: "g", + [key8]: "h", + }; + expect(counter).toBe(8); + expect(object.a).toBe("a"); + expect(object.b).toBe("b"); + expect(object.c).toBe("c"); + expect(object.d).toBe("d"); + expect(object[0]).toBe("e"); + expect(object[2]).toBe("f"); + expect(object[4]).toBe("g"); + expect(object[8]).toBe("h"); + expect(Object.getOwnPropertyNames(object) + "").toBe( + ["0", "2", "4", "8", "a", "b", "c", "d"] + "" + ); +});