From 40fa4a761bf5f2082335382f21c2c87c6cb72219 Mon Sep 17 00:00:00 2001 From: "ricow@chromium.org" Date: Wed, 15 Jun 2011 06:44:57 +0000 Subject: [PATCH] Correctly set ReadOnly flag on indexed properties when using the API Set method (fixes issue 1470) Review URL: http://codereview.chromium.org/7149015 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@8286 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/runtime.cc | 34 ++++++++++++++++++++++++++++++++++ test/cctest/test-api.cc | 22 ++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/src/runtime.cc b/src/runtime.cc index 079a9b89dc..a50303b65f 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -3936,6 +3936,29 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DefineOrRedefineDataProperty) { } +// Special case for elements if any of the flags are true. +// If elements are in fast case we always implicitly assume that: +// DONT_DELETE: false, DONT_ENUM: false, READ_ONLY: false. +static MaybeObject* NormalizeObjectSetElement(Isolate* isolate, + Handle js_object, + uint32_t index, + Handle value, + PropertyAttributes attr) { + // Normalize the elements to enable attributes on the property. + NormalizeElements(js_object); + Handle dictionary(js_object->element_dictionary()); + // Make sure that we never go back to fast case. + dictionary->set_requires_slow_elements(); + PropertyDetails details = PropertyDetails(attr, NORMAL); + Handle extended_dictionary = + NumberDictionarySet(dictionary, index, value, details); + if (*extended_dictionary != *dictionary) { + js_object->set_elements(*extended_dictionary); + } + return *value; +} + + MaybeObject* Runtime::SetObjectProperty(Isolate* isolate, Handle object, Handle key, @@ -3971,6 +3994,10 @@ MaybeObject* Runtime::SetObjectProperty(Isolate* isolate, return *value; } + if (((attr & (DONT_DELETE | DONT_ENUM | READ_ONLY)) != 0)) { + return NormalizeObjectSetElement(isolate, js_object, index, value, attr); + } + Handle result = SetElement(js_object, index, value, strict_mode); if (result.is_null()) return Failure::Exception(); return *value; @@ -3979,6 +4006,13 @@ MaybeObject* Runtime::SetObjectProperty(Isolate* isolate, if (key->IsString()) { Handle result; if (Handle::cast(key)->AsArrayIndex(&index)) { + if (((attr & (DONT_DELETE | DONT_ENUM | READ_ONLY)) != 0)) { + return NormalizeObjectSetElement(isolate, + js_object, + index, + value, + attr); + } result = SetElement(js_object, index, value, strict_mode); } else { Handle key_string = Handle::cast(key); diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index a1ff562c2b..c2e5382c38 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -14480,3 +14480,25 @@ THREADED_TEST(CallAPIFunctionOnNonObject) { TryCatch try_catch; CompileRun("f.call(2)"); } + + +// Regression test for issue 1470. +THREADED_TEST(ReadOnlyIndexedProperties) { + v8::HandleScope scope; + Local templ = ObjectTemplate::New(); + + LocalContext context; + Local obj = templ->NewInstance(); + context->Global()->Set(v8_str("obj"), obj); + obj->Set(v8_str("1"), v8_str("DONT_CHANGE"), v8::ReadOnly); + obj->Set(v8_str("1"), v8_str("foobar")); + CHECK_EQ(v8_str("DONT_CHANGE"), obj->Get(v8_str("1"))); + obj->Set(v8_num(2), v8_str("DONT_CHANGE"), v8::ReadOnly); + obj->Set(v8_num(2), v8_str("foobar")); + CHECK_EQ(v8_str("DONT_CHANGE"), obj->Get(v8_num(2))); + + // Test non-smi case. + obj->Set(v8_str("2000000000"), v8_str("DONT_CHANGE"), v8::ReadOnly); + obj->Set(v8_str("2000000000"), v8_str("foobar")); + CHECK_EQ(v8_str("DONT_CHANGE"), obj->Get(v8_str("2000000000"))); +}