From 2dcef89a6f3a79a0efc2c1e260029a5b26c646a9 Mon Sep 17 00:00:00 2001 From: Sean Leary Date: Sat, 21 Dec 2024 09:50:52 -0600 Subject: [PATCH] Code review action items - add comments and consistent error messages for strict mode --- src/main/java/org/json/JSONArray.java | 32 ++++++++------ src/main/java/org/json/JSONObject.java | 21 ++++++---- .../org/json/JSONParserConfiguration.java | 20 +++++---- src/main/java/org/json/JSONTokener.java | 7 +++- .../java/org/json/junit/JSONArrayTest.java | 2 +- .../java/org/json/junit/JSONObjectTest.java | 2 +- .../junit/JSONParserConfigurationTest.java | 42 ++++++++++++------- 7 files changed, 77 insertions(+), 49 deletions(-) diff --git a/src/main/java/org/json/JSONArray.java b/src/main/java/org/json/JSONArray.java index 759acd7..6458ab2 100644 --- a/src/main/java/org/json/JSONArray.java +++ b/src/main/java/org/json/JSONArray.java @@ -67,8 +67,10 @@ public class JSONArray implements Iterable { */ private final ArrayList myArrayList; + // strict mode checks after constructor require access to this object private JSONTokener jsonTokener; + // strict mode checks after constructor require access to this object private JSONParserConfiguration jsonParserConfiguration; /** @@ -138,8 +140,16 @@ public class JSONArray implements Iterable { throw x.syntaxError("Expected a ',' or ']'"); } if (nextChar == ']') { + // trailing commas are not allowed in strict mode if (jsonParserConfiguration.isStrictMode()) { - throw x.syntaxError("Expected another array element"); + throw x.syntaxError("Strict mode error: Expected another array element"); + } + return; + } + if (nextChar == ',') { + // consecutive commas are not allowed in strict mode + if (jsonParserConfiguration.isStrictMode()) { + throw x.syntaxError("Strict mode error: Expected a valid array element"); } return; } @@ -166,12 +176,10 @@ public class JSONArray implements Iterable { */ public JSONArray(String source) throws JSONException { this(source, new JSONParserConfiguration()); - if (this.jsonParserConfiguration.isStrictMode()) { - char c = jsonTokener.nextClean(); - if (c != 0) { - throw jsonTokener.syntaxError(String.format("invalid character '%s' found after end of array", c)); - - } + // Strict mode does not allow trailing chars + if (this.jsonParserConfiguration.isStrictMode() && + this.jsonTokener.nextClean() != 0) { + throw jsonTokener.syntaxError("Strict mode error: Unparsed characters found at end of input text"); } } @@ -188,12 +196,10 @@ public class JSONArray implements Iterable { */ public JSONArray(String source, JSONParserConfiguration jsonParserConfiguration) throws JSONException { this(new JSONTokener(source), jsonParserConfiguration); - if (this.jsonParserConfiguration.isStrictMode()) { - char c = jsonTokener.nextClean(); - if (c != 0) { - throw jsonTokener.syntaxError(String.format("invalid character '%s' found after end of array", c)); - - } + // Strict mode does not allow trailing chars + if (this.jsonParserConfiguration.isStrictMode() && + this.jsonTokener.nextClean() != 0) { + throw jsonTokener.syntaxError("Strict mode error: Unparsed characters found at end of input text"); } } diff --git a/src/main/java/org/json/JSONObject.java b/src/main/java/org/json/JSONObject.java index f49bab3..3bb6da7 100644 --- a/src/main/java/org/json/JSONObject.java +++ b/src/main/java/org/json/JSONObject.java @@ -152,8 +152,10 @@ public class JSONObject { */ public static final Object NULL = new Null(); + // strict mode checks after constructor require access to this object private JSONTokener jsonTokener; + // strict mode checks after constructor require access to this object private JSONParserConfiguration jsonParserConfiguration; /** @@ -268,13 +270,15 @@ public class JSONObject { switch (x.nextClean()) { case ';': + // In strict mode semicolon is not a valid separator if (jsonParserConfiguration.isStrictMode()) { - throw x.syntaxError("Invalid character ';' found in object in strict mode"); + throw x.syntaxError("Strict mode error: Invalid character ';' found"); } case ',': if (x.nextClean() == '}') { + // trailing commas are not allowed in strict mode if (jsonParserConfiguration.isStrictMode()) { - throw x.syntaxError("Expected another object element"); + throw x.syntaxError("Strict mode error: Expected another object element"); } return; } @@ -452,9 +456,10 @@ public class JSONObject { */ public JSONObject(String source) throws JSONException { this(source, new JSONParserConfiguration()); + // Strict mode does not allow trailing chars if (this.jsonParserConfiguration.isStrictMode() && this.jsonTokener.nextClean() != 0) { - throw new JSONException("Unparsed characters found at end of input text"); + throw new JSONException("Strict mode error: Unparsed characters found at end of input text"); } } @@ -474,12 +479,10 @@ public class JSONObject { */ public JSONObject(String source, JSONParserConfiguration jsonParserConfiguration) throws JSONException { this(new JSONTokener(source), jsonParserConfiguration); - if (this.jsonParserConfiguration.isStrictMode()) { - char c = jsonTokener.nextClean(); - if (c != 0) { - throw jsonTokener.syntaxError(String.format("invalid character '%s' found after end of array", c)); - - } + // Strict mode does not allow trailing chars + if (this.jsonParserConfiguration.isStrictMode() && + this.jsonTokener.nextClean() != 0) { + throw new JSONException("Strict mode error: Unparsed characters found at end of input text"); } } diff --git a/src/main/java/org/json/JSONParserConfiguration.java b/src/main/java/org/json/JSONParserConfiguration.java index 46996cb..f5d0660 100644 --- a/src/main/java/org/json/JSONParserConfiguration.java +++ b/src/main/java/org/json/JSONParserConfiguration.java @@ -64,6 +64,18 @@ public class JSONParserConfiguration extends ParserConfiguration { return clone; } + /** + * Sets the strict mode configuration for the JSON parser with default true value + *

+ * When strict mode is enabled, the parser will throw a JSONException if it encounters an invalid character + * immediately following the final ']' character in the input. This is useful for ensuring strict adherence to the + * JSON syntax, as any characters after the final closing bracket of a JSON array are considered invalid. + * @return a new JSONParserConfiguration instance with the updated strict mode setting + */ + public JSONParserConfiguration withStrictMode() { + return withStrictMode(true); + } + /** * Sets the strict mode configuration for the JSON parser. *

@@ -92,13 +104,7 @@ public class JSONParserConfiguration extends ParserConfiguration { } /** - * Retrieves the current strict mode setting of the JSON parser. - *

- * Strict mode, when enabled, instructs the parser to throw a JSONException if it encounters an invalid character - * immediately following the final ']' character in the input. This ensures strict adherence to the JSON syntax, as - * any characters after the final closing bracket of a JSON array are considered invalid. - * - * @return the current strict mode setting. True if strict mode is enabled, false otherwise. + * @return the current strict mode setting. */ public boolean isStrictMode() { return this.strictMode; diff --git a/src/main/java/org/json/JSONTokener.java b/src/main/java/org/json/JSONTokener.java index 2fcc24a..d4c780e 100644 --- a/src/main/java/org/json/JSONTokener.java +++ b/src/main/java/org/json/JSONTokener.java @@ -32,6 +32,7 @@ public class JSONTokener { /** the number of characters read in the previous line. */ private long characterPreviousLine; + // access to this object is required for strict mode checking private JSONParserConfiguration jsonParserConfiguration; /** @@ -443,10 +444,11 @@ public class JSONTokener { Object nextSimpleValue(char c) { String string; + // Strict mode only allows strings with explicit double quotes if (jsonParserConfiguration != null && jsonParserConfiguration.isStrictMode() && c == '\'') { - throw this.syntaxError("Single quote wrap not allowed in strict mode"); + throw this.syntaxError("Strict mode error: Single quoted strings are not allowed"); } switch (c) { case '"': @@ -477,10 +479,11 @@ public class JSONTokener { throw this.syntaxError("Missing value"); } Object obj = JSONObject.stringToValue(string); + // Strict mode only allows strings with explicit double quotes if (jsonParserConfiguration != null && jsonParserConfiguration.isStrictMode() && obj instanceof String) { - throw this.syntaxError(String.format("Value '%s' is not surrounded by quotes", obj)); + throw this.syntaxError(String.format("Strict mode error: Value '%s' is not surrounded by quotes", obj)); } return obj; } diff --git a/src/test/java/org/json/junit/JSONArrayTest.java b/src/test/java/org/json/junit/JSONArrayTest.java index ed7c9ba..e6d7147 100644 --- a/src/test/java/org/json/junit/JSONArrayTest.java +++ b/src/test/java/org/json/junit/JSONArrayTest.java @@ -481,7 +481,7 @@ public class JSONArrayTest { System.out.println("Skipping JSONArrayTest unquotedText() when strictMode default is true"); } else { String str = "[value1, something!, (parens), foo@bar.com, 23, 23+45]"; - JSONArray jsonArray = new JSONArray(str); + JSONArray jsonArray = new JSONArray(str); List expected = Arrays.asList("value1", "something!", "(parens)", "foo@bar.com", 23, "23+45"); assertEquals(expected, jsonArray.toList()); } diff --git a/src/test/java/org/json/junit/JSONObjectTest.java b/src/test/java/org/json/junit/JSONObjectTest.java index ad4974b..7f121f3 100644 --- a/src/test/java/org/json/junit/JSONObjectTest.java +++ b/src/test/java/org/json/junit/JSONObjectTest.java @@ -83,7 +83,7 @@ public class JSONObjectTest { Singleton.getInstance().setSomeInt(0); Singleton.getInstance().setSomeString(null); } - + /** * Tests that the similar method is working as expected. */ diff --git a/src/test/java/org/json/junit/JSONParserConfigurationTest.java b/src/test/java/org/json/junit/JSONParserConfigurationTest.java index 84c3911..422c90c 100644 --- a/src/test/java/org/json/junit/JSONParserConfigurationTest.java +++ b/src/test/java/org/json/junit/JSONParserConfigurationTest.java @@ -184,7 +184,8 @@ public class JSONParserConfigurationTest { .withStrictMode(true); String testCase = "[badString]"; JSONException je = assertThrows(JSONException.class, () -> new JSONArray(testCase, jsonParserConfiguration)); - assertEquals("Value 'badString' is not surrounded by quotes at 10 [character 11 line 1]", je.getMessage()); + assertEquals("Strict mode error: Value 'badString' is not surrounded by quotes at 10 [character 11 line 1]", + je.getMessage()); } @Test @@ -193,7 +194,8 @@ public class JSONParserConfigurationTest { .withStrictMode(true); String testCase = "{\"a0\":badString}"; JSONException je = assertThrows(JSONException.class, () -> new JSONObject(testCase, jsonParserConfiguration)); - assertEquals("Value 'badString' is not surrounded by quotes at 15 [character 16 line 1]", je.getMessage()); + assertEquals("Strict mode error: Value 'badString' is not surrounded by quotes at 15 [character 16 line 1]", + je.getMessage()); } @Test @@ -289,7 +291,8 @@ public class JSONParserConfigurationTest { String testCase = "[1,2];[3,4]"; JSONException je = assertThrows("expected non-compliant array but got instead: " + testCase, JSONException.class, () -> new JSONArray(testCase, jsonParserConfiguration)); - assertEquals("invalid character ';' found after end of array at 6 [character 7 line 1]", je.getMessage()); + assertEquals("Strict mode error: Unparsed characters found at end of input text at 6 [character 7 line 1]", + je.getMessage()); } @Test @@ -299,7 +302,7 @@ public class JSONParserConfigurationTest { String testCase = "{\"a0\":[1,2];\"a1\":[3,4]}"; JSONException je = assertThrows("expected non-compliant array but got instead: " + testCase, JSONException.class, () -> new JSONObject(testCase, jsonParserConfiguration)); - assertEquals("Invalid character ';' found in object in strict mode at 12 [character 13 line 1]", je.getMessage()); + assertEquals("Strict mode error: Invalid character ';' found at 12 [character 13 line 1]", je.getMessage()); } @Test @@ -309,7 +312,8 @@ public class JSONParserConfigurationTest { String testCase = "[\"1\",\"2\"];[3,4]"; JSONException je = assertThrows("expected non-compliant array but got instead: " + testCase, JSONException.class, () -> new JSONArray(testCase, jsonParserConfiguration)); - assertEquals("invalid character ';' found after end of array at 10 [character 11 line 1]", je.getMessage()); + assertEquals("Strict mode error: Unparsed characters found at end of input text at 10 [character 11 line 1]", + je.getMessage()); } @Test @@ -319,7 +323,7 @@ public class JSONParserConfigurationTest { String testCase = "{\"a0\":[\"1\",\"2\"];\"a1\":[3,4]}"; JSONException je = assertThrows("expected non-compliant array but got instead: " + testCase, JSONException.class, () -> new JSONObject(testCase, jsonParserConfiguration)); - assertEquals("Invalid character ';' found in object in strict mode at 16 [character 17 line 1]", je.getMessage()); + assertEquals("Strict mode error: Invalid character ';' found at 16 [character 17 line 1]", je.getMessage()); } @Test @@ -329,7 +333,8 @@ public class JSONParserConfigurationTest { String testCase = "[{\"test\": implied}]"; JSONException je = assertThrows("expected non-compliant array but got instead: " + testCase, JSONException.class, () -> new JSONArray(testCase, jsonParserConfiguration)); - assertEquals("Value 'implied' is not surrounded by quotes at 17 [character 18 line 1]", je.getMessage()); + assertEquals("Strict mode error: Value 'implied' is not surrounded by quotes at 17 [character 18 line 1]", + je.getMessage()); } @Test @@ -339,7 +344,8 @@ public class JSONParserConfigurationTest { String testCase = "{\"a0\":{\"test\": implied}]}"; JSONException je = assertThrows("expected non-compliant array but got instead: " + testCase, JSONException.class, () -> new JSONObject(testCase, jsonParserConfiguration)); - assertEquals("Value 'implied' is not surrounded by quotes at 22 [character 23 line 1]", je.getMessage()); + assertEquals("Strict mode error: Value 'implied' is not surrounded by quotes at 22 [character 23 line 1]", + je.getMessage()); } @Test @@ -381,13 +387,13 @@ public class JSONParserConfigurationTest { "Expected a ',' or ']' at 10 [character 11 line 1]", jeOne.getMessage()); assertEquals( - "Single quote wrap not allowed in strict mode at 2 [character 3 line 1]", + "Strict mode error: Single quoted strings are not allowed at 2 [character 3 line 1]", jeTwo.getMessage()); assertEquals( - "Single quote wrap not allowed in strict mode at 2 [character 3 line 1]", + "Strict mode error: Single quoted strings are not allowed at 2 [character 3 line 1]", jeThree.getMessage()); assertEquals( - "Single quote wrap not allowed in strict mode at 3 [character 4 line 1]", + "Strict mode error: Single quoted strings are not allowed at 3 [character 4 line 1]", jeFour.getMessage()); } @@ -414,13 +420,13 @@ public class JSONParserConfigurationTest { "Expected a ':' after a key at 10 [character 11 line 1]", jeOne.getMessage()); assertEquals( - "Single quote wrap not allowed in strict mode at 2 [character 3 line 1]", + "Strict mode error: Single quoted strings are not allowed at 2 [character 3 line 1]", jeTwo.getMessage()); assertEquals( - "Single quote wrap not allowed in strict mode at 6 [character 7 line 1]", + "Strict mode error: Single quoted strings are not allowed at 6 [character 7 line 1]", jeThree.getMessage()); assertEquals( - "Single quote wrap not allowed in strict mode at 2 [character 3 line 1]", + "Strict mode error: Single quoted strings are not allowed at 2 [character 3 line 1]", jeFour.getMessage()); } @@ -467,7 +473,8 @@ public class JSONParserConfigurationTest { JSONException je = assertThrows("expected non-compliant array but got instead: " + testCase, JSONException.class, () -> new JSONArray(testCase, jsonParserConfiguration)); - assertEquals("Value 'test' is not surrounded by quotes at 6 [character 7 line 1]", je.getMessage()); + assertEquals("Strict mode error: Value 'test' is not surrounded by quotes at 6 [character 7 line 1]", + je.getMessage()); } @Test @@ -479,7 +486,8 @@ public class JSONParserConfigurationTest { JSONException je = assertThrows("expected non-compliant json but got instead: " + testCase, JSONException.class, () -> new JSONObject(testCase, jsonParserConfiguration)); - assertEquals("Value 'test' is not surrounded by quotes at 5 [character 6 line 1]", je.getMessage()); + assertEquals("Strict mode error: Value 'test' is not surrounded by quotes at 5 [character 6 line 1]", + je.getMessage()); } /** @@ -492,6 +500,8 @@ public class JSONParserConfigurationTest { return Arrays.asList( "[1],", "[1,]", + "[,]", + "[,,]", "[[1],\"sa\",[2]]a", "[1],\"dsa\": \"test\"", "[[a]]",