Fix #269

FasterXML · Mar 31, 2021 · b4db541 · b4db541
1 parent e697c72
commit b4db541
Show file tree

Hide file tree

Showing 6 changed files with 222 additions and 12 deletions.
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
@@ -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
@@ -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
@@ -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
@@ -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<Long, String> map;
+
+        public TestData269 setMap(Map<Long, String> map) {
+            this.map = map;
+            return this;
+        }
+
+        public Map<Long, String> 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<Long, String> 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
@@ -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
@@ -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`