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

Disable Java Map accessing by properties #836

Merged
merged 3 commits into from
Feb 20, 2021
Merged

Disable Java Map accessing by properties #836

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
87795ab
Disable Java Map accessing by properties
tuchida Feb 2, 2021
4322742
Example of FEATURE_ENABLE_JAVA_MAP_ACCESS usage
tuchida Feb 19, 2021
dc43827
Use Context.getCurrentContext()
tuchida Feb 19, 2021
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
20 changes: 20 additions & 0 deletions src/org/mozilla/javascript/Context.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,26 @@ public class Context
*/
public static final int FEATURE_ENABLE_XML_SECURE_PARSING = 20;

/**
* Configure whether the entries in a Java Map can be accessed by properties.
*
* WARNING: This feature is similar to the one in Nashorn, but incomplete.
*
* 1. A entry has priority over method.
*
* map.put("put", "abc");
* map.put; // abc
* map.put("put", "efg"); // ERROR
*
* 2. The distinction between numeric keys and string keys is ambiguous.
*
* map.put('1', 123);
* map['1']; // Not found. This means `map[1]`.
*
* @since 1.7 Release 14
*/
gbrail marked this conversation as resolved.
Show resolved Hide resolved
public static final int FEATURE_ENABLE_JAVA_MAP_ACCESS = 21;

public static final String languageVersionProperty = "language version";
public static final String errorReporterProperty = "error reporter";

Expand Down
5 changes: 4 additions & 1 deletion src/org/mozilla/javascript/ContextFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,9 @@ protected boolean hasFeature(Context cx, int featureIndex)

case Context.FEATURE_ENABLE_XML_SECURE_PARSING:
return true;

case Context.FEATURE_ENABLE_JAVA_MAP_ACCESS:
return false;
}
// It is a bug to call the method with unknown featureIndex
throw new IllegalArgumentException(String.valueOf(featureIndex));
Expand Down Expand Up @@ -604,4 +607,4 @@ public final Context enterContext(Context cx)
{
return Context.enter(cx, this);
}
}
}
67 changes: 45 additions & 22 deletions src/org/mozilla/javascript/NativeJavaMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,63 +25,86 @@ public String getClassName() {
return "JavaMap";
}


@Override
public boolean has(String name, Scriptable start) {
if (map.containsKey(name)) {
return true;
Context cx = Context.getContext();
Copy link
Collaborator

Choose a reason for hiding this comment

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

"getContext" will throw a RuntimeError if there is no Context on the stack. That's usually incorrect but I know that there's code out there that might do it. Could you please replace these tests with:

Context cx = Context.getCurrentContext();
if (cx != null && cx.hasFeature(Context.FEATURE_ENABLE_JAVA_MAP_ACCESS))

(or make a helper function in Context or ScriptRuntime since we do that ll the time!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dc43827
I fixed NativeJavaMap and NativeJavaList since I based it on #713.

if (cx.hasFeature(Context.FEATURE_ENABLE_JAVA_MAP_ACCESS)) {
if (map.containsKey(name)) {
return true;
}
}
return super.has(name, start);
}

@Override
public boolean has(int index, Scriptable start) {
if (map.containsKey(Integer.valueOf(index))) {
return true;
Context cx = Context.getContext();
if (cx.hasFeature(Context.FEATURE_ENABLE_JAVA_MAP_ACCESS)) {
if (map.containsKey(Integer.valueOf(index))) {
return true;
}
}
return super.has(index, start);
}

@Override
public Object get(String name, Scriptable start) {
if (map.containsKey(name)) {
Context cx = Context.getContext();
Object obj = map.get(name);
return cx.getWrapFactory().wrap(cx, this, obj, obj.getClass());
Context cx = Context.getContext();
if (cx.hasFeature(Context.FEATURE_ENABLE_JAVA_MAP_ACCESS)) {
if (map.containsKey(name)) {
Object obj = map.get(name);
return cx.getWrapFactory().wrap(cx, this, obj, obj.getClass());
}
}
return super.get(name, start);
}

@Override
public Object get(int index, Scriptable start) {
if (map.containsKey(Integer.valueOf(index))) {
Context cx = Context.getContext();
Object obj = map.get(Integer.valueOf(index));
return cx.getWrapFactory().wrap(cx, this, obj, obj.getClass());
Context cx = Context.getContext();
if (cx.hasFeature(Context.FEATURE_ENABLE_JAVA_MAP_ACCESS)) {
if (map.containsKey(Integer.valueOf(index))) {
Object obj = map.get(Integer.valueOf(index));
return cx.getWrapFactory().wrap(cx, this, obj, obj.getClass());
}
}
return super.get(index, start);
}

@Override
public void put(String name, Scriptable start, Object value) {
map.put(name, Context.jsToJava(value, Object.class));
Context cx = Context.getContext();
if (cx.hasFeature(Context.FEATURE_ENABLE_JAVA_MAP_ACCESS)) {
map.put(name, Context.jsToJava(value, Object.class));
} else {
super.put(name, start, value);
}
}

@Override
public void put(int index, Scriptable start, Object value) {
map.put(Integer.valueOf(index), Context.jsToJava(value, Object.class));
Context cx = Context.getContext();
if (cx.hasFeature(Context.FEATURE_ENABLE_JAVA_MAP_ACCESS)) {
map.put(Integer.valueOf(index), Context.jsToJava(value, Object.class));
} else {
super.put(index, start, value);
}
}

@Override
public Object[] getIds() {
List<Object> ids = new ArrayList<>(map.size());
for (Object key : map.keySet()) {
if (key instanceof Integer) {
ids.add((Integer)key);
} else {
ids.add(ScriptRuntime.toString(key));
Context cx = Context.getContext();
if (cx.hasFeature(Context.FEATURE_ENABLE_JAVA_MAP_ACCESS)) {
List<Object> ids = new ArrayList<>(map.size());
for (Object key : map.keySet()) {
if (key instanceof Integer) {
ids.add((Integer)key);
} else {
ids.add(ScriptRuntime.toString(key));
}
}
return ids.toArray();
}
return ids.toArray();
return super.getIds();
}
}
95 changes: 64 additions & 31 deletions testsrc/org/mozilla/javascript/tests/NativeJavaMapTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,43 +9,55 @@
import junit.framework.TestCase;
import org.mozilla.javascript.Context;
import org.mozilla.javascript.ContextFactory;
import org.mozilla.javascript.EvaluatorException;
import org.mozilla.javascript.NativeArray;
import org.mozilla.javascript.NativeJavaMethod;
import org.mozilla.javascript.Scriptable;
import org.mozilla.javascript.tools.shell.Global;

import java.util.HashMap;
import java.util.Map;
import java.util.function.Function;

import static org.junit.Assert.*;

/**
* From @makusuko (Markus Sunela), imported from PR https://github.com/mozilla/rhino/pull/561
*/
public class NativeJavaMapTest extends TestCase {
protected final Global global = new Global();
private final ContextFactory contextFactoryWithMapAccess = new ContextFactory() {
@Override
protected boolean hasFeature(Context cx, int featureIndex) {
if (featureIndex == Context.FEATURE_ENABLE_JAVA_MAP_ACCESS) {
return true;
}
return super.hasFeature(cx, featureIndex);
}
};

public NativeJavaMapTest() {
global.init(ContextFactory.getGlobal());
}


public void testAccessingJavaMapIntegerValues() {
Map<Number, Number> map = new HashMap<>();
map.put(0, 1);
map.put(1, 2);
map.put(2, 3);

assertEquals(2, runScriptAsInt("value[1]", map));
assertEquals(3, runScriptAsInt("value[2]", map));
assertEquals(2, runScriptAsInt("value[1]", map, true));
assertEquals(3, runScriptAsInt("value[2]", map, true));
}

public void testJavaMethodCalls() {
Map<String, Number> map = new HashMap<>();
map.put("a", 1);
map.put("b", 2);
map.put("c", 3);
assertEquals(3, runScriptAsInt("value.size()", map));
assertEquals(1, runScriptAsInt("value.get('a')", map));
assertEquals(4, runScriptAsInt("value.put('d', 4);value.size()", map));
assertEquals(3, runScriptAsInt("value.size()", map, true));
assertEquals(1, runScriptAsInt("value.get('a')", map, true));
assertEquals(4, runScriptAsInt("value.put('d', 4);value.size()", map, true));
}

public void testUpdatingJavaMapIntegerValues() {
Expand All @@ -54,8 +66,8 @@ public void testUpdatingJavaMapIntegerValues() {
map.put(1,2);
map.put(2,3);

assertEquals(2, runScriptAsInt("value[1]", map));
assertEquals(5, runScriptAsInt("value[1]=5;value[1]", map));
assertEquals(2, runScriptAsInt("value[1]", map, true));
assertEquals(5, runScriptAsInt("value[1]=5;value[1]", map, true));
assertEquals(5, map.get(1).intValue());
}

Expand All @@ -65,10 +77,10 @@ public void testAccessingJavaMapStringValues() {
map.put("b", "b");
map.put("c", "c");

assertEquals("b", runScriptAsString("value['b']", map));
assertEquals("c", runScriptAsString("value['c']", map));
assertEquals("b", runScriptAsString("value.b", map));
assertEquals("c", runScriptAsString("value.c", map));
assertEquals("b", runScriptAsString("value['b']", map, true));
assertEquals("c", runScriptAsString("value['c']", map, true));
assertEquals("b", runScriptAsString("value.b", map, true));
assertEquals("c", runScriptAsString("value.c", map, true));
}

public void testUpdatingJavaMapStringValues() {
Expand All @@ -77,66 +89,87 @@ public void testUpdatingJavaMapStringValues() {
map.put("b", "b");
map.put("c", "c");

assertEquals("b", runScriptAsString("value['b']", map));
assertEquals("b", runScriptAsString("value.b", map));
assertEquals("f", runScriptAsString("value['b']=\"f\";value['b']", map));
assertEquals("b", runScriptAsString("value['b']", map, true));
assertEquals("b", runScriptAsString("value.b", map, true));
assertEquals("f", runScriptAsString("value['b']=\"f\";value['b']", map, true));
assertEquals("f", map.get("b"));
assertEquals("g", runScriptAsString("value.b=\"g\";value.b", map));
assertEquals("g", runScriptAsString("value.b=\"g\";value.b", map, true));
}

public void testAccessMapInMap() {
Map<String, Map<String, String>> map = new HashMap<>();
map.put("a", new HashMap<>());
map.get("a").put("a", "a");

assertEquals("a", runScriptAsString("value['a']['a']", map));
assertEquals("a", runScriptAsString("value.a.a", map));
assertEquals("a", runScriptAsString("value['a']['a']", map, true));
assertEquals("a", runScriptAsString("value.a.a", map, true));
}

public void testUpdatingMapInMap() {
Map<String, Map<String, String>> map = new HashMap<>();
map.put("a", new HashMap<>());
map.get("a").put("a", "a");

assertEquals("a", runScriptAsString("value['a']['a']", map));
assertEquals("a", runScriptAsString("value.a.a", map));
assertEquals("b", runScriptAsString("value.a.a = 'b';value.a.a", map));
assertEquals("a", runScriptAsString("value['a']['a']", map, true));
assertEquals("a", runScriptAsString("value.a.a", map, true));
assertEquals("b", runScriptAsString("value.a.a = 'b';value.a.a", map, true));
}

public void testKeys() {
Map<String, String> map = new HashMap<>();
NativeArray resEmpty = (NativeArray) runScript("Object.keys(value)", map, Function.identity());
NativeArray resEmpty = (NativeArray) runScript("Object.keys(value)", map, true);
assertEquals(0, resEmpty.size());

map.put("a", "a");
map.put("b", "b");
map.put("c", "c");

NativeArray res = (NativeArray) runScript("Object.keys(value)", map, Function.identity());
NativeArray res = (NativeArray) runScript("Object.keys(value)", map, true);
assertEquals(3, res.size());
assertTrue(res.contains("a"));
assertTrue(res.contains("b"));
assertTrue(res.contains("c"));

Map<Integer, String> mapInt = new HashMap<>();
mapInt.put(42, "test");
NativeArray resInt = (NativeArray) runScript("Object.keys(value)", mapInt, Function.identity());
NativeArray resInt = (NativeArray) runScript("Object.keys(value)", mapInt, true);
assertTrue(resInt.contains("42")); // Object.keys always return Strings as key
}

private int runScriptAsInt(String scriptSourceText, Object value) {
return runScript(scriptSourceText, value, Context::toNumber).intValue();
public void testJavaMapWithoutAccessEntries() {
Map<Object, Object> map = new HashMap<>();
map.put(0, 1);
map.put("put", "method");
map.put("a", "abc");

assertThrows(EvaluatorException.class, () -> runScript("value[0]", map, false));
assertTrue(runScript("value.put", map, false) instanceof NativeJavaMethod);
assertThrows(EvaluatorException.class, () -> runScript("value['a'] = 0", map, false));
assertEquals(false, runScript("'a' in value", map, false));
assertEquals(true, runScript("Object.keys(value).includes('getClass')", map, false));
}

private int runScriptAsInt(String scriptSourceText, Object value, boolean enableJavaMapAccess) {
return runScript(scriptSourceText, value, Context::toNumber, enableJavaMapAccess).intValue();
}

private String runScriptAsString(String scriptSourceText, Object value) {
return runScript(scriptSourceText, value, Context::toString);
private String runScriptAsString(String scriptSourceText, Object value, boolean enableJavaMapAccess) {
return runScript(scriptSourceText, value, Context::toString, enableJavaMapAccess);
}

private <T> T runScript(String scriptSourceText, Object value, Function<Object, T> convert) {
return ContextFactory.getGlobal().call(context -> {
private Object runScript(String scriptSourceText, Object value, boolean enableJavaMapAccess) {
return runScript(scriptSourceText, value, Function.identity(), enableJavaMapAccess);
}

private <T> T runScript(String scriptSourceText, Object value, Function<Object, T> convert, boolean enableJavaMapAccess) {
return getContextFactory(enableJavaMapAccess).call(context -> {
Scriptable scope = context.initStandardObjects(global);
scope.put("value", scope, Context.javaToJS(value, scope));
return convert.apply(context.evaluateString(scope, scriptSourceText, "", 1, null));
});
}
}

private ContextFactory getContextFactory(boolean enableJavaMapAccess) {
return enableJavaMapAccess ? contextFactoryWithMapAccess : ContextFactory.getGlobal();
}
}