diff --git a/src/main/java/org/json/JSONArray.java b/src/main/java/org/json/JSONArray.java index 0d7fde9df..fad76b44b 100644 --- a/src/main/java/org/json/JSONArray.java +++ b/src/main/java/org/json/JSONArray.java @@ -14,6 +14,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Set; /** @@ -1750,18 +1751,25 @@ public JSONObject toJSONObject(JSONArray names) throws JSONException { * Make a JSON text of this JSONArray. For compactness, no unnecessary * whitespace is added. If it is not possible to produce a syntactically * correct JSON text then null will be returned instead. This could occur if - * the array contains an invalid number. + * the array contains an invalid number. Cyclic references throw + * {@link JSONException}; other serialization failures still return null. *

* Warning: This method assumes that the data structure is acyclical. * * * @return a printable, displayable, transmittable representation of the * array. + * @throws JSONException if a cyclic reference is detected during serialization */ @Override public String toString() { try { return this.toString(0); + } catch (JSONException e) { + if (JSONObject.isCyclicReferenceException(e)) { + throw e; + } + return null; } catch (Exception e) { return null; } @@ -1848,13 +1856,21 @@ public Writer write(Writer writer) throws JSONException { @SuppressWarnings("resource") public Writer write(Writer writer, int indentFactor, int indent) throws JSONException { + return write(writer, indentFactor, indent, JSONObject.newSerializationStack()); + } + + Writer write(Writer writer, int indentFactor, int indent, Set serializationStack) + throws JSONException { + if (!serializationStack.add(this)) { + throw JSONObject.cyclicReferenceDuringSerializationException(); + } try { boolean needsComma = false; int length = this.length(); writer.write('['); if (length == 1) { - writeArrayAttempt(writer, indentFactor, indent, 0); + writeArrayAttempt(writer, indentFactor, indent, 0, serializationStack); } else if (length != 0) { final int newIndent = indent + indentFactor; @@ -1866,7 +1882,7 @@ public Writer write(Writer writer, int indentFactor, int indent) writer.write('\n'); } JSONObject.indent(writer, newIndent); - writeArrayAttempt(writer, indentFactor, newIndent, i); + writeArrayAttempt(writer, indentFactor, newIndent, i, serializationStack); needsComma = true; } if (indentFactor > 0) { @@ -1878,6 +1894,8 @@ public Writer write(Writer writer, int indentFactor, int indent) return writer; } catch (IOException e) { throw new JSONException(e); + } finally { + serializationStack.remove(this); } } @@ -1892,10 +1910,16 @@ public Writer write(Writer writer, int indentFactor, int indent) * @param i * Index in array to be added */ - private void writeArrayAttempt(Writer writer, int indentFactor, int indent, int i) { + private void writeArrayAttempt(Writer writer, int indentFactor, int indent, int i, + Set serializationStack) { try { JSONObject.writeValue(writer, this.myArrayList.get(i), - indentFactor, indent); + indentFactor, indent, serializationStack); + } catch (JSONException e) { + if (JSONObject.isCyclicReferenceException(e)) { + throw e; + } + throw new JSONException("Unable to write JSONArray value at index: " + i, e); } catch (Exception e) { throw new JSONException("Unable to write JSONArray value at index: " + i, e); } diff --git a/src/main/java/org/json/JSONObject.java b/src/main/java/org/json/JSONObject.java index bcd218e5d..6b53e8573 100644 --- a/src/main/java/org/json/JSONObject.java +++ b/src/main/java/org/json/JSONObject.java @@ -2953,7 +2953,8 @@ public JSONArray toJSONArray(JSONArray names) throws JSONException { /** * Make a JSON text of this JSONObject. For compactness, no whitespace is * added. If this would not result in a syntactically correct JSON text, - * then null will be returned instead. + * then null will be returned instead. Cyclic references throw + * {@link JSONException}; other serialization failures still return null. *

* Warning: This method assumes that the data structure is acyclical. * @@ -2962,11 +2963,17 @@ public JSONArray toJSONArray(JSONArray names) throws JSONException { * of the object, beginning with { (left * brace) and ending with } (right * brace). + * @throws JSONException if a cyclic reference is detected during serialization */ @Override public String toString() { try { return this.toString(0); + } catch (JSONException e) { + if (isCyclicReferenceException(e)) { + throw e; + } + return null; } catch (Exception e) { return null; } @@ -3140,9 +3147,45 @@ public Writer write(Writer writer) throws JSONException { return this.write(writer, 0, 0); } + static Set newSerializationStack() { + return Collections.newSetFromMap(new IdentityHashMap()); + } + + static final String CYCLIC_REFERENCE_MESSAGE = + "Cyclic reference detected during serialization"; + + static final class CyclicReferenceException extends JSONException { + private static final long serialVersionUID = 1L; + + CyclicReferenceException() { + super(CYCLIC_REFERENCE_MESSAGE); + } + } + + static JSONException cyclicReferenceDuringSerializationException() { + return new CyclicReferenceException(); + } + + static boolean isCyclicReferenceException(Throwable throwable) { + while (throwable != null) { + if (throwable instanceof CyclicReferenceException) { + return true; + } + throwable = throwable.getCause(); + } + return false; + } + @SuppressWarnings("resource") static final Writer writeValue(Writer writer, Object value, int indentFactor, int indent) throws JSONException, IOException { + return writeValue(writer, value, indentFactor, indent, newSerializationStack()); + } + + @SuppressWarnings("resource") + static final Writer writeValue(Writer writer, Object value, + int indentFactor, int indent, Set serializationStack) + throws JSONException, IOException { if (value == null || value.equals(null)) { writer.write("null"); } else if (value instanceof JSONString) { @@ -3160,17 +3203,17 @@ static final Writer writeValue(Writer writer, Object value, } else if (value instanceof Enum) { writer.write(quote(((Enum)value).name())); } else if (value instanceof JSONObject) { - ((JSONObject) value).write(writer, indentFactor, indent); + ((JSONObject) value).write(writer, indentFactor, indent, serializationStack); } else if (value instanceof JSONArray) { - ((JSONArray) value).write(writer, indentFactor, indent); + ((JSONArray) value).write(writer, indentFactor, indent, serializationStack); } else if (value instanceof Map) { Map map = (Map) value; - new JSONObject(map).write(writer, indentFactor, indent); + new JSONObject(map).write(writer, indentFactor, indent, serializationStack); } else if (value instanceof Collection) { Collection coll = (Collection) value; - new JSONArray(coll).write(writer, indentFactor, indent); + new JSONArray(coll).write(writer, indentFactor, indent, serializationStack); } else if (value.getClass().isArray()) { - new JSONArray(value).write(writer, indentFactor, indent); + new JSONArray(value).write(writer, indentFactor, indent, serializationStack); } else { quote(value.toString(), writer); } @@ -3248,6 +3291,14 @@ static final void indent(Writer writer, int indent) throws IOException { @SuppressWarnings("resource") public Writer write(Writer writer, int indentFactor, int indent) throws JSONException { + return write(writer, indentFactor, indent, newSerializationStack()); + } + + Writer write(Writer writer, int indentFactor, int indent, Set serializationStack) + throws JSONException { + if (!serializationStack.add(this)) { + throw cyclicReferenceDuringSerializationException(); + } try { boolean needsComma = false; final int length = this.length(); @@ -3262,14 +3313,16 @@ public Writer write(Writer writer, int indentFactor, int indent) writer.write(' '); } // might throw an exception - attemptWriteValue(writer, indentFactor, indent, entry, key); + attemptWriteValue(writer, indentFactor, indent, entry, key, serializationStack); } else if (length != 0) { - writeContent(writer, indentFactor, indent, needsComma); + writeContent(writer, indentFactor, indent, needsComma, serializationStack); } writer.write('}'); return writer; } catch (IOException exception) { throw new JSONException(exception); + } finally { + serializationStack.remove(this); } } @@ -3286,7 +3339,8 @@ public Writer write(Writer writer, int indentFactor, int indent) * @throws IOException * If something goes wrong */ - private void writeContent(Writer writer, int indentFactor, int indent, boolean needsComma) throws IOException { + private void writeContent(Writer writer, int indentFactor, int indent, boolean needsComma, + Set serializationStack) throws IOException { final int newIndent = indent + indentFactor; for (final Entry entry : this.entrySet()) { if (needsComma) { @@ -3302,7 +3356,7 @@ private void writeContent(Writer writer, int indentFactor, int indent, boolean n if (indentFactor > 0) { writer.write(' '); } - attemptWriteValue(writer, indentFactor, newIndent, entry, key); + attemptWriteValue(writer, indentFactor, newIndent, entry, key, serializationStack); needsComma = true; } if (indentFactor > 0) { @@ -3327,9 +3381,15 @@ private void writeContent(Writer writer, int indentFactor, int indent, boolean n * occurs */ - private static void attemptWriteValue(Writer writer, int indentFactor, int indent, Entry entry, String key) { + private static void attemptWriteValue(Writer writer, int indentFactor, int indent, Entry entry, + String key, Set serializationStack) { try{ - writeValue(writer, entry.getValue(), indentFactor, indent); + writeValue(writer, entry.getValue(), indentFactor, indent, serializationStack); + } catch (JSONException e) { + if (isCyclicReferenceException(e)) { + throw e; + } + throw new JSONException("Unable to write JSONObject value for key: " + key, e); } catch (Exception e) { throw new JSONException("Unable to write JSONObject value for key: " + key, e); } diff --git a/src/test/java/org/json/Issue1056CyclicSerializationTest.java b/src/test/java/org/json/Issue1056CyclicSerializationTest.java new file mode 100644 index 000000000..a3475aad8 --- /dev/null +++ b/src/test/java/org/json/Issue1056CyclicSerializationTest.java @@ -0,0 +1,59 @@ +package org.json; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertThrows; + +import org.junit.Test; + +/** + * Tests cyclic-reference detection during serialization. Lives in {@code org.json} + * so tests can assert {@link JSONObject.CyclicReferenceException} by type. + */ +public class Issue1056CyclicSerializationTest { + + @Test + public void selfReferentialJSONObject() { + JSONObject jo = new JSONObject(); + jo.put("key", "value"); + jo.put("self", jo); + JSONException ex = assertThrows(JSONException.class, jo::toString); + assertTrue(ex instanceof JSONObject.CyclicReferenceException); + } + + @Test + public void mutualJSONObjectCycle() { + JSONObject a = new JSONObject(); + JSONObject b = new JSONObject(); + a.put("b", b); + b.put("a", a); + JSONException ex = assertThrows(JSONException.class, a::toString); + assertTrue(ex instanceof JSONObject.CyclicReferenceException); + } + + @Test + public void mixedJSONObjectJSONArrayCycle() { + JSONObject jo = new JSONObject(); + JSONArray arr = new JSONArray(); + jo.put("arr", arr); + arr.put(jo); + JSONException ex = assertThrows(JSONException.class, jo::toString); + assertTrue(ex instanceof JSONObject.CyclicReferenceException); + } + + @Test + public void selfReferentialJSONArray() { + JSONArray arr = new JSONArray(); + arr.put("x"); + arr.put(arr); + JSONException ex = assertThrows(JSONException.class, arr::toString); + assertTrue(ex instanceof JSONObject.CyclicReferenceException); + } + + @Test + public void valueToStringOnCyclicJSONObject() { + JSONObject jo = new JSONObject(); + jo.put("self", jo); + JSONException ex = assertThrows(JSONException.class, () -> JSONObject.valueToString(jo)); + assertTrue(ex instanceof JSONObject.CyclicReferenceException); + } +} diff --git a/src/test/java/org/json/junit/JSONArrayTest.java b/src/test/java/org/json/junit/JSONArrayTest.java index c54b61795..d79ca27da 100644 --- a/src/test/java/org/json/junit/JSONArrayTest.java +++ b/src/test/java/org/json/junit/JSONArrayTest.java @@ -1526,6 +1526,12 @@ public void testRecursiveDepthArrayFor900Levels() { } } + @Test + public void issue1056BrokenToStringStillReturnsNullFromToString() { + JSONArray arr = new JSONArray().put(new JSONObject().put("k", new org.json.junit.data.BrokenToString())); + assertNull(arr.toString()); + } + @Test(expected = JSONException.class) public void testRecursiveDepthArrayFor1001Levels() { ArrayList array = buildNestedArray(1001); diff --git a/src/test/java/org/json/junit/JSONObjectTest.java b/src/test/java/org/json/junit/JSONObjectTest.java index 6b692789e..28c929901 100644 --- a/src/test/java/org/json/junit/JSONObjectTest.java +++ b/src/test/java/org/json/junit/JSONObjectTest.java @@ -3517,6 +3517,24 @@ public void issue743SerializationMap() { String jsonString = object.toString(); } + @Test + public void issue1056BrokenToStringStillReturnsNullFromToString() { + JSONObject jo = new JSONObject().put("k", new BrokenToString()); + assertNull(jo.toString()); + } + + @Test + public void issue1056DiamondSharedChildStillSerializes() { + JSONObject parent = new JSONObject(); + JSONObject child = new JSONObject().put("x", 1); + parent.put("a", child); + parent.put("b", child); + String json = parent.toString(); + assertTrue(json.contains("\"a\":")); + assertTrue(json.contains("\"b\":")); + assertTrue(json.contains("\"x\":1")); + } + @Test(expected = JSONException.class) public void testCircularReferenceMultipleLevel() { HashMap inside = new HashMap<>();