From b4db5419b9a1690f16cb0e511bb440eea58a35fe Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 31 Mar 2021 11:01:26 -0700 Subject: [PATCH] Fix #269 --- .../dataformat/cbor/CBORGenerator.java | 11 +- .../jackson/dataformat/cbor/CBORParser.java | 13 ++ .../cbor/mapper/NumberBeanTest.java | 137 +++++++++++++++++- .../cbor/mapper/NumberMap269Test.java | 63 ++++++++ release-notes/CREDITS-2.x | 5 +- release-notes/VERSION-2.x | 5 + 6 files changed, 222 insertions(+), 12 deletions(-) create mode 100644 cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/mapper/NumberMap269Test.java diff --git a/cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORGenerator.java b/cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORGenerator.java index 880e585a9..35fc012d1 100644 --- a/cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORGenerator.java +++ b/cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORGenerator.java @@ -690,10 +690,13 @@ private final void _writeIntFull(int markerBase, int i) throws IOException // Helper method that works like `writeNumber(long)` but DOES NOT // check internal output state. It does, however, check need for minimization - private final void _writeLongNoCheck(long l) throws IOException { + private final void _writeLongNoCheck(long l) throws IOException + { if (_cfgMinimalInts) { if (l >= 0) { - if (l <= 0x100000000L) { + // 31-Mar-2021, tatu: [dataformats-cbor#269] Incorrect boundary check, + // was off by one, resulting in truncation to 0 + if (l < 0x100000000L) { _writeIntMinimal(PREFIX_TYPE_INT_POS, (int) l); return; } @@ -962,7 +965,9 @@ public void writeNumber(long l) throws IOException { _verifyValueWrite("write number"); if (_cfgMinimalInts) { // maybe 32 bits is enough? if (l >= 0) { - if (l <= 0x100000000L) { + // 31-Mar-2021, tatu: [dataformats-cbor#269] Incorrect boundary check, + // was off by one, resulting in truncation to 0 + if (l < 0x100000000L) { _writeIntMinimal(PREFIX_TYPE_INT_POS, (int) l); return; } 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 06a69fa68..3ad2065bc 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 @@ -817,6 +817,19 @@ protected String _numberToName(int ch, boolean neg) throws IOException break; case 26: i = _decode32Bits(); + // [dataformats-binary#269] (and earlier [dataformats-binary#30]), + // got some edge case to consider + if (i < 0) { + long l; + if (neg) { + long unsignedBase = (long) i & 0xFFFFFFFFL; + l = -unsignedBase - 1L; + } else { + l = (long) i; + l = l & 0xFFFFFFFFL; + } + return String.valueOf(l); + } break; case 27: { diff --git a/cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/mapper/NumberBeanTest.java b/cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/mapper/NumberBeanTest.java index 99a81eecc..50f01f823 100644 --- a/cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/mapper/NumberBeanTest.java +++ b/cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/mapper/NumberBeanTest.java @@ -1,6 +1,14 @@ package com.fasterxml.jackson.dataformat.cbor.mapper; +import java.io.ByteArrayOutputStream; +import java.math.BigDecimal; +import java.math.BigInteger; + +import com.fasterxml.jackson.annotation.JsonUnwrapped; + import com.fasterxml.jackson.databind.ObjectMapper; + +import com.fasterxml.jackson.dataformat.cbor.CBORGenerator; import com.fasterxml.jackson.dataformat.cbor.CBORTestBase; public class NumberBeanTest extends CBORTestBase @@ -19,6 +27,23 @@ static class LongBean { protected LongBean() { } } + static class NumberWrapper { + public Number nr; + } + + // // Copied form "JDKNumberDeserTest", related to BigDecimal precision + // // retaining through buffering + + // [databind#2784] + static class BigDecimalHolder2784 { + public BigDecimal value; + } + + static class NestedBigDecimalHolder2784 { + @JsonUnwrapped + public BigDecimalHolder2784 holder; + } + /* /********************************************************** /* Test methods @@ -46,15 +71,28 @@ public void testLongRoundTrip() throws Exception 100L, -200L, 5000L, -3600L, Integer.MIN_VALUE, Integer.MAX_VALUE, - 1L + Integer.MAX_VALUE, -1L + Integer.MIN_VALUE, - 2330462449L, // from [dataformats-binary#30] - Long.MIN_VALUE, Long.MAX_VALUE - }) { - LongBean input = new LongBean(v); - byte[] b = MAPPER.writeValueAsBytes(input); - LongBean result = MAPPER.readValue(b, LongBean.class); - assertEquals(input.value, result.value); + 1L + Integer.MAX_VALUE, -1L + Integer.MIN_VALUE + }) { + _testLongRoundTrip(v); } + + _testLongRoundTrip(2330462449L); // from [dataformats-binary#30] + _testLongRoundTrip(0xFFFFFFFFL); // max positive uint32 + _testLongRoundTrip(-0xFFFFFFFFL); + _testLongRoundTrip(0x100000000L); + _testLongRoundTrip(-0x100000000L); + _testLongRoundTrip(0x100000001L); + _testLongRoundTrip(-0x100000001L); + _testLongRoundTrip(Long.MIN_VALUE); + _testLongRoundTrip(Long.MAX_VALUE); + } + + private void _testLongRoundTrip(long v) throws Exception + { + LongBean input = new LongBean(v); + byte[] b = MAPPER.writeValueAsBytes(input); + LongBean result = MAPPER.readValue(b, LongBean.class); + assertEquals(input.value, result.value); } // for [dataformats-binary#32] coercion of Float into Double @@ -67,4 +105,87 @@ public void testUntypedWithFloat() throws Exception assertEquals(Float.class, result[0].getClass()); assertEquals(input[0], result[0]); } + + public void testNumberTypeRetainingInt() throws Exception + { + NumberWrapper result; + ByteArrayOutputStream bytes; + + bytes = new ByteArrayOutputStream(); + try (CBORGenerator g = cborGenerator(bytes)) { + g.writeStartObject(); + g.writeNumberField("nr", 123); + g.writeEndObject(); + } + result = MAPPER.readValue(bytes.toByteArray(), NumberWrapper.class); + assertEquals(Integer.valueOf(123), result.nr); + + bytes = new ByteArrayOutputStream(); + try (CBORGenerator g = cborGenerator(bytes)) { + g.writeStartObject(); + g.writeNumberField("nr", Long.MAX_VALUE); + g.writeEndObject(); + } + result = MAPPER.readValue(bytes.toByteArray(), NumberWrapper.class); + assertEquals(Long.valueOf(Long.MAX_VALUE), result.nr); + + bytes = new ByteArrayOutputStream(); + try (CBORGenerator g = cborGenerator(bytes)) { + g.writeStartObject(); + g.writeNumberField("nr", BigInteger.valueOf(-42L)); + g.writeEndObject(); + } + result = MAPPER.readValue(bytes.toByteArray(), NumberWrapper.class); + assertEquals(BigInteger.valueOf(-42L), result.nr); + } + + public void testNumberTypeRetainingFP() throws Exception + { + NumberWrapper result; + ByteArrayOutputStream bytes; + + bytes = new ByteArrayOutputStream(); + try (CBORGenerator g = cborGenerator(bytes)) { + g.writeStartObject(); + g.writeNumberField("nr", 0.25f); + g.writeEndObject(); + } + result = MAPPER.readValue(bytes.toByteArray(), NumberWrapper.class); + assertEquals(Float.valueOf(0.25f), result.nr); + + bytes = new ByteArrayOutputStream(); + try (CBORGenerator g = cborGenerator(bytes)) { + g.writeStartObject(); + g.writeNumberField("nr", 0.5); + g.writeEndObject(); + } + result = MAPPER.readValue(bytes.toByteArray(), NumberWrapper.class); + assertEquals(Double.valueOf(0.5), result.nr); + + bytes = new ByteArrayOutputStream(); + try (CBORGenerator g = cborGenerator(bytes)) { + g.writeStartObject(); + g.writeNumberField("nr", new BigDecimal("0.100")); + g.writeEndObject(); + } + result = MAPPER.readValue(bytes.toByteArray(), NumberWrapper.class); + assertEquals(new BigDecimal("0.100"), result.nr); + } + + // [databind#2784] + public void testBigDecimalWithBuffering() throws Exception + { + final BigDecimal VALUE = new BigDecimal("5.00"); + // Need to generate by hand since JSON would not indicate desire for BigDecimal + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + try (CBORGenerator g = cborGenerator(bytes)) { + g.writeStartObject(); + g.writeNumberField("value", VALUE); + g.writeEndObject(); + } + + NestedBigDecimalHolder2784 result = MAPPER.readValue(bytes.toByteArray(), + NestedBigDecimalHolder2784.class); + assertEquals(VALUE, result.holder.value); + } } diff --git a/cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/mapper/NumberMap269Test.java b/cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/mapper/NumberMap269Test.java new file mode 100644 index 000000000..e203f8cd2 --- /dev/null +++ b/cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/mapper/NumberMap269Test.java @@ -0,0 +1,63 @@ +package com.fasterxml.jackson.dataformat.cbor.mapper; + +import java.util.*; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.dataformat.cbor.CBORTestBase; + +// [dataformats-binary#269] +public class NumberMap269Test extends CBORTestBase +{ + static class TestData269 { + Map map; + + public TestData269 setMap(Map map) { + this.map = map; + return this; + } + + public Map getMap() { + return map; + } + } + + /* + /********************************************************** + /* Test methods + /********************************************************** + */ + + private final ObjectMapper MAPPER = cborMapper(); + + // [dataformats-binary#269] + public void testInt32BoundaryWithMapKey() throws Exception + { + // First, with specific reported combo: + _testInt32BoundaryWithMapKey(4294967296L, -4294967296L); + + // and then systematically couple of others (actually overlapping but...) + final long MAX_POS_UINT32 = 0xFFFFFFFFL; + final long MAX_POS_UINT32_PLUS_1 = MAX_POS_UINT32 + 1L; + + _testInt32BoundaryWithMapKey(MAX_POS_UINT32, -MAX_POS_UINT32); + _testInt32BoundaryWithMapKey(MAX_POS_UINT32_PLUS_1, + -MAX_POS_UINT32_PLUS_1); + + _testInt32BoundaryWithMapKey(MAX_POS_UINT32_PLUS_1 + 1L, + -MAX_POS_UINT32_PLUS_1 - 1L); + } + + private void _testInt32BoundaryWithMapKey(long key1, long key2) throws Exception + { + Map map = new LinkedHashMap<>(); + map.put(key1, "hello"); + map.put(key2, "world"); + TestData269 input = new TestData269().setMap(map); + + byte[] cborDoc = MAPPER.writeValueAsBytes(input); + + TestData269 result = MAPPER.readValue(cborDoc, TestData269.class); + + assertEquals(input.map, result.map); + } +} diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index 790996c4b..9cb7a9f19 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -140,4 +140,7 @@ Jonas Konrad (yawkat@github) * Reported, contributed fix for #201: `CBORGenerator.Feature.WRITE_MINIMAL_INTS` does not write most compact form for all integers (2.11.0) - + +Dylan (Quantum64@github) +* Reported #269: CBOR loses `Map` entries with specific `long` Map key values (32-bit boundary) + (2.11.5 / 2.12.0) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 93ee2f652..85b8de0be 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -9,6 +9,11 @@ Modules: === Releases === ------------------------------------------------------------------------ +Not yet releasaed: + +#269: CBOR loses `Map` entries with specific `long` Map key values (32-bit boundary) + (reported by Quantum64@github) + 2.11.4 (12-Dec-2020) #186: (cbor) Eager allocation of byte buffer can cause `java.lang.OutOfMemoryError`