diff --git a/src/main/java/org/json/JSONArray.java b/src/main/java/org/json/JSONArray.java index 3be3e1451..59588a5d4 100644 --- a/src/main/java/org/json/JSONArray.java +++ b/src/main/java/org/json/JSONArray.java @@ -15,6 +15,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Set; /** @@ -149,11 +150,27 @@ public JSONArray(String source) throws JSONException { * A Collection. */ public JSONArray(Collection collection) { + this(collection, null); + } + + /** + * Construct a JSONArray from a Collection, with an objectsRecord + * set to identify recursively defined objects. + * + * It's package-private because it is called exclusively by + * {@code JSONObject#wrap(Object, Set)}. + * + * @param collection + * A Collection. + * @param objectsRecord + * The set of already recorded objects + */ + JSONArray(Collection collection, Set objectsRecord) { if (collection == null) { this.myArrayList = new ArrayList(); } else { this.myArrayList = new ArrayList(collection.size()); - this.addAll(collection, true); + this.addAll(collection, true, objectsRecord); } } @@ -1577,7 +1594,8 @@ public List toList() { public boolean isEmpty() { return this.myArrayList.isEmpty(); } - + + /** * Add a collection's elements to the JSONArray. * @@ -1589,10 +1607,26 @@ public boolean isEmpty() { * */ private void addAll(Collection collection, boolean wrap) { + this.addAll(collection, wrap, null); + } + + /** + * In addition to adding a collection to the JSONArray in the manner of {@code #addAll(Collection, boolean)}, + * propagates the objectsRecord to {@code JSONObject#wrap(Object, Set)} to identify recursively defined objects. + * + * @param collection + * A Collection. + * @param wrap + * {@code true} to call {@link JSONObject#wrap(Object)} for each item, + * {@code false} to add the items directly + * @param objectsRecord + * The set of already recorded objects + */ + private void addAll(Collection collection, boolean wrap, Set objectsRecord) { this.myArrayList.ensureCapacity(this.myArrayList.size() + collection.size()); if (wrap) { for (Object o: collection){ - this.put(JSONObject.wrap(o)); + this.put(JSONObject.wrap(o, objectsRecord)); } } else { for (Object o: collection){ diff --git a/src/main/java/org/json/JSONObject.java b/src/main/java/org/json/JSONObject.java index 68d302d56..2dd53a232 100644 --- a/src/main/java/org/json/JSONObject.java +++ b/src/main/java/org/json/JSONObject.java @@ -1545,6 +1545,20 @@ && isValidMethodName(method.getName())) { // itself DFS recursive if (objectsRecord.contains(result)) { throw recursivelyDefinedObjectException(key); + } else if (result instanceof Collection) { + Collection collection = (Collection) result; + // We're modifying the collection to check it, + // so we want to not affect the original collection. + Set set = Collections.newSetFromMap(new IdentityHashMap()); + set.addAll(collection); + + for (Object object : objectsRecord) { + if (object instanceof Collection + && set.removeAll((Collection) object) + || set.remove(object)) { + throw recursivelyDefinedObjectException(key); + } + } } objectsRecord.add(result); @@ -2452,7 +2466,20 @@ public static Object wrap(Object object) { return wrap(object, null); } - private static Object wrap(Object object, Set objectsRecord) { + /** + * Wraps the object much like {@code #wrap(Object)}, but propagates an objectsRecord set + * to identify recursively defined objects. + * + * It's package private because other classes in this package may need to further propagate + * the set. + * + * @param object + * The object to wrap + * @param objectsRecord + * The set of already recorded objects + * @return The wrapped value + */ + static Object wrap(Object object, Set objectsRecord) { try { if (NULL.equals(object)) { return NULL; @@ -2470,7 +2497,7 @@ private static Object wrap(Object object, Set objectsRecord) { if (object instanceof Collection) { Collection coll = (Collection) object; - return new JSONArray(coll); + return new JSONArray(coll, objectsRecord); } if (object.getClass().isArray()) { return new JSONArray(object); diff --git a/src/test/java/org/json/junit/JSONObjectRecursiveExceptionTest.java b/src/test/java/org/json/junit/JSONObjectRecursiveExceptionTest.java new file mode 100644 index 000000000..b597afd98 --- /dev/null +++ b/src/test/java/org/json/junit/JSONObjectRecursiveExceptionTest.java @@ -0,0 +1,75 @@ +package org.json.junit; + +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.hamcrest.core.StringContains; +import org.json.JSONException; +import org.json.JSONObject; +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +public class JSONObjectRecursiveExceptionTest { + public static class RecursiveList { + public List getRecursiveList() { + return Collections.singletonList(this); + } + } + + public static class RecursiveArray { + public RecursiveArray[] getRecursiveArray() { + return new RecursiveArray[] {this}; + } + } + + public static class RecursiveMap { + public Map getRecursiveMap() { + Map map = new HashMap(); + map.put("test", this); + return map; + } + } + + public static class Recursive { + public Recursive getSelf() { + return this; + } + } + + @Rule + public ExpectedException exceptionRule = ExpectedException.none(); + + @Before + public void before() { + exceptionRule.expect(JSONException.class); + exceptionRule.expectMessage(StringContains.containsString("JavaBean object contains recursively defined member variable of key")); + } + + + @Test + public void testRecursiveList() { + new JSONObject(new RecursiveList()); + } + + @Ignore + @Test + public void testRecursiveArray() { + new JSONObject(new RecursiveArray()); + } + + @Ignore + @Test + public void testRecursiveMap() { + new JSONObject(new RecursiveMap()); + } + + @Test + public void testRecursive() { + new JSONObject(new Recursive()); + } +}