Closed Bug 1413794 Opened 2 years ago Closed Last year

Typed array [[Set]] wrongly inspects the receiver when a canonical numeric string is passed as property name

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: Waldo, Assigned: evilpie)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Testcase:

  var receiver =
    new Proxy({},
              { getOwnPropertyDescriptor(p) { print("called"); throw 42; } });
  var ta = new Uint8Array(1);
  assertEq(Reflect.set(ta, 0, 47, receiver), true);
  assertEq(ta[0], 47);

Expected behavior is to pass.  Actual behavior is to print "called" and throw.

We might have been spec-compliant at some time in the past, because es6draft behaves identically.  But es6draft hasn't been updated in over a year, and

https://tc39.github.io/ecma262/#sec-reflect.set
https://tc39.github.io/ecma262/#sec-integer-indexed-exotic-objects-set-p-v-receiver
https://tc39.github.io/ecma262/#sec-integerindexedelementset

seem pretty clear that if the property name is a numeric index, this should entirely ignore the receiver.

Or is my spec-reading ability completely atrophied from half a year ago?
It was changed in the spec in https://github.com/tc39/ecma262/issues/347, which in turn points back at bug 1216621 explaining SM already works as proposed in the spec PR. Curious!
Assignee: nobody → evilpies
Attachment #8989044 - Flags: review?(andrebargull)
Comment on attachment 8989044 [details] [diff] [review]
Typed array [[Set]] wrongly inspects the receiver when a canonical numeric string is passed as property name

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

Thanks!

::: js/src/vm/NativeObject.cpp
@@ +2678,3 @@
>  {
> +    // ES2019 draft rev e7dc63fb5d1c26beada9ffc12dc78aa6548f1fb5
> +    // 9.4.5.9 IntegerIndexedElementSet

Nit: Move spec reference above the method header.

@@ +2739,4 @@
>          if (pobj->denseElementsAreFrozen())
>              return result.fail(JSMSG_READ_ONLY);
>  
> +        // TypedArray [[Set]] ignores the receiver completely.

I think it makes sense to move this block above the frozen elements check, given that TypedArrays cannot have frozen elements. WDYT?
Attachment #8989044 - Flags: review?(andrebargull) → review+
(In reply to André Bargull [:anba] from comment #3)
> Comment on attachment 8989044 [details] [diff] [review]
> Typed array [[Set]] wrongly inspects the receiver when a canonical numeric
> string is passed as property name
> 
> Review of attachment 8989044 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks!
Thanks for the quick review.
> 
> ::: js/src/vm/NativeObject.cpp
> @@ +2678,3 @@
> >  {
> > +    // ES2019 draft rev e7dc63fb5d1c26beada9ffc12dc78aa6548f1fb5
> > +    // 9.4.5.9 IntegerIndexedElementSet
> 
> Nit: Move spec reference above the method header.
> 
> @@ +2739,4 @@
> >          if (pobj->denseElementsAreFrozen())
> >              return result.fail(JSMSG_READ_ONLY);
> >  
> > +        // TypedArray [[Set]] ignores the receiver completely.
> 
> I think it makes sense to move this block above the frozen elements check,
> given that TypedArrays cannot have frozen elements. WDYT?

Makes sense.
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/846e6b6678b6
Typed array [[Set]] wrongly inspects the receiver when a canonical numeric string is passed as property name. r=anba
https://hg.mozilla.org/mozilla-central/rev/846e6b6678b6
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Depends on: 1502889
You need to log in before you can comment on or make changes to this bug.