From d7a6f6c1dfd97879ef67fc07f88ee0e83d894d8c Mon Sep 17 00:00:00 2001 From: Bradley Hess Date: Fri, 28 Oct 2016 15:06:32 -0400 Subject: [PATCH] Add JsonParser feature to ignore a trailing comma (fixes #118, #323) --- .../fasterxml/jackson/core/JsonParser.java | 25 +- .../core/json/ReaderBasedJsonParser.java | 75 +++-- .../core/json/UTF8DataInputJsonParser.java | 39 ++- .../core/json/UTF8StreamJsonParser.java | 92 +++-- .../jackson/core/read/TrailingCommasTest.java | 316 ++++++++++++++++++ 5 files changed, 455 insertions(+), 92 deletions(-) create mode 100644 src/test/java/com/fasterxml/jackson/core/read/TrailingCommasTest.java diff --git a/src/main/java/com/fasterxml/jackson/core/JsonParser.java b/src/main/java/com/fasterxml/jackson/core/JsonParser.java index 2fd00111c9..661cad852c 100644 --- a/src/main/java/com/fasterxml/jackson/core/JsonParser.java +++ b/src/main/java/com/fasterxml/jackson/core/JsonParser.java @@ -227,7 +227,30 @@ public enum Feature { * * @since 2.8 */ - ALLOW_MISSING_VALUES(false) + ALLOW_MISSING_VALUES(false), + + /** + * Feature that determines whether {@link JsonParser} will allow for a single trailing + * comma following the final value (in an Array) or member (in an Object). These commas + * will simply be ignored. + *

+ * For example, when this feature is enabled, [true,true,] is equivalent to + * [true, true] and {"a": true,} is equivalent to + * {"a": true}. + *

+ * When combined with ALLOW_MISSING_VALUES, this feature takes priority, and + * the final trailing comma in an array declaration does not imply a missing + * (null) value. For example, when both ALLOW_MISSING_VALUES + * and ALLOW_TRAILING_COMMA are enabled, [true,true,] is + * equivalent to [true, true], and [true,true,,] is equivalent to + * [true, true, null]. + *

+ * Since the JSON specification does not permit trailing commas, this is a non-standard + * feature, and as such disabled by default. + * + * @since 2.9 + */ + ALLOW_TRAILING_COMMA(false) ; /** diff --git a/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java b/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java index dc80f62e6f..b27de2e774 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java +++ b/src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java @@ -652,26 +652,20 @@ public final JsonToken nextToken() throws IOException _binaryValue = null; // Closing scope? - if (i == INT_RBRACKET) { - _updateLocation(); - if (!_parsingContext.inArray()) { - _reportMismatchedEndMarker(i, '}'); - } - _parsingContext = _parsingContext.clearAndGetParent(); - return (_currToken = JsonToken.END_ARRAY); - } - if (i == INT_RCURLY) { - _updateLocation(); - if (!_parsingContext.inObject()) { - _reportMismatchedEndMarker(i, ']'); - } - _parsingContext = _parsingContext.clearAndGetParent(); - return (_currToken = JsonToken.END_OBJECT); + if (i == INT_RBRACKET || i == INT_RCURLY) { + _closeScope(i); + return _currToken; } // Nope: do we then expect a comma? if (_parsingContext.expectComma()) { i = _skipComma(i); + + // Was that a trailing comma? + if (isEnabled(Feature.ALLOW_TRAILING_COMMA) && (i == INT_RBRACKET || i == INT_RCURLY)) { + _closeScope(i); + return _currToken; + } } /* And should we now have a name? Always true for Object contexts, since @@ -811,26 +805,20 @@ public boolean nextFieldName(SerializableString sstr) throws IOException } _binaryValue = null; - if (i == INT_RBRACKET) { - _updateLocation(); - if (!_parsingContext.inArray()) { - _reportMismatchedEndMarker(i, '}'); - } - _parsingContext = _parsingContext.clearAndGetParent(); - _currToken = JsonToken.END_ARRAY; - return false; - } - if (i == INT_RCURLY) { - _updateLocation(); - if (!_parsingContext.inObject()) { - _reportMismatchedEndMarker(i, ']'); - } - _parsingContext = _parsingContext.clearAndGetParent(); - _currToken = JsonToken.END_OBJECT; + // Closing scope? + if (i == INT_RBRACKET || i == INT_RCURLY) { + _closeScope(i); return false; } + if (_parsingContext.expectComma()) { i = _skipComma(i); + + // Was that a trailing comma? + if (isEnabled(Feature.ALLOW_TRAILING_COMMA) && (i == INT_RBRACKET || i == INT_RCURLY)) { + _closeScope(i); + return false; + } } if (!_parsingContext.inObject()) { @@ -2834,4 +2822,29 @@ protected void _reportInvalidToken(String matchedPart, String msg) throws IOExce } _reportError("Unrecognized token '"+sb.toString()+"': was expecting "+msg); } + + /* + /********************************************************** + /* Internal methods, other + /********************************************************** + */ + + private void _closeScope(int i) throws JsonParseException { + if (i == INT_RBRACKET) { + _updateLocation(); + if (!_parsingContext.inArray()) { + _reportMismatchedEndMarker(i, '}'); + } + _parsingContext = _parsingContext.clearAndGetParent(); + _currToken = JsonToken.END_ARRAY; + } + if (i == INT_RCURLY) { + _updateLocation(); + if (!_parsingContext.inObject()) { + _reportMismatchedEndMarker(i, ']'); + } + _parsingContext = _parsingContext.clearAndGetParent(); + _currToken = JsonToken.END_OBJECT; + } + } } diff --git a/src/main/java/com/fasterxml/jackson/core/json/UTF8DataInputJsonParser.java b/src/main/java/com/fasterxml/jackson/core/json/UTF8DataInputJsonParser.java index 8bc3789ac6..de5d081252 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/UTF8DataInputJsonParser.java +++ b/src/main/java/com/fasterxml/jackson/core/json/UTF8DataInputJsonParser.java @@ -575,19 +575,9 @@ public JsonToken nextToken() throws IOException _tokenInputRow = _currInputRow; // Closing scope? - if (i == INT_RBRACKET) { - if (!_parsingContext.inArray()) { - _reportMismatchedEndMarker(i, '}'); - } - _parsingContext = _parsingContext.clearAndGetParent(); - return (_currToken = JsonToken.END_ARRAY); - } - if (i == INT_RCURLY) { - if (!_parsingContext.inObject()) { - _reportMismatchedEndMarker(i, ']'); - } - _parsingContext = _parsingContext.clearAndGetParent(); - return (_currToken = JsonToken.END_OBJECT); + if (i == INT_RBRACKET || i == INT_RCURLY) { + _closeScope(i); + return _currToken; } // Nope: do we then expect a comma? @@ -596,6 +586,12 @@ public JsonToken nextToken() throws IOException _reportUnexpectedChar(i, "was expecting comma to separate "+_parsingContext.typeDesc()+" entries"); } i = _skipWS(); + + // Was that a trailing comma? + if (isEnabled(Feature.ALLOW_TRAILING_COMMA) && (i == INT_RBRACKET || i == INT_RCURLY)) { + _closeScope(i); + return _currToken; + } } /* And should we now have a name? Always true for @@ -2788,6 +2784,23 @@ public JsonLocation getCurrentLocation() { /********************************************************** */ + private void _closeScope(int i) throws JsonParseException { + if (i == INT_RBRACKET) { + if (!_parsingContext.inArray()) { + _reportMismatchedEndMarker(i, '}'); + } + _parsingContext = _parsingContext.clearAndGetParent(); + _currToken = JsonToken.END_ARRAY; + } + if (i == INT_RCURLY) { + if (!_parsingContext.inObject()) { + _reportMismatchedEndMarker(i, ']'); + } + _parsingContext = _parsingContext.clearAndGetParent(); + _currToken = JsonToken.END_OBJECT; + } + } + /** * Helper method needed to fix [Issue#148], masking of 0x00 character */ diff --git a/src/main/java/com/fasterxml/jackson/core/json/UTF8StreamJsonParser.java b/src/main/java/com/fasterxml/jackson/core/json/UTF8StreamJsonParser.java index 5a0dcdafb5..23f52bf4a4 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/UTF8StreamJsonParser.java +++ b/src/main/java/com/fasterxml/jackson/core/json/UTF8StreamJsonParser.java @@ -738,21 +738,9 @@ public JsonToken nextToken() throws IOException _binaryValue = null; // Closing scope? - if (i == INT_RBRACKET) { - _updateLocation(); - if (!_parsingContext.inArray()) { - _reportMismatchedEndMarker(i, '}'); - } - _parsingContext = _parsingContext.clearAndGetParent(); - return (_currToken = JsonToken.END_ARRAY); - } - if (i == INT_RCURLY) { - _updateLocation(); - if (!_parsingContext.inObject()) { - _reportMismatchedEndMarker(i, ']'); - } - _parsingContext = _parsingContext.clearAndGetParent(); - return (_currToken = JsonToken.END_OBJECT); + if (i == INT_RBRACKET || i == INT_RCURLY) { + _closeScope(i); + return _currToken; } // Nope: do we then expect a comma? @@ -761,6 +749,12 @@ public JsonToken nextToken() throws IOException _reportUnexpectedChar(i, "was expecting comma to separate "+_parsingContext.typeDesc()+" entries"); } i = _skipWS(); + + // Was that a trailing comma? + if (isEnabled(Feature.ALLOW_TRAILING_COMMA) && (i == INT_RBRACKET || i == INT_RCURLY)) { + _closeScope(i); + return _currToken; + } } /* And should we now have a name? Always true for @@ -930,22 +924,8 @@ public boolean nextFieldName(SerializableString str) throws IOException _binaryValue = null; // Closing scope? - if (i == INT_RBRACKET) { - _updateLocation(); - if (!_parsingContext.inArray()) { - _reportMismatchedEndMarker(i, '}'); - } - _parsingContext = _parsingContext.clearAndGetParent(); - _currToken = JsonToken.END_ARRAY; - return false; - } - if (i == INT_RCURLY) { - _updateLocation(); - if (!_parsingContext.inObject()) { - _reportMismatchedEndMarker(i, ']'); - } - _parsingContext = _parsingContext.clearAndGetParent(); - _currToken = JsonToken.END_OBJECT; + if (i == INT_RBRACKET || i == INT_RCURLY) { + _closeScope(i); return false; } @@ -955,6 +935,12 @@ public boolean nextFieldName(SerializableString str) throws IOException _reportUnexpectedChar(i, "was expecting comma to separate "+_parsingContext.typeDesc()+" entries"); } i = _skipWS(); + + // Was that a trailing comma? + if (isEnabled(Feature.ALLOW_TRAILING_COMMA) && (i == INT_RBRACKET || i == INT_RCURLY)) { + _closeScope(i); + return false; + } } if (!_parsingContext.inObject()) { @@ -1017,22 +1003,8 @@ public String nextFieldName() throws IOException } _binaryValue = null; - if (i == INT_RBRACKET) { - _updateLocation(); - if (!_parsingContext.inArray()) { - _reportMismatchedEndMarker(i, '}'); - } - _parsingContext = _parsingContext.clearAndGetParent(); - _currToken = JsonToken.END_ARRAY; - return null; - } - if (i == INT_RCURLY) { - _updateLocation(); - if (!_parsingContext.inObject()) { - _reportMismatchedEndMarker(i, ']'); - } - _parsingContext = _parsingContext.clearAndGetParent(); - _currToken = JsonToken.END_OBJECT; + if (i == INT_RBRACKET || i == INT_RCURLY) { + _closeScope(i); return null; } @@ -1042,7 +1014,14 @@ public String nextFieldName() throws IOException _reportUnexpectedChar(i, "was expecting comma to separate "+_parsingContext.typeDesc()+" entries"); } i = _skipWS(); + + // Was that a trailing comma? + if (isEnabled(Feature.ALLOW_TRAILING_COMMA) && (i == INT_RBRACKET || i == INT_RCURLY)) { + _closeScope(i); + return null; + } } + if (!_parsingContext.inObject()) { _updateLocation(); _nextTokenNotInObject(i); @@ -3733,6 +3712,25 @@ private final void _updateNameLocation() /********************************************************** */ + private void _closeScope(int i) throws JsonParseException { + if (i == INT_RBRACKET) { + _updateLocation(); + if (!_parsingContext.inArray()) { + _reportMismatchedEndMarker(i, '}'); + } + _parsingContext = _parsingContext.clearAndGetParent(); + _currToken = JsonToken.END_ARRAY; + } + if (i == INT_RCURLY) { + _updateLocation(); + if (!_parsingContext.inObject()) { + _reportMismatchedEndMarker(i, ']'); + } + _parsingContext = _parsingContext.clearAndGetParent(); + _currToken = JsonToken.END_OBJECT; + } + } + /** * Helper method needed to fix [Issue#148], masking of 0x00 character */ diff --git a/src/test/java/com/fasterxml/jackson/core/read/TrailingCommasTest.java b/src/test/java/com/fasterxml/jackson/core/read/TrailingCommasTest.java new file mode 100644 index 0000000000..972b6505f2 --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/core/read/TrailingCommasTest.java @@ -0,0 +1,316 @@ +package com.fasterxml.jackson.core.read; + +import com.fasterxml.jackson.core.BaseTest; +import com.fasterxml.jackson.core.JsonFactory; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonParser.Feature; +import com.fasterxml.jackson.core.JsonToken; +import com.fasterxml.jackson.core.json.UTF8DataInputJsonParser; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; + +@RunWith(Parameterized.class) +public class TrailingCommasTest extends BaseTest { + + private final JsonFactory factory; + private final HashSet features; + private final int mode; + + public TrailingCommasTest(int mode, List features) { + this.factory = new JsonFactory(); + this.features = new HashSet(features); + + for (JsonParser.Feature feature : features) { + factory.enable(feature); + } + + this.mode = mode; + } + + @Parameterized.Parameters(name = "Mode {0}, Features {1}") + public static Collection getTestCases() { + ArrayList cases = new ArrayList(); + + for (int mode : ALL_MODES) { + cases.add(new Object[]{mode, Collections.emptyList()}); + cases.add(new Object[]{mode, Arrays.asList(Feature.ALLOW_MISSING_VALUES)}); + cases.add(new Object[]{mode, Arrays.asList(Feature.ALLOW_TRAILING_COMMA)}); + cases.add(new Object[]{mode, Arrays.asList(Feature.ALLOW_MISSING_VALUES, Feature.ALLOW_TRAILING_COMMA)}); + } + + return cases; + } + + @Test + public void testArrayBasic() throws Exception { + String json = "[\"a\", \"b\"]"; + + JsonParser p = createParser(factory, mode, json); + + assertEquals(JsonToken.START_ARRAY, p.nextToken()); + + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + assertEquals("a", p.getText()); + + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + assertEquals("b", p.getText()); + + assertEquals(JsonToken.END_ARRAY, p.nextToken()); + assertEnd(p); + } + + @Test + public void testArrayInnerComma() throws Exception { + String json = "[\"a\",, \"b\"]"; + + JsonParser p = createParser(factory, mode, json); + + assertEquals(JsonToken.START_ARRAY, p.nextToken()); + + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + assertEquals("a", p.getText()); + + if (!features.contains(Feature.ALLOW_MISSING_VALUES)) { + assertUnexpected(p, ','); + return; + } + + assertToken(JsonToken.VALUE_NULL, p.nextToken()); + + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + assertEquals("b", p.getText()); + + assertEquals(JsonToken.END_ARRAY, p.nextToken()); + assertEnd(p); + } + + @Test + public void testArrayLeadingComma() throws Exception { + String json = "[,\"a\", \"b\"]"; + + JsonParser p = createParser(factory, mode, json); + + assertEquals(JsonToken.START_ARRAY, p.nextToken()); + + if (!features.contains(Feature.ALLOW_MISSING_VALUES)) { + assertUnexpected(p, ','); + return; + } + + assertToken(JsonToken.VALUE_NULL, p.nextToken()); + + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + assertEquals("a", p.getText()); + + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + assertEquals("b", p.getText()); + + assertEquals(JsonToken.END_ARRAY, p.nextToken()); + assertEnd(p); + } + + @Test + public void testArrayTrailingComma() throws Exception { + String json = "[\"a\", \"b\",]"; + + JsonParser p = createParser(factory, mode, json); + + assertEquals(JsonToken.START_ARRAY, p.nextToken()); + + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + assertEquals("a", p.getText()); + + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + assertEquals("b", p.getText()); + + // ALLOW_TRAILING_COMMA takes priority over ALLOW_MISSING_VALUES + if (features.contains(Feature.ALLOW_TRAILING_COMMA)) { + assertToken(JsonToken.END_ARRAY, p.nextToken()); + assertEnd(p); + } else if (features.contains(Feature.ALLOW_MISSING_VALUES)) { + assertToken(JsonToken.VALUE_NULL, p.nextToken()); + assertToken(JsonToken.END_ARRAY, p.nextToken()); + assertEnd(p); + } else { + assertUnexpected(p, ']'); + } + } + + @Test + public void testArrayTrailingCommas() throws Exception { + String json = "[\"a\", \"b\",,]"; + + JsonParser p = createParser(factory, mode, json); + + assertEquals(JsonToken.START_ARRAY, p.nextToken()); + + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + assertEquals("a", p.getText()); + + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + assertEquals("b", p.getText()); + + // ALLOW_TRAILING_COMMA takes priority over ALLOW_MISSING_VALUES + if (features.contains(Feature.ALLOW_MISSING_VALUES) && + features.contains(Feature.ALLOW_TRAILING_COMMA)) { + assertToken(JsonToken.VALUE_NULL, p.nextToken()); + assertToken(JsonToken.END_ARRAY, p.nextToken()); + assertEnd(p); + } else if (features.contains(Feature.ALLOW_MISSING_VALUES)) { + assertToken(JsonToken.VALUE_NULL, p.nextToken()); + assertToken(JsonToken.VALUE_NULL, p.nextToken()); + assertToken(JsonToken.END_ARRAY, p.nextToken()); + assertEnd(p); + } else { + assertUnexpected(p, ','); + } + } + + @Test + public void testArrayTrailingCommasTriple() throws Exception { + String json = "[\"a\", \"b\",,,]"; + + JsonParser p = createParser(factory, mode, json); + + assertEquals(JsonToken.START_ARRAY, p.nextToken()); + + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + assertEquals("a", p.getText()); + + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + assertEquals("b", p.getText()); + + // ALLOW_TRAILING_COMMA takes priority over ALLOW_MISSING_VALUES + if (features.contains(Feature.ALLOW_MISSING_VALUES) && + features.contains(Feature.ALLOW_TRAILING_COMMA)) { + assertToken(JsonToken.VALUE_NULL, p.nextToken()); + assertToken(JsonToken.VALUE_NULL, p.nextToken()); + assertToken(JsonToken.END_ARRAY, p.nextToken()); + assertEnd(p); + } else if (features.contains(Feature.ALLOW_MISSING_VALUES)) { + assertToken(JsonToken.VALUE_NULL, p.nextToken()); + assertToken(JsonToken.VALUE_NULL, p.nextToken()); + assertToken(JsonToken.VALUE_NULL, p.nextToken()); + assertToken(JsonToken.END_ARRAY, p.nextToken()); + assertEnd(p); + } else { + assertUnexpected(p, ','); + } + } + + @Test + public void testObjectBasic() throws Exception { + String json = "{\"a\": true, \"b\": false}"; + + JsonParser p = createParser(factory, mode, json); + + assertEquals(JsonToken.START_OBJECT, p.nextToken()); + + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertEquals("a", p.getText()); + assertToken(JsonToken.VALUE_TRUE, p.nextToken()); + + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertEquals("b", p.getText()); + assertToken(JsonToken.VALUE_FALSE, p.nextToken()); + + assertEquals(JsonToken.END_OBJECT, p.nextToken()); + assertEnd(p); + } + + @Test + public void testObjectInnerComma() throws Exception { + String json = "{\"a\": true,, \"b\": false}"; + + JsonParser p = createParser(factory, mode, json); + + assertEquals(JsonToken.START_OBJECT, p.nextToken()); + + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertEquals("a", p.getText()); + assertToken(JsonToken.VALUE_TRUE, p.nextToken()); + + assertUnexpected(p, ','); + } + + @Test + public void testObjectLeadingComma() throws Exception { + String json = "{,\"a\": true, \"b\": false}"; + + JsonParser p = createParser(factory, mode, json); + + assertEquals(JsonToken.START_OBJECT, p.nextToken()); + + assertUnexpected(p, ','); + } + + @Test + public void testObjectTrailingComma() throws Exception { + String json = "{\"a\": true, \"b\": false,}"; + + JsonParser p = createParser(factory, mode, json); + + assertEquals(JsonToken.START_OBJECT, p.nextToken()); + + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertEquals("a", p.getText()); + assertToken(JsonToken.VALUE_TRUE, p.nextToken()); + + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertEquals("b", p.getText()); + assertToken(JsonToken.VALUE_FALSE, p.nextToken()); + + if (features.contains(Feature.ALLOW_TRAILING_COMMA)) { + assertToken(JsonToken.END_OBJECT, p.nextToken()); + assertEnd(p); + } else { + assertUnexpected(p, '}'); + } + } + + @Test + public void testObjectTrailingCommas() throws Exception { + String json = "{\"a\": true, \"b\": false,,}"; + + JsonParser p = createParser(factory, mode, json); + + assertEquals(JsonToken.START_OBJECT, p.nextToken()); + + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertEquals("a", p.getText()); + assertToken(JsonToken.VALUE_TRUE, p.nextToken()); + + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertEquals("b", p.getText()); + assertToken(JsonToken.VALUE_FALSE, p.nextToken()); + + assertUnexpected(p, ','); + } + + private void assertEnd(JsonParser p) throws IOException { + // Issue #325 + if (!(p instanceof UTF8DataInputJsonParser)) { + JsonToken next = p.nextToken(); + assertNull("expected end of stream but found " + next, next); + } + } + + private void assertUnexpected(JsonParser p, char c) throws IOException { + try { + p.nextToken(); + fail("No exception thrown"); + } catch (Exception e) { + verifyException(e, String.format("Unexpected character ('%s' (code %d))", c, (int) c)); + } + } +}