Bug 1317307: Throw a TypeError when attempting to change array.length to accessor property. r=till
authorAndré Bargull <andre.bargull@gmail.com>
Mon, 14 Nov 2016 12:27:52 -0800
changeset 322734 55b6cacff79119a3aa4c16366c68db41ec1d05a1
parent 322733 7a2af053b06aa116eb3bac3370e082897d91e531
child 322735 94615002eb6e15e49ddc93aa49f89f064e882b6d
push id21
push usermaklebus@msu.edu
push dateThu, 01 Dec 2016 06:22:08 +0000
reviewerstill
bugs1317307
milestone53.0a1
Bug 1317307: Throw a TypeError when attempting to change array.length to accessor property. r=till
js/src/jsarray.cpp
js/src/tests/ecma_5/Array/redefine-length-accessor.js
js/src/vm/NativeObject.cpp
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -546,26 +546,27 @@ js::ArraySetLength(JSContext* cx, Handle
     MOZ_ASSERT(id == NameToId(cx->names().length));
 
     if (!arr->maybeCopyElementsForWrite(cx))
         return false;
 
     // Step 1.
     uint32_t newLen;
     if (attrs & JSPROP_IGNORE_VALUE) {
+        MOZ_ASSERT(value.isUndefined());
+
         // The spec has us calling OrdinaryDefineOwnProperty if
         // Desc.[[Value]] is absent, but our implementation is so different that
         // this is impossible. Instead, set newLen to the current length and
         // proceed to step 9.
         newLen = arr->length();
     } else {
         // Step 2 is irrelevant in our implementation.
 
         // Steps 3-7.
-        MOZ_ASSERT_IF(attrs & JSPROP_IGNORE_VALUE, value.isUndefined());
         if (!CanonicalizeArrayLengthValue(cx, value, &newLen))
             return false;
 
         // Step 8 is irrelevant in our implementation.
     }
 
     // Steps 9-11.
     bool lengthIsWritable = arr->lengthIsWritable();
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_5/Array/redefine-length-accessor.js
@@ -0,0 +1,42 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+var absent = {};
+
+var getterValues = [absent, undefined, function(){}];
+var setterValues = [absent, undefined, function(){}];
+var configurableValues = [absent, true, false];
+var enumerableValues = [absent, true, false];
+
+function CreateDescriptor(getter, setter, enumerable, configurable) {
+  var descriptor = {};
+  if (getter !== absent)
+    descriptor.get = getter;
+  if (setter !== absent)
+    descriptor.set = setter;
+  if (configurable !== absent)
+    descriptor.configurable = configurable;
+  if (enumerable !== absent)
+    descriptor.enumerable = enumerable;
+  return descriptor;
+}
+
+getterValues.forEach(function(getter) {
+  setterValues.forEach(function(setter) {
+    enumerableValues.forEach(function(enumerable) {
+      configurableValues.forEach(function(configurable) {
+        var descriptor = CreateDescriptor(getter, setter, enumerable, configurable);
+        if (!("get" in descriptor || "set" in descriptor))
+          return;
+
+        assertThrowsInstanceOf(function() {
+          Object.defineProperty([], "length", descriptor);
+        }, TypeError);
+      });
+    });
+  });
+});
+
+if (typeof reportCompare === "function")
+  reportCompare(0, 0);
--- a/js/src/vm/NativeObject.cpp
+++ b/js/src/vm/NativeObject.cpp
@@ -1367,28 +1367,31 @@ DefinePropertyIsRedundant(ExclusiveConte
 
 bool
 js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId id,
                          Handle<PropertyDescriptor> desc_,
                          ObjectOpResult& result)
 {
     desc_.assertValid();
 
-    // Section numbers and step numbers below refer to ES6 draft rev 36
-    // (17 March 2015).
+    // Section numbers and step numbers below refer to ES2016.
     //
     // This function aims to implement 9.1.6 [[DefineOwnProperty]] as well as
     // the [[DefineOwnProperty]] methods described in 9.4.2.1 (arrays), 9.4.4.2
     // (arguments), and 9.4.5.3 (typed array views).
 
     // Dispense with custom behavior of exotic native objects first.
     if (obj->is<ArrayObject>()) {
         // 9.4.2.1 step 2. Redefining an array's length is very special.
         Rooted<ArrayObject*> arr(cx, &obj->as<ArrayObject>());
         if (id == NameToId(cx->names().length)) {
+            // 9.1.6.3 ValidateAndApplyPropertyDescriptor, step 7.a.
+            if (desc_.isAccessorDescriptor())
+                return result.fail(JSMSG_CANT_REDEFINE_PROP);
+
             if (!cx->shouldBeJSContext())
                 return false;
             return ArraySetLength(cx->asJSContext(), arr, id, desc_.attributes(), desc_.value(),
                                   result);
         }
 
         // 9.4.2.1 step 3. Don't extend a fixed-length array.
         uint32_t index;
@@ -1418,17 +1421,17 @@ js::NativeDefineProperty(ExclusiveContex
             if ((desc_.attributes() & JSPROP_RESOLVING) == 0)
                 obj->as<ArgumentsObject>().markIteratorOverridden();
         } else if (JSID_IS_INT(id)) {
             if ((desc_.attributes() & JSPROP_RESOLVING) == 0)
                 obj->as<ArgumentsObject>().markElementOverridden();
         }
     }
 
-    // 9.1.6.1 OrdinaryDefineOwnProperty steps 1-2.
+    // 9.1.6.1 OrdinaryDefineOwnProperty step 1.
     RootedShape shape(cx);
     if (desc_.attributes() & JSPROP_RESOLVING) {
         // We are being called from a resolve or enumerate hook to reify a
         // lazily-resolved property. To avoid reentering the resolve hook and
         // recursing forever, skip the resolve hook when doing this lookup.
         NativeLookupOwnPropertyNoResolve(cx, obj, id, &shape);
     } else {
         if (!NativeLookupOwnProperty<CanGC>(cx, obj, id, &shape))