From bd373c84ff906d7062ea023010a5f77c9f22c73b Mon Sep 17 00:00:00 2001 From: arimu1 <19286898+arimu1@users.noreply.github.com> Date: Wed, 2 Sep 2026 22:49:52 +0700 Subject: [PATCH 1/2] Detect cyclic references during JSONObject/JSONArray serialization Thread an identity-based serialization stack through write paths so programmatic cycles throw JSONException instead of StackOverflowError. Re-throw cyclic-reference JSONException from toString() while preserving existing null-return behavior for other serialization failures. Fixes #1056 --- src/main/java/org/json/JSONArray.java | 27 +++++++-- src/main/java/org/json/JSONObject.java | 59 +++++++++++++++---- .../java/org/json/junit/JSONArrayTest.java | 8 +++ .../java/org/json/junit/JSONObjectTest.java | 36 +++++++++++ 4 files changed, 115 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/json/JSONArray.java b/src/main/java/org/json/JSONArray.java index 0d7fde9df..08255a3bc 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; /** @@ -1762,6 +1763,8 @@ public JSONObject toJSONObject(JSONArray names) throws JSONException { public String toString() { try { return this.toString(0); + } catch (JSONException e) { + throw e; } catch (Exception e) { return null; } @@ -1848,13 +1851,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 new JSONException(JSONObject.CYCLIC_REFERENCE_MESSAGE); + } 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 +1877,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 +1889,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 +1905,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.CYCLIC_REFERENCE_MESSAGE.equals(e.getMessage())) { + 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..c52ce0a7a 100644 --- a/src/main/java/org/json/JSONObject.java +++ b/src/main/java/org/json/JSONObject.java @@ -2967,6 +2967,8 @@ public JSONArray toJSONArray(JSONArray names) throws JSONException { public String toString() { try { return this.toString(0); + } catch (JSONException e) { + throw e; } catch (Exception e) { return null; } @@ -3140,9 +3142,27 @@ 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"; + + private static JSONException cyclicReferenceDuringSerializationException() { + return new JSONException(CYCLIC_REFERENCE_MESSAGE); + } + @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 +3180,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 +3268,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 +3290,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 +3316,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 +3333,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 +3358,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 (CYCLIC_REFERENCE_MESSAGE.equals(e.getMessage())) { + 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/junit/JSONArrayTest.java b/src/test/java/org/json/junit/JSONArrayTest.java index c54b61795..ce23a1968 100644 --- a/src/test/java/org/json/junit/JSONArrayTest.java +++ b/src/test/java/org/json/junit/JSONArrayTest.java @@ -1526,6 +1526,14 @@ public void testRecursiveDepthArrayFor900Levels() { } } + @Test(expected = JSONException.class) + public void issue1056SelfReferentialJSONArray() { + JSONArray arr = new JSONArray(); + arr.put("x"); + arr.put(arr); + 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..ce996133e 100644 --- a/src/test/java/org/json/junit/JSONObjectTest.java +++ b/src/test/java/org/json/junit/JSONObjectTest.java @@ -3517,6 +3517,42 @@ public void issue743SerializationMap() { String jsonString = object.toString(); } + @Test(expected = JSONException.class) + public void issue1056SelfReferentialJSONObject() { + JSONObject jo = new JSONObject(); + jo.put("key", "value"); + jo.put("self", jo); + jo.toString(); + } + + @Test(expected = JSONException.class) + public void issue1056MutualJSONObjectCycle() { + JSONObject a = new JSONObject(); + JSONObject b = new JSONObject(); + a.put("b", b); + b.put("a", a); + a.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 issue1056ValueToStringOnCyclicJSONObject() { + JSONObject jo = new JSONObject(); + jo.put("self", jo); + JSONObject.valueToString(jo); + } + @Test(expected = JSONException.class) public void testCircularReferenceMultipleLevel() { HashMap inside = new HashMap<>(); From 4d225d21bd3adbdc1d811f8b5b1b6d6ffe2ef776 Mon Sep 17 00:00:00 2001 From: arimu1 <19286898+arimu1@users.noreply.github.com> Date: Wed, 2 Sep 2026 23:05:30 +0700 Subject: [PATCH 2/2] fix(serialize): use typed cyclic exception and preserve toString null contract Add package-private CyclicReferenceException and detect cycles via instanceof instead of message matching. Re-throw only cyclic failures from no-arg toString(); other JSONException paths still return null. Strengthen tests with typed assertions, BrokenToString null checks, and mixed object/array cycle. Fixes review feedback on #1075 --- src/main/java/org/json/JSONArray.java | 13 ++-- src/main/java/org/json/JSONObject.java | 33 +++++++++-- .../Issue1056CyclicSerializationTest.java | 59 +++++++++++++++++++ .../java/org/json/junit/JSONArrayTest.java | 10 ++-- .../java/org/json/junit/JSONObjectTest.java | 26 ++------ 5 files changed, 104 insertions(+), 37 deletions(-) create mode 100644 src/test/java/org/json/Issue1056CyclicSerializationTest.java diff --git a/src/main/java/org/json/JSONArray.java b/src/main/java/org/json/JSONArray.java index 08255a3bc..fad76b44b 100644 --- a/src/main/java/org/json/JSONArray.java +++ b/src/main/java/org/json/JSONArray.java @@ -1751,20 +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) { - throw e; + if (JSONObject.isCyclicReferenceException(e)) { + throw e; + } + return null; } catch (Exception e) { return null; } @@ -1857,7 +1862,7 @@ public Writer write(Writer writer, int indentFactor, int indent) Writer write(Writer writer, int indentFactor, int indent, Set serializationStack) throws JSONException { if (!serializationStack.add(this)) { - throw new JSONException(JSONObject.CYCLIC_REFERENCE_MESSAGE); + throw JSONObject.cyclicReferenceDuringSerializationException(); } try { boolean needsComma = false; @@ -1911,7 +1916,7 @@ private void writeArrayAttempt(Writer writer, int indentFactor, int indent, int JSONObject.writeValue(writer, this.myArrayList.get(i), indentFactor, indent, serializationStack); } catch (JSONException e) { - if (JSONObject.CYCLIC_REFERENCE_MESSAGE.equals(e.getMessage())) { + if (JSONObject.isCyclicReferenceException(e)) { throw 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 c52ce0a7a..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,13 +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) { - throw e; + if (isCyclicReferenceException(e)) { + throw e; + } + return null; } catch (Exception e) { return null; } @@ -3149,8 +3154,26 @@ static Set newSerializationStack() { static final String CYCLIC_REFERENCE_MESSAGE = "Cyclic reference detected during serialization"; - private static JSONException cyclicReferenceDuringSerializationException() { - return new JSONException(CYCLIC_REFERENCE_MESSAGE); + 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") @@ -3363,7 +3386,7 @@ private static void attemptWriteValue(Writer writer, int indentFactor, int inden try{ writeValue(writer, entry.getValue(), indentFactor, indent, serializationStack); } catch (JSONException e) { - if (CYCLIC_REFERENCE_MESSAGE.equals(e.getMessage())) { + if (isCyclicReferenceException(e)) { throw 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 ce23a1968..d79ca27da 100644 --- a/src/test/java/org/json/junit/JSONArrayTest.java +++ b/src/test/java/org/json/junit/JSONArrayTest.java @@ -1526,12 +1526,10 @@ public void testRecursiveDepthArrayFor900Levels() { } } - @Test(expected = JSONException.class) - public void issue1056SelfReferentialJSONArray() { - JSONArray arr = new JSONArray(); - arr.put("x"); - arr.put(arr); - arr.toString(); + @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) diff --git a/src/test/java/org/json/junit/JSONObjectTest.java b/src/test/java/org/json/junit/JSONObjectTest.java index ce996133e..28c929901 100644 --- a/src/test/java/org/json/junit/JSONObjectTest.java +++ b/src/test/java/org/json/junit/JSONObjectTest.java @@ -3517,21 +3517,10 @@ public void issue743SerializationMap() { String jsonString = object.toString(); } - @Test(expected = JSONException.class) - public void issue1056SelfReferentialJSONObject() { - JSONObject jo = new JSONObject(); - jo.put("key", "value"); - jo.put("self", jo); - jo.toString(); - } - - @Test(expected = JSONException.class) - public void issue1056MutualJSONObjectCycle() { - JSONObject a = new JSONObject(); - JSONObject b = new JSONObject(); - a.put("b", b); - b.put("a", a); - a.toString(); + @Test + public void issue1056BrokenToStringStillReturnsNullFromToString() { + JSONObject jo = new JSONObject().put("k", new BrokenToString()); + assertNull(jo.toString()); } @Test @@ -3546,13 +3535,6 @@ public void issue1056DiamondSharedChildStillSerializes() { assertTrue(json.contains("\"x\":1")); } - @Test(expected = JSONException.class) - public void issue1056ValueToStringOnCyclicJSONObject() { - JSONObject jo = new JSONObject(); - jo.put("self", jo); - JSONObject.valueToString(jo); - } - @Test(expected = JSONException.class) public void testCircularReferenceMultipleLevel() { HashMap inside = new HashMap<>();