Changeset 207226 in webkit

Timestamp:

Oct 12, 2016 11:27:50 AM (8 years ago)

Author:

mark.lam@apple.com

Message:

Array.prototype.slice should not modify frozen objects.
​https://bugs.webkit.org/show_bug.cgi?id=163338

Reviewed by Filip Pizlo.

JSTests:

• stress/array-slice-on-frozen-object.js: Added.

Source/JavaScriptCore:

1. The ES6 spec for Array.prototype.slice (​https://tc39.github.io/ecma262/#sec-array.prototype.slice) states that it uses the CreateDataPropertyOrThrow() (​https://tc39.github.io/ecma262/#sec-createdatapropertyorthrow) to add items to the result array. The spec for CreateDataPropertyOrThrow states:

"This abstract operation creates a property whose attributes are set to the
same defaults used for properties created by the ECMAScript language assignment
operator. Normally, the property will not already exist. If it does exist and
is not configurable or if O is not extensible, DefineOwnProperty? will
return false causing this operation to throw a TypeError exception."

2. Array.prototype.slice also uses a Set function (​https://tc39.github.io/ecma262/#sec-set-o-p-v-throw) to set the "length" property and passes true for the Throw argument. Ultimately, it ends up calling the OrdinarySet function (​https://tc39.github.io/ecma262/#sec-ordinaryset) that will fail if the property is not writable. This failure should result in a TypeError being thrown in Set.

Since the properties of frozen objects are not extensible, not configurable,
and not writeable, Array.prototype.slice should fail to write to the result
array if it is frozen.

If the source array being sliced has 1 or more elements, (1) will take effect
when we try to set the element in the non-writeable result obj.
If the source array being sliced has 0 elements, we will not set any elements and
(1) will not trigger. Subsequently, (2) will take effect when we will try to
set the length of the result obj.

• runtime/ArrayPrototype.cpp:

(JSC::putLength):
(JSC::setLength):
(JSC::arrayProtoFuncSlice):
(JSC::arrayProtoFuncSplice):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/array-slice-on-frozen-object.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/ArrayPrototype.cpp (modified) (4 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2016-10-12  Mark Lam  <mark.lam@apple.com>
+
+        Array.prototype.slice should not modify frozen objects.
+        https://bugs.webkit.org/show_bug.cgi?id=163338
+
+        Reviewed by Filip Pizlo.
+
+        * stress/array-slice-on-frozen-object.js: Added.
+
 2016-10-11  Mark Lam  <mark.lam@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-10-12  Mark Lam  <mark.lam@apple.com>
+
+        Array.prototype.slice should not modify frozen objects.
+        https://bugs.webkit.org/show_bug.cgi?id=163338
+
+        Reviewed by Filip Pizlo.
+
+        1. The ES6 spec for Array.prototype.slice
+           (https://tc39.github.io/ecma262/#sec-array.prototype.slice) states that it uses
+           the CreateDataPropertyOrThrow()
+           (https://tc39.github.io/ecma262/#sec-createdatapropertyorthrow) to add items to
+           the result array.  The spec for CreateDataPropertyOrThrow states:
+
+           "This abstract operation creates a property whose attributes are set to the
+           same defaults used for properties created by the ECMAScript language assignment
+           operator. Normally, the property will not already exist. If it does exist and
+           is not configurable or if O is not extensible, [[DefineOwnProperty]] will
+           return false causing this operation to throw a TypeError exception."
+
+        2. Array.prototype.slice also uses a Set function
+           (https://tc39.github.io/ecma262/#sec-set-o-p-v-throw) to set the "length"
+           property and passes true for the Throw argument.  Ultimately, it ends up
+           calling the OrdinarySet function
+           (https://tc39.github.io/ecma262/#sec-ordinaryset) that will fail if the
+           property is not writable.  This failure should result in a TypeError being
+           thrown in Set.
+
+           Since the properties of frozen objects are not extensible, not configurable,
+           and not writeable, Array.prototype.slice should fail to write to the result
+           array if it is frozen.
+
+        If the source array being sliced has 1 or more elements, (1) will take effect
+        when we try to set the element in the non-writeable result obj.
+        If the source array being sliced has 0 elements, we will not set any elements and
+        (1) will not trigger.  Subsequently, (2) will take effect when we will try to
+        set the length of the result obj.
+
+        * runtime/ArrayPrototype.cpp:
+        (JSC::putLength):
+        (JSC::setLength):
+        (JSC::arrayProtoFuncSlice):
+        (JSC::arrayProtoFuncSplice):
+
 2016-10-12  Filip Pizlo  <fpizlo@apple.com>
 

trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp

@@ -164 +164 @@
 }
 
-static ALWAYS_INLINE void putLength(ExecState* exec, VM& vm, JSObject* obj, JSValue value)
+static ALWAYS_INLINE bool putLength(ExecState* exec, VM& vm, JSObject* obj, JSValue value)
 {
     PutPropertySlot slot(obj);
-    obj->methodTable()->put(obj, exec, vm.propertyNames->length, value, slot);
+    return obj->methodTable()->put(obj, exec, vm.propertyNames->length, value, slot);
 }
 
 static ALWAYS_INLINE void setLength(ExecState* exec, VM& vm, JSObject* obj, unsigned value)
 {
-    if (isJSArray(obj))
-        jsCast<JSArray*>(obj)->setLength(exec, value);
-    putLength(exec, vm, obj, jsNumber(value));
+    auto scope = DECLARE_THROW_SCOPE(vm);
+    static const bool throwException = true;
+    if (isJSArray(obj)) {
+        jsCast<JSArray*>(obj)->setLength(exec, value, throwException);
+        RETURN_IF_EXCEPTION(scope, void());
+    }
+    bool success = putLength(exec, vm, obj, jsNumber(value));
+    RETURN_IF_EXCEPTION(scope, void());
+    if (UNLIKELY(!success))
+        throwTypeError(exec, scope, ASCIILiteral(ReadonlyPropertyWriteError));
 }
 
@@ -875 +882 @@
         RETURN_IF_EXCEPTION(scope, encodedJSValue());
         if (v)
-            result->putDirectIndex(exec, n, v);
-    }
+            result->putDirectIndex(exec, n, v, 0, PutDirectIndexShouldThrow);
+    }
+    scope.release();
     setLength(exec, vm, result, n);
     return JSValue::encode(result);
@@ -908 +916 @@
 
         setLength(exec, vm, result, 0);
+        RETURN_IF_EXCEPTION(scope, encodedJSValue());
+        scope.release();
         setLength(exec, vm, thisObj, length);
         return JSValue::encode(result);
@@ -973 +983 @@
     }
     
+    scope.release();
     setLength(exec, vm, thisObj, length - deleteCount + additionalArgs);
     return JSValue::encode(result);