diff --git a/src/main/java/org/json/JSONObject.java b/src/main/java/org/json/JSONObject.java index 761df1df0..fd02919f2 100644 --- a/src/main/java/org/json/JSONObject.java +++ b/src/main/java/org/json/JSONObject.java @@ -45,6 +45,7 @@ of this software and associated documentation files (the "Software"), to deal import java.util.Map.Entry; import java.util.ResourceBundle; import java.util.Set; +import java.util.HashSet; import java.util.regex.Pattern; /** @@ -296,6 +297,7 @@ public JSONObject(Map m) { } final Object value = e.getValue(); if (value != null) { + checkForCyclicDependency(value); this.map.put(String.valueOf(e.getKey()), wrap(value)); } } @@ -1520,6 +1522,9 @@ public String optString(String key, String defaultValue) { * the bean */ private void populateMap(Object bean) { + + checkForCyclicDependency(bean); + Class klass = bean.getClass(); // If klass is a System class then set includeSuperClass to false. @@ -2676,4 +2681,79 @@ private static JSONException wrongValueFormatException( "JSONObject[" + quote(key) + "] is not a " + valueType + " (" + value + ")." , cause); } + + /** + * + * @param value, the root node to check for the validity of child nodes + */ + private void checkForCyclicDependency(Object value) { + checkForCyclicDependency(value, new HashSet()); + } + + /** + * @param value, the root node to check for the validity of child nodes + * @param setOfInstanceVariables, the path from root to {@param value} which acts as the ancestors to the current and child nodes. + * @throws JSONException, if there exists a cyclical dependency from ancestorial root to any of the child nodes. + */ + private static void checkForCyclicDependency(Object value, Set setOfInstanceVariables) throws JSONException { + Class klass = value.getClass(); + + // If klass is a System class then set includeSuperClass to false. + boolean includeSuperClass = klass.getClassLoader() != null; + + Method[] methods = includeSuperClass ? klass.getMethods() : klass.getDeclaredMethods(); + for (final Method method : methods) { + final int modifiers = method.getModifiers(); + if (Modifier.isPublic(modifiers) + && !Modifier.isStatic(modifiers) + && method.getParameterTypes().length == 0 + && !method.isBridge() + && method.getReturnType() != Void.TYPE + && isValidMethodName(method.getName())) { + final String key = getKeyNameFromMethod(method); + if (key != null && !key.isEmpty()) { + try { + final Object result = method.invoke(value); + if (result != null) { + // 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 + if (result instanceof Closeable) { + try { + ((Closeable) result).close(); + } catch (IOException ignore) { + } + } + + if (setOfInstanceVariables.contains(result)) { + throw cyclicDependencyFormatException(key); + } + + //adding the currently checked object to the ancestor path + setOfInstanceVariables.add(result); + //DFS type search for checking depdendency. + checkForCyclicDependency(result, setOfInstanceVariables); + + //removed the currently checked object from ancestor path for different root to leaf path + setOfInstanceVariables.remove(result); + } + } catch (IllegalAccessException ignore) { + } catch (IllegalArgumentException ignore) { + } catch (InvocationTargetException ignore) { + } + } + } + } + } + + /** + * Create a new JSONException in a common format for cyclic dependency. + * + * @return JSONException that can be thrown. + */ + private static JSONException cyclicDependencyFormatException(String key) { + return new JSONException( + "JSONObject[" + quote(key) + "] cannot be written because of a Cyclic Dependency"); + } + } diff --git a/src/test/java/org/json/junit/JSONObjectTest.java b/src/test/java/org/json/junit/JSONObjectTest.java index 94c3e4b8f..09e5a87bc 100644 --- a/src/test/java/org/json/junit/JSONObjectTest.java +++ b/src/test/java/org/json/junit/JSONObjectTest.java @@ -58,24 +58,7 @@ of this software and associated documentation files (the "Software"), to deal import org.json.JSONPointerException; import org.json.JSONTokener; import org.json.XML; -import org.json.junit.data.BrokenToString; -import org.json.junit.data.ExceptionalBean; -import org.json.junit.data.Fraction; -import org.json.junit.data.GenericBean; -import org.json.junit.data.GenericBeanInt; -import org.json.junit.data.MyBean; -import org.json.junit.data.MyBeanCustomName; -import org.json.junit.data.MyBeanCustomNameSubClass; -import org.json.junit.data.MyBigNumberBean; -import org.json.junit.data.MyEnum; -import org.json.junit.data.MyEnumField; -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.Singleton; -import org.json.junit.data.SingletonEnum; -import org.json.junit.data.WeirdList; +import org.json.junit.data.*; import org.junit.Test; import com.jayway.jsonpath.Configuration; @@ -3094,8 +3077,8 @@ public void testGenericBean() { final JSONObject jo = new JSONObject(bean); assertEquals(jo.keySet().toString(), 8, jo.length()); assertEquals(42, jo.get("genericValue")); - assertEquals("Expected the getter to only be called once", - 1, bean.genericGetCounter); + assertEquals("Expected the getter to only be called twice (to detect cycles and to get object)", + 2, bean.genericGetCounter); assertEquals(0, bean.genericSetCounter); } @@ -3109,8 +3092,8 @@ public void testGenericIntBean() { final JSONObject jo = new JSONObject(bean); assertEquals(jo.keySet().toString(), 10, jo.length()); assertEquals(42, jo.get("genericValue")); - assertEquals("Expected the getter to only be called once", - 1, bean.genericGetCounter); + assertEquals("Expected the getter to only be called twice (to detect cycles and to get object)", + 2, bean.genericGetCounter); assertEquals(0, bean.genericSetCounter); } @@ -3241,4 +3224,18 @@ public void jsonObjectClearMethodTest() { assertTrue("expected jsonObject.length() == 0", jsonObject.length() == 0); //Check if its length is 0 jsonObject.getInt("key1"); //Should throws org.json.JSONException: JSONObject["asd"] not found } + + @Test(expected = JSONException.class) + public void jsonObjectForCyclicDependency() { + DependencyClassOuter obj = new DependencyClassOuter(); + new JSONObject(obj); + } + + @Test(expected = JSONException.class) + public void jsonObjectMapForCyclicDependency() { + DependencyClassOuter obj = new DependencyClassOuter(); + Map testMap = new HashMap<>(); + testMap.put("test", obj); + new JSONObject(testMap); + } } diff --git a/src/test/java/org/json/junit/data/DependencyClassInner.java b/src/test/java/org/json/junit/data/DependencyClassInner.java new file mode 100644 index 000000000..dd2bc5e48 --- /dev/null +++ b/src/test/java/org/json/junit/data/DependencyClassInner.java @@ -0,0 +1,13 @@ +package org.json.junit.data; + +public class DependencyClassInner { + private final DependencyClassOuter dependencyClassOuter; + + DependencyClassInner(DependencyClassOuter dependencyClassOuter) { + this.dependencyClassOuter = dependencyClassOuter; + } + + public DependencyClassOuter getDependencyClassOuter() { + return dependencyClassOuter; + } +} diff --git a/src/test/java/org/json/junit/data/DependencyClassOuter.java b/src/test/java/org/json/junit/data/DependencyClassOuter.java new file mode 100644 index 000000000..1e876bbed --- /dev/null +++ b/src/test/java/org/json/junit/data/DependencyClassOuter.java @@ -0,0 +1,8 @@ +package org.json.junit.data; + +public class DependencyClassOuter { + + public DependencyClassInner getDependencyClassInner() { + return new DependencyClassInner(this); + } +}