Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix scope for defineOwnProperties #1226

Closed
wants to merge 1 commit into from
Closed

fix scope for defineOwnProperties #1226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions src/org/mozilla/javascript/AbstractEcmaObjectOperations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,21 @@ static boolean hasOwnProperty(Context cx, Object o, Object property) {
* Implementation of Abstract Object operation testIntegrityLevel as defined by EcmaScript
*
* @param cx
* @param scope the current scope.
* @param o
* @param level
* @return boolean
* @see <a
* href="https://262.ecma-international.org/11.0/#sec-testintegritylevel">TestIntegrityLevel</a>
*/
static boolean testIntegrityLevel(Context cx, Object o, INTEGRITY_LEVEL level) {
static boolean testIntegrityLevel(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm, the idea of all the methods in this class is that they implement the abstract object operations as defined by EcmaScript (with the addition of Context as first param): testIntegretyLevel in the EcmaScript specification doesn't have a scope parameter

Copy link
Collaborator Author

@rbri rbri May 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my point of view scope is like context, we need to add this to the params of many methods.

Copy link
Collaborator

@p-bakker p-bakker May 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the (top level) scope should be derivable from the ScriptableObject.

I think the issue here is that the prototype of the function is a NativeObject instance on which the parentScope isn;t set accordingly, hence it ends up being null in the ScriptableObject.getOwnPropertyDescriptor(...) method.

Adding obj.setParentScope(this); after the construction of the NativeObject instance in BaseFunction.setupDefaultPrototype() solves that. Then in ScriptableObject.getOwnPropertyDescriptor(...) the getParentSCope call starts working properly, by always returning a proper scope and the problem guess away

Did a quick test with this approach and it seems to solve the problem

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable, will have a look and maybe prepare another PR,

Context cx, Scriptable scope, Object o, INTEGRITY_LEVEL level) {
ScriptableObject obj = ScriptableObject.ensureScriptableObject(o);

if (obj.isExtensible()) return false;

for (Object name : obj.getIds(true, true)) {
ScriptableObject desc = obj.getOwnPropertyDescriptor(cx, name);
ScriptableObject desc = obj.getOwnPropertyDescriptor(cx, scope, name);
if (Boolean.TRUE.equals(desc.get("configurable"))) return false;

if (level == INTEGRITY_LEVEL.FROZEN
Expand All @@ -81,13 +83,15 @@ static boolean testIntegrityLevel(Context cx, Object o, INTEGRITY_LEVEL level) {
* Implementation of Abstract Object operation setIntegrityLevel as defined by EcmaScript
*
* @param cx
* @param scope the current scope.
* @param o
* @param level
* @return boolean
* @see <a
* href="https://262.ecma-international.org/11.0/#sec-setintegritylevel">SetIntegrityLevel</a>
*/
static boolean setIntegrityLevel(Context cx, Object o, INTEGRITY_LEVEL level) {
static boolean setIntegrityLevel(
Context cx, Scriptable scope, Object o, INTEGRITY_LEVEL level) {
/*
1. Assert: Type(O) is Object.
2. Assert: level is either sealed or frozen.
Expand Down Expand Up @@ -128,13 +132,13 @@ static boolean setIntegrityLevel(Context cx, Object o, INTEGRITY_LEVEL level) {
obj.preventExtensions();

for (Object key : obj.getIds(true, true)) {
ScriptableObject desc = obj.getOwnPropertyDescriptor(cx, key);
ScriptableObject desc = obj.getOwnPropertyDescriptor(cx, scope, key);

if (level == INTEGRITY_LEVEL.SEALED) {
if (Boolean.TRUE.equals(desc.get("configurable"))) {
desc.put("configurable", desc, Boolean.FALSE);

obj.defineOwnProperty(cx, key, desc, false);
obj.defineOwnProperty(cx, scope, key, desc, false);
}
} else {
if (ScriptableObject.isDataDescriptor(desc)
Expand All @@ -144,7 +148,7 @@ static boolean setIntegrityLevel(Context cx, Object o, INTEGRITY_LEVEL level) {
if (Boolean.TRUE.equals(desc.get("configurable"))) {
desc.put("configurable", desc, Boolean.FALSE);
}
obj.defineOwnProperty(cx, key, desc, false);
obj.defineOwnProperty(cx, scope, key, desc, false);
}
}

Expand Down
16 changes: 7 additions & 9 deletions src/org/mozilla/javascript/Arguments.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,37 +322,35 @@ Object[] getIds(boolean getNonEnumerable, boolean getSymbols) {
}

@Override
protected ScriptableObject getOwnPropertyDescriptor(Context cx, Object id) {
protected ScriptableObject getOwnPropertyDescriptor(Context cx, Scriptable scope, Object id) {
if (ScriptRuntime.isSymbol(id) || id instanceof Scriptable) {
return super.getOwnPropertyDescriptor(cx, id);
return super.getOwnPropertyDescriptor(cx, scope, id);
}

double d = ScriptRuntime.toNumber(id);
int index = (int) d;
if (d != index) {
return super.getOwnPropertyDescriptor(cx, id);
return super.getOwnPropertyDescriptor(cx, scope, id);
}
Object value = arg(index);
if (value == NOT_FOUND) {
return super.getOwnPropertyDescriptor(cx, id);
return super.getOwnPropertyDescriptor(cx, scope, id);
}
if (sharedWithActivation(index)) {
value = getFromActivation(index);
}
if (super.has(index, this)) { // the descriptor has been redefined
ScriptableObject desc = super.getOwnPropertyDescriptor(cx, id);
ScriptableObject desc = super.getOwnPropertyDescriptor(cx, scope, id);
desc.put("value", desc, value);
return desc;
}
Scriptable scope = getParentScope();
if (scope == null) scope = this;
return buildDataDescriptor(scope, value, EMPTY);
}

@Override
protected void defineOwnProperty(
Context cx, Object id, ScriptableObject desc, boolean checkValid) {
super.defineOwnProperty(cx, id, desc, checkValid);
Context cx, Scriptable scope, Object id, ScriptableObject desc, boolean checkValid) {
super.defineOwnProperty(cx, scope, id, desc, checkValid);
if (ScriptRuntime.isSymbol(id)) {
return;
}
Expand Down
4 changes: 2 additions & 2 deletions src/org/mozilla/javascript/ArrowFunction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ public ArrowFunction(
throwing.put("configurable", throwing, Boolean.FALSE);
throwing.preventExtensions();

this.defineOwnProperty(cx, "caller", throwing, false);
this.defineOwnProperty(cx, "arguments", throwing, false);
this.defineOwnProperty(cx, scope, "caller", throwing, false);
this.defineOwnProperty(cx, scope, "arguments", throwing, false);
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions src/org/mozilla/javascript/BoundFunction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ public BoundFunction(
throwing.put("configurable", throwing, Boolean.FALSE);
throwing.preventExtensions();

this.defineOwnProperty(cx, "caller", throwing, false);
this.defineOwnProperty(cx, "arguments", throwing, false);
this.defineOwnProperty(cx, scope, "caller", throwing, false);
this.defineOwnProperty(cx, scope, "arguments", throwing, false);
}

@Override
Expand Down
12 changes: 6 additions & 6 deletions src/org/mozilla/javascript/IdScriptableObject.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,7 @@ private IdFunctionObject newIdFunction(

@Override
protected void defineOwnProperty(
Context cx, Object key, ScriptableObject desc, boolean checkValid) {
Context cx, Scriptable scope, Object key, ScriptableObject desc, boolean checkValid) {
if (key instanceof String) {
String name = (String) key;
int info = findInstanceIdInfo(name);
Expand All @@ -863,7 +863,7 @@ protected void defineOwnProperty(
delete(id); // it will be replaced with a slot
} else {
checkPropertyDefinition(desc);
ScriptableObject current = getOwnPropertyDescriptor(cx, key);
ScriptableObject current = getOwnPropertyDescriptor(cx, scope, key);
checkPropertyChange(name, current, desc);
int attr = (info >>> 16);
Object value = getProperty(desc, "value");
Expand All @@ -884,7 +884,7 @@ protected void defineOwnProperty(
prototypeValues.delete(id); // it will be replaced with a slot
} else {
checkPropertyDefinition(desc);
ScriptableObject current = getOwnPropertyDescriptor(cx, key);
ScriptableObject current = getOwnPropertyDescriptor(cx, scope, key);
checkPropertyChange(name, current, desc);
int attr = prototypeValues.getAttributes(id);
Object value = getProperty(desc, "value");
Expand All @@ -909,12 +909,12 @@ protected void defineOwnProperty(
}
}
}
super.defineOwnProperty(cx, key, desc, checkValid);
super.defineOwnProperty(cx, scope, key, desc, checkValid);
}

@Override
protected ScriptableObject getOwnPropertyDescriptor(Context cx, Object id) {
ScriptableObject desc = super.getOwnPropertyDescriptor(cx, id);
protected ScriptableObject getOwnPropertyDescriptor(Context cx, Scriptable scope, Object id) {
ScriptableObject desc = super.getOwnPropertyDescriptor(cx, scope, id);
if (desc == null) {
if (id instanceof String) {
desc = getBuiltInDescriptor((String) id);
Expand Down
8 changes: 4 additions & 4 deletions src/org/mozilla/javascript/NativeArray.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,20 +668,20 @@ public int getAttributes(int index) {
}

@Override
protected ScriptableObject getOwnPropertyDescriptor(Context cx, Object id) {
protected ScriptableObject getOwnPropertyDescriptor(Context cx, Scriptable scope, Object id) {
if (dense != null) {
int index = toDenseIndex(id);
if (0 <= index && index < dense.length && dense[index] != NOT_FOUND) {
Object value = dense[index];
return defaultIndexPropertyDescriptor(value);
}
}
return super.getOwnPropertyDescriptor(cx, id);
return super.getOwnPropertyDescriptor(cx, scope, id);
}

@Override
protected void defineOwnProperty(
Context cx, Object id, ScriptableObject desc, boolean checkValid) {
Context cx, Scriptable scope, Object id, ScriptableObject desc, boolean checkValid) {
long index = toArrayIndex(id);
if (index >= length) {
length = index + 1;
Expand All @@ -705,7 +705,7 @@ protected void defineOwnProperty(
}
}

super.defineOwnProperty(cx, id, desc, checkValid);
super.defineOwnProperty(cx, scope, id, desc, checkValid);

if (id instanceof String && ((String) id).equals("length")) {
lengthAttr =
Expand Down
2 changes: 1 addition & 1 deletion src/org/mozilla/javascript/NativeMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ static void init(Context cx, Scriptable scope, boolean sealed) {
desc.put("enumerable", desc, Boolean.FALSE);
desc.put("configurable", desc, Boolean.TRUE);
desc.put("get", desc, obj.get(NativeSet.GETSIZE, obj));
obj.defineOwnProperty(cx, "size", desc);
obj.defineOwnProperty(cx, scope, "size", desc);

if (sealed) {
obj.sealObject();
Expand Down
18 changes: 9 additions & 9 deletions src/org/mozilla/javascript/NativeObject.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ public Object execIdCall(
Scriptable s = getCompatibleObject(cx, scope, arg);
ScriptableObject obj = ensureScriptableObject(s);
Object nameArg = args.length < 2 ? Undefined.instance : args[1];
Scriptable desc = obj.getOwnPropertyDescriptor(cx, nameArg);
Scriptable desc = obj.getOwnPropertyDescriptor(cx, scope, nameArg);
return desc == null ? Undefined.instance : desc;
}
case ConstructorId_getOwnPropertyDescriptors:
Expand All @@ -521,7 +521,7 @@ public Object execIdCall(

ScriptableObject descs = (ScriptableObject) cx.newObject(scope);
for (Object key : obj.getIds(true, true)) {
Scriptable desc = obj.getOwnPropertyDescriptor(cx, key);
Scriptable desc = obj.getOwnPropertyDescriptor(cx, scope, key);
if (desc == null) {
continue;
} else if (key instanceof Symbol) {
Expand All @@ -541,7 +541,7 @@ public Object execIdCall(
Object name = args.length < 2 ? Undefined.instance : args[1];
Object descArg = args.length < 3 ? Undefined.instance : args[2];
ScriptableObject desc = ensureScriptableObject(descArg);
obj.defineOwnProperty(cx, name, desc);
obj.defineOwnProperty(cx, scope, name, desc);
return obj;
}
case ConstructorId_isExtensible:
Expand Down Expand Up @@ -573,7 +573,7 @@ public Object execIdCall(
ScriptableObject obj = ensureScriptableObject(arg);
Object propsObj = args.length < 2 ? Undefined.instance : args[1];
Scriptable props = Context.toObject(propsObj, scope);
obj.defineOwnProperties(cx, ensureScriptableObject(props));
obj.defineOwnProperties(cx, scope, ensureScriptableObject(props));
return obj;
}
case ConstructorId_create:
Expand All @@ -587,7 +587,7 @@ public Object execIdCall(

if (args.length > 1 && !Undefined.isUndefined(args[1])) {
Scriptable props = Context.toObject(args[1], scope);
newObject.defineOwnProperties(cx, ensureScriptableObject(props));
newObject.defineOwnProperties(cx, scope, ensureScriptableObject(props));
}

return newObject;
Expand All @@ -601,7 +601,7 @@ public Object execIdCall(
}

return AbstractEcmaObjectOperations.testIntegrityLevel(
cx, arg, AbstractEcmaObjectOperations.INTEGRITY_LEVEL.SEALED);
cx, scope, arg, AbstractEcmaObjectOperations.INTEGRITY_LEVEL.SEALED);
}
case ConstructorId_isFrozen:
{
Expand All @@ -612,7 +612,7 @@ public Object execIdCall(
}

return AbstractEcmaObjectOperations.testIntegrityLevel(
cx, arg, AbstractEcmaObjectOperations.INTEGRITY_LEVEL.FROZEN);
cx, scope, arg, AbstractEcmaObjectOperations.INTEGRITY_LEVEL.FROZEN);
}
case ConstructorId_seal:
{
Expand All @@ -623,7 +623,7 @@ public Object execIdCall(
}

AbstractEcmaObjectOperations.setIntegrityLevel(
cx, arg, AbstractEcmaObjectOperations.INTEGRITY_LEVEL.SEALED);
cx, scope, arg, AbstractEcmaObjectOperations.INTEGRITY_LEVEL.SEALED);

return arg;
}
Expand All @@ -636,7 +636,7 @@ public Object execIdCall(
}

AbstractEcmaObjectOperations.setIntegrityLevel(
cx, arg, AbstractEcmaObjectOperations.INTEGRITY_LEVEL.FROZEN);
cx, scope, arg, AbstractEcmaObjectOperations.INTEGRITY_LEVEL.FROZEN);

return arg;
}
Expand Down
2 changes: 1 addition & 1 deletion src/org/mozilla/javascript/NativePromise.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public static void init(Context cx, Scriptable scope, boolean sealed) {
0,
(Context lcx, Scriptable lscope, Scriptable thisObj, Object[] args) ->
constructor));
constructor.defineOwnProperty(cx, SymbolKey.SPECIES, speciesDescriptor, false);
constructor.defineOwnProperty(cx, scope, SymbolKey.SPECIES, speciesDescriptor, false);

constructor.definePrototypeMethod(
scope,
Expand Down
2 changes: 1 addition & 1 deletion src/org/mozilla/javascript/NativeSet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ static void init(Context cx, Scriptable scope, boolean sealed) {
desc.put("enumerable", desc, Boolean.FALSE);
desc.put("configurable", desc, Boolean.TRUE);
desc.put("get", desc, obj.get(GETSIZE, obj));
obj.defineOwnProperty(cx, "size", desc);
obj.defineOwnProperty(cx, scope, "size", desc);

if (sealed) {
obj.sealObject();
Expand Down
4 changes: 2 additions & 2 deletions src/org/mozilla/javascript/NativeString.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,7 @@ protected Object[] getIds(boolean nonEnumerable, boolean getSymbols) {
}

@Override
protected ScriptableObject getOwnPropertyDescriptor(Context cx, Object id) {
protected ScriptableObject getOwnPropertyDescriptor(Context cx, Scriptable scope, Object id) {
if (!(id instanceof Symbol)
&& (cx != null)
&& (cx.getLanguageVersion() >= Context.VERSION_ES6)) {
Expand All @@ -847,7 +847,7 @@ protected ScriptableObject getOwnPropertyDescriptor(Context cx, Object id) {
return defaultIndexPropertyDescriptor(value);
}
}
return super.getOwnPropertyDescriptor(cx, id);
return super.getOwnPropertyDescriptor(cx, scope, id);
}

private ScriptableObject defaultIndexPropertyDescriptor(Object value) {
Expand Down
4 changes: 2 additions & 2 deletions src/org/mozilla/javascript/ScriptRuntime.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4739,9 +4739,9 @@ public static Scriptable getTemplateLiteralCallSite(
}

AbstractEcmaObjectOperations.setIntegrityLevel(
cx, rawObj, AbstractEcmaObjectOperations.INTEGRITY_LEVEL.FROZEN);
cx, scope, rawObj, AbstractEcmaObjectOperations.INTEGRITY_LEVEL.FROZEN);
AbstractEcmaObjectOperations.setIntegrityLevel(
cx, siteObj, AbstractEcmaObjectOperations.INTEGRITY_LEVEL.FROZEN);
cx, scope, siteObj, AbstractEcmaObjectOperations.INTEGRITY_LEVEL.FROZEN);

strings[index] = siteObj;

Expand Down
Loading