Closed Bug 1308735 Opened 5 years ago Closed 3 years ago

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


(Core :: JavaScript Engine, defect, P3)




Tracking Status
firefox62 --- fixed


(Reporter: Oriol, Assigned: evilpie)


(Blocks 1 open bug)


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


(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 [§][1] and [§][2] I expected false:

> § If intIndex ≥ length, return false.

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

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

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

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
Use soft-fail mechansim for out-of-bounds typed array indexes. r=jorendorff
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
I've mentioned this here 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 :)
Depends on: 1496475
You need to log in before you can comment on or make changes to this bug.