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.

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

Should ScriptableObject.sealObject also prevent modifications of parent and prototype #1085

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
6 changes: 1 addition & 5 deletions src/org/mozilla/javascript/ScriptRuntime.java
Original file line number Diff line number Diff line change
Expand Up @@ -146,15 +146,11 @@ public static ScriptableObject initSafeStandardObjects(
scope.associateValue(LIBRARY_SCOPE_KEY, scope);
(new ClassCache()).associate(scope);

BaseFunction.init(scope, sealed);
NativeObject.init(scope, sealed);
BaseFunction.init(scope, sealed);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When these two were swapped, the setPrototype some lines below is not needed


Scriptable objectProto = ScriptableObject.getObjectPrototype(scope);

// Function.prototype.__proto__ should be Object.prototype
Scriptable functionProto = ScriptableObject.getClassPrototype(scope, "Function");
functionProto.setPrototype(objectProto);

// Set the prototype of the object passed in if need be
if (scope.getPrototype() == null) scope.setPrototype(objectProto);

Expand Down
2 changes: 2 additions & 0 deletions src/org/mozilla/javascript/ScriptableObject.java
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,7 @@ public Scriptable getPrototype() {
/** Sets the prototype of the object. */
@Override
public void setPrototype(Scriptable m) {
checkNotSealed("__proto__", 0);
prototypeObject = m;
}

Expand All @@ -692,6 +693,7 @@ public Scriptable getParentScope() {
/** Sets the parent (enclosing) scope of the object. */
@Override
public void setParentScope(Scriptable m) {
checkNotSealed("__parent__", 0);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would prevent modification of SpecialRefs (TODO: Should I make 2 constants SpecialRef.PARENT / SpecialRef.PROTO)?

parentScopeObject = m;
}

Expand Down
100 changes: 78 additions & 22 deletions testsrc/org/mozilla/javascript/tests/SealedSharedScopeTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import static org.junit.Assert.fail;

import java.util.Locale;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
Expand All @@ -21,6 +20,7 @@
import org.mozilla.javascript.IdFunctionObject;
import org.mozilla.javascript.ImporterTopLevel;
import org.mozilla.javascript.Scriptable;
import org.mozilla.javascript.ScriptableObject;
import org.mozilla.javascript.Wrapper;

@RunWith(BlockJUnit4ClassRunner.class)
Expand All @@ -35,6 +35,20 @@ public class SealedSharedScopeTest {
public void setUp() throws Exception {
Context tmpCtx = Context.enter();
sharedScope = new ImporterTopLevel(tmpCtx, true);

tmpCtx.evaluateString(
sharedScope,
"jsObj = {'bar': 42};\n"
// Some tests...
// + "Object.defineProperties(jsObj, { baz : { writable: true, value: 'aaa' }});\n"
// + "Object.seal(jsObj);"
,
"init",
1,
null);
// Note: Object.seal != ScriptableObject.sealObject
((ScriptableObject) sharedScope.get("jsObj", sharedScope)).sealObject();

sharedScope.sealObject();
Context.exit();

Expand All @@ -61,17 +75,14 @@ private Object evaluateString(Scriptable scope, String source) {
}

/**
* Test should verify if JavaImporter can import
* java.util.Date/java.sql.Date without colliding with internal Date
* function.
* Test should verify if JavaImporter can import java.util.Date/java.sql.Date without colliding
* with internal Date function.
*/
@Test
public void importClassWithImporter() throws Exception {
Object o;
evaluateString(scope1, "var imp1 = new JavaImporter();\n"
+ "imp1.importClass(java.util.Date);");
evaluateString(scope1, "var imp2 = new JavaImporter();\n"
+ "imp2.importClass(java.sql.Date);");
evaluateString(scope1, "var imp1 = new JavaImporter();\nimp1.importClass(java.util.Date);");
evaluateString(scope1, "var imp2 = new JavaImporter();\nimp2.importClass(java.sql.Date);");
o = evaluateString(scope1, "imp1.Date");
assertEquals(java.util.Date.class, o);

Expand All @@ -82,22 +93,19 @@ public void importClassWithImporter() throws Exception {
assertEquals(IdFunctionObject.class, o.getClass());

o = evaluateString(scope2, "typeof imp1"); // scope 2 has
// no imp1
// no imp1
assertEquals("undefined", o);
}

/**
* Test should verify if JavaImporter can import
* java.util.Date/java.sql.Date without colliding with internal Date
* function.
* Test should verify if JavaImporter can import java.util.Date/java.sql.Date without colliding
* with internal Date function.
*/
@Test
public void importPackageWithImporter() throws Exception {
Object o;
evaluateString(scope1, "var imp1 = new JavaImporter();\n"
+ "imp1.importPackage(java.util);");
evaluateString(scope1, "var imp2 = new JavaImporter();\n"
+ "imp2.importPackage(java.sql);");
evaluateString(scope1, "var imp1 = new JavaImporter();\nimp1.importPackage(java.util);");
evaluateString(scope1, "var imp2 = new JavaImporter();\nimp2.importPackage(java.sql);");
o = evaluateString(scope1, "imp1.Date");
assertEquals(java.util.Date.class, o);

Expand All @@ -108,7 +116,7 @@ public void importPackageWithImporter() throws Exception {
assertEquals(IdFunctionObject.class, o.getClass());

o = evaluateString(scope2, "typeof imp1 == 'undefined'"); // scope 2 has
// no imp1
// no imp1
assertTrue((Boolean) o);
}

Expand All @@ -123,8 +131,7 @@ public void importClassWithScope() throws Exception {
o = evaluateString(scope2, "Name");
assertEquals(javax.xml.soap.Name.class, o);

o = evaluateString(sharedScope, "typeof Name"); // JavaScript "Statement"
// function
o = evaluateString(sharedScope, "typeof Name"); // JavaScript "Statement" function
assertEquals("undefined", o);
}

Expand All @@ -139,8 +146,7 @@ public void importPackageWithScope() throws Exception {
o = evaluateString(scope2, "Name");
assertEquals(javax.xml.soap.Name.class, o);

o = evaluateString(sharedScope, "typeof Name"); // JavaScript "Statement"
// function
o = evaluateString(sharedScope, "typeof Name"); // JavaScript "Statement" function
assertEquals("undefined", o);
}

Expand All @@ -158,9 +164,59 @@ public void importClassSucceedsOnScope() throws Exception {
evaluateString(scope2, "Locale.getDefault()");
fail("EcmaError expected");
} catch (EcmaError e) {
assertEquals("ReferenceError: \"Locale\" is not defined. (test#1)",
assertEquals("ReferenceError: \"Locale\" is not defined. (test#1)", e.getMessage());
}
}

@Test
public void testSealedJsModifyProp() {
String s = evaluateString(scope1, "jsObj").toString();
assertEquals("[object Object]", s);

try {
evaluateString(scope1, "'use strict';jsObj.bar = 3");
fail("EvaluatorException expected");
} catch (EvaluatorException e) {
assertEquals(
"Cannot modify a property of a sealed object: bar. (test#1)", e.getMessage());
}
}

@Test
public void testSealedJsAddProp() {
String s = evaluateString(scope1, "jsObj").toString();
assertEquals("[object Object]", s);

try {
evaluateString(scope1, "'use strict';jsObj.foo = 3");
fail("EvaluatorException expected");
} catch (EvaluatorException e) {
assertEquals(
"Cannot modify a property of a sealed object: foo. (test#1)", e.getMessage());
}
}

@Test
public void testSealedJsModifyProto() {
try {
evaluateString(scope1, "jsObj.__proto__ = {}");
fail("EvaluatorException expected");
} catch (EvaluatorException e) {
assertEquals(
"Cannot modify a property of a sealed object: __proto__. (test#1)",
e.getMessage());
}
}

@Test
public void testSealedJsModifyParent() {
try {
evaluateString(scope1, "jsObj.__parent__ = {}");
fail("EvaluatorException expected");
} catch (EvaluatorException e) {
assertEquals(
"Cannot modify a property of a sealed object: __parent__. (test#1)",
e.getMessage());
}
}
}