From d165590809f31e1c325b23002413dd739689b4ef Mon Sep 17 00:00:00 2001 From: Daniel Bertalan Date: Sun, 2 Jul 2023 20:33:58 +0200 Subject: [PATCH] LibJS/Bytecode: Do not coerce the receiver to Object for internal_set This makes the behavior of `Symbol` correct in strict mode, wherein if the receiver is a symbol primitive, assigning new properties should throw a TypeError. --- Userland/Libraries/LibJS/Bytecode/Op.cpp | 15 ++++++++------- .../LibJS/Tests/builtins/Symbol/Symbol.js | 8 ++++++++ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/Userland/Libraries/LibJS/Bytecode/Op.cpp b/Userland/Libraries/LibJS/Bytecode/Op.cpp index 1fee5143ad..1112eedc4f 100644 --- a/Userland/Libraries/LibJS/Bytecode/Op.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Op.cpp @@ -48,8 +48,9 @@ DeprecatedString Instruction::to_deprecated_string(Bytecode::Executable const& e namespace JS::Bytecode::Op { -static ThrowCompletionOr put_by_property_key(VM& vm, Object* object, Value value, PropertyKey name, PropertyKind kind) +static ThrowCompletionOr put_by_property_key(VM& vm, Value base, Value value, PropertyKey name, PropertyKind kind) { + auto object = TRY(base.to_object(vm)); if (kind == PropertyKind::Getter || kind == PropertyKind::Setter) { // The generator should only pass us functions for getters and setters. VERIFY(value.is_function()); @@ -70,9 +71,9 @@ static ThrowCompletionOr put_by_property_key(VM& vm, Object* object, Value break; } case PropertyKind::KeyValue: { - bool succeeded = TRY(object->internal_set(name, value, object)); + bool succeeded = TRY(object->internal_set(name, value, base)); if (!succeeded && vm.in_strict_mode()) - return vm.throw_completion(ErrorType::ReferenceNullishSetProperty, name, TRY_OR_THROW_OOM(vm, value.to_string_without_side_effects())); + return vm.throw_completion(ErrorType::ReferenceNullishSetProperty, name, TRY_OR_THROW_OOM(vm, base.to_string_without_side_effects())); break; } case PropertyKind::Spread: @@ -548,9 +549,9 @@ ThrowCompletionOr PutById::execute_impl(Bytecode::Interpreter& interpreter auto& vm = interpreter.vm(); // NOTE: Get the value from the accumulator before side effects have a chance to overwrite it. auto value = interpreter.accumulator(); - auto object = TRY(interpreter.reg(m_base).to_object(vm)); + auto base = interpreter.reg(m_base); PropertyKey name = interpreter.current_executable().get_identifier(m_property); - TRY(put_by_property_key(vm, object, value, name, m_kind)); + TRY(put_by_property_key(vm, base, value, name, m_kind)); interpreter.accumulator() = value; return {}; } @@ -1017,10 +1018,10 @@ ThrowCompletionOr PutByValue::execute_impl(Bytecode::Interpreter& interpre // NOTE: Get the value from the accumulator before side effects have a chance to overwrite it. auto value = interpreter.accumulator(); - auto object = TRY(interpreter.reg(m_base).to_object(vm)); + auto base = interpreter.reg(m_base); auto property_key = TRY(interpreter.reg(m_property).to_property_key(vm)); - TRY(put_by_property_key(vm, object, value, property_key, m_kind)); + TRY(put_by_property_key(vm, base, value, property_key, m_kind)); interpreter.accumulator() = value; return {}; } diff --git a/Userland/Libraries/LibJS/Tests/builtins/Symbol/Symbol.js b/Userland/Libraries/LibJS/Tests/builtins/Symbol/Symbol.js index 2533f593c7..b947d28f4c 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/Symbol/Symbol.js +++ b/Userland/Libraries/LibJS/Tests/builtins/Symbol/Symbol.js @@ -17,3 +17,11 @@ test("constructing symbol from symbol is an error", () => { Symbol(Symbol("foo")); }).toThrowWithMessage(TypeError, "Cannot convert symbol to string"); }); + +test("setting new properties on a symbol is an error in strict mode", () => { + "use strict"; + var symbol = Symbol("foo"); + expect(() => { + symbol.bar = 42; + }).toThrowWithMessage(TypeError, "Cannot set property 'bar' of Symbol(foo)"); +});