1308735 - [[DefineOwnProperty]] and [[Set]] don't fail for TypeArrays

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Oriol Brufau [:Oriol]]

Run this code:

    "use strict";
    var arr = new Uint8Array(0);
    Reflect.defineProperty(arr, 1, {value:2}); // true
    Reflect.set(arr, 1, 2);                    // true
    arr[1] = 2;                                // 2

Reflect methods return true but according to [§9.4.5.3][1] and [§9.4.5.9][2] I expected false:

> §9.4.5.3-3.b.vi. If intIndex ≥ length, return false.

> §9.4.5.9-9. If index < 0 or index ≥ length, return false.

And therefore, the assignment should fail in strict mode [§6.2.3.2][3]

> §6.2.3.2-6.b. Let succeeded be ? base.[[Set]](GetReferencedName(V), W, GetThisValue(V)).
> §6.2.3.2-6.c. If succeeded is false and IsStrictReference(V) is true, throw a TypeError exception.

[1]: http://www.ecma-international.org/ecma-262/7.0/#sec-integer-indexed-exotic-objects-defineownproperty-p-desc
[2]: http://www.ecma-international.org/ecma-262/7.0/#sec-integerindexedelementset
[3]: http://www.ecma-international.org/ecma-262/7.0/#sec-putvalue

Comment 1

[Tom S]

Chrome returns false for Reflect.defineProperty and true for Reflect.set. The assignment doesn't fail. For this and related bugs I doubt ES6 is actually web-compatible. We should test other browsers.

Comment 2

[Tom S]

Attachment: Use soft-fail mechansim for typed arrays

Using the soft-fail mechanism here means, that Reflect.parse, Reflect.defineProperty and ObjectOrReflectDefineProperty (internally) will return false. We won't actually throw though when assigning in strict mode.

Comment 3

[Jason Orendorff [:jorendorff]]

Comment on attachment 8979713 [details] [diff] [review]
Use soft-fail mechansim for typed arrays

Review of attachment 8979713 [details] [diff] [review]:
-----------------------------------------------------------------

r=me. Thanks!

::: js/src/vm/NativeObject.cpp
@@ +1933,3 @@
>              MOZ_ASSERT(index >= obj->as<TypedArrayObject>().length());
>  
>              // We (wrongly) ignore out of range defines.

Is this comment still correct?

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/05cefb55b142
Use soft-fail mechansim for out-of-bounds typed array indexes. r=jorendorff

Comment 5

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/05cefb55b142

Comment 6

[Florian Scholz (Open Web Docs)]

I've mentioned this here https://developer.mozilla.org/en-US/Firefox/Releases/62#JavaScript but I don't think it is important enough to talk about it in the typed array or Reflect reference docs. 
I would appreciate if you could double-check my summary of this change. Thanks :)