793210 - getOwnPropertyDescriptor proxy trap reverses the logic for new properties on non-extensible objects

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Simon Lindholm]

User Agent: Mozilla/5.0 (X11; Linux i686; rv:18.0) Gecko/18.0 Firefox/18.0
Build ID: 20120921030601

Steps to reproduce:

1.
var target = {};
Object.getOwnPropertyDescriptor(new Proxy(target, {
  getOwnPropertyDescriptor: function () {
    return {value: 2, configurable: true};
   }
}), 'foo');

2.
var target = {};
Object.preventExtensions(target);
Object.getOwnPropertyDescriptor(new Proxy(target, {
  getOwnPropertyDescriptor: function () {
    return {value: 2, configurable: true};
   }
}), 'foo');


Actual results:

The first thing throws "TypeError: proxy can't report a new property on a non-extensible object", but shouldn't. The second thing should throw, but doesn't.

TrapGetOwnProperty in jsproxy.cpp does:
    // step 9
    if (target->isExtensible() && !isFixed) {
        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_CANT_REPORT_NEW);
        return false;
    }

but per spec the condition should be

    (!target->isExtensible() && !isFixed)

Comment 1

[Simon Lindholm]

The patch for bug 795903 should also fix this.

Comment 2

[Simon Lindholm]

Any update on this (or bug 795903)? It breaking proxies pretty badly, and should be a simple fix.

Comment 3

[Eric Faust [:efaust]]

Attachment: Fix

Wtf. Here's a fix.

Comment 4

[Jason Orendorff [:jorendorff]]

Comment on attachment 8388745 [details] [diff] [review]
Fix

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

Add the test case to one of the test suites. r=me with that.

Of course proxies still need work, but thanks for knocking this one out.

Comment 5

[Eric Faust [:efaust]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/79d8037bf2ed

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/79d8037bf2ed