From 5b00e1e303df526e4e30bedfaeb6cf4b3f8473c5 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Fri, 4 Oct 2024 16:47:36 +0200 Subject: [PATCH 1/2] Skip importing same class twice Change logic for duplicate Java imports so that it is not necessarily an error when the same class is imported multiple times, but instead we silently return the same class without re-importing. --- .../mozilla/javascript/ImporterTopLevel.java | 5 +- .../javascript/tests/ImportClassTest.java | 106 ++++++++++++++++++ .../tests/SealedSharedScopeTest.java | 7 +- 3 files changed, 113 insertions(+), 5 deletions(-) create mode 100644 tests/src/test/java/org/mozilla/javascript/tests/ImportClassTest.java diff --git a/rhino/src/main/java/org/mozilla/javascript/ImporterTopLevel.java b/rhino/src/main/java/org/mozilla/javascript/ImporterTopLevel.java index 2a28f116cb..8b64358de8 100644 --- a/rhino/src/main/java/org/mozilla/javascript/ImporterTopLevel.java +++ b/rhino/src/main/java/org/mozilla/javascript/ImporterTopLevel.java @@ -219,7 +219,10 @@ private static void importClass(Scriptable scope, NativeJavaClass cl) { String s = cl.getClassObject().getName(); String n = s.substring(s.lastIndexOf('.') + 1); Object val = scope.get(n, scope); - if (val != NOT_FOUND && val != cl) { + if (val != NOT_FOUND) { + if (val.equals(cl)) { + return; // do not redefine same class + } throw Context.reportRuntimeErrorById("msg.prop.defined", n); } // defineProperty(n, cl, DONTENUM); diff --git a/tests/src/test/java/org/mozilla/javascript/tests/ImportClassTest.java b/tests/src/test/java/org/mozilla/javascript/tests/ImportClassTest.java new file mode 100644 index 0000000000..355d246a7b --- /dev/null +++ b/tests/src/test/java/org/mozilla/javascript/tests/ImportClassTest.java @@ -0,0 +1,106 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +/** */ +package org.mozilla.javascript.tests; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.UUID; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.mozilla.javascript.Context; +import org.mozilla.javascript.ContextFactory; +import org.mozilla.javascript.ImporterTopLevel; +import org.mozilla.javascript.NativeJavaClass; +import org.mozilla.javascript.Script; +import org.mozilla.javascript.Scriptable; +import org.mozilla.javascript.ScriptableObject; +import org.mozilla.javascript.UniqueTag; +import org.mozilla.javascript.drivers.TestUtils; +import org.mozilla.javascript.tools.shell.Global; +import org.mozilla.javascript.tools.shell.ShellContextFactory; + +/** + * @author donnamalayeri + */ +public class ImportClassTest { + + protected final Global global = new Global(); + + public ImportClassTest() { + global.init(contextFactory); + } + + @Before + public void setUp() { + TestUtils.setGlobalContextFactory(contextFactory); + } + + @After + public void tearDown() { + TestUtils.setGlobalContextFactory(null); + } + + private ContextFactory contextFactory = + new ShellContextFactory() { + @Override + protected boolean hasFeature(Context cx, int featureIndex) { + if (featureIndex == Context.FEATURE_ENHANCED_JAVA_ACCESS) return true; + return super.hasFeature(cx, featureIndex); + } + }; + + @Test + public void importPackageAndClass() { + Object result = + runScript( + "importPackage(java.util);\n" + + "UUID.randomUUID();\n" // calls getPkgProperty("UUID", global, + // false) + + "importClass(java.util.UUID);\n" + + "UUID.randomUUID();\n"); // calls getPkgProperty("UUID", + // NativeJavaPackage, true) + assertTrue(Context.jsToJava(result, UUID.class) instanceof UUID); + } + + @Test + public void importInSameContext() { + Object result = runScript("importClass(java.util.UUID);UUID.randomUUID();"); + assertTrue(Context.jsToJava(result, UUID.class) instanceof UUID); + result = runScript("importClass(java.util.UUID);UUID.randomUUID();"); + assertTrue(Context.jsToJava(result, UUID.class) instanceof UUID); + } + + @Test + public void importMultipleTimes() { + Utils.runWithAllOptimizationLevels(cx -> { + ScriptableObject sharedScope = cx.initStandardObjects(); + //sharedScope.sealObject(); code below will try to modify sealed object + + Script script = cx.compileString("importClass(java.util.UUID);true", "TestScript", 1, null); + + for (int i = 0; i < 3; i++) { + Scriptable scope = new ImporterTopLevel(cx); + scope.setParentScope(sharedScope); + script.exec(cx, scope); + assertEquals(UniqueTag.NOT_FOUND, sharedScope.get("UUID", sharedScope)); + assertTrue(scope.get("UUID", scope) instanceof NativeJavaClass); + } + return null; + }); + } + + private Object runScript(final String scriptSourceText) { + + return contextFactory.call( + context -> { + Script script = context.compileString(scriptSourceText, "", 1, null); + Object exec = script.exec(context, global); + return exec; + }); + } +} diff --git a/tests/src/test/java/org/mozilla/javascript/tests/SealedSharedScopeTest.java b/tests/src/test/java/org/mozilla/javascript/tests/SealedSharedScopeTest.java index 7888e35c06..c334487638 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/SealedSharedScopeTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/SealedSharedScopeTest.java @@ -21,6 +21,7 @@ import org.mozilla.javascript.EvaluatorException; import org.mozilla.javascript.IdFunctionObject; import org.mozilla.javascript.ImporterTopLevel; +import org.mozilla.javascript.NativeObject; import org.mozilla.javascript.Scriptable; import org.mozilla.javascript.Wrapper; @@ -41,12 +42,10 @@ public void setUp() throws Exception { ctx = Context.enter(); ctx.setLanguageVersion(Context.VERSION_DEFAULT); - scope1 = ctx.newObject(sharedScope); + scope1 = new NativeObject(); scope1.setPrototype(sharedScope); - scope1.setParentScope(null); - scope2 = ctx.newObject(sharedScope); + scope2 = new NativeObject(); scope2.setPrototype(sharedScope); - scope2.setParentScope(null); } @After From 931c2b6fe777f876387bf6d1998682812d7e406f Mon Sep 17 00:00:00 2001 From: Greg Brail Date: Mon, 23 Dec 2024 11:59:35 -0800 Subject: [PATCH 2/2] Fix up shared scope usage to match how it's documented. --- .../javascript/tests/ImportClassTest.java | 34 +++++++++++-------- .../tests/SealedSharedScopeTest.java | 7 ++-- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/tests/src/test/java/org/mozilla/javascript/tests/ImportClassTest.java b/tests/src/test/java/org/mozilla/javascript/tests/ImportClassTest.java index 355d246a7b..745bd92627 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/ImportClassTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/ImportClassTest.java @@ -77,21 +77,25 @@ public void importInSameContext() { @Test public void importMultipleTimes() { - Utils.runWithAllOptimizationLevels(cx -> { - ScriptableObject sharedScope = cx.initStandardObjects(); - //sharedScope.sealObject(); code below will try to modify sealed object - - Script script = cx.compileString("importClass(java.util.UUID);true", "TestScript", 1, null); - - for (int i = 0; i < 3; i++) { - Scriptable scope = new ImporterTopLevel(cx); - scope.setParentScope(sharedScope); - script.exec(cx, scope); - assertEquals(UniqueTag.NOT_FOUND, sharedScope.get("UUID", sharedScope)); - assertTrue(scope.get("UUID", scope) instanceof NativeJavaClass); - } - return null; - }); + Utils.runWithAllModes( + cx -> { + ScriptableObject sharedScope = cx.initStandardObjects(); + // sharedScope.sealObject(); // code below will try to modify sealed object + + Script script = + cx.compileString( + "importClass(java.util.UUID);true", "TestScript", 1, null); + + for (int i = 0; i < 3; i++) { + Scriptable scope = new ImporterTopLevel(cx); + scope.setPrototype(sharedScope); + scope.setParentScope(null); + script.exec(cx, scope); + assertEquals(UniqueTag.NOT_FOUND, sharedScope.get("UUID", sharedScope)); + assertTrue(scope.get("UUID", scope) instanceof NativeJavaClass); + } + return null; + }); } private Object runScript(final String scriptSourceText) { diff --git a/tests/src/test/java/org/mozilla/javascript/tests/SealedSharedScopeTest.java b/tests/src/test/java/org/mozilla/javascript/tests/SealedSharedScopeTest.java index c334487638..7888e35c06 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/SealedSharedScopeTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/SealedSharedScopeTest.java @@ -21,7 +21,6 @@ import org.mozilla.javascript.EvaluatorException; import org.mozilla.javascript.IdFunctionObject; import org.mozilla.javascript.ImporterTopLevel; -import org.mozilla.javascript.NativeObject; import org.mozilla.javascript.Scriptable; import org.mozilla.javascript.Wrapper; @@ -42,10 +41,12 @@ public void setUp() throws Exception { ctx = Context.enter(); ctx.setLanguageVersion(Context.VERSION_DEFAULT); - scope1 = new NativeObject(); + scope1 = ctx.newObject(sharedScope); scope1.setPrototype(sharedScope); - scope2 = new NativeObject(); + scope1.setParentScope(null); + scope2 = ctx.newObject(sharedScope); scope2.setPrototype(sharedScope); + scope2.setParentScope(null); } @After