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

Allow JSON.stringify to operate on Java objects #860

Merged
merged 1 commit into from
Apr 14, 2021
Merged
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
35 changes: 30 additions & 5 deletions src/org/mozilla/javascript/NativeJSON.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Stack;

import org.mozilla.javascript.json.JsonParser;
Expand Down Expand Up @@ -287,13 +288,33 @@ private static Object str(Object key, Scriptable holder,
new Object[] { key, value });
}


if (value instanceof NativeNumber) {
value = Double.valueOf(ScriptRuntime.toNumber(value));
} else if (value instanceof NativeString) {
value = ScriptRuntime.toString(value);
} else if (value instanceof NativeBoolean) {
value = ((NativeBoolean) value).getDefaultValue(ScriptRuntime.BooleanClass);
} else if (value instanceof NativeJavaObject) {
value = ((NativeJavaObject) value).unwrap();
if (value instanceof Map) {
Map<?,?> map = (Map<?,?>) value;
NativeObject nObj = new NativeObject();
map.forEach((k, v) -> {
if (k instanceof CharSequence) {
nObj.put(((CharSequence) k).toString(), nObj, v);
}
});
value = nObj;
}
else {
if (value instanceof Collection<?>) {
Collection<?> col = (Collection<?>) value;
value = col.toArray(new Object[col.size()]);
}
if (value instanceof Object[]) {
value = new NativeArray((Object[]) value);
}
}
}

if (value == null) return "null";
Expand All @@ -314,11 +335,15 @@ private static Object str(Object key, Scriptable holder,
return "null";
}

if (value instanceof Scriptable && !(value instanceof Callable)) {
if (value instanceof NativeArray) {
return ja((NativeArray) value, state);
if (value instanceof Scriptable) {
if (!(value instanceof Callable)) {
if (value instanceof NativeArray) {
return ja((NativeArray) value, state);
}
return jo((Scriptable) value, state);
}
return jo((Scriptable) value, state);
} else if (!Undefined.isUndefined(value)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm worried that throwing this exception will break backward compatibility. Previously, someone with a complex object who is trying to stringify it might end up with a weird result, and now they will get a less weird result. But if there's a chance that people who were previously getting weird results will now get an exception, then we could easily break a lot of code. Unless the ECMA spec says that we should throw an exception in this case, then do we really need it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current code sees it as a NativeJavaObject, which is Scriptable, and so it sends it down the path to try to stringify it as a javascript object, because it is not Callable. This is throwing the StackOverflowError mentioned in the linked issue. I believe the ECMA spec says that all Objects that are not Functions should throw a TypeError if they are cyclic or return a JSON string representing an object.

Since I am unwrapping the NativeJavaObject, it is no longer Scriptable. I think at this point it either has to be Undefined or some other Java Object that can't be serialized, so the options are to either throw a more descriptive exception than it had been throwing, return Undefined, or return "{}".

I believe prior to 4c76ef2 it never made it this far and threw the other exception described in the linked issue:

Java class "[Ljava.lang.reflect.Constructor;" has no public instance field or method named "toJSON"

That commit let the object make it through to the replacer, but created a worse exception if a replacer did not exist to do something with the object.

I think my TypeError is in line with the historical behavior before the StackOverflowError, but I am open to suggestions. This also mimics the behavior of BigInt, which throws a TypeError when you try to stringify it without a replacer or adding a custom BigInt.prototype.toJSON method.

throw ScriptRuntime.typeErrorById("msg.json.cant.serialize", value.getClass().getName());
}

return Undefined.instance;
Expand Down
4 changes: 4 additions & 0 deletions src/org/mozilla/javascript/resources/Messages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,10 @@ msg.invalid.date =\
msg.toisostring.must.return.primitive =\
toISOString must return a primitive value, but instead returned "{0}"

# NativeJSON
msg.json.cant.serialize =\
Do not know how to serialize a {0}

# Parser
msg.got.syntax.errors = \
Compilation produced {0} syntax errors.
Expand Down
147 changes: 147 additions & 0 deletions testsrc/jstests/stringify-java-objects.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
load('testsrc/assert.js');

function replacer(key, value) {
var javatype = value instanceof java.lang.Object ? value.getClass().name : null;
return javatype ? 'replaced: ' + javatype : value;
}

// java.lang.String
var javaString = new java.lang.String('test');

var obj = {test: javaString};
var expected = JSON.stringify({test: 'replaced: java.lang.String'});
var actual = JSON.stringify(obj, replacer);
assertEquals(expected, actual);

var obj = {test: javaString};
var expected = JSON.stringify({test: 'test'});
var actual = JSON.stringify(obj);
assertEquals(expected, actual);

// java.lang.Double
var javaDouble = java.lang.Double.valueOf(12.34);

var obj = {test: javaDouble};
var expected = JSON.stringify({test: 'replaced: java.lang.Double'});
var actual = JSON.stringify(obj, replacer);
assertEquals(expected, actual);

var obj = {test: javaDouble};
var expected = JSON.stringify({test: 12.34});
var actual = JSON.stringify(obj);
assertEquals(expected, actual);

// java.lang.Boolean
var javaBoolean = java.lang.Boolean.valueOf(false);

var obj = {test: javaBoolean};
var expected = JSON.stringify({test: 'replaced: java.lang.Boolean'});
var actual = JSON.stringify(obj, replacer);
assertEquals(expected, actual);

var obj = {test: javaBoolean};
var expected = JSON.stringify({test: false});
var actual = JSON.stringify(obj);
assertEquals(expected, actual);

// java.util.Collection
var javaCollection = new java.util.LinkedHashSet();
javaCollection.add('test');
javaCollection.add({nested: 'jsObj'});

var obj = {test: javaCollection};
var expected = JSON.stringify({test: 'replaced: java.util.LinkedHashSet'});
var actual = JSON.stringify(obj, replacer);
assertEquals(expected, actual);

var obj = {test: javaCollection};
var expected = JSON.stringify({test: ['test', {nested: 'jsObj'}]});
var actual = JSON.stringify(obj);
assertEquals(expected, actual);

// java Array
var javaArray = new java.lang.String('a,b,c').split(',');

var obj = {test: javaArray};
var expected = JSON.stringify({test: 'replaced: [Ljava.lang.String;'});
var actual = JSON.stringify(obj, replacer);
assertEquals(expected, actual);

var obj = {test: javaArray};
var expected = JSON.stringify({test: ['a','b','c']});
var actual = JSON.stringify(obj);
assertEquals(expected, actual);

// java Map
var javaMap = new java.util.HashMap();
javaMap.put(new java.lang.Object(), 'property skipped if key is not string-like');
javaMap.put('te' + 'st', 55);

var obj = {test: javaMap};
var expected = JSON.stringify({test: 'replaced: java.util.HashMap'});
var actual = JSON.stringify(obj, replacer);
assertEquals(expected, actual);

var obj = javaMap;
var expected = JSON.stringify({test: 55});
var actual = JSON.stringify(obj);
assertEquals(expected, actual);

// complex object
var obj = {
array: javaArray,
boxed: [javaDouble, javaBoolean],
objects: {
plainJS: {test: 1},
emptyMap: java.util.Collections.EMPTY_MAP,
otherMap: javaMap
}
};
var expected = JSON.stringify({
array: ['a','b','c'],
boxed: [12.34, false],
objects: {
plainJS: {test: 1},
emptyMap: {},
otherMap: {test: 55}
}
});
var actual = JSON.stringify(obj);
assertEquals(expected, actual);

// other Java object
var javaObject = new java.net.URI('test://other/java/object');

var obj = {test: javaObject};
var expected = JSON.stringify({test: 'replaced: java.net.URI'});
var actual = JSON.stringify(obj, replacer);
assertEquals(expected, actual);

var obj = {test: javaObject};
assertThrows(()=>JSON.stringify(obj), TypeError);

// JavaAdapter with toJSON
var javaObject = new JavaAdapter(java.lang.Object, {
toJSON: _ => ({javaAdapter: true})
});

var obj = javaObject;
var expected = JSON.stringify({javaAdapter: true});
var actual = JSON.stringify(obj, replacer);
assertEquals(expected, actual);

// JavaAdapter without toJSON
var javaObject = new JavaAdapter(java.lang.Object, {
toString: () => 'just an object'
});

var obj = {test: javaObject};
var expected = /^replaced: adapter\d+$/;
var actual = JSON.parse(JSON.stringify(obj, replacer)).test;
assertEquals("string", typeof actual);
assertTrue(expected.test(actual));

var obj = {test: javaObject};
assertThrows(()=>JSON.stringify(obj), TypeError);

"success"
10 changes: 10 additions & 0 deletions testsrc/org/mozilla/javascript/tests/StringifyJavaObjectsTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package org.mozilla.javascript.tests;

import org.mozilla.javascript.Context;
import org.mozilla.javascript.drivers.LanguageVersion;
import org.mozilla.javascript.drivers.RhinoTest;
import org.mozilla.javascript.drivers.ScriptTestsBase;

@RhinoTest("testsrc/jstests/stringify-java-objects.js")
@LanguageVersion(Context.VERSION_ES6)
public class StringifyJavaObjectsTest extends ScriptTestsBase {}