From 2cf1592f621cc2f08ae0dc5817b99242f41e316b Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 30 May 2025 17:41:49 -0700 Subject: [PATCH 1/4] More testing wrt core#1434 (verifying 2.x behavior) --- .../dataformat/avro/AvroNumberTest.java | 18 +++++++++++++ .../jackson/dataformat/cbor/CBORParser.java | 2 ++ .../CBORNumberParsingGetType1433Test.java | 2 -- .../jackson/dataformat/ion/IonParserTest.java | 25 +++++++++++++++++++ .../dataformat/protobuf/ReadSimpleTest.java | 16 ++++++++++++ .../dataformat/smile/SmileParserBase.java | 2 ++ .../SmileNumberParsingGetType1433Test.java | 2 -- 7 files changed, 63 insertions(+), 4 deletions(-) diff --git a/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/AvroNumberTest.java b/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/AvroNumberTest.java index 98de4c7c0..6b0efbc21 100644 --- a/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/AvroNumberTest.java +++ b/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/AvroNumberTest.java @@ -9,6 +9,7 @@ import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.core.JsonParser.NumberType; import com.fasterxml.jackson.core.JsonParser.NumberTypeFP; +import com.fasterxml.jackson.core.exc.StreamReadException; import com.fasterxml.jackson.core.JsonToken; import com.fasterxml.jackson.core.io.SerializedString; import com.fasterxml.jackson.dataformat.avro.testsupport.LimitingInputStream; @@ -69,8 +70,12 @@ public void testNumberCoercions() throws Exception byte[] bytes = MAPPER.writer(schema).writeValueAsBytes(input); JsonParser p = MAPPER.getFactory() .createParser(LimitingInputStream.wrap(bytes, 42)); + p.setSchema(schema); + + _verifyGetNumberTypeFail(p, "null"); assertToken(JsonToken.START_OBJECT, p.nextToken()); + _verifyGetNumberTypeFail(p, "START_OBJECT"); assertTrue(p.nextFieldName(new SerializedString("i"))); assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken()); @@ -122,6 +127,19 @@ public void testNumberCoercions() throws Exception assertEquals(Double.valueOf(input.d), p.getNumberValue()); assertToken(JsonToken.END_OBJECT, p.nextToken()); + _verifyGetNumberTypeFail(p, "END_OBJECT"); p.close(); + + _verifyGetNumberTypeFail(p, "null"); + } + + private void _verifyGetNumberTypeFail(JsonParser p, String token) throws Exception + { + try { + p.getNumberType(); + fail("Should not pass"); + } catch (StreamReadException e) { + verifyException(e, "Current token ("+token+") not numeric, can not use numeric"); + } } } diff --git a/cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORParser.java b/cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORParser.java index 81e83d502..441b4620c 100644 --- a/cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORParser.java +++ b/cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORParser.java @@ -769,6 +769,8 @@ public void close() throws IOException { if (!_closed) { _closed = true; _symbols.release(); + // 30-May-2025, tatu: was missing before 2.20 + _currToken = null; try { _closeInput(); } finally { diff --git a/cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/parse/CBORNumberParsingGetType1433Test.java b/cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/parse/CBORNumberParsingGetType1433Test.java index ddf9604f5..47ebca612 100644 --- a/cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/parse/CBORNumberParsingGetType1433Test.java +++ b/cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/parse/CBORNumberParsingGetType1433Test.java @@ -51,8 +51,6 @@ void getNumberType() throws Exception _verifyGetNumberTypeFail(p, "VALUE_TRUE"); assertToken(JsonToken.END_ARRAY, p.nextToken()); _verifyGetNumberTypeFail(p, "END_ARRAY"); - assertNull(p.nextToken()); - _verifyGetNumberTypeFail(p, "null"); p.close(); _verifyGetNumberTypeFail(p, "null"); } diff --git a/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/IonParserTest.java b/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/IonParserTest.java index 5f498903e..cec9fa9c6 100644 --- a/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/IonParserTest.java +++ b/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/IonParserTest.java @@ -20,6 +20,7 @@ import com.amazon.ion.*; import com.amazon.ion.system.IonSystemBuilder; + import org.junit.jupiter.api.Test; import com.fasterxml.jackson.core.*; @@ -36,6 +37,10 @@ public void testGetNumberTypeAndValue() throws Exception { Integer intValue = Integer.MAX_VALUE; IonValue ionInt = ion.newInt(intValue); IonParser intParser = new IonFactory().createParser(ionInt); + + // initially no current token so: + _verifyGetNumberTypeFail(intParser, "null"); + assertEquals(JsonToken.VALUE_NUMBER_INT, intParser.nextToken()); assertEquals(JsonParser.NumberType.INT, intParser.getNumberType()); assertEquals(JsonParser.NumberTypeFP.UNKNOWN, intParser.getNumberTypeFP()); @@ -44,14 +49,17 @@ public void testGetNumberTypeAndValue() throws Exception { Long longValue = Long.MAX_VALUE; IonValue ionLong = ion.newInt(longValue); IonParser longParser = new IonFactory().createParser(ionLong); + _verifyGetNumberTypeFail(longParser, "null"); assertEquals(JsonToken.VALUE_NUMBER_INT, longParser.nextToken()); assertEquals(JsonParser.NumberType.LONG, longParser.getNumberType()); assertEquals(JsonParser.NumberTypeFP.UNKNOWN, intParser.getNumberTypeFP()); assertEquals(longValue, longParser.getNumberValue()); + assertNull(longParser.nextToken()); BigInteger bigIntValue = new BigInteger(Long.MAX_VALUE + "1"); IonValue ionBigInt = ion.newInt(bigIntValue); IonParser bigIntParser = new IonFactory().createParser(ionBigInt); + _verifyGetNumberTypeFail(bigIntParser, "null"); assertEquals(JsonToken.VALUE_NUMBER_INT, bigIntParser.nextToken()); assertEquals(JsonParser.NumberType.BIG_INTEGER, bigIntParser.getNumberType()); assertEquals(JsonParser.NumberTypeFP.UNKNOWN, intParser.getNumberTypeFP()); @@ -60,6 +68,7 @@ public void testGetNumberTypeAndValue() throws Exception { Double decimalValue = Double.MAX_VALUE; IonValue ionDecimal = ion.newDecimal(decimalValue); IonParser decimalParser = new IonFactory().createParser(ionDecimal); + _verifyGetNumberTypeFail(decimalParser, "null"); assertEquals(JsonToken.VALUE_NUMBER_FLOAT, decimalParser.nextToken()); assertEquals(JsonParser.NumberType.BIG_DECIMAL, decimalParser.getNumberType()); assertEquals(JsonParser.NumberTypeFP.BIG_DECIMAL, decimalParser.getNumberTypeFP()); @@ -68,6 +77,7 @@ public void testGetNumberTypeAndValue() throws Exception { Double floatValue = Double.MAX_VALUE; IonValue ionFloat = ion.newFloat(floatValue); IonParser floatParser = new IonFactory().createParser(ionFloat); + _verifyGetNumberTypeFail(floatParser, "null"); assertEquals(JsonToken.VALUE_NUMBER_FLOAT, floatParser.nextToken()); assertEquals(JsonParser.NumberType.DOUBLE, floatParser.getNumberType()); // [dataformats-binary#490]: float coerces to double @@ -77,6 +87,7 @@ public void testGetNumberTypeAndValue() throws Exception { BigDecimal bigDecimalValue = new BigDecimal(Double.MAX_VALUE + "1"); IonValue ionBigDecimal = ion.newDecimal(bigDecimalValue); IonParser bigDecimalParser = new IonFactory().createParser(ionBigDecimal); + _verifyGetNumberTypeFail(bigDecimalParser, "null"); assertEquals(JsonToken.VALUE_NUMBER_FLOAT, bigDecimalParser.nextToken()); assertEquals(JsonParser.NumberType.BIG_DECIMAL, bigDecimalParser.getNumberType()); assertEquals(JsonParser.NumberTypeFP.BIG_DECIMAL, bigDecimalParser.getNumberTypeFP()); @@ -139,9 +150,11 @@ public void testIonExceptionIsWrapped() throws IOException { IonFactory f = new IonFactory(); try (IonParser parser = (IonParser) f.createParser("[ 12, true ) ]")) { assertEquals(JsonToken.START_ARRAY, parser.nextToken()); + _verifyGetNumberTypeFail(parser, "START_ARRAY"); assertEquals(JsonToken.VALUE_NUMBER_INT, parser.nextValue()); assertEquals(12, parser.getIntValue()); assertEquals(JsonToken.VALUE_TRUE, parser.nextValue()); + _verifyGetNumberTypeFail(parser, "VALUE_TRUE"); parser.nextValue(); } }); @@ -193,4 +206,16 @@ public void testUnknownSymbolExceptionForAnnotationIsWrapped() throws IOExceptio } }); } + + // 30-May-2025, tatu: NOTE: IonParser implements `JsonParser.getNumberType()` way + // documented before 2.19 ("null for non-numeric types") unlike all other backends + // (which throws StreamReadException). In 2.19.1 Javadocs were updated to reflect + // actual behavior, so in theory IonParser is non-compliant. However, for compatibility + // reasons we'll let that be. + // Further, as per [jackson-core#1434], Jackson 3.0 will actually change definition + // to NOT throw an exception but return null -- what Ion already does. + private void _verifyGetNumberTypeFail(JsonParser p, String token) throws Exception + { + assertNull(p.getNumberType()); + } } diff --git a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/ReadSimpleTest.java b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/ReadSimpleTest.java index bfec52d69..6c60ec0c2 100644 --- a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/ReadSimpleTest.java +++ b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/ReadSimpleTest.java @@ -8,6 +8,7 @@ import com.fasterxml.jackson.core.*; import com.fasterxml.jackson.core.JsonParser.NumberType; import com.fasterxml.jackson.core.JsonParser.NumberTypeFP; +import com.fasterxml.jackson.core.exc.StreamReadException; import com.fasterxml.jackson.databind.*; import com.fasterxml.jackson.dataformat.protobuf.schema.ProtobufSchema; import com.fasterxml.jackson.dataformat.protobuf.schema.ProtobufSchemaLoader; @@ -93,8 +94,11 @@ public void testReadPointIntStreaming() throws Exception // actually let's also try via streaming parser JsonParser p = MAPPER.getFactory().createParser(bytes); p.setSchema(schema); + + _verifyGetNumberTypeFail(p, "null"); assertToken(JsonToken.START_OBJECT, p.nextToken()); assertNull(p.currentName()); + _verifyGetNumberTypeFail(p, "START_OBJECT"); assertToken(JsonToken.FIELD_NAME, p.nextToken()); assertEquals("x", p.currentName()); assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken()); @@ -109,8 +113,10 @@ public void testReadPointIntStreaming() throws Exception assertEquals(input.y, p.getIntValue()); assertToken(JsonToken.END_OBJECT, p.nextToken()); assertNull(p.currentName()); + _verifyGetNumberTypeFail(p, "END_OBJECT"); p.close(); assertNull(p.currentName()); + _verifyGetNumberTypeFail(p, "null"); } @Test @@ -469,4 +475,14 @@ public void testStringArraySimpleLowLimit() throws Exception "unexpected message: " + message); } } + + private void _verifyGetNumberTypeFail(JsonParser p, String token) throws Exception + { + try { + p.getNumberType(); + fail("Should not pass"); + } catch (StreamReadException e) { + verifyException(e, "Current token ("+token+") not numeric, can not use numeric"); + } + } } diff --git a/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParserBase.java b/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParserBase.java index 2c18ff8be..bc3a3e959 100644 --- a/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParserBase.java +++ b/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParserBase.java @@ -397,6 +397,8 @@ public final void close() throws IOException { _closed = true; _inputEnd = 0; _symbols.release(); + // 30-May-2025, tatu: was missing before 2.20 + _currToken = null; try { _closeInput(); } finally { diff --git a/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/parse/SmileNumberParsingGetType1433Test.java b/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/parse/SmileNumberParsingGetType1433Test.java index 5787ff886..bfe0d30b3 100644 --- a/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/parse/SmileNumberParsingGetType1433Test.java +++ b/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/parse/SmileNumberParsingGetType1433Test.java @@ -51,8 +51,6 @@ void getNumberType() throws Exception _verifyGetNumberTypeFail(p, "VALUE_TRUE"); assertToken(JsonToken.END_ARRAY, p.nextToken()); _verifyGetNumberTypeFail(p, "END_ARRAY"); - assertNull(p.nextToken()); - _verifyGetNumberTypeFail(p, "null"); p.close(); _verifyGetNumberTypeFail(p, "null"); } From eeaed56cad93e00a2ad9fd4eb09b97df47e15bd0 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 30 May 2025 17:49:50 -0700 Subject: [PATCH 2/4] Minor tweaks --- .../com/fasterxml/jackson/dataformat/cbor/CBORParser.java | 4 +++- .../com/fasterxml/jackson/dataformat/ion/IonParser.java | 8 ++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORParser.java b/cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORParser.java index 441b4620c..f9c1a6798 100644 --- a/cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORParser.java +++ b/cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORParser.java @@ -770,7 +770,9 @@ public void close() throws IOException { _closed = true; _symbols.release(); // 30-May-2025, tatu: was missing before 2.20 - _currToken = null; + if (JsonParser.Feature.CLEAR_CURRENT_TOKEN_ON_CLOSE.enabledIn(_features)) { + _currToken = null; + } try { _closeInput(); } finally { diff --git a/ion/src/main/java/com/fasterxml/jackson/dataformat/ion/IonParser.java b/ion/src/main/java/com/fasterxml/jackson/dataformat/ion/IonParser.java index 1948b9fe3..6d58344de 100644 --- a/ion/src/main/java/com/fasterxml/jackson/dataformat/ion/IonParser.java +++ b/ion/src/main/java/com/fasterxml/jackson/dataformat/ion/IonParser.java @@ -254,6 +254,10 @@ public boolean isClosed() { @Override public void close() throws IOException { if (!_closed) { + // 30-May-2025, tatu: was missing before 2.20 + if (JsonParser.Feature.CLEAR_CURRENT_TOKEN_ON_CLOSE.enabledIn(_features)) { + _currToken = null; + } // should only close if manage the resource if (_ioContext.isResourceManaged()) { Object src = _ioContext.contentReference().getRawContent(); @@ -448,6 +452,10 @@ private void _verifyIsNumberToken() throws IOException } } + // NOTE: Ion implementation follows original (up to 2.19) JsonParser Javadocs + // behavior (if non-number, return `null`), which is different from other + // backends (if non-number, throw exception). But since 3.0 will switch to + // "return null for non-numbers", Ion behavior is retained in 2.20+. @Override public NumberType getNumberType() throws IOException { From b2158e05fb2dfd34ff580aa7e3462b814f2d4fb9 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 30 May 2025 17:51:12 -0700 Subject: [PATCH 3/4] ... --- .../fasterxml/jackson/dataformat/smile/SmileParserBase.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParserBase.java b/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParserBase.java index bc3a3e959..59446c12f 100644 --- a/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParserBase.java +++ b/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParserBase.java @@ -398,7 +398,9 @@ public final void close() throws IOException { _inputEnd = 0; _symbols.release(); // 30-May-2025, tatu: was missing before 2.20 - _currToken = null; + if (JsonParser.Feature.CLEAR_CURRENT_TOKEN_ON_CLOSE.enabledIn(_features)) { + _currToken = null; + } try { _closeInput(); } finally { From 3475b17b743cdd8c0a65c01cf7f9fd0c3be64a53 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Sat, 31 May 2025 09:28:23 -0700 Subject: [PATCH 4/4] Fix ProtobufParser too --- .../jackson/dataformat/protobuf/ProtobufParser.java | 10 ++++++++++ .../jackson/dataformat/protobuf/ReadSimpleTest.java | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java index cb398768f..e13d71c62 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java @@ -448,6 +448,10 @@ public void close() throws IOException _state = STATE_CLOSED; if (!_closed) { _closed = true; + // 30-May-2025, tatu: was missing before 2.20 + if (JsonParser.Feature.CLEAR_CURRENT_TOKEN_ON_CLOSE.enabledIn(_features)) { + _currToken = null; + } try { _closeInput(); } finally { @@ -461,6 +465,12 @@ public void close() throws IOException _parsingContext = _parsingContext.getParent(); } _parsingContext.setCurrentName(null); + } else { + // 31-May-2025, tatu: To work around [dataformats-binary#598], + // need to forcibly clear current token even in this case + if (JsonParser.Feature.CLEAR_CURRENT_TOKEN_ON_CLOSE.enabledIn(_features)) { + _currToken = null; + } } } diff --git a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/ReadSimpleTest.java b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/ReadSimpleTest.java index 6c60ec0c2..b85abcce9 100644 --- a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/ReadSimpleTest.java +++ b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/ReadSimpleTest.java @@ -92,7 +92,7 @@ public void testReadPointIntStreaming() throws Exception assertEquals(6, bytes.length); // actually let's also try via streaming parser - JsonParser p = MAPPER.getFactory().createParser(bytes); + JsonParser p = MAPPER.createParser(bytes); p.setSchema(schema); _verifyGetNumberTypeFail(p, "null");