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

Reading configuration from config files #1722

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Q: Should we change this to

Suggested change
if (RhinoConfig.get("rhino.use_java_policy_security", false)) {
if (RhinoConfig.get("rhino.useJavaPolicySecurity", false)) {

A: Probably not, as this will break other things!
And JavaPolicySecurity has to be removed anyway in the future (see #1353)

initJavaPolicySecuritySupport();
}
} catch (SecurityException ex) {
Expand Down
4 changes: 4 additions & 0 deletions rhino/rhino-config-for-this-module.config
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this PR is looking very good, but why do we have this file and the other .config files in the top-level directory? I suppose that we need one just to test the file reading part of the codebase, but since they won't be used by the runtime or tests otherwise, do we really need all of them? That's really my only concern with this PR at this point. Thanks!

Original file line number Diff line number Diff line change
@@ -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
7 changes: 7 additions & 0 deletions rhino/rhino-test.config
Original file line number Diff line number Diff line change
@@ -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

2 changes: 2 additions & 0 deletions rhino/src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
27 changes: 4 additions & 23 deletions rhino/src/main/java/org/mozilla/javascript/RhinoException.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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
}
}
}
6 changes: 4 additions & 2 deletions rhino/src/main/java/org/mozilla/javascript/Token.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

package org.mozilla.javascript;

import org.mozilla.javascript.config.RhinoConfig;

/**
* This class implements the JavaScript scanner.
*
Expand All @@ -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. */
Expand Down
101 changes: 101 additions & 0 deletions rhino/src/main/java/org/mozilla/javascript/config/RhinoConfig.java
Original file line number Diff line number Diff line change
@@ -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>) RhinoProperties::init);

/**
* Returns the property as string.
*
* <p>If the value is not set, <code>defaultVaule</code> 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.
*
* <p>If the property is set to any of the enum names (case-insensitive), this enum value is
* returned, otherwise <code>defaultValue</code> is returned.
*
* <p>Note: <code>defaultValue</code> must be specified to derive the enum class and its values.
*/
@SuppressWarnings("unchecked")
public static <T extends Enum<T>> T get(String property, T defaultValue) {
Objects.requireNonNull(defaultValue, "defaultValue must not be null");
Object ret = INSTANCE.get(property);
if (ret != null) {
Class<T> enumType = (Class<T>) 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.
*
* <p>A property is true, if it is either <code>Boolean.TRUE</code> or if and only if the string
* representation is equal to {@code "true"} (case-insensitive). If the property is not set,
* <code>defaultValue</code> is returned
*
* <p>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.
*
* <p>if the property is not set or not a valid integer value, <code>defaultValue</code> 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;
}
}
Loading