From 9216a193660cc9edd5e0070296d71408ab18349a Mon Sep 17 00:00:00 2001 From: rikkarth Date: Sat, 27 Apr 2024 22:14:35 +0100 Subject: [PATCH] feat(#877): improved JSONArray and JSONTokener logic JSONArray construction improved to recursive validation JSONTokener implemented smallCharMemory and array level for improved validation Added new test cases and minor test case adaption --- src/main/java/org/json/JSONArray.java | 118 ++++++++---------- src/main/java/org/json/JSONTokener.java | 50 +++++++- .../java/org/json/junit/JSONArrayTest.java | 6 +- .../junit/JSONParserConfigurationTest.java | 50 +++++++- 4 files changed, 148 insertions(+), 76 deletions(-) diff --git a/src/main/java/org/json/JSONArray.java b/src/main/java/org/json/JSONArray.java index 7177b65..bcd95f9 100644 --- a/src/main/java/org/json/JSONArray.java +++ b/src/main/java/org/json/JSONArray.java @@ -96,87 +96,75 @@ public class JSONArray implements Iterable { */ public JSONArray(JSONTokener x, JSONParserConfiguration jsonParserConfiguration) throws JSONException { this(); - if (x.nextClean() != '[') { + char nextChar = x.nextClean(); + + // check first character, if not '[' throw JSONException + if (nextChar != '[') { throw x.syntaxError("A JSONArray text must start with '['"); } - char nextChar = x.nextClean(); - if (nextChar == 0) { - // array is unclosed. No ']' found, instead EOF - throw x.syntaxError("Expected a ',' or ']'"); - } - if (nextChar != ']') { - x.back(); - for (;;) { - if (x.nextClean() == ',') { - x.back(); - this.myArrayList.add(JSONObject.NULL); - } else { - x.back(); - this.myArrayList.add(x.nextValue(jsonParserConfiguration)); + parseTokener(x, jsonParserConfiguration); // runs recursively + + } + + private void parseTokener(JSONTokener x, JSONParserConfiguration jsonParserConfiguration) { + boolean strictMode = jsonParserConfiguration.isStrictMode(); + + char cursor = x.nextClean(); + + switch (cursor) { + case 0: + throwErrorIfEoF(x); + break; + case ',': + cursor = x.nextClean(); + + throwErrorIfEoF(x); + + if (cursor == ']') { + break; } - switch (x.nextClean()) { - case 0: - // array is unclosed. No ']' found, instead EOF - throw x.syntaxError("Expected a ',' or ']'"); - case ',': - nextChar = x.nextClean(); - if (nextChar == 0) { - // array is unclosed. No ']' found, instead EOF - throw x.syntaxError("Expected a ',' or ']'"); - } - if (nextChar == ']') { - return; - } - x.back(); - break; - case ']': - if (jsonParserConfiguration.isStrictMode()) { - nextChar = x.nextClean(); - if (nextChar == ','){ - x.back(); - return; - } + x.back(); - if (nextChar == ']'){ - x.back(); - return; - } + parseTokener(x, jsonParserConfiguration); + break; + case ']': + if (strictMode) { + cursor = x.nextClean(); + boolean isNotEoF = !x.end(); - if (nextChar != 0) { - throw x.syntaxError("invalid character found after end of array: " + nextChar); - } - } + if (isNotEoF && x.getArrayLevel() == 0) { + throw x.syntaxError(String.format("invalid character '%s' found after end of array", cursor)); + } - return; - default: - throw x.syntaxError("Expected a ',' or ']'"); + x.back(); } - } - } + break; + default: + x.back(); + boolean currentCharIsQuote = x.getPrevious() == '"'; + boolean quoteIsNotNextToValidChar = x.getPreviousChar() != ',' && x.getPreviousChar() != '['; - if (jsonParserConfiguration.isStrictMode()) { - validateInput(x); + if (strictMode && currentCharIsQuote && quoteIsNotNextToValidChar) { + throw x.syntaxError(String.format("invalid character '%s' found after end of array", cursor)); + } + + this.myArrayList.add(x.nextValue(jsonParserConfiguration)); + parseTokener(x, jsonParserConfiguration); } } /** - * Checks if Array adheres to strict mode guidelines, if not, throws JSONException providing back the input in the - * error message. + * Throws JSONException if JSONTokener has reached end of file, usually when array is unclosed. No ']' found, + * instead EoF. * - * @param x tokener used to examine input. - * @throws JSONException if input is not compliant with strict mode guidelines; + * @param x the JSONTokener being evaluated. + * @throws JSONException if JSONTokener has reached end of file. */ - private void validateInput(JSONTokener x) { - char cursor = x.getPrevious(); - - boolean isEndOfArray = cursor == ']'; - char nextChar = x.nextClean(); - boolean nextCharacterIsNotEoF = nextChar != 0; - - if (isEndOfArray && nextCharacterIsNotEoF) { - throw x.syntaxError(String.format("Provided Array is not compliant with strict mode guidelines: '%s'", nextChar)); + private void throwErrorIfEoF(JSONTokener x) { + if (x.end()) { + throw x.syntaxError(String.format("Expected a ',' or ']' but instead found '%s'", x.getPrevious())); } } diff --git a/src/main/java/org/json/JSONTokener.java b/src/main/java/org/json/JSONTokener.java index 00b9fbe..106c3cb 100644 --- a/src/main/java/org/json/JSONTokener.java +++ b/src/main/java/org/json/JSONTokener.java @@ -2,6 +2,8 @@ package org.json; import java.io.*; import java.nio.charset.Charset; +import java.util.ArrayList; +import java.util.List; /* Public Domain. @@ -31,6 +33,8 @@ public class JSONTokener { private boolean usePrevious; /** the number of characters read in the previous line. */ private long characterPreviousLine; + private final List smallCharMemory; + private int arrayLevel = 0; /** @@ -49,6 +53,7 @@ public class JSONTokener { this.character = 1; this.characterPreviousLine = 0; this.line = 1; + this.smallCharMemory = new ArrayList<>(2); } @@ -186,6 +191,46 @@ public class JSONTokener { return this.previous; } + private void insertCharacterInCharMemory(Character c) { + boolean foundSameCharRef = checkForEqualCharRefInMicroCharMemory(c); + if(foundSameCharRef){ + return; + } + + if(smallCharMemory.size() < 2){ + smallCharMemory.add(c); + return; + } + + smallCharMemory.set(0, smallCharMemory.get(1)); + smallCharMemory.remove(1); + smallCharMemory.add(c); + } + + private boolean checkForEqualCharRefInMicroCharMemory(Character c) { + boolean isNotEmpty = !smallCharMemory.isEmpty(); + if (isNotEmpty) { + Character lastChar = smallCharMemory.get(smallCharMemory.size() - 1); + return c.compareTo(lastChar) == 0; + } + + // list is empty so there's no equal characters + return false; + } + + /** + * Retrieves the previous char from memory. + * + * @return previous char stored in memory. + */ + public char getPreviousChar() { + return smallCharMemory.get(0); + } + + public int getArrayLevel(){ + return this.arrayLevel; + } + /** * Get the last character read from the input or '\0' if nothing has been read yet. * @return the last character read from the input. @@ -263,7 +308,6 @@ public class JSONTokener { return new String(chars); } - /** * Get the next char in the string, skipping whitespace. * @throws JSONException Thrown if there is an error reading the source string. @@ -273,6 +317,7 @@ public class JSONTokener { for (;;) { char c = this.next(); if (c == 0 || c > ' ') { + insertCharacterInCharMemory(c); return c; } } @@ -441,6 +486,7 @@ public class JSONTokener { case '[': this.back(); try { + this.arrayLevel++; return new JSONArray(this, jsonParserConfiguration); } catch (StackOverflowError e) { throw new JSONException("JSON Array or Object depth too large to process.", e); @@ -531,7 +577,7 @@ public class JSONTokener { return value; } - throw new JSONException(String.format("Value is not surrounded by quotes: %s", value)); + throw this.syntaxError(String.format("Value '%s' is not surrounded by quotes", value)); } /** diff --git a/src/test/java/org/json/junit/JSONArrayTest.java b/src/test/java/org/json/junit/JSONArrayTest.java index fcaa8ce..510e546 100644 --- a/src/test/java/org/json/junit/JSONArrayTest.java +++ b/src/test/java/org/json/junit/JSONArrayTest.java @@ -142,7 +142,7 @@ public class JSONArrayTest { assertNull("Should throw an exception", new JSONArray("[")); } catch (JSONException e) { assertEquals("Expected an exception message", - "Expected a ',' or ']' at 1 [character 2 line 1]", + "Expected a ',' or ']' but instead found '[' at 1 [character 2 line 1]", e.getMessage()); } } @@ -157,7 +157,7 @@ public class JSONArrayTest { assertNull("Should throw an exception", new JSONArray("[\"test\"")); } catch (JSONException e) { assertEquals("Expected an exception message", - "Expected a ',' or ']' at 7 [character 8 line 1]", + "Expected a ',' or ']' but instead found '\"' at 7 [character 8 line 1]", e.getMessage()); } } @@ -172,7 +172,7 @@ public class JSONArrayTest { assertNull("Should throw an exception", new JSONArray("[\"test\",")); } catch (JSONException e) { assertEquals("Expected an exception message", - "Expected a ',' or ']' at 8 [character 9 line 1]", + "Expected a ',' or ']' but instead found ',' at 8 [character 9 line 1]", e.getMessage()); } } diff --git a/src/test/java/org/json/junit/JSONParserConfigurationTest.java b/src/test/java/org/json/junit/JSONParserConfigurationTest.java index 97e55da..6904542 100644 --- a/src/test/java/org/json/junit/JSONParserConfigurationTest.java +++ b/src/test/java/org/json/junit/JSONParserConfigurationTest.java @@ -46,6 +46,17 @@ public class JSONParserConfigurationTest { () -> new JSONArray(testCase, jsonParserConfiguration))); } + @Test + public void givenEmptyArray_testStrictModeTrue_shouldNotThrowJsonException(){ + JSONParserConfiguration jsonParserConfiguration = new JSONParserConfiguration() + .withStrictMode(true); + + String testCase = "[]"; + + JSONArray jsonArray = new JSONArray(testCase, jsonParserConfiguration); + System.out.println(jsonArray); + } + @Test public void givenValidDoubleArray_testStrictModeTrue_shouldNotThrowJsonException() { JSONParserConfiguration jsonParserConfiguration = new JSONParserConfiguration() @@ -63,6 +74,30 @@ public class JSONParserConfigurationTest { assertTrue(arrayShouldContainBooleanAt0.get(0) instanceof Boolean); } + @Test + public void givenValidEmptyArrayInsideArray_testStrictModeTrue_shouldNotThrowJsonException(){ + JSONParserConfiguration jsonParserConfiguration = new JSONParserConfiguration() + .withStrictMode(true); + + String testCase = "[[]]"; + + JSONArray jsonArray = new JSONArray(testCase, jsonParserConfiguration); + + assertEquals(testCase, jsonArray.toString()); + } + + @Test + public void givenValidEmptyArrayInsideArray_testStrictModeFalse_shouldNotThrowJsonException(){ + JSONParserConfiguration jsonParserConfiguration = new JSONParserConfiguration() + .withStrictMode(false); + + String testCase = "[[]]"; + + JSONArray jsonArray = new JSONArray(testCase, jsonParserConfiguration); + + assertEquals(testCase, jsonArray.toString()); + } + @Test public void givenInvalidString_testStrictModeTrue_shouldThrowJsonException() { JSONParserConfiguration jsonParserConfiguration = new JSONParserConfiguration() @@ -72,7 +107,7 @@ public class JSONParserConfigurationTest { JSONException je = assertThrows(JSONException.class, () -> new JSONArray(testCase, jsonParserConfiguration)); - assertEquals("Value is not surrounded by quotes: badString", je.getMessage()); + assertEquals("Value 'badString' is not surrounded by quotes at 10 [character 11 line 1]", je.getMessage()); } @Test @@ -121,7 +156,7 @@ public class JSONParserConfigurationTest { 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("invalid character ';' found after end of array at 6 [character 7 line 1]", je.getMessage()); } @Test @@ -134,7 +169,7 @@ public class JSONParserConfigurationTest { 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("invalid character ';' found after end of array at 10 [character 11 line 1]", je.getMessage()); } @Test @@ -147,7 +182,7 @@ public class JSONParserConfigurationTest { JSONException je = assertThrows("expected non-compliant array but got instead: " + testCase, JSONException.class, () -> new JSONArray(testCase, jsonParserConfiguration)); - assertEquals("Value is not surrounded by quotes: implied", je.getMessage()); + assertEquals("Value 'implied' is not surrounded by quotes at 17 [character 18 line 1]", je.getMessage()); } @Test @@ -206,7 +241,7 @@ public class JSONParserConfigurationTest { JSONException jeTwo = assertThrows(JSONException.class, () -> new JSONArray(testCaseTwo, jsonParserConfiguration)); - assertEquals("Expected a ',' or ']' at 10 [character 11 line 1]", jeOne.getMessage()); + assertEquals("Unterminated string. Character with int code 0 is not allowed within a quoted string. at 15 [character 16 line 1]", jeOne.getMessage()); assertEquals("Unterminated string. Character with int code 0 is not allowed within a quoted string. at 15 [character 16 line 1]", jeTwo.getMessage()); } @@ -220,7 +255,7 @@ public class JSONParserConfigurationTest { JSONException je = assertThrows("expected non-compliant array but got instead: " + testCase, JSONException.class, () -> new JSONArray(testCase, jsonParserConfiguration)); - assertEquals(String.format("Value is not surrounded by quotes: %s", "test"), je.getMessage()); + assertEquals("Value 'test' is not surrounded by quotes at 6 [character 7 line 1]", je.getMessage()); } @Test @@ -251,6 +286,9 @@ public class JSONParserConfigurationTest { */ private List getNonCompliantJSONList() { return Arrays.asList( + "[1],", + "[[1]\"sa\",[2]]a", + "[1],\"dsa\": \"test\"", "[[a]]", "[]asdf", "[]]",