Browse Source

LibJS: Avoid ToPropertyKey for spreading in PutByValue(WithThis)

This is not we're supposed to do according to https://tc39.es/ecma262/#sec-runtime-semantics-propertydefinitionevaluation
Furthermore, this was observable by ToPrimitive looking up toString and
valueOf and potentially calling them if they exist. The big ticket
issue however is that for objects without toString and valueOf, such as
null-proto objects, this would unexpectedly throw.
Luke Wilde 1 year ago
parent
commit
2aaae6fc70

+ 2 - 2
Userland/Libraries/LibJS/Bytecode/Op.cpp

@@ -1183,7 +1183,7 @@ ThrowCompletionOr<void> PutByValue::execute_impl(Bytecode::Interpreter& interpre
 
 
     auto base = interpreter.reg(m_base);
     auto base = interpreter.reg(m_base);
 
 
-    auto property_key = TRY(interpreter.reg(m_property).to_property_key(vm));
+    auto property_key = m_kind != PropertyKind::Spread ? TRY(interpreter.reg(m_property).to_property_key(vm)) : PropertyKey {};
     TRY(put_by_property_key(vm, base, base, value, property_key, m_kind));
     TRY(put_by_property_key(vm, base, base, value, property_key, m_kind));
     interpreter.accumulator() = value;
     interpreter.accumulator() = value;
     return {};
     return {};
@@ -1198,7 +1198,7 @@ ThrowCompletionOr<void> PutByValueWithThis::execute_impl(Bytecode::Interpreter&
 
 
     auto base = interpreter.reg(m_base);
     auto base = interpreter.reg(m_base);
 
 
-    auto property_key = TRY(interpreter.reg(m_property).to_property_key(vm));
+    auto property_key = m_kind != PropertyKind::Spread ? TRY(interpreter.reg(m_property).to_property_key(vm)) : PropertyKey {};
     TRY(put_by_property_key(vm, base, interpreter.reg(m_this_value), value, property_key, m_kind));
     TRY(put_by_property_key(vm, base, interpreter.reg(m_this_value), value, property_key, m_kind));
     interpreter.accumulator() = value;
     interpreter.accumulator() = value;
     return {};
     return {};

+ 21 - 1
Userland/Libraries/LibJS/Tests/object-spread.js

@@ -144,7 +144,7 @@ describe("modification of spreadable objects during spread", () => {
         expect(Object.getOwnPropertyNames(result)).toContain("bar");
         expect(Object.getOwnPropertyNames(result)).toContain("bar");
     });
     });
 
 
-    test.xfail("spreading array", () => {
+    test("spreading array", () => {
         const array = [0];
         const array = [0];
         array[2] = 2;
         array[2] = 2;
         array[999] = 999;
         array[999] = 999;
@@ -192,3 +192,23 @@ test("allows assignment expressions", () => {
     expect("({ ...a ??= 'hello' })").toEval();
     expect("({ ...a ??= 'hello' })").toEval();
     expect("function* test() { return ({ ...yield a }); }").toEval();
     expect("function* test() { return ({ ...yield a }); }").toEval();
 });
 });
+
+test("spreading null-proto objects", () => {
+    const obj = {
+        __proto__: null,
+        hello: "world",
+        friends: "well hello",
+        toString() {
+            expect().fail("called toString()");
+        },
+        valueOf() {
+            expect().fail("called valueOf()");
+        },
+    };
+    let res;
+    expect(() => {
+        res = { ...obj };
+    }).not.toThrow();
+    expect(res).toHaveProperty("hello", "world");
+    expect(res).toHaveProperty("friends", "well hello");
+});