Bug 596031 - 'this' is wrong in getters and setters when a proxy object is on the prototype chain. r=brendan/jorendorff/gal

--HG--
extra : rebase_source : 609b96c4b461e33f0f79dc74f714edfa882d1da0
This commit is contained in:
Blake Kaplan 2010-10-29 10:42:35 -07:00
parent 3c02390373
commit c0e2873909
14 changed files with 165 additions and 62 deletions

View File

@ -688,7 +688,7 @@ js_GetDenseArrayElementValue(JSContext *cx, JSObject *obj, jsid id, Value *vp)
}
static JSBool
array_getProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp)
array_getProperty(JSContext *cx, JSObject *obj, JSObject *receiver, jsid id, Value *vp)
{
uint32 i;

View File

@ -738,6 +738,21 @@ CallJSPropertyOpSetter(JSContext *cx, js::PropertyOp op, JSObject *obj, jsid id,
return op(cx, obj, id, vp);
}
inline bool
CallSetter(JSContext *cx, JSObject *obj, jsid id, PropertyOp op, uintN attrs, uintN shortid,
js::Value *vp)
{
if (attrs & JSPROP_SETTER)
return ExternalGetOrSet(cx, obj, id, CastAsObjectJsval(op), JSACC_WRITE, 1, vp, vp);
if (attrs & JSPROP_GETTER)
return js_ReportGetterOnlyAssignment(cx);
if (attrs & JSPROP_SHORTID)
id = INT_TO_JSID(shortid);
return CallJSPropertyOpSetter(cx, op, obj, id, vp);
}
} /* namespace js */
#endif /* jscntxtinlines_h___ */

View File

@ -3023,7 +3023,7 @@ with_LookupProperty(JSContext *cx, JSObject *obj, jsid id, JSObject **objp,
}
static JSBool
with_GetProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp)
with_GetProperty(JSContext *cx, JSObject *obj, JSObject *receiver, jsid id, Value *vp)
{
return obj->getProto()->getProperty(cx, id, vp);
}
@ -4933,7 +4933,7 @@ js_NativeSet(JSContext *cx, JSObject *obj, const Shape *shape, bool added, Value
}
static JS_ALWAYS_INLINE bool
js_GetPropertyHelperWithShapeInline(JSContext *cx, JSObject *obj, jsid id,
js_GetPropertyHelperWithShapeInline(JSContext *cx, JSObject *obj, JSObject *receiver, jsid id,
uintN getHow, Value *vp,
const Shape **shapeOut, JSObject **holderOut)
{
@ -5019,8 +5019,11 @@ js_GetPropertyHelperWithShapeInline(JSContext *cx, JSObject *obj, jsid id,
return JS_TRUE;
}
if (!obj2->isNative())
return obj2->getProperty(cx, id, vp);
if (!obj2->isNative()) {
return obj2->isProxy()
? JSProxy::get(cx, obj2, receiver, id, vp)
: obj2->getProperty(cx, id, vp);
}
shape = (Shape *) prop;
*shapeOut = shape;
@ -5031,39 +5034,41 @@ js_GetPropertyHelperWithShapeInline(JSContext *cx, JSObject *obj, jsid id,
}
/* This call site is hot -- use the always-inlined variant of js_NativeGet(). */
if (!js_NativeGetInline(cx, obj, obj2, shape, getHow, vp))
if (!js_NativeGetInline(cx, receiver, obj2, shape, getHow, vp))
return JS_FALSE;
return JS_TRUE;
}
extern bool
js_GetPropertyHelperWithShape(JSContext *cx, JSObject *obj, jsid id,
bool
js_GetPropertyHelperWithShape(JSContext *cx, JSObject *obj, JSObject *receiver, jsid id,
uint32 getHow, Value *vp,
const Shape **shapeOut, JSObject **holderOut)
{
return js_GetPropertyHelperWithShapeInline(cx, obj, id, getHow, vp, shapeOut, holderOut);
return js_GetPropertyHelperWithShapeInline(cx, obj, receiver, id, getHow, vp,
shapeOut, holderOut);
}
static JS_ALWAYS_INLINE JSBool
js_GetPropertyHelperInline(JSContext *cx, JSObject *obj, jsid id, uint32 getHow, Value *vp)
js_GetPropertyHelperInline(JSContext *cx, JSObject *obj, JSObject *receiver, jsid id,
uint32 getHow, Value *vp)
{
const Shape *shape;
JSObject *holder;
return js_GetPropertyHelperWithShapeInline(cx, obj, id, getHow, vp, &shape, &holder);
}
extern JSBool
js_GetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, uint32 getHow, Value *vp)
{
return js_GetPropertyHelperInline(cx, obj, id, getHow, vp);
return js_GetPropertyHelperWithShapeInline(cx, obj, receiver, id, getHow, vp, &shape, &holder);
}
JSBool
js_GetProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp)
js_GetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, uint32 getHow, Value *vp)
{
return js_GetPropertyHelperInline(cx, obj, obj, id, getHow, vp);
}
JSBool
js_GetProperty(JSContext *cx, JSObject *obj, JSObject *receiver, jsid id, Value *vp)
{
/* This call site is hot -- use the always-inlined variant of js_GetPropertyHelper(). */
return js_GetPropertyHelperInline(cx, obj, id, JSGET_METHOD_BARRIER, vp);
return js_GetPropertyHelperInline(cx, obj, receiver, id, JSGET_METHOD_BARRIER, vp);
}
JSBool
@ -5099,7 +5104,7 @@ js_GetMethod(JSContext *cx, JSObject *obj, jsid id, uintN getHow, Value *vp)
if (obj->isXML())
return js_GetXMLMethod(cx, obj, id, vp);
#endif
return op(cx, obj, id, vp);
return op(cx, obj, obj, id, vp);
}
JS_FRIEND_API(bool)
@ -5180,8 +5185,26 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, uintN defineHow,
if (protoIndex < 0)
return JS_FALSE;
if (prop) {
if (!pobj->isNative())
if (!pobj->isNative()) {
if (pobj->isProxy()) {
AutoPropertyDescriptorRooter pd(cx);
if (!pobj->getProxyHandler()->getPropertyDescriptor(cx, pobj, id, true, &pd))
return false;
if (pd.attrs & JSPROP_SHARED)
return CallSetter(cx, obj, id, pd.setter, pd.attrs, pd.shortid, vp);
if (pd.attrs & JSPROP_READONLY) {
if (strict)
return obj->reportReadOnly(cx, id);
if (JS_HAS_STRICT_OPTION(cx))
return obj->reportReadOnly(cx, id, JSREPORT_STRICT | JSREPORT_WARNING);
return true;
}
}
prop = NULL;
}
} else {
/* We should never add properties to lexical blocks. */
JS_ASSERT(!obj->isBlock());

View File

@ -209,7 +209,13 @@ js_DefineProperty(JSContext *cx, JSObject *obj, jsid id, const js::Value *value,
js::PropertyOp getter, js::PropertyOp setter, uintN attrs);
extern JSBool
js_GetProperty(JSContext *cx, JSObject *obj, jsid id, js::Value *vp);
js_GetProperty(JSContext *cx, JSObject *obj, JSObject *receiver, jsid id, js::Value *vp);
inline JSBool
js_GetProperty(JSContext *cx, JSObject *obj, jsid id, js::Value *vp)
{
return js_GetProperty(cx, obj, obj, id, vp);
}
namespace js {
@ -1064,9 +1070,13 @@ struct JSObject : js::gc::Cell {
return (op ? op : js_DefineProperty)(cx, this, id, &value, getter, setter, attrs);
}
JSBool getProperty(JSContext *cx, jsid id, js::Value *vp) {
JSBool getProperty(JSContext *cx, JSObject *receiver, jsid id, js::Value *vp) {
js::PropertyIdOp op = getOps()->getProperty;
return (op ? op : js_GetProperty)(cx, this, id, vp);
return (op ? op : (js::PropertyIdOp)js_GetProperty)(cx, this, receiver, id, vp);
}
JSBool getProperty(JSContext *cx, jsid id, js::Value *vp) {
return getProperty(cx, this, id, vp);
}
JSBool setProperty(JSContext *cx, jsid id, js::Value *vp, JSBool strict) {
@ -1085,7 +1095,7 @@ struct JSObject : js::gc::Cell {
}
JSBool deleteProperty(JSContext *cx, jsid id, js::Value *rval, JSBool strict) {
js::StrictPropertyIdOp op = getOps()->deleteProperty;
js::DeleteIdOp op = getOps()->deleteProperty;
return (op ? op : js_DeleteProperty)(cx, this, id, rval, strict);
}
@ -1569,8 +1579,9 @@ extern JSBool
js_GetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, uint32 getHow, js::Value *vp);
extern bool
js_GetPropertyHelperWithShape(JSContext *cx, JSObject *obj, jsid id, uint32 getHow,
js::Value *vp, const js::Shape **shapeOut, JSObject **holderOut);
js_GetPropertyHelperWithShape(JSContext *cx, JSObject *obj, JSObject *receiver, jsid id,
uint32 getHow, js::Value *vp,
const js::Shape **shapeOut, JSObject **holderOut);
extern JSBool
js_GetOwnPropertyDescriptor(JSContext *cx, JSObject *obj, jsid id, js::Value *vp);

View File

@ -127,7 +127,7 @@ JSProxyHandler::get(JSContext *cx, JSObject *proxy, JSObject *receiver, jsid id,
return true;
}
if (desc.attrs & JSPROP_GETTER) {
return ExternalGetOrSet(cx, proxy, id, CastAsObjectJsval(desc.getter),
return ExternalGetOrSet(cx, receiver, id, CastAsObjectJsval(desc.getter),
JSACC_READ, 0, NULL, vp);
}
if (!(desc.attrs & JSPROP_SHARED))
@ -136,7 +136,7 @@ JSProxyHandler::get(JSContext *cx, JSObject *proxy, JSObject *receiver, jsid id,
vp->setUndefined();
if (desc.attrs & JSPROP_SHORTID)
id = INT_TO_JSID(desc.shortid);
return CallJSPropertyOp(cx, desc.getter, proxy, id, vp);
return CallJSPropertyOp(cx, desc.getter, receiver, id, vp);
}
bool
@ -148,43 +148,29 @@ JSProxyHandler::set(JSContext *cx, JSObject *proxy, JSObject *receiver, jsid id,
return false;
/* The control-flow here differs from ::get() because of the fall-through case below. */
if (desc.obj) {
if (desc.setter && ((desc.attrs & JSPROP_SETTER) || desc.setter != PropertyStub)) {
if (desc.attrs & JSPROP_SETTER) {
return ExternalGetOrSet(cx, proxy, id, CastAsObjectJsval(desc.setter),
JSACC_WRITE, 1, vp, vp);
}
if (desc.attrs & JSPROP_SHORTID)
id = INT_TO_JSID(desc.shortid);
return CallJSPropertyOpSetter(cx, desc.setter, proxy, id, vp);
}
if (desc.setter && ((desc.attrs & JSPROP_SETTER) || desc.setter != PropertyStub))
return CallSetter(cx, receiver, id, desc.setter, desc.attrs, desc.shortid, vp);
if (desc.attrs & JSPROP_READONLY)
return true;
desc.value = *vp;
return defineProperty(cx, proxy, id, &desc);
return defineProperty(cx, receiver, id, &desc);
}
if (!getPropertyDescriptor(cx, proxy, id, true, &desc))
return false;
if (desc.obj) {
if (desc.setter && ((desc.attrs & JSPROP_SETTER) || desc.setter != PropertyStub)) {
if (desc.attrs & JSPROP_SETTER) {
return ExternalGetOrSet(cx, proxy, id, CastAsObjectJsval(desc.setter),
JSACC_WRITE, 1, vp, vp);
}
if (desc.attrs & JSPROP_SHORTID)
id = INT_TO_JSID(desc.shortid);
return CallJSPropertyOpSetter(cx, desc.setter, proxy, id, vp);
}
if (desc.setter && ((desc.attrs & JSPROP_SETTER) || desc.setter != PropertyStub))
return CallSetter(cx, receiver, id, desc.setter, desc.attrs, desc.shortid, vp);
if (desc.attrs & JSPROP_READONLY)
return true;
/* fall through */
}
desc.obj = proxy;
desc.obj = receiver;
desc.value = *vp;
desc.attrs = JSPROP_ENUMERATE;
desc.getter = NULL;
desc.setter = NULL;
desc.shortid = 0;
return defineProperty(cx, proxy, id, &desc);
return defineProperty(cx, receiver, id, &desc);
}
bool
@ -862,15 +848,15 @@ proxy_DefineProperty(JSContext *cx, JSObject *obj, jsid id, const Value *value,
}
static JSBool
proxy_GetProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp)
proxy_GetProperty(JSContext *cx, JSObject *obj, JSObject *receiver, jsid id, Value *vp)
{
return JSProxy::get(cx, obj, obj, id, vp);
return JSProxy::get(cx, obj, receiver, id, vp);
}
static JSBool
proxy_SetProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp, JSBool strict)
{
// TODO: throwing away strict
// FIXME (bug 596351): throwing away strict.
return JSProxy::set(cx, obj, obj, id, vp);
}

View File

@ -12032,7 +12032,7 @@ GetPropertyByName(JSContext* cx, JSObject* obj, JSString** namep, Value* vp, PIC
/* Delegate to the op, if present. */
PropertyIdOp op = obj->getOps()->getProperty;
if (op) {
bool result = op(cx, obj, id, vp);
bool result = op(cx, obj, obj, id, vp);
if (!result)
SetBuiltinError(cx);
return cx->tracerState->builtinStatus == 0;
@ -12047,7 +12047,8 @@ GetPropertyByName(JSContext* cx, JSObject* obj, JSString** namep, Value* vp, PIC
const Shape *shape;
JSObject *holder;
if (!js_GetPropertyHelperWithShape(cx, obj, id, JSGET_METHOD_BARRIER, vp, &shape, &holder)) {
if (!js_GetPropertyHelperWithShape(cx, obj, obj, id, JSGET_METHOD_BARRIER, vp, &shape,
&holder)) {
SetBuiltinError(cx);
return false;
}

View File

@ -493,7 +493,7 @@ class TypedArrayTemplate
}
static JSBool
obj_getProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp)
obj_getProperty(JSContext *cx, JSObject *obj, JSObject *receiver, jsid id, Value *vp)
{
ThisTypeArray *tarray = ThisTypeArray::fromJSObject(obj);
JS_ASSERT(tarray);

View File

@ -898,10 +898,12 @@ typedef JSBool
(* DefinePropOp)(JSContext *cx, JSObject *obj, jsid id, const Value *value,
PropertyOp getter, PropertyOp setter, uintN attrs);
typedef JSBool
(* PropertyIdOp)(JSContext *cx, JSObject *obj, jsid id, Value *vp);
(* PropertyIdOp)(JSContext *cx, JSObject *obj, JSObject *receiver, jsid id, Value *vp);
typedef JSBool
(* StrictPropertyIdOp)(JSContext *cx, JSObject *obj, jsid id, Value *vp, JSBool strict);
typedef JSBool
(* DeleteIdOp)(JSContext *cx, JSObject *obj, jsid id, Value *vp, JSBool strict);
typedef JSBool
(* CallOp)(JSContext *cx, uintN argc, Value *vp);
typedef JSBool
(* LookupPropOp)(JSContext *cx, JSObject *obj, jsid id, JSObject **objp,
@ -999,7 +1001,7 @@ struct ObjectOps {
js::StrictPropertyIdOp setProperty;
js::AttributesOp getAttributes;
js::AttributesOp setAttributes;
js::StrictPropertyIdOp deleteProperty;
js::DeleteIdOp deleteProperty;
js::NewEnumerateOp enumerate;
js::TypeOfOp typeOf;
js::TraceOp trace;

View File

@ -204,13 +204,14 @@ JSWrapper::hasOwn(JSContext *cx, JSObject *wrapper, jsid id, bool *bp)
bool
JSWrapper::get(JSContext *cx, JSObject *wrapper, JSObject *receiver, jsid id, Value *vp)
{
GET(JS_GetPropertyById(cx, wrappedObject(wrapper), id, Jsvalify(vp)));
GET(wrappedObject(wrapper)->getProperty(cx, receiver, id, vp));
}
bool
JSWrapper::set(JSContext *cx, JSObject *wrapper, JSObject *receiver, jsid id, Value *vp)
{
SET(JS_SetPropertyById(cx, wrappedObject(wrapper), id, Jsvalify(vp)));
// FIXME (bug 596351): Need deal with strict mode.
SET(wrappedObject(wrapper)->setProperty(cx, id, vp, false));
}
bool

View File

@ -4743,7 +4743,7 @@ xml_defineProperty(JSContext *cx, JSObject *obj, jsid id, const Value *v,
}
static JSBool
xml_getProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp)
xml_getProperty(JSContext *cx, JSObject *obj, JSObject *receiver, jsid id, Value *vp)
{
if (JSID_IS_DEFAULT_XML_NAMESPACE(id)) {
vp->setUndefined();

View File

@ -12,6 +12,7 @@ script scripted-proxies.js
script array-length-protochange.js
script parseInt-octal.js
script proxy-enumerateOwn-duplicates.js
skip-if(!xulRuntime.shell) script proxy-proto-setter.js
skip-if(!xulRuntime.shell) script reflect-parse.js
script destructure-accessor.js
script censor-strict-caller.js

View File

@ -0,0 +1,49 @@
// Any copyright is dedicated to the Public Domain.
// http://creativecommons.org/licenses/publicdomain/
// Contributor: Blake Kaplan
function expect(actual, arg) {
reportCompare(expect.expected, actual, arg);
}
var window = { set x(y) { expect(this, y) }, y: 4 };
expect.expected = window;
window.x = "setting through a setter directly";
window.y = 5;
reportCompare(5, window.y, "setting properties works");
var easy = { easy: 'yes', __proto__: window }
expect.expected = easy;
easy.x = "setting through a setter all-native on prototype";
easy.y = 6;
reportCompare(5, window.y, "window.y remains as it was");
reportCompare(6, easy.y, "shadowing works properly");
var sandbox = evalcx('');
sandbox.window = window;
sandbox.print = print;
sandbox.expect = expect;
var hard = evalcx('Object.create(window)', sandbox);
expect.expected = hard;
hard.x = "a setter through proxy -> native";
hard.y = 6;
reportCompare(5, window.y, "window.y remains as it was through a proxy");
reportCompare(6, hard.y, "shadowing works on proxies");
hard.__proto__ = { 'passed': true }
reportCompare(true, hard.passed, "can set proxy.__proto__ through a native");
var inverse = evalcx('({ set x(y) { expect(this, y); }, y: 4 })', sandbox);
expect.expected = inverse;
inverse.x = "setting through a proxy directly";
inverse.y = 5;
reportCompare(5, inverse.y, "setting properties works on proxies");
var inversehard = Object.create(inverse);
expect.expected = inversehard;
inversehard.x = "setting through a setter with a proxy on the proto chain";
inversehard.y = 6;
reportCompare(5, inverse.y, "inverse.y remains as it was");
reportCompare(6, inversehard.y, "shadowing works native -> proxy");
inversehard.__proto__ = { 'passed': true }
reportCompare(true, inversehard.passed, "can set native.__proto__ through a proxy");

View File

@ -12,7 +12,7 @@ script regress-551763-0.js
script regress-551763-1.js
script regress-551763-2.js
script regress-555246-0.js
fails-if(xulRuntime.shell) script regress-555246-1.js
script regress-555246-1.js
script regress-559438.js
script regress-560101.js
script regress-560998-1.js
@ -48,3 +48,4 @@ script regress-598176.js
script regress-600067.js
script regress-600137.js
script regress-602621.js
fails-if(!xulRuntime.shell) script regress-607863.js

View File

@ -0,0 +1,13 @@
/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/*
* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/licenses/publicdomain/
*/
var sandbox = evalcx('');
var foreign = evalcx('({ get f() this, set x(v) { result = this } })', sandbox);
var local = Object.create(foreign);
reportCompare(local, local.f, "this should be set correctly in getters");
local.x = 42;
reportCompare(local, sandbox.result, "this should be set correctly in setters");