Closed Bug 793210 Opened 12 years ago Closed 10 years ago

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

Categories

(Core :: JavaScript Engine, defect)

18 Branch
x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: simon.lindholm10, Assigned: efaust)

References

Details

(Whiteboard: [js:p2][firebug-p1])

Attachments

(1 file)

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)
Assignee: general → ejpbruel
QA Contact: ejpbruel
Whiteboard: [js:p2]
The patch for bug 795903 should also fix this.
Whiteboard: [js:p2] → [js:p2][firebug-p1]
Any update on this (or bug 795903)? It breaking proxies pretty badly, and should be a simple fix.
Blocks: 703537
See Also: → 806299
See Also: → 795903
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(jwalden+bmo)
Flags: needinfo?(jorendorff)
Attached patch FixSplinter Review
Wtf. Here's a fix.
Assignee: ejpbruel → efaustbmo
Status: NEW → ASSIGNED
Attachment #8388745 - Flags: review?(jorendorff)
Flags: needinfo?(jwalden+bmo)
Flags: needinfo?(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.
Attachment #8388745 - Flags: review?(jorendorff) → review+
https://hg.mozilla.org/mozilla-central/rev/79d8037bf2ed
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: