Closed Bug 1308735 Opened 3 years ago Closed Last year

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

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: Oriol, Assigned: evilpie)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, triage-deferred)

Attachments

(1 file)

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
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.
Keywords: triage-deferred
Priority: -- → P3
Depends on: 1329321
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.
Assignee: nobody → evilpies
Attachment #8979713 - Flags: review?(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?
Attachment #8979713 - Flags: review?(jorendorff) → review+
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/05cefb55b142
Use soft-fail mechansim for out-of-bounds typed array indexes. r=jorendorff
https://hg.mozilla.org/mozilla-central/rev/05cefb55b142
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
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 :)
You need to log in before you can comment on or make changes to this bug.