Closed
Bug 1308735
Opened 8 years ago
Closed 6 years ago
[[DefineOwnProperty]] and [[Set]] don't fail for TypeArrays
Categories
(Core :: JavaScript Engine, defect, P3)
Core
JavaScript Engine
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)
3.92 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•7 years ago
|
||
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.
Blocks: es6typedarray
Updated•7 years ago
|
Keywords: triage-deferred
Priority: -- → P3
Assignee | ||
Comment 2•6 years ago
|
||
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 3•6 years ago
|
||
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
Comment 5•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/05cefb55b142
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Updated•6 years ago
|
Keywords: dev-doc-needed
Comment 6•6 years ago
|
||
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 :)
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•