From 6a1152cd446230b192b7d5b056cc1cb3877be684 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 2 Jan 2018 17:23:16 -0800 Subject: [PATCH] Fix #1729 --- release-notes/CREDITS | 2 + release-notes/VERSION-2.x | 2 + .../jackson/databind/util/TokenBuffer.java | 89 +++++++++++++++++-- .../convert/TestArrayConversions.java | 32 +++---- .../databind/util/TestTokenBuffer.java | 55 +++++++++++- 5 files changed, 159 insertions(+), 21 deletions(-) diff --git a/release-notes/CREDITS b/release-notes/CREDITS index 723a7c18bb..c97a87390b 100644 --- a/release-notes/CREDITS +++ b/release-notes/CREDITS @@ -643,6 +643,8 @@ Kevin Gallardo (newkek@github) * Reported #1658: Infinite recursion when deserializing a class extending a Map, with a recursive value type (2.8.10) + * Reported #1729: Integer bounds verification when calling `TokenBuffer.getIntValue()` + (2.9.4) Lukas Euler * Reported #1735: Missing type checks when using polymorphic type ids diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 983bac11da..87d3a4c286 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -5,6 +5,8 @@ Project: jackson-databind 2.9.4 (not yet released) +#1729: Integer bounds verification when calling `TokenBuffer.getIntValue()` + (reported by Kevin G) #1854: NPE deserializing collection with `@JsonCreator` and `ACCEPT_CASE_INSENSITIVE_PROPERTIES` (reported by rue-jw@github) #1855: Blacklist for more serialization gadgets (dbcp/tomcat, spring) diff --git a/src/main/java/com/fasterxml/jackson/databind/util/TokenBuffer.java b/src/main/java/com/fasterxml/jackson/databind/util/TokenBuffer.java index b42dc0f26e..25ffa89197 100644 --- a/src/main/java/com/fasterxml/jackson/databind/util/TokenBuffer.java +++ b/src/main/java/com/fasterxml/jackson/databind/util/TokenBuffer.java @@ -1573,16 +1573,22 @@ public float getFloatValue() throws IOException { @Override public int getIntValue() throws IOException { - // optimize common case: - if (_currToken == JsonToken.VALUE_NUMBER_INT) { - return ((Number) _currentObject()).intValue(); + Number n = (_currToken == JsonToken.VALUE_NUMBER_INT) ? + ((Number) _currentObject()) : getNumberValue(); + if ((n instanceof Integer) || _smallerThanInt(n)) { + return n.intValue(); } - return getNumberValue().intValue(); + return _convertNumberToInt(n); } @Override public long getLongValue() throws IOException { - return getNumberValue().longValue(); + Number n = (_currToken == JsonToken.VALUE_NUMBER_INT) ? + ((Number) _currentObject()) : getNumberValue(); + if ((n instanceof Long) || _smallerThanLong(n)) { + return n.longValue(); + } + return _convertNumberToLong(n); } @Override @@ -1623,6 +1629,79 @@ public final Number getNumberValue() throws IOException { +value.getClass().getName()); } + private final boolean _smallerThanInt(Number n) { + return (n instanceof Short) || (n instanceof Byte); + } + + private final boolean _smallerThanLong(Number n) { + return (n instanceof Integer) || (n instanceof Short) || (n instanceof Byte); + } + + /* 02-Jan-2017, tatu: Modified from method(s) in `ParserBase` + */ + + protected int _convertNumberToInt(Number n) throws IOException + { + if (n instanceof Long) { + long l = n.longValue(); + int result = (int) l; + if (((long) result) != l) { + reportOverflowInt(); + } + return result; + } + if (n instanceof BigInteger) { + BigInteger big = (BigInteger) n; + if (BI_MIN_INT.compareTo(big) > 0 + || BI_MAX_INT.compareTo(big) < 0) { + reportOverflowInt(); + } + } else if ((n instanceof Double) || (n instanceof Float)) { + double d = n.doubleValue(); + // Need to check boundaries + if (d < MIN_INT_D || d > MAX_INT_D) { + reportOverflowInt(); + } + return (int) d; + } else if (n instanceof BigDecimal) { + BigDecimal big = (BigDecimal) n; + if (BD_MIN_INT.compareTo(big) > 0 + || BD_MAX_INT.compareTo(big) < 0) { + reportOverflowInt(); + } + } else { + _throwInternal(); + } + return n.intValue(); + } + + protected long _convertNumberToLong(Number n) throws IOException + { + if (n instanceof BigInteger) { + BigInteger big = (BigInteger) n; + if (BI_MIN_LONG.compareTo(big) > 0 + || BI_MAX_LONG.compareTo(big) < 0) { + reportOverflowLong(); + } + } else if ((n instanceof Double) || (n instanceof Float)) { + double d = n.doubleValue(); + // Need to check boundaries + if (d < MIN_LONG_D || d > MAX_LONG_D) { + reportOverflowLong(); + } + return (int) d; + } else if (n instanceof BigDecimal) { + BigDecimal big = (BigDecimal) n; + if (BD_MIN_LONG.compareTo(big) > 0 + || BD_MAX_LONG.compareTo(big) < 0) { + reportOverflowLong(); + } + } else { + _throwInternal(); + } + return n.longValue(); + } + /* /********************************************************** /* Public API, access to token information, other diff --git a/src/test/java/com/fasterxml/jackson/databind/convert/TestArrayConversions.java b/src/test/java/com/fasterxml/jackson/databind/convert/TestArrayConversions.java index a4156eb094..fbc4751bb3 100644 --- a/src/test/java/com/fasterxml/jackson/databind/convert/TestArrayConversions.java +++ b/src/test/java/com/fasterxml/jackson/databind/convert/TestArrayConversions.java @@ -14,16 +14,19 @@ public class TestArrayConversions final static String OVERFLOW_MSG_BYTE = "out of range of Java byte"; final static String OVERFLOW_MSG = "overflow"; - final ObjectMapper mapper = new ObjectMapper(); + final static String OVERFLOW_MSG_INT = "out of range of int"; + final static String OVERFLOW_MSG_LONG = "out of range of long"; + + final ObjectMapper MAPPER = new ObjectMapper(); public void testNullXform() throws Exception { /* when given null, null should be returned without conversion * (Java null has no type) */ - assertNull(mapper.convertValue(null, Integer.class)); - assertNull(mapper.convertValue(null, String.class)); - assertNull(mapper.convertValue(null, byte[].class)); + assertNull(MAPPER.convertValue(null, Integer.class)); + assertNull(MAPPER.convertValue(null, String.class)); + assertNull(MAPPER.convertValue(null, byte[].class)); } /** @@ -72,7 +75,7 @@ public void testIntArrayToX() throws Exception List expNums = _numberList(data, data.length); // Alas, due to type erasure, need to use TypeRef, not just class - List actNums = mapper.convertValue(data, new TypeReference>() {}); + List actNums = MAPPER.convertValue(data, new TypeReference>() {}); assertEquals(expNums, actNums); } @@ -84,7 +87,7 @@ public void testLongArrayToX() throws Exception verifyLongArrayConversion(data, int[].class); List expNums = _numberList(data, data.length); - List actNums = mapper.convertValue(data, new TypeReference>() {}); + List actNums = MAPPER.convertValue(data, new TypeReference>() {}); assertEquals(expNums, actNums); } @@ -92,21 +95,21 @@ public void testOverflows() { // Byte overflow try { - mapper.convertValue(new int[] { 1000 }, byte[].class); + MAPPER.convertValue(new int[] { 1000 }, byte[].class); } catch (IllegalArgumentException e) { verifyException(e, OVERFLOW_MSG_BYTE); } // Short overflow try { - mapper.convertValue(new int[] { -99999 }, short[].class); + MAPPER.convertValue(new int[] { -99999 }, short[].class); } catch (IllegalArgumentException e) { verifyException(e, OVERFLOW_MSG); } // Int overflow try { - mapper.convertValue(new long[] { Long.MAX_VALUE }, int[].class); + MAPPER.convertValue(new long[] { Long.MAX_VALUE }, int[].class); } catch (IllegalArgumentException e) { - verifyException(e, OVERFLOW_MSG); + verifyException(e, OVERFLOW_MSG_INT); } // Longs need help of BigInteger... BigInteger biggie = BigInteger.valueOf(Long.MAX_VALUE); @@ -114,13 +117,12 @@ public void testOverflows() List l = new ArrayList(); l.add(biggie); try { - mapper.convertValue(l, int[].class); + MAPPER.convertValue(l, long[].class); } catch (IllegalArgumentException e) { - verifyException(e, OVERFLOW_MSG); + verifyException(e, OVERFLOW_MSG_LONG); } - } - + /* /******************************************************** /* Helper methods @@ -171,7 +173,7 @@ private T _convert(Object input, Class outputType) // must be a primitive array, like "int[].class" if (!outputType.isArray()) throw new IllegalArgumentException(); if (!outputType.getComponentType().isPrimitive()) throw new IllegalArgumentException(); - T result = mapper.convertValue(input, outputType); + T result = MAPPER.convertValue(input, outputType); // sanity check first: assertNotNull(result); assertEquals(outputType, result.getClass()); diff --git a/src/test/java/com/fasterxml/jackson/databind/util/TestTokenBuffer.java b/src/test/java/com/fasterxml/jackson/databind/util/TestTokenBuffer.java index 5f77017df6..e785be7284 100644 --- a/src/test/java/com/fasterxml/jackson/databind/util/TestTokenBuffer.java +++ b/src/test/java/com/fasterxml/jackson/databind/util/TestTokenBuffer.java @@ -6,6 +6,7 @@ import java.util.UUID; import com.fasterxml.jackson.core.*; +import com.fasterxml.jackson.core.JsonParser.NumberType; import com.fasterxml.jackson.core.io.SerializedString; import com.fasterxml.jackson.core.util.JsonParserSequence; @@ -124,7 +125,59 @@ public void testSimpleNumberWrites() throws IOException p.close(); buf.close(); } - + + // [databind#1729] + public void testNumberOverflowInt() throws IOException + { + try (TokenBuffer buf = new TokenBuffer(null, false)) { + long big = 1L + Integer.MAX_VALUE; + buf.writeNumber(big); + try (JsonParser p = buf.asParser()) { + assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken()); + assertEquals(NumberType.LONG, p.getNumberType()); + try { + p.getIntValue(); + fail("Expected failure for `int` overflow"); + } catch (JsonParseException e) { + verifyException(e, "Numeric value ("+big+") out of range of int"); + } + } + } + // and ditto for coercion. + try (TokenBuffer buf = new TokenBuffer(null, false)) { + long big = 1L + Integer.MAX_VALUE; + buf.writeNumber(String.valueOf(big)); + try (JsonParser p = buf.asParser()) { + // NOTE: oddity of buffering, no inspection of "real" type if given String... + assertToken(JsonToken.VALUE_NUMBER_FLOAT, p.nextToken()); + try { + p.getIntValue(); + fail("Expected failure for `int` overflow"); + } catch (JsonParseException e) { + verifyException(e, "Numeric value ("+big+") out of range of int"); + } + } + } + } + + public void testNumberOverflowLong() throws IOException + { + try (TokenBuffer buf = new TokenBuffer(null, false)) { + BigInteger big = BigInteger.valueOf(Long.MAX_VALUE).add(BigInteger.ONE); + buf.writeNumber(big); + try (JsonParser p = buf.asParser()) { + assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken()); + assertEquals(NumberType.BIG_INTEGER, p.getNumberType()); + try { + p.getLongValue(); + fail("Expected failure for `long` overflow"); + } catch (JsonParseException e) { + verifyException(e, "Numeric value ("+big+") out of range of long"); + } + } + } + } + public void testParentContext() throws IOException { TokenBuffer buf = new TokenBuffer(null, false); // no ObjectCodec