Merge pull request #645 from Zetmas/feature/issue#632

Detect and handle circular references when parsing Java beans

src/main/java/org/json/JSONObject.java

@@ -39,6 +39,7 @@ import java.math.BigInteger;
 import java.util.Collection;
 import java.util.Enumeration;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Locale;
 import java.util.Map;
@@ -365,6 +366,11 @@ public class JSONObject {
         this.populateMap(bean);
     }
 
+    private JSONObject(Object bean, Set<Object> objectsRecord) {
+        this();
+        this.populateMap(bean, objectsRecord);
+    }
+
     /**
      * Construct a JSONObject from an Object, using reflection to find the
      * public members. The resulting JSONObject's keys will be the strings from
@@ -1520,6 +1526,10 @@ public class JSONObject {
      *            the bean
      */
     private void populateMap(Object bean) {
+        populateMap(bean, new HashSet<Object>());
+    }
+
+    private void populateMap(Object bean, Set<Object> objectsRecord) {
         Class<?> klass = bean.getClass();
 
         // If klass is a System class then set includeSuperClass to false.
@@ -1540,10 +1550,22 @@ public class JSONObject {
                     try {
                         final Object result = method.invoke(bean);
                         if (result != null) {
-                            this.map.put(key, wrap(result));
+                            // check cyclic dependency and throw error if needed
+                            // the wrap and populateMap combination method is 
+                            // itself DFS recursive
+                            if (objectsRecord.contains(result)) {
+                                throw recursivelyDefinedObjectException(key);
+                            }
+                            
+                            objectsRecord.add(result);
+
+                            this.map.put(key, wrap(result, objectsRecord));
+
+                            objectsRecord.remove(result);
+
                             // we don't use the result anywhere outside of wrap
                             // if it's a resource we should be sure to close it
-                            // after calling toString
+                            // after calling toString 
                             if (result instanceof Closeable) {
                                 try {
                                     ((Closeable) result).close();
@@ -2431,6 +2453,10 @@ public class JSONObject {
      * @return The wrapped value
      */
     public static Object wrap(Object object) {
+        return wrap(object, null);
+    }
+
+    private static Object wrap(Object object, Set<Object> objectsRecord) {
         try {
             if (NULL.equals(object)) {
                 return NULL;
@@ -2465,7 +2491,15 @@ public class JSONObject {
                     || object.getClass().getClassLoader() == null) {
                 return object.toString();
             }
-            return new JSONObject(object);
+            if (objectsRecord != null) {
+                return new JSONObject(object, objectsRecord);
+            }
+            else {
+                return new JSONObject(object);
+            }
+        }
+        catch (JSONException exception) {
+            throw exception;
         } catch (Exception exception) {
             return null;
         }
@@ -2676,4 +2710,15 @@ public class JSONObject {
                 "JSONObject[" + quote(key) + "] is not a " + valueType + " (" + value + ")."
                 , cause);
     }
+
+    /**
+     * Create a new JSONException in a common format for recursive object definition.
+     * @param key name of the key
+     * @return JSONException that can be thrown.
+     */
+    private static JSONException recursivelyDefinedObjectException(String key) {
+        return new JSONException(
+            "JavaBean object contains recursively defined member variable of key " + quote(key)
+        );
+    }
 }

src/test/java/org/json/junit/JSONObjectTest.java

@@ -73,6 +73,7 @@ import org.json.junit.data.MyJsonString;
 import org.json.junit.data.MyNumber;
 import org.json.junit.data.MyNumberContainer;
 import org.json.junit.data.MyPublicClass;
+import org.json.junit.data.RecursiveBean;
 import org.json.junit.data.Singleton;
 import org.json.junit.data.SingletonEnum;
 import org.json.junit.data.WeirdList;
@@ -3218,6 +3219,99 @@ public class JSONObjectTest {
         jsonObject.put(null, new Object());
         fail("Expected an exception");
     }
+    @Test(expected=JSONException.class)
+    public void testSelfRecursiveObject() {
+        // A -> A ...
+        RecursiveBean ObjA = new RecursiveBean("ObjA");
+        ObjA.setRef(ObjA);
+        new JSONObject(ObjA);
+        fail("Expected an exception");
+    }
+    @Test(expected=JSONException.class)
+    public void testLongSelfRecursiveObject() {
+        // B -> A -> A ...
+        RecursiveBean ObjA = new RecursiveBean("ObjA");
+        RecursiveBean ObjB = new RecursiveBean("ObjB");
+        ObjB.setRef(ObjA);
+        ObjA.setRef(ObjA);
+        new JSONObject(ObjB);
+        fail("Expected an exception");
+    }
+    @Test(expected=JSONException.class)
+    public void testSimpleRecursiveObject() {
+        // B -> A -> B ...
+        RecursiveBean ObjA = new RecursiveBean("ObjA");
+        RecursiveBean ObjB = new RecursiveBean("ObjB");
+        ObjB.setRef(ObjA);
+        ObjA.setRef(ObjB);
+        new JSONObject(ObjA);
+        fail("Expected an exception");
+    }
+    @Test(expected=JSONException.class)
+    public void testLongRecursiveObject() {
+        // D -> C -> B -> A -> D ...
+        RecursiveBean ObjA = new RecursiveBean("ObjA");
+        RecursiveBean ObjB = new RecursiveBean("ObjB");
+        RecursiveBean ObjC = new RecursiveBean("ObjC");
+        RecursiveBean ObjD = new RecursiveBean("ObjD");
+        ObjC.setRef(ObjB);
+        ObjB.setRef(ObjA);
+        ObjD.setRef(ObjC);
+        ObjA.setRef(ObjD);
+        new JSONObject(ObjB);
+        fail("Expected an exception");
+    }
+    @Test(expected=JSONException.class)
+    public void testRepeatObjectRecursive() {
+        // C -> B -> A -> D -> C ...
+        //        -> D -> C ...
+        RecursiveBean ObjA = new RecursiveBean("ObjA");
+        RecursiveBean ObjB = new RecursiveBean("ObjB");
+        RecursiveBean ObjC = new RecursiveBean("ObjC");
+        RecursiveBean ObjD = new RecursiveBean("ObjD");
+        ObjC.setRef(ObjB);
+        ObjB.setRef(ObjA);
+        ObjB.setRef2(ObjD);
+        ObjA.setRef(ObjD);
+        ObjD.setRef(ObjC);
+        new JSONObject(ObjC);
+        fail("Expected an exception");
+    }
+    @Test
+    public void testRepeatObjectNotRecursive() {
+        // C -> B -> A
+        //        -> A
+        RecursiveBean ObjA = new RecursiveBean("ObjA");
+        RecursiveBean ObjB = new RecursiveBean("ObjB");
+        RecursiveBean ObjC = new RecursiveBean("ObjC");
+        ObjC.setRef(ObjA);
+        ObjB.setRef(ObjA);
+        ObjB.setRef2(ObjA);
+        new JSONObject(ObjC);
+        new JSONObject(ObjB);
+        new JSONObject(ObjA);
+    }
+    @Test
+    public void testLongRepeatObjectNotRecursive() {
+        // C -> B -> A -> D -> E
+        //        -> D -> E
+        RecursiveBean ObjA = new RecursiveBean("ObjA");
+        RecursiveBean ObjB = new RecursiveBean("ObjB");
+        RecursiveBean ObjC = new RecursiveBean("ObjC");
+        RecursiveBean ObjD = new RecursiveBean("ObjD");
+        RecursiveBean ObjE = new RecursiveBean("ObjE");
+        ObjC.setRef(ObjB);
+        ObjB.setRef(ObjA);
+        ObjB.setRef2(ObjD);
+        ObjA.setRef(ObjD);
+        ObjD.setRef(ObjE);
+        new JSONObject(ObjC);
+        new JSONObject(ObjB);
+        new JSONObject(ObjA);
+        new JSONObject(ObjD);
+        new JSONObject(ObjE);
+    }
+
 
     @Test
     public void testIssue548ObjectWithEmptyJsonArray() {

src/test/java/org/json/junit/data/RecursiveBean.java

@@ -0,0 +1,23 @@
+package org.json.junit.data;
+
+/**
+ * test class for verifying if recursively defined bean can be correctly identified
+ * @author Zetmas
+ *
+ */
+public class RecursiveBean {
+    private String name;
+    private Object reference;
+    private Object reference2;
+    public String getName() { return name; }
+    public Object getRef() {return reference;}
+    public Object getRef2() {return reference2;}
+    public void setRef(Object refObj) {reference = refObj;}
+    public void setRef2(Object refObj) {reference2 = refObj;}
+
+    public RecursiveBean(String name) {
+        this.name = name;
+        reference = null;
+        reference2 = null;
+    }
+}