Skip to content

Commit c619ad4

Browse files
IdanHolinusg
authored andcommitted
LibJS: Allow no-op define property calls on non-configurable objects
This brings us slightly closer to the specification's 10.1.6.3 ValidateAndApplyPropertyDescriptor.
1 parent 0c8dce6 commit c619ad4

File tree

2 files changed

+31
-8
lines changed

2 files changed

+31
-8
lines changed

Userland/Libraries/LibJS/Runtime/Object.cpp

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -657,11 +657,35 @@ bool Object::put_own_property(const StringOrSymbol& property_name, Value value,
657657
VERIFY(metadata.has_value());
658658
}
659659

660-
if (!new_property && mode == PutOwnPropertyMode::DefineProperty && !metadata.value().attributes.is_configurable() && attributes != metadata.value().attributes) {
661-
dbgln_if(OBJECT_DEBUG, "Disallow reconfig of non-configurable property");
662-
if (throw_exceptions)
663-
vm().throw_exception<TypeError>(global_object(), ErrorType::DescChangeNonConfigurable, property_name.to_display_string());
664-
return false;
660+
auto value_here = m_storage[metadata.value().offset];
661+
if (!new_property && mode == PutOwnPropertyMode::DefineProperty && !metadata.value().attributes.is_configurable()) {
662+
if ((attributes.has_configurable() && attributes.is_configurable()) || (attributes.has_enumerable() && attributes.is_enumerable() != metadata.value().attributes.is_enumerable())) {
663+
dbgln_if(OBJECT_DEBUG, "Disallow reconfig of non-configurable property");
664+
if (throw_exceptions)
665+
vm().throw_exception<TypeError>(global_object(), ErrorType::DescChangeNonConfigurable, property_name.to_display_string());
666+
return false;
667+
}
668+
669+
if (value_here.is_accessor() != value.is_accessor()) {
670+
dbgln_if(OBJECT_DEBUG, "Disallow reconfig of non-configurable property");
671+
if (throw_exceptions)
672+
vm().throw_exception<TypeError>(global_object(), ErrorType::DescChangeNonConfigurable, property_name.to_display_string());
673+
return false;
674+
}
675+
676+
if (!value_here.is_accessor() && !metadata.value().attributes.is_writable() && ((attributes.has_writable() && attributes.is_writable()) || (!value.is_empty() && !same_value(value, value_here)))) {
677+
dbgln_if(OBJECT_DEBUG, "Disallow reconfig of non-configurable property");
678+
if (throw_exceptions)
679+
vm().throw_exception<TypeError>(global_object(), ErrorType::DescChangeNonConfigurable, property_name.to_display_string());
680+
return false;
681+
}
682+
683+
if (value_here.is_accessor() && ((attributes.has_setter() && value.as_accessor().setter() != value_here.as_accessor().setter()) || (attributes.has_getter() && value.as_accessor().getter() != value_here.as_accessor().getter()))) {
684+
dbgln_if(OBJECT_DEBUG, "Disallow reconfig of non-configurable property");
685+
if (throw_exceptions)
686+
vm().throw_exception<TypeError>(global_object(), ErrorType::DescChangeNonConfigurable, property_name.to_display_string());
687+
return false;
688+
}
665689
}
666690

667691
if (mode == PutOwnPropertyMode::DefineProperty && attributes != metadata.value().attributes) {
@@ -675,7 +699,6 @@ bool Object::put_own_property(const StringOrSymbol& property_name, Value value,
675699
dbgln_if(OBJECT_DEBUG, "Reconfigured property {}, new shape says offset is {} and my storage capacity is {}", property_name.to_display_string(), metadata.value().offset, m_storage.size());
676700
}
677701

678-
auto value_here = m_storage[metadata.value().offset];
679702
if (!new_property && mode == PutOwnPropertyMode::Put && !value_here.is_accessor() && !metadata.value().attributes.is_writable()) {
680703
dbgln_if(OBJECT_DEBUG, "Disallow write to non-writable property");
681704
if (throw_exceptions && vm().in_strict_mode())

Userland/Libraries/LibJS/Tests/builtins/Object/Object.defineProperty.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ describe("errors", () => {
166166
Object.defineProperty(o, "foo", { value: 1, writable: true, enumerable: true });
167167

168168
expect(() => {
169-
Object.defineProperty(o, "foo", { value: 2, writable: false, enumerable: true });
169+
Object.defineProperty(o, "foo", { value: 2, writable: true, enumerable: false });
170170
}).toThrowWithMessage(
171171
TypeError,
172172
"Cannot change attributes of non-configurable property 'foo'"
@@ -179,7 +179,7 @@ describe("errors", () => {
179179
Object.defineProperty(o, s, { value: 1, writable: true, enumerable: true });
180180

181181
expect(() => {
182-
Object.defineProperty(o, s, { value: 2, writable: false, enumerable: true });
182+
Object.defineProperty(o, s, { value: 2, writable: true, enumerable: false });
183183
}).toThrowWithMessage(
184184
TypeError,
185185
"Cannot change attributes of non-configurable property 'Symbol(foo)'"

0 commit comments

Comments
 (0)