From 1d04e2dd7c7c8fbb18548a32578298ec1195aec0 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Mon, 3 Feb 2025 10:56:03 +0100 Subject: [PATCH 1/5] Add RhinoConfig parser/loader Spotbugs --- rhino/src/main/java/module-info.java | 2 + .../javascript/config/RhinoConfig.java | 101 +++++++++++ .../javascript/config/RhinoProperties.java | 162 ++++++++++++++++++ .../config/RhinoPropertiesLoader.java | 13 ++ 4 files changed, 278 insertions(+) create mode 100644 rhino/src/main/java/org/mozilla/javascript/config/RhinoConfig.java create mode 100644 rhino/src/main/java/org/mozilla/javascript/config/RhinoProperties.java create mode 100644 rhino/src/main/java/org/mozilla/javascript/config/RhinoPropertiesLoader.java diff --git a/rhino/src/main/java/module-info.java b/rhino/src/main/java/module-info.java index 32b36988d2..a855737fdc 100644 --- a/rhino/src/main/java/module-info.java +++ b/rhino/src/main/java/module-info.java @@ -12,9 +12,11 @@ exports org.mozilla.javascript.serialize; exports org.mozilla.javascript.typedarrays; exports org.mozilla.javascript.xml; + exports org.mozilla.javascript.config; uses org.mozilla.javascript.RegExpProxy; uses org.mozilla.javascript.xml.XMLLoader; + uses org.mozilla.javascript.config.RhinoPropertiesLoader; provides org.mozilla.javascript.RegExpProxy with org.mozilla.javascript.regexp.RegExpImpl; diff --git a/rhino/src/main/java/org/mozilla/javascript/config/RhinoConfig.java b/rhino/src/main/java/org/mozilla/javascript/config/RhinoConfig.java new file mode 100644 index 0000000000..52d8f2b972 --- /dev/null +++ b/rhino/src/main/java/org/mozilla/javascript/config/RhinoConfig.java @@ -0,0 +1,101 @@ +package org.mozilla.javascript.config; + +import java.security.AccessController; +import java.security.PrivilegedAction; +import java.util.Objects; + +/** + * RhinoConfig provides typesafe and static access methods to a {@link RhinoProperties} default + * instance. + * + * @author Roland Praml, Foconis Analytics GmbH + */ +public class RhinoConfig { + + private static final RhinoProperties INSTANCE = + // We still assume, that a security manager could be in place! + AccessController.doPrivileged( + (PrivilegedAction) RhinoProperties::init); + + /** + * Returns the property as string. + * + *

If the value is not set, defaultVaule is returned. + */ + private static String get(String property, String defaultValue) { + Object ret = INSTANCE.get(property); + if (ret != null) { + return ret.toString(); + } + return defaultValue; + } + + /** Returns the property as string with null as default. */ + public static String get(String property) { + return get(property, (String) null); + } + + /** + * Returns the property as enum. + * + *

If the property is set to any of the enum names (case-insensitive), this enum value is + * returned, otherwise defaultValue is returned. + * + *

Note: defaultValue must be specified to derive the enum class and its values. + */ + @SuppressWarnings("unchecked") + public static > T get(String property, T defaultValue) { + Objects.requireNonNull(defaultValue, "defaultValue must not be null"); + Object ret = INSTANCE.get(property); + if (ret != null) { + Class enumType = (Class) defaultValue.getDeclaringClass(); + // We make a case insentive lookup here. + for (T enm : enumType.getEnumConstants()) { + if (enm.name().equalsIgnoreCase(ret.toString())) { + return enm; + } + } + } + return defaultValue; + } + + /** + * Returns the property as boolean. + * + *

A property is true, if it is either Boolean.TRUE or if and only if the string + * representation is equal to {@code "true"} (case-insensitive). If the property is not set, + * defaultValue is returned + * + *

Same behaviour as {@link Boolean#getBoolean(String)} + */ + public static boolean get(String property, boolean defaultValue) { + Object ret = INSTANCE.get(property); + if (ret instanceof Boolean) { + return (Boolean) ret; + } else if (ret == null) { + return defaultValue; + } + + return Boolean.parseBoolean(ret.toString()); + } + + /** + * Returns the property as integer. + * + *

if the property is not set or not a valid integer value, defaultValue is + * returned. + */ + public static int get(String property, int defaultValue) { + Object ret = INSTANCE.get(property); + if (ret instanceof Number) { + return ((Number) ret).intValue(); + } else if (ret != null) { + try { + return Integer.decode(ret.toString()); + } catch (NumberFormatException e) { + // ignore invalid values. See Integer.getInteger + } + } + return defaultValue; + } +} diff --git a/rhino/src/main/java/org/mozilla/javascript/config/RhinoProperties.java b/rhino/src/main/java/org/mozilla/javascript/config/RhinoProperties.java new file mode 100644 index 0000000000..c399845b4e --- /dev/null +++ b/rhino/src/main/java/org/mozilla/javascript/config/RhinoProperties.java @@ -0,0 +1,162 @@ +package org.mozilla.javascript.config; + +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Properties; +import java.util.ServiceLoader; + +/** + * Utility class to read current rhino configuration from various properties. + * + *

Rhino properties typically begins with "rhino." (properties) or "RHINO_" (env) + * + *

You can override this behaviour and implement a {@link RhinoPropertiesLoader} and register it + * as service. If no loader was found, the configuration is read from these locations by default: + * + *

    + *
  1. rhino.config file from current class' classpath + *
  2. rhino.config file from current threas's classpath + *
  3. rhino.config file from current directory + *
  4. rhino-test.config file from current class' classpath + *
  5. rhino-test.config file from current threas's classpath + *
  6. rhino-test.config file from current directory + *
  7. env variables starting with "RHINO_" (underscores are replaced by '.' and string is + *
  8. System-properties starting with "rhino." + *
+ * + *

(the later config can override previous ones) + * + *

The config files are in UTF-8 format and all keys in this configuration are case-insensitive + * and dot/underscore-insensitive. + * + *

This means, "rhino.use_java_policy_security=true" is equvalent to + * "RHINO_USE_JAVA_POLICY_SECURITY=TRUE" + * + * @author Roland Praml, Foconis Analytics GmbH + */ +public class RhinoProperties { + + private static String[] CONFIG_FILES = {"rhino.config", "rhino-test.config"}; + private List> configs = new ArrayList<>(); + + /** + * Initializes the default, used in RhinoConfig. If there is RhinoPropertiesLoader present, then + * this loader has full control. Ohterwise, properties are loaded from default locations. + */ + static RhinoProperties init() { + RhinoProperties props = new RhinoProperties(); + Iterator loader = + ServiceLoader.load(RhinoPropertiesLoader.class).iterator(); + if (loader.hasNext()) { + while (loader.hasNext()) { + loader.next().load(props); + } + } else { + props.loadDefaults(); + } + return props; + } + + /** Load properties from the default locations. */ + public void loadDefaults() { + ClassLoader classLoader = RhinoProperties.class.getClassLoader(); + ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); + + for (String configFile : CONFIG_FILES) { + loadFromClasspath(classLoader, configFile); + loadFromClasspath(contextClassLoader, configFile); + loadFromFile(new File(configFile)); + } + + addConfig(System.getenv()); + addConfig(System.getProperties()); + } + + /** Loads the configuration from the given file. */ + public void loadFromFile(File config) { + if (!config.exists()) { + return; + } + try (InputStream in = new FileInputStream(config)) { + Properties props = new Properties(); + props.load(new InputStreamReader(in, StandardCharsets.UTF_8)); + addConfig(props); + } catch (IOException e) { + System.err.println( + "Rhino: Error loading configuration from " + + config.getAbsolutePath() + + ": " + + e.getMessage()); + } + } + + /** Loads the configuration from classpath. */ + public void loadFromClasspath(ClassLoader cl, String location) { + if (cl != null) { + loadFromResource(cl.getResource(location)); + } + } + + /** Loads the configuration from the given resource. */ + public void loadFromResource(URL resource) { + if (resource == null) { + return; + } + try (InputStream in = resource.openStream()) { + Properties props = new Properties(); + props.load(new InputStreamReader(in, StandardCharsets.UTF_8)); + addConfig(props); + } catch (IOException e) { + System.err.println( + "Rhino: Error loading configuration from " + resource + ": " + e.getMessage()); + } + } + + /** Adds a config map. Later added configs are overriding previous ones */ + public void addConfig(Map config) { + configs.add(0, config); + } + + /** + * Tries to find the property in the maps. It tries the property first, then it tries the camel + * upper version. + */ + public Object get(String property) { + Objects.requireNonNull(property, "property must not be null"); + for (Map map : configs) { + Object ret = map.get(property); + if (ret != null) { + return ret; + } + ret = map.get(toCamelUpper(property)); + if (ret != null) { + return ret; + } + } + return null; + } + + /** converts camelCaseStrings like "rhino.printICode" to "RHINO_PRINT_ICODE". */ + private String toCamelUpper(String property) { + String s = property.replace('.', '_'); + StringBuilder sb = new StringBuilder(s.length() + 5); + for (int i = 0; i < s.length(); ++i) { + char c = s.charAt(i); + if (i > 0 && Character.isUpperCase(c) && Character.isLowerCase(s.charAt(i - 1))) { + sb.append('_'); + } + sb.append(Character.toUpperCase(c)); + } + return sb.toString(); + } +} diff --git a/rhino/src/main/java/org/mozilla/javascript/config/RhinoPropertiesLoader.java b/rhino/src/main/java/org/mozilla/javascript/config/RhinoPropertiesLoader.java new file mode 100644 index 0000000000..0580fccf39 --- /dev/null +++ b/rhino/src/main/java/org/mozilla/javascript/config/RhinoPropertiesLoader.java @@ -0,0 +1,13 @@ +package org.mozilla.javascript.config; + +/** + * Optional Loader interface for loading properties. You can override the default loading mechanism + * by providing your implementation as service. + * + * @author Roland Praml, Foconis Analytics GmbH + */ +public interface RhinoPropertiesLoader { + + /** Classes can add their configs to rhinoProperties. */ + void load(RhinoProperties properties); +} From 24d18e111de0eb1f38fa208c3c9b0f6496c2fe9d Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Mon, 3 Feb 2025 10:59:00 +0100 Subject: [PATCH 2/5] Add testcases for RhinoConfig --- rhino/rhino-config-for-this-module.config | 4 + rhino/rhino-test.config | 7 + .../javascript/tests/RhinoPropertiesTest.java | 145 ++++++++++++++++++ ...la.javascript.config.RhinoPropertiesLoader | 1 + .../src/test/resources/rhino-testcase.config | 2 + rhino/src/test/resources/rhino.config | 7 + 6 files changed, 166 insertions(+) create mode 100644 rhino/rhino-config-for-this-module.config create mode 100644 rhino/rhino-test.config create mode 100644 rhino/src/test/java/org/mozilla/javascript/tests/RhinoPropertiesTest.java create mode 100644 rhino/src/test/resources/META-INF/services/org.mozilla.javascript.config.RhinoPropertiesLoader create mode 100644 rhino/src/test/resources/rhino-testcase.config create mode 100644 rhino/src/test/resources/rhino.config diff --git a/rhino/rhino-config-for-this-module.config b/rhino/rhino-config-for-this-module.config new file mode 100644 index 0000000000..4ecd74b03a --- /dev/null +++ b/rhino/rhino-config-for-this-module.config @@ -0,0 +1,4 @@ +# This file is loaded by the RhinoPropertiesTest and can be used to modify values in this module +# +# rhino.printTrees=true +# rhino.printICode=true \ No newline at end of file diff --git a/rhino/rhino-test.config b/rhino/rhino-test.config new file mode 100644 index 0000000000..91b0c4ec6e --- /dev/null +++ b/rhino/rhino-test.config @@ -0,0 +1,7 @@ +# THIS FILE IS NOT USED BY DEFAULT +# There is a RhinoPropertiesLoader in this maven module +# If you want to reconfigure something, please use rhino-config-for-this-module.config (see RhinoPropertiesTest) + +testconfig.bar=value4-mod +TESTCONFIG_BAZ=value5 + diff --git a/rhino/src/test/java/org/mozilla/javascript/tests/RhinoPropertiesTest.java b/rhino/src/test/java/org/mozilla/javascript/tests/RhinoPropertiesTest.java new file mode 100644 index 0000000000..98cb21f147 --- /dev/null +++ b/rhino/src/test/java/org/mozilla/javascript/tests/RhinoPropertiesTest.java @@ -0,0 +1,145 @@ +package org.mozilla.javascript.tests; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +import java.io.File; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.mozilla.javascript.config.RhinoConfig; +import org.mozilla.javascript.config.RhinoProperties; +import org.mozilla.javascript.config.RhinoPropertiesLoader; + +/** + * Testcase for various property loading mechanism.
+ * Note: Testing is a bit difficult, so to test the serviceLoader mechanism, we have to register a + * RhinoPropertiesLoader in this maven module. Unfortunately, this means, that all other tests in + * this module will also use this loader.
+ * The loader does NOT load the properties from the default locations as mentioned in the + * documentation. So, do not wonder, if setting config values in 'rhino.config' or + * 'rhino-test.config' do not take effect. Instead use `rhino-config-for-this-module.config`. + * + * @author Roland Praml, Foconis Analytics GmbH + */ +public class RhinoPropertiesTest { + + /** This loader is used for all tests in this maven module! */ + public static class Loader implements RhinoPropertiesLoader { + + public static enum TestEnum { + valueA, + VALUEB, + valuec + } + + @Override + public void load(RhinoProperties properties) { + + properties.addConfig(Map.of("foo.bar", "baz1")); + properties.addConfig( + Map.of( + "foo.bar", "baz2", + "foo.someBoolean1", "true", + "foo.someBoolean2", "TRUE", + "foo.someBoolean3", "1", + "foo.someInteger1", "42", + "foo.someInteger2", 42, + "foo.someAValue", true, + "FOO_SOME_BVALUE", 42, + "FOO_ENUM1", "valuea")); + properties.addConfig( + Map.of( + "FOO_ENUM1", "valuea", + "FOO_ENUM2", "VALUEA", + "FOO_ENUM3", "VALUEB", + "FOO_ENUM4", "VALUEC", + "FOO_ENUM5", "VALUED")); + // load module independent file. + properties.loadFromFile(new File("rhino-config-for-this-module.config")); + } + } + + /** Tests the loader initialization and all methods in RhinoConfig. */ + @Test + void testLoaderInit() { + // override test + assertEquals("baz2", RhinoConfig.get("foo.bar")); + + // normal getters (with correct type) + assertEquals(true, RhinoConfig.get("foo.someBoolean1", false)); + assertEquals(true, RhinoConfig.get("foo.someBoolean2", false)); + assertEquals(false, RhinoConfig.get("foo.someBoolean3", false)); + assertEquals(false, RhinoConfig.get("foo.someOtherBoolean", false)); + + // integer test + assertEquals(42, RhinoConfig.get("foo.someInteger1", 21)); + assertEquals(42, RhinoConfig.get("foo.someInteger2", 21)); + assertEquals(21, RhinoConfig.get("foo.someOtherInteger", 21)); + + // camelCase-Tests + assertEquals(true, RhinoConfig.get("foo.someAValue", false)); + assertEquals(42, RhinoConfig.get("foo.someBValue", 21)); + + // enums + assertEquals(Loader.TestEnum.valueA, RhinoConfig.get("foo.enum1", Loader.TestEnum.VALUEB)); + assertEquals(Loader.TestEnum.valueA, RhinoConfig.get("foo.enum2", Loader.TestEnum.VALUEB)); + assertEquals(Loader.TestEnum.VALUEB, RhinoConfig.get("foo.enum3", Loader.TestEnum.valueA)); + assertEquals(Loader.TestEnum.valuec, RhinoConfig.get("foo.enum4", Loader.TestEnum.valueA)); + assertEquals(Loader.TestEnum.valueA, RhinoConfig.get("foo.enum5", Loader.TestEnum.valueA)); + assertEquals(Loader.TestEnum.valueA, RhinoConfig.get("foo.enum6", Loader.TestEnum.valueA)); + } + + /** Tests explicit loading a file from classpath. */ + @Test + void testClasspathLoad() { + RhinoProperties properties = new RhinoProperties(); + properties.loadFromClasspath(getClass().getClassLoader(), "rhino-testcase.config"); + assertEquals("value1", properties.get("testconfig.foo")); + assertEquals("value2", properties.get("testconfig.bar")); + } + + /** Tests explicit loading a file. */ + @Test + void testFileLoad() { + RhinoProperties properties = new RhinoProperties(); + properties.loadFromFile(new File("src/test/resources/rhino-testcase.config")); + assertEquals("value1", properties.get("testconfig.foo")); + assertEquals("value2", properties.get("testconfig.bar")); + } + + @Test + void testDefaultLoad() { + System.setProperty("some.system.value", "value6"); + try { + RhinoProperties properties = new RhinoProperties(); + properties.loadDefaults(); + assertEquals("value3", properties.get("testconfig.foo")); + assertEquals("value4-mod", properties.get("testconfig.bar")); + assertEquals("value5", properties.get("testconfig.baz")); + assertEquals("value6", properties.get("some.system.value")); + } finally { + System.clearProperty("some.system.value"); + } + } + + /** System properties */ + @Test + void testSystemOverride() { + System.setProperty("testconfig.foo", "system-wins"); + try { + RhinoProperties properties = new RhinoProperties(); + properties.loadDefaults(); + assertEquals("system-wins", properties.get("testconfig.foo")); + } finally { + System.clearProperty("testconfig.foo"); + } + } + + @Test + void testEnv() { + RhinoProperties properties = new RhinoProperties(); + properties.loadDefaults(); + // TODO: can/shoud we set an environment value. so check, if we can read PATH + assertNotNull(properties.get("path")); + } +} diff --git a/rhino/src/test/resources/META-INF/services/org.mozilla.javascript.config.RhinoPropertiesLoader b/rhino/src/test/resources/META-INF/services/org.mozilla.javascript.config.RhinoPropertiesLoader new file mode 100644 index 0000000000..7bd8eccbec --- /dev/null +++ b/rhino/src/test/resources/META-INF/services/org.mozilla.javascript.config.RhinoPropertiesLoader @@ -0,0 +1 @@ +org.mozilla.javascript.tests.RhinoPropertiesTest$Loader diff --git a/rhino/src/test/resources/rhino-testcase.config b/rhino/src/test/resources/rhino-testcase.config new file mode 100644 index 0000000000..88edfb882a --- /dev/null +++ b/rhino/src/test/resources/rhino-testcase.config @@ -0,0 +1,2 @@ +testconfig.foo=value1 +TESTCONFIG_BAR=value2 diff --git a/rhino/src/test/resources/rhino.config b/rhino/src/test/resources/rhino.config new file mode 100644 index 0000000000..3f0821e22d --- /dev/null +++ b/rhino/src/test/resources/rhino.config @@ -0,0 +1,7 @@ +# THIS FILE IS NOT USED BY DEFAULT +# There is a RhinoPropertiesLoader in this maven module +# If you want to reconfigure something, please see RhinoPropertiesTest + +testconfig.foo=value3 +TESTCONFIG_BAR=value4 + From a25ad32d5dbbecdc553462a08f24f231b02587f0 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Mon, 3 Feb 2025 11:06:03 +0100 Subject: [PATCH 3/5] Read config values from RhinoConfig --- .../mozilla/javascript/tools/shell/Main.java | 3 ++- .../mozilla/javascript/RhinoException.java | 27 +++---------------- .../java/org/mozilla/javascript/Token.java | 6 +++-- .../javascript/optimizer/DefaultLinker.java | 11 ++------ 4 files changed, 12 insertions(+), 35 deletions(-) diff --git a/rhino-tools/src/main/java/org/mozilla/javascript/tools/shell/Main.java b/rhino-tools/src/main/java/org/mozilla/javascript/tools/shell/Main.java index 698b5b382f..60cb08f00b 100644 --- a/rhino-tools/src/main/java/org/mozilla/javascript/tools/shell/Main.java +++ b/rhino-tools/src/main/java/org/mozilla/javascript/tools/shell/Main.java @@ -39,6 +39,7 @@ import org.mozilla.javascript.SecurityController; import org.mozilla.javascript.commonjs.module.ModuleScope; import org.mozilla.javascript.commonjs.module.Require; +import org.mozilla.javascript.config.RhinoConfig; import org.mozilla.javascript.tools.SourceReader; import org.mozilla.javascript.tools.ToolErrorReporter; @@ -132,7 +133,7 @@ public void quit(Context cx, int exitCode) { */ public static void main(String args[]) { try { - if (Boolean.getBoolean("rhino.use_java_policy_security")) { + if (RhinoConfig.get("rhino.use_java_policy_security", false)) { initJavaPolicySecuritySupport(); } } catch (SecurityException ex) { diff --git a/rhino/src/main/java/org/mozilla/javascript/RhinoException.java b/rhino/src/main/java/org/mozilla/javascript/RhinoException.java index 8bccbf6c2e..a88e800c71 100644 --- a/rhino/src/main/java/org/mozilla/javascript/RhinoException.java +++ b/rhino/src/main/java/org/mozilla/javascript/RhinoException.java @@ -10,11 +10,11 @@ import java.io.FilenameFilter; import java.io.PrintStream; import java.io.PrintWriter; -import java.security.AccessControlException; import java.util.ArrayList; import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.mozilla.javascript.config.RhinoConfig; /** The class of exceptions thrown by the JavaScript engine. */ public abstract class RhinoException extends RuntimeException { @@ -221,11 +221,11 @@ static String formatStackTrace(ScriptStackElement[] stack, String message) { /** * Get a string representing the script stack of this exception. * - * @deprecated the filter argument is ignored as we are able to recognize script stack elements - * by our own. Use #getScriptStackTrace() instead. * @param filter ignored * @return a script stack dump * @since 1.6R6 + * @deprecated the filter argument is ignored as we are able to recognize script stack elements + * by our own. Use #getScriptStackTrace() instead. */ @Deprecated public String getScriptStackTrace(FilenameFilter filter) { @@ -375,7 +375,7 @@ public static StackStyle getStackStyle() { private static final long serialVersionUID = 1883500631321581169L; // Just for testing! - private static StackStyle stackStyle = StackStyle.RHINO; + private static StackStyle stackStyle = RhinoConfig.get("rhino.stack.style", StackStyle.RHINO); private String sourceName; private int lineNumber; @@ -384,23 +384,4 @@ public static StackStyle getStackStyle() { Object interpreterStackInfo; int[] interpreterLineData; - - // Allow us to override default stack style for debugging. - static { - try { - String style = System.getProperty("rhino.stack.style"); - if (style != null) { - if ("Rhino".equalsIgnoreCase(style)) { - stackStyle = StackStyle.RHINO; - } else if ("Mozilla".equalsIgnoreCase(style)) { - stackStyle = StackStyle.MOZILLA; - } else if ("V8".equalsIgnoreCase(style)) { - stackStyle = StackStyle.V8; - } - } - } catch (AccessControlException ace) { - // ignore. We will land here, if SecurityManager is in place and error is lazily - // initialized - } - } } diff --git a/rhino/src/main/java/org/mozilla/javascript/Token.java b/rhino/src/main/java/org/mozilla/javascript/Token.java index 2fac729e63..128ccca048 100644 --- a/rhino/src/main/java/org/mozilla/javascript/Token.java +++ b/rhino/src/main/java/org/mozilla/javascript/Token.java @@ -6,6 +6,8 @@ package org.mozilla.javascript; +import org.mozilla.javascript.config.RhinoConfig; + /** * This class implements the JavaScript scanner. * @@ -24,8 +26,8 @@ public static enum CommentType { } // debug flags - public static final boolean printTrees = false; - static final boolean printICode = false; + public static final boolean printTrees = RhinoConfig.get("rhino.printTrees", false); + static final boolean printICode = RhinoConfig.get("rhino.printICode", false); static final boolean printNames = printTrees || printICode; /** Token types. These values correspond to JSTokenType values in jsscan.c. */ diff --git a/rhino/src/main/java/org/mozilla/javascript/optimizer/DefaultLinker.java b/rhino/src/main/java/org/mozilla/javascript/optimizer/DefaultLinker.java index 57e0697a98..7f2ab7bec8 100644 --- a/rhino/src/main/java/org/mozilla/javascript/optimizer/DefaultLinker.java +++ b/rhino/src/main/java/org/mozilla/javascript/optimizer/DefaultLinker.java @@ -14,6 +14,7 @@ import org.mozilla.javascript.ScriptRuntime; import org.mozilla.javascript.Scriptable; import org.mozilla.javascript.Token; +import org.mozilla.javascript.config.RhinoConfig; /** * This linker is the last one in the chain, and as such it must be able to link every type of @@ -22,15 +23,7 @@ */ @SuppressWarnings("AndroidJdkLibsChecker") class DefaultLinker implements GuardingDynamicLinker { - static final boolean DEBUG; - - static { - String debugVal = System.getProperty("RHINO_DEBUG_LINKER"); - if (debugVal == null) { - debugVal = System.getenv("RHINO_DEBUG_LINKER"); - } - DEBUG = Boolean.parseBoolean(debugVal); - } + static final boolean DEBUG = RhinoConfig.get("rhino.debugLinker", false); @Override public GuardedInvocation getGuardedInvocation(LinkRequest req, LinkerServices svc) From 5799ee23b1621fc955cea212826d625ffe47b5a0 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Mon, 3 Feb 2025 12:07:15 +0100 Subject: [PATCH 4/5] RhinoProperties has debug flag now --- .../javascript/config/RhinoProperties.java | 44 +++++++++++++++---- tests/build.gradle | 7 +++ tests/rhino-test.config | 22 ++++++++++ 3 files changed, 64 insertions(+), 9 deletions(-) create mode 100644 tests/rhino-test.config diff --git a/rhino/src/main/java/org/mozilla/javascript/config/RhinoProperties.java b/rhino/src/main/java/org/mozilla/javascript/config/RhinoProperties.java index c399845b4e..758d151fe8 100644 --- a/rhino/src/main/java/org/mozilla/javascript/config/RhinoProperties.java +++ b/rhino/src/main/java/org/mozilla/javascript/config/RhinoProperties.java @@ -48,6 +48,8 @@ public class RhinoProperties { private static String[] CONFIG_FILES = {"rhino.config", "rhino-test.config"}; private List> configs = new ArrayList<>(); + // this allows debugging via system property "rhino.debugConfig" + private final boolean debug = Boolean.getBoolean("rhino.debugConfig"); /** * Initializes the default, used in RhinoConfig. If there is RhinoPropertiesLoader present, then @@ -59,9 +61,16 @@ static RhinoProperties init() { ServiceLoader.load(RhinoPropertiesLoader.class).iterator(); if (loader.hasNext()) { while (loader.hasNext()) { - loader.next().load(props); + RhinoPropertiesLoader next = loader.next(); + if (props.debug) { + System.out.println("Rhino: using loader " + next.getClass().getName()); + } + next.load(props); } } else { + if (props.debug) { + System.out.println("Rhino: no loader found. Loading defaults"); + } props.loadDefaults(); } return props; @@ -77,8 +86,13 @@ public void loadDefaults() { loadFromClasspath(contextClassLoader, configFile); loadFromFile(new File(configFile)); } - + if (debug) { + System.out.println("Rhino: loading configuration from System.getEnv()"); + } addConfig(System.getenv()); + if (debug) { + System.out.println("Rhino: loading configuration from System.getProperties()"); + } addConfig(System.getProperties()); } @@ -87,6 +101,9 @@ public void loadFromFile(File config) { if (!config.exists()) { return; } + if (debug) { + System.out.println("Rhino: loading configuration from " + config.getAbsolutePath()); + } try (InputStream in = new FileInputStream(config)) { Properties props = new Properties(); props.load(new InputStreamReader(in, StandardCharsets.UTF_8)); @@ -112,6 +129,9 @@ public void loadFromResource(URL resource) { if (resource == null) { return; } + if (debug) { + System.out.println("Rhino: loading configuration from " + resource); + } try (InputStream in = resource.openStream()) { Properties props = new Properties(); props.load(new InputStreamReader(in, StandardCharsets.UTF_8)); @@ -124,6 +144,9 @@ public void loadFromResource(URL resource) { /** Adds a config map. Later added configs are overriding previous ones */ public void addConfig(Map config) { + if (debug) { + System.out.println("Rhino: added " + config.size() + " values"); + } configs.add(0, config); } @@ -134,13 +157,16 @@ public void addConfig(Map config) { public Object get(String property) { Objects.requireNonNull(property, "property must not be null"); for (Map map : configs) { - Object ret = map.get(property); - if (ret != null) { - return ret; - } - ret = map.get(toCamelUpper(property)); - if (ret != null) { - return ret; + String key = property; + for (int i = 0; i < 2; i++) { + Object ret = map.get(key); + if (ret != null) { + if (debug) { + System.out.println("Rhino: get(" + key + ")=" + ret); + } + return ret; + } + key = toCamelUpper(property); } } return null; diff --git a/tests/build.gradle b/tests/build.gradle index 5dc5580002..71d5ef4d1d 100644 --- a/tests/build.gradle +++ b/tests/build.gradle @@ -60,6 +60,13 @@ test { systemProperty 'updateTest262properties', System.getProperty('updateTest262properties') } + System.properties.each { k,v-> + if (k.startsWith("rhino.")) { + systemProperty k, v + } + } + + jacoco { // These particular tests produce Jacoco exceptions because of their size excludes = ['**perlstress**', '**start_unicode**'] diff --git a/tests/rhino-test.config b/tests/rhino-test.config new file mode 100644 index 0000000000..b9c73b5014 --- /dev/null +++ b/tests/rhino-test.config @@ -0,0 +1,22 @@ +# This is the rhino configuration for tests. +# Normally, this file is used to list the avaliable options with default values +# +# You may change values for debug purposes. +# To debug configuration loading issues, you may add "-Drhino.debugConfig=true" gradle option + +# Stack style. Available values RHINO, MOZILLA, MOZILLA_LF, V8 +# +# default: +# rhino.stack.style=rhino + +# various compile/linker flags: +# +# default: +# rhino.printTrees=false +# rhino.printICode=false +# rhino.debugLinker=false + +# Rhino-Shell +# +# default: +# rhino.use_java_policy_security=false \ No newline at end of file From a5a7a1a6046ac53c2d9a6b55c8c4f1b71e9a1986 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Mon, 3 Feb 2025 13:27:46 +0100 Subject: [PATCH 5/5] Better debug logging --- .../javascript/config/RhinoProperties.java | 57 +++++++++---------- 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/rhino/src/main/java/org/mozilla/javascript/config/RhinoProperties.java b/rhino/src/main/java/org/mozilla/javascript/config/RhinoProperties.java index 758d151fe8..84675b4b8d 100644 --- a/rhino/src/main/java/org/mozilla/javascript/config/RhinoProperties.java +++ b/rhino/src/main/java/org/mozilla/javascript/config/RhinoProperties.java @@ -62,15 +62,11 @@ static RhinoProperties init() { if (loader.hasNext()) { while (loader.hasNext()) { RhinoPropertiesLoader next = loader.next(); - if (props.debug) { - System.out.println("Rhino: using loader " + next.getClass().getName()); - } + props.logDebug("Using loader %s", next.getClass().getName()); next.load(props); } } else { - if (props.debug) { - System.out.println("Rhino: no loader found. Loading defaults"); - } + props.logDebug("No loader found. Loading defaults"); props.loadDefaults(); } return props; @@ -86,13 +82,10 @@ public void loadDefaults() { loadFromClasspath(contextClassLoader, configFile); loadFromFile(new File(configFile)); } - if (debug) { - System.out.println("Rhino: loading configuration from System.getEnv()"); - } + logDebug("loading configuration from System.getEnv()"); addConfig(System.getenv()); - if (debug) { - System.out.println("Rhino: loading configuration from System.getProperties()"); - } + + logDebug("Rhino: loading configuration from System.getProperties()"); addConfig(System.getProperties()); } @@ -101,19 +94,16 @@ public void loadFromFile(File config) { if (!config.exists()) { return; } - if (debug) { - System.out.println("Rhino: loading configuration from " + config.getAbsolutePath()); - } + + logDebug("loading configuration from %s", config.getAbsoluteFile()); try (InputStream in = new FileInputStream(config)) { Properties props = new Properties(); props.load(new InputStreamReader(in, StandardCharsets.UTF_8)); addConfig(props); } catch (IOException e) { - System.err.println( - "Rhino: Error loading configuration from " - + config.getAbsolutePath() - + ": " - + e.getMessage()); + logError( + "Error loading configuration from %s: %s", + config.getAbsoluteFile(), e.getMessage()); } } @@ -129,24 +119,20 @@ public void loadFromResource(URL resource) { if (resource == null) { return; } - if (debug) { - System.out.println("Rhino: loading configuration from " + resource); - } + + logDebug("Rhino: loading configuration from %s", resource); try (InputStream in = resource.openStream()) { Properties props = new Properties(); props.load(new InputStreamReader(in, StandardCharsets.UTF_8)); addConfig(props); } catch (IOException e) { - System.err.println( - "Rhino: Error loading configuration from " + resource + ": " + e.getMessage()); + logError("Error loading configuration from %s: %s", resource, e.getMessage()); } } /** Adds a config map. Later added configs are overriding previous ones */ public void addConfig(Map config) { - if (debug) { - System.out.println("Rhino: added " + config.size() + " values"); - } + logDebug("added %d values", config.size()); configs.add(0, config); } @@ -161,9 +147,7 @@ public Object get(String property) { for (int i = 0; i < 2; i++) { Object ret = map.get(key); if (ret != null) { - if (debug) { - System.out.println("Rhino: get(" + key + ")=" + ret); - } + logDebug("get(%s)=%s", key, ret); return ret; } key = toCamelUpper(property); @@ -185,4 +169,15 @@ private String toCamelUpper(String property) { } return sb.toString(); } + + private void logDebug(String msg, Object... args) { + if (!debug) { + return; + } + System.out.println("[Rhino] " + String.format(msg, args)); + } + + private void logError(String msg, Object... args) { + System.err.println("[Rhino] " + String.format(msg, args)); + } }