Skip to content

Commit

Permalink
Work on fix for #201 based on #200
Browse files Browse the repository at this point in the history
  • Loading branch information
@cowtowncoder
cowtowncoder committed Apr 8, 2020
1 parent 8dfb021 commit 6cedcc4
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 23 deletions.
59 changes: 49 additions & 10 deletions cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORGenerator.java
View file
Expand Up @@ -114,9 +114,6 @@ public int getMask() {
*/
private final static int MIN_BUFFER_LENGTH = (3 * 256) + 2;

private final static long MIN_INT_AS_LONG = (long) Integer.MIN_VALUE;
private final static long MAX_INT_AS_LONG = (long) Integer.MAX_VALUE;

/**
* Special value that is use to keep tracks of arrays and maps opened with infinite length
*/
Expand Down Expand Up @@ -671,10 +668,48 @@ private final void _writeIntNoCheck(int i) throws IOException {
_outputBuffer[_outputTail++] = b0;
}

private final void _writeIntMinimal(int markerBase, int i) throws IOException
{
_ensureRoomForOutput(5);
byte b0;
if (i >= 0) {
if (i < 24) {
_outputBuffer[_outputTail++] = (byte) (markerBase + i);
return;
}
if (i <= 0xFF) {
_outputBuffer[_outputTail++] = (byte) (markerBase + SUFFIX_UINT8_ELEMENTS);
_outputBuffer[_outputTail++] = (byte) i;
return;
}
b0 = (byte) i;
i >>= 8;
if (i <= 0xFF) {
_outputBuffer[_outputTail++] = (byte) (markerBase + SUFFIX_UINT16_ELEMENTS);
_outputBuffer[_outputTail++] = (byte) i;
_outputBuffer[_outputTail++] = b0;
return;
}
} else {
b0 = (byte) i;
i >>= 8;
}
_outputBuffer[_outputTail++] = (byte) (markerBase + SUFFIX_UINT32_ELEMENTS);
_outputBuffer[_outputTail++] = (byte) (i >> 16);
_outputBuffer[_outputTail++] = (byte) (i >> 8);
_outputBuffer[_outputTail++] = (byte) i;
_outputBuffer[_outputTail++] = b0;
}

private final void _writeLongNoCheck(long l) throws IOException {
if (_cfgMinimalInts) {
if (l <= MAX_INT_AS_LONG && l >= MIN_INT_AS_LONG) {
_writeIntNoCheck((int) l);
if (l >= 0) {
if (l <= 0x100000000L) {
_writeIntMinimal(PREFIX_TYPE_INT_POS, (int) l);
return;
}
} else if (l >= -0x100000000L) {
_writeIntMinimal(PREFIX_TYPE_INT_NEG, (int) (-l - 1));
return;
}
}
Expand Down Expand Up @@ -935,14 +970,18 @@ public void writeNumber(int i) throws IOException {

@Override
public void writeNumber(long l) throws IOException {
if (_cfgMinimalInts) {
// First: maybe 32 bits is enough?
if (l <= MAX_INT_AS_LONG && l >= MIN_INT_AS_LONG) {
writeNumber((int) l);
_verifyValueWrite("write number");
if (_cfgMinimalInts) { // maybe 32 bits is enough?
if (l >= 0) {
if (l <= 0x100000000L) {
_writeIntMinimal(PREFIX_TYPE_INT_POS, (int) l);
return;
}
} else if (l >= -0x100000000L) {
_writeIntMinimal(PREFIX_TYPE_INT_NEG, (int) (-l - 1));
return;
}
}
_verifyValueWrite("write number");
_ensureRoomForOutput(9);
if (l < 0L) {
l += 1;
Expand Down
52 changes: 43 additions & 9 deletions cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/gen/ArrayGenerationTest.java
View file
Expand Up @@ -3,7 +3,10 @@
import java.io.ByteArrayOutputStream;

import com.fasterxml.jackson.core.*;
import com.fasterxml.jackson.core.JsonParser.NumberType;
import com.fasterxml.jackson.dataformat.cbor.CBORFactory;
import com.fasterxml.jackson.dataformat.cbor.CBORGenerator;
import com.fasterxml.jackson.dataformat.cbor.CBORParser;
import com.fasterxml.jackson.dataformat.cbor.CBORTestBase;

/**
Expand All @@ -15,23 +18,54 @@ public class ArrayGenerationTest extends CBORTestBase

public void testIntArray() throws Exception
{
_testIntArray(false);
_testIntArray(true);
_testIntArray();
}

public void testLongArray() throws Exception
{
_testLongArray(false);
_testLongArray(true);
_testLongArray();
}

public void testDoubleArray() throws Exception
{
_testDoubleArray(false);
_testDoubleArray(true);
_testDoubleArray();
}

private void _testIntArray(boolean useBytes) throws Exception {
public void testMinimalIntValues2() throws Exception
{
// Array with 2 values that can't be passed as `int`s but DO fit
// CBOR 5-byte int (sign + 32-bits)
final long[] input = new long[] {
0xffffffffL, // max value that fits
-0xffffffffL - 1 // min value that fits
};
ByteArrayOutputStream bytes = new ByteArrayOutputStream();
CBORGenerator gen = FACTORY.createGenerator(bytes);
assertTrue(gen.isEnabled(CBORGenerator.Feature.WRITE_MINIMAL_INTS));
gen.writeArray(input, 0, 2);
gen.close();

// With default settings, should get:
byte[] encoded = bytes.toByteArray();
assertEquals(11, encoded.length);

// then verify contents

CBORParser p = FACTORY.createParser(encoded);
assertToken(JsonToken.START_ARRAY, p.nextToken());
assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken());
assertEquals(NumberType.LONG, p.getNumberType());
assertEquals(input[0], p.getLongValue());
assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken());
assertEquals(NumberType.LONG, p.getNumberType());
assertEquals(input[1], p.getLongValue());
assertToken(JsonToken.END_ARRAY, p.nextToken());
p.close();

// but then also check without minimization
}

private void _testIntArray() throws Exception {
// first special cases of 0, 1 values
_testIntArray(0, 0, 0);
_testIntArray(0, 1, 1);
Expand All @@ -52,7 +86,7 @@ private void _testIntArray(boolean useBytes) throws Exception {
_testIntArray(7777, 0, 1);
}

private void _testLongArray(boolean useBytes) throws Exception {
private void _testLongArray() throws Exception {
// first special cases of 0, 1 values
_testLongArray(0, 0, 0);
_testLongArray(0, 1, 1);
Expand All @@ -73,7 +107,7 @@ private void _testLongArray(boolean useBytes) throws Exception {
_testLongArray(6110, 0, 1);
}

private void _testDoubleArray(boolean useBytes) throws Exception {
private void _testDoubleArray() throws Exception {
// first special cases of 0, 1 values
_testDoubleArray(0, 0, 0);
_testDoubleArray(0, 1, 1);
Expand Down
38 changes: 34 additions & 4 deletions cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/gen/GeneratorSimpleTest.java
View file
Expand Up @@ -71,21 +71,48 @@ public void testMinimalIntValues() throws Exception

out = new ByteArrayOutputStream();
gen = cborGenerator(out);
gen.writeNumber(Integer.MAX_VALUE);
gen.writeNumber((long) Integer.MAX_VALUE);
gen.close();
_verifyBytes(out.toByteArray(),
(byte) (CBORConstants.PREFIX_TYPE_INT_POS + 26),
(byte) 0x7F, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF);

out = new ByteArrayOutputStream();
gen = cborGenerator(out);
gen.writeNumber(Integer.MIN_VALUE);
gen.writeNumber((long) Integer.MIN_VALUE);
gen.close();
_verifyBytes(out.toByteArray(),
(byte) (CBORConstants.PREFIX_TYPE_INT_NEG + 26),
(byte) 0x7F, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF);
}

// [dataformats-binary#201]
public void testMinimalIntValues2() throws Exception
{
ByteArrayOutputStream out;
CBORGenerator gen;

out = new ByteArrayOutputStream();
gen = cborGenerator(out);
// -1 if coerced as int, BUT cbor encoding can fit in 32-bit integer since
// sign is indicated by prefix
gen.writeNumber(0xffffffffL);
gen.close();
_verifyBytes(out.toByteArray(),
(byte) (CBORConstants.PREFIX_TYPE_INT_POS + 26),
(byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF);

out = new ByteArrayOutputStream();
gen = cborGenerator(out);
// similarly for negative numbers, we have 32-bit value bits, not 31
// as with Java int
gen.writeNumber(-0xffffffffL - 1);
gen.close();
_verifyBytes(out.toByteArray(),
(byte) (CBORConstants.PREFIX_TYPE_INT_NEG + 26),
(byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF);
}

public void testIntValues() throws Exception
{
// first, single-byte
Expand Down Expand Up @@ -156,12 +183,15 @@ public void testLongValues() throws Exception
{
ByteArrayOutputStream out = new ByteArrayOutputStream();
CBORGenerator gen = cborGenerator(out);
long l = -1L + Integer.MIN_VALUE;

// 07-Apr-2020, tatu: Since minimization enabled by default,
// need to use value that can not be minimized
long l = ((long) Integer.MIN_VALUE) << 2;
gen.writeNumber(l);
gen.close();
byte[] b = out.toByteArray();
assertEquals((byte) (CBORConstants.PREFIX_TYPE_INT_NEG + 27), b[0]);
assertEquals(9, b.length);
assertEquals((byte) (CBORConstants.PREFIX_TYPE_INT_NEG + 27), b[0]);
// could test full contents, but for now this shall suffice
assertEquals(0, b[1]);
assertEquals(0, b[2]);
Expand Down
6 changes: 6 additions & 0 deletions release-notes/CREDITS-2.x
View file
Expand Up @@ -126,3 +126,9 @@ Binh Tran (ankel@github)
* Reported, contributed fix for #192: (ion) Allow `IonObjectMapper` with class name annotation introspector
to deserialize generic subtypes
(2.11.0)
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)
3 changes: 3 additions & 0 deletions release-notes/VERSION-2.x
View file
Expand Up @@ -19,6 +19,9 @@ Project: jackson-datatypes-binaryModules:
(contributed by Bryan H)
#198: `jackson-databind` should not be full dependency for (cbor, protobuf, smile)
modules
#201: `CBORGenerator.Feature.WRITE_MINIMAL_INTS` does not write most compact form
for all integers
(reported by Jonas K)
- `AvroGenerator` overrides `getOutputContext()` properly
2.10.3 (03-Mar-2020)
Expand Down

0 comments on commit 6cedcc4

Please sign in to comment.