mirror of
https://github.com/LadybirdBrowser/ladybird.git
synced 2024-11-26 17:40:27 +00:00
LibJS: Fix crash in Object.{freeze,seal}() with indexed properties
This was failing to take two things into account: - When constructing a PropertyName from a value, it won't automatically convert to Type::Number for something like string "0", even though that's how things work internally, since indexed properties are stored separately. This will be improved in a future patch, it's a footgun and should happen automatically. - Those can't be looked up on the shape, we have to go through the indexed properties instead. Additionally it now operates on the shape or indexed properties directly as define_property() was overly strict and would throw if a property was already non-configurable. Fixes #6469.
This commit is contained in:
parent
085816645f
commit
ac3e7ef791
Notes:
sideshowbarker
2024-07-18 19:21:48 +09:00
Author: https://github.com/linusg Commit: https://github.com/SerenityOS/serenity/commit/ac3e7ef7917 Pull-request: https://github.com/SerenityOS/serenity/pull/6510
3 changed files with 72 additions and 5 deletions
|
@ -164,11 +164,20 @@ bool Object::prevent_extensions()
|
|||
bool Object::set_integrity_level(IntegrityLevel level)
|
||||
{
|
||||
// FIXME: This feels clunky and should get nicer abstractions.
|
||||
auto update_property = [this](auto& property_name, auto attributes) {
|
||||
auto metadata = shape().lookup(property_name.to_string_or_symbol());
|
||||
VERIFY(metadata.has_value());
|
||||
auto value = get_direct(metadata->offset);
|
||||
define_property(property_name, value, metadata->attributes.bits() & attributes);
|
||||
auto update_property = [this](auto& property_name, auto new_attributes) {
|
||||
if (property_name.is_number()) {
|
||||
auto value_and_attributes = m_indexed_properties.get(nullptr, property_name.as_number(), false).value();
|
||||
auto value = value_and_attributes.value;
|
||||
auto attributes = value_and_attributes.attributes.bits() & new_attributes;
|
||||
m_indexed_properties.put(nullptr, property_name.as_number(), value, attributes, false);
|
||||
} else {
|
||||
auto metadata = shape().lookup(property_name.to_string_or_symbol()).value();
|
||||
auto attributes = metadata.attributes.bits() & new_attributes;
|
||||
if (m_shape->is_unique())
|
||||
m_shape->reconfigure_property_in_unique_shape(property_name.to_string_or_symbol(), attributes);
|
||||
else
|
||||
set_shape(*m_shape->create_configure_transition(property_name.to_string_or_symbol(), attributes));
|
||||
}
|
||||
};
|
||||
|
||||
auto& vm = this->vm();
|
||||
|
@ -184,6 +193,11 @@ bool Object::set_integrity_level(IntegrityLevel level)
|
|||
case IntegrityLevel::Sealed:
|
||||
for (auto& key : keys) {
|
||||
auto property_name = PropertyName::from_value(global_object(), key);
|
||||
if (property_name.is_string()) {
|
||||
i32 property_index = property_name.as_string().to_int().value_or(-1);
|
||||
if (property_index >= 0)
|
||||
property_name = property_index;
|
||||
}
|
||||
update_property(property_name, ~Attribute::Configurable);
|
||||
if (vm.exception())
|
||||
return {};
|
||||
|
@ -192,6 +206,11 @@ bool Object::set_integrity_level(IntegrityLevel level)
|
|||
case IntegrityLevel::Frozen:
|
||||
for (auto& key : keys) {
|
||||
auto property_name = PropertyName::from_value(global_object(), key);
|
||||
if (property_name.is_string()) {
|
||||
i32 property_index = property_name.as_string().to_int().value_or(-1);
|
||||
if (property_index >= 0)
|
||||
property_name = property_index;
|
||||
}
|
||||
auto property_descriptor = get_own_property_descriptor(property_name);
|
||||
VERIFY(property_descriptor.has_value());
|
||||
u8 attributes = property_descriptor->is_accessor_descriptor()
|
||||
|
|
|
@ -47,4 +47,27 @@ describe("normal behavior", () => {
|
|||
o.foo = "baz";
|
||||
expect(o.foo).toBe("bar");
|
||||
});
|
||||
|
||||
// #6469
|
||||
test("works with indexed properties", () => {
|
||||
const a = ["foo"];
|
||||
expect(a[0]).toBe("foo");
|
||||
Object.freeze(a);
|
||||
a[0] = "bar";
|
||||
expect(a[0]).toBe("foo");
|
||||
});
|
||||
|
||||
test("works with properties that are already non-configurable", () => {
|
||||
const o = {};
|
||||
Object.defineProperty(o, "foo", {
|
||||
value: "bar",
|
||||
configurable: false,
|
||||
writable: true,
|
||||
enumerable: true,
|
||||
});
|
||||
expect(o.foo).toBe("bar");
|
||||
Object.freeze(o);
|
||||
o.foo = "baz";
|
||||
expect(o.foo).toBe("bar");
|
||||
});
|
||||
});
|
||||
|
|
|
@ -47,4 +47,29 @@ describe("normal behavior", () => {
|
|||
o.foo = "baz";
|
||||
expect(o.foo).toBe("baz");
|
||||
});
|
||||
|
||||
// #6469
|
||||
test("works with indexed properties", () => {
|
||||
const a = ["foo"];
|
||||
expect(a[0]).toBe("foo");
|
||||
Object.seal(a);
|
||||
a[0] = "bar";
|
||||
a[1] = "baz";
|
||||
expect(a[0]).toBe("bar");
|
||||
expect(a[1]).toBeUndefined();
|
||||
});
|
||||
|
||||
test("works with properties that are already non-configurable", () => {
|
||||
const o = {};
|
||||
Object.defineProperty(o, "foo", {
|
||||
value: "bar",
|
||||
configurable: false,
|
||||
writable: true,
|
||||
enumerable: true,
|
||||
});
|
||||
expect(o.foo).toBe("bar");
|
||||
Object.seal(o);
|
||||
o.foo = "baz";
|
||||
expect(o.foo).toBe("baz");
|
||||
});
|
||||
});
|
||||
|
|
Loading…
Reference in a new issue