From 8f13bd20c54a55e254c4b1061e30294f48db40ba Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 12 Jun 2020 18:20:38 -0700 Subject: [PATCH] More progress, now with `byte` coercions --- .../deser/std/NumberDeserializers.java | 55 +---- .../std/PrimitiveArrayDeserializers.java | 2 +- .../databind/deser/std/StdDeserializer.java | 202 ++++++++++++++---- .../convert/TestArrayConversions.java | 4 +- .../databind/deser/jdk/JDKScalarsTest.java | 13 +- 5 files changed, 177 insertions(+), 99 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/NumberDeserializers.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/NumberDeserializers.java index 5cf8739273..efe0153685 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/std/NumberDeserializers.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/std/NumberDeserializers.java @@ -254,7 +254,7 @@ public static class ByteDeserializer final static ByteDeserializer primitiveInstance = new ByteDeserializer(Byte.TYPE, (byte) 0); final static ByteDeserializer wrapperInstance = new ByteDeserializer(Byte.class, null); - + public ByteDeserializer(Class cls, Byte nvl) { super(cls, LogicalType.Integer, nvl, (byte) 0); @@ -266,58 +266,7 @@ public Byte deserialize(JsonParser p, DeserializationContext ctxt) throws IOExce if (p.hasToken(JsonToken.VALUE_NUMBER_INT)) { return p.getByteValue(); } - return _parseByte(p, ctxt); - } - - protected Byte _parseByte(JsonParser p, DeserializationContext ctxt) throws IOException - { - JsonToken t = p.currentToken(); - if (t == JsonToken.VALUE_STRING) { // let's do implicit re-parse - String text = p.getText(); - CoercionAction act = _checkFromStringCoercion(ctxt, text); - if (act == CoercionAction.AsNull) { - return (Byte) getNullValue(ctxt); - } - if (act == CoercionAction.AsEmpty) { - return (Byte) getEmptyValue(ctxt); - } - text = text.trim(); - if (_hasTextualNull(text)) { - return (Byte) _coerceTextualNull(ctxt, _primitive); - } - int value; - try { - value = NumberInput.parseInt(text); - } catch (IllegalArgumentException iae) { - return (Byte) ctxt.handleWeirdStringValue(_valueClass, text, - "not a valid Byte value"); - } - // So far so good: but does it fit? - // as per [JACKSON-804], allow range up to 255, inclusive - if (_byteOverflow(value)) { - return (Byte) ctxt.handleWeirdStringValue(_valueClass, text, - "overflow, value cannot be represented as 8-bit value"); - // fall-through for deferred fails - } - return Byte.valueOf((byte) value); - } - if (t == JsonToken.VALUE_NUMBER_FLOAT) { - if (!ctxt.isEnabled(DeserializationFeature.ACCEPT_FLOAT_AS_INT)) { - _failDoubleToIntCoercion(p, ctxt, "Byte"); - } - return p.getByteValue(); - } - if (t == JsonToken.VALUE_NULL) { - return (Byte) _coerceNullToken(ctxt, _primitive); - } - // [databind#381] - if (t == JsonToken.START_ARRAY) { - return _deserializeFromArray(p, ctxt); - } - if (t == JsonToken.VALUE_NUMBER_INT) { // shouldn't usually be called with it but - return p.getByteValue(); - } - return (Byte) ctxt.handleUnexpectedToken(_valueClass, p); + return _parseByte(p, ctxt, _valueClass); } } diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/PrimitiveArrayDeserializers.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/PrimitiveArrayDeserializers.java index 35dc22c769..20c87d35b3 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/std/PrimitiveArrayDeserializers.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/std/PrimitiveArrayDeserializers.java @@ -508,7 +508,7 @@ public byte[] deserialize(JsonParser p, DeserializationContext ctxt) throws IOEx _verifyNullForPrimitive(ctxt); value = (byte) 0; } else { - value = _parseBytePrimitive(p, ctxt); + value = _parseBytePrimitive(ctxt, p, handledType()); } } if (ix >= chunk.length) { diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/StdDeserializer.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/StdDeserializer.java index 678e516f2e..7f15e81a54 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/std/StdDeserializer.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/std/StdDeserializer.java @@ -7,6 +7,7 @@ import com.fasterxml.jackson.annotation.Nulls; import com.fasterxml.jackson.core.*; +import com.fasterxml.jackson.core.JsonParser.NumberType; import com.fasterxml.jackson.core.exc.InputCoercionException; import com.fasterxml.jackson.core.io.NumberInput; @@ -369,7 +370,8 @@ protected final boolean _parseBooleanPrimitive(DeserializationContext ctxt, JsonParser p, Class targetType) throws IOException { - JsonToken t = p.currentToken(); + final JsonToken t = p.currentToken(); + // usually caller should have handled but: if (t == JsonToken.VALUE_TRUE) return true; if (t == JsonToken.VALUE_FALSE) return false; if (t == JsonToken.VALUE_NULL) { @@ -393,7 +395,7 @@ protected final boolean _parseBooleanPrimitive(DeserializationContext ctxt, return false; } if (act == CoercionAction.AsEmpty) { - return (Boolean) getEmptyValue(ctxt); + return false; } text = text.trim(); // [databind#422]: Allow aliases @@ -411,14 +413,13 @@ protected final boolean _parseBooleanPrimitive(DeserializationContext ctxt, "only \"true\" or \"false\" recognized"); return Boolean.TRUE.equals(b); } - // [databind#381] + // 12-Jun-2020, tatu: For some reason calling `_deserializeFromArray()` won't work so: if (t == JsonToken.START_ARRAY && ctxt.isEnabled(DeserializationFeature.UNWRAP_SINGLE_VALUE_ARRAYS)) { p.nextToken(); final boolean parsed = _parseBooleanPrimitive(ctxt, p, targetType); _verifyEndArrayForSingle(p, ctxt); return parsed; } - // Otherwise, no can do: return ((Boolean) ctxt.handleUnexpectedToken(targetType, p)).booleanValue(); } @@ -426,7 +427,10 @@ protected final Boolean _parseBoolean(DeserializationContext ctxt, JsonParser p, Class targetType) throws IOException { - JsonToken t = p.currentToken(); + final JsonToken t = p.currentToken(); + // usually caller should have handled but: + if (t == JsonToken.VALUE_TRUE) return true; + if (t == JsonToken.VALUE_FALSE) return false; if (t == JsonToken.VALUE_NULL) { return (Boolean) _coerceNullToken(ctxt, false); } @@ -459,14 +463,7 @@ protected final Boolean _parseBoolean(DeserializationContext ctxt, return (Boolean) ctxt.handleWeirdStringValue(_valueClass, text, "only \"true\" or \"false\" recognized"); } - // usually caller should have handled but: - if (t == JsonToken.VALUE_TRUE) { - return Boolean.TRUE; - } - if (t == JsonToken.VALUE_FALSE) { - return Boolean.FALSE; - } - if (t == JsonToken.START_ARRAY) { // unwrapping? + if (t == JsonToken.START_ARRAY) { // unwrapping / from-empty-array coercion? return (Boolean) _deserializeFromArray(p, ctxt); } // Otherwise, no can do: @@ -487,22 +484,129 @@ protected boolean _parseBooleanFromInt(JsonParser p, DeserializationContext ctxt return !"0".equals(p.getText()); } - protected final byte _parseBytePrimitive(JsonParser p, DeserializationContext ctxt) + protected final byte _parseBytePrimitive(DeserializationContext ctxt, JsonParser p, + Class targetType) throws IOException { - int value = _parseIntPrimitive(p, ctxt); - // So far so good: but does it fit? - if (_byteOverflow(value)) { - Number v = (Number) ctxt.handleWeirdStringValue(_valueClass, String.valueOf(value), - "overflow, value cannot be represented as 8-bit value"); - return _nonNullNumber(v).byteValue(); + final JsonToken t = p.currentToken(); + if (t == JsonToken.VALUE_NUMBER_INT) return p.getByteValue(); + if (t == JsonToken.VALUE_NULL) { + return (Byte) _coerceNullToken(ctxt, true); + } + if (t == JsonToken.VALUE_STRING) { // let's do implicit re-parse + String text = p.getText(); + CoercionAction act = _checkFromStringCoercion(ctxt, text, + LogicalType.Integer, targetType); + if (act == CoercionAction.AsNull) { +// _verifyNullForPrimitiveCoercion(ctxt, text); + return (byte) 0; // no need to check as does not come from `null`, explicit coercion + } + if (act == CoercionAction.AsEmpty) { + return (byte) 0; + } + text = text.trim(); + if (_hasTextualNull(text)) { + _verifyNullForPrimitiveCoercion(ctxt, text); + return (byte) 0; + } + int value; + try { + value = NumberInput.parseInt(text); + } catch (IllegalArgumentException iae) { + return (Byte) ctxt.handleWeirdStringValue(_valueClass, text, + "not a valid Byte value"); + } + // So far so good: but does it fit? + // as per [JACKSON-804], allow range up to 255, inclusive + if (_byteOverflow(value)) { + return (Byte) ctxt.handleWeirdStringValue(_valueClass, text, + "overflow, value cannot be represented as 8-bit value"); + // fall-through for deferred fails + } + return (byte) value; + } + if (t == JsonToken.VALUE_NUMBER_FLOAT) { + CoercionAction act = _checkFloatToIntCoercion(ctxt, p, targetType); + if (act == CoercionAction.AsNull) { + return (byte) 0; + } + if (act == CoercionAction.AsEmpty) { + return (byte) 0; + } + return p.getByteValue(); + } + // 12-Jun-2020, tatu: For some reason calling `_deserializeFromArray()` won't work so: + if (t == JsonToken.START_ARRAY && ctxt.isEnabled(DeserializationFeature.UNWRAP_SINGLE_VALUE_ARRAYS)) { + p.nextToken(); + final byte parsed = _parseBytePrimitive(ctxt, p, targetType); + _verifyEndArrayForSingle(p, ctxt); + return parsed; + } + return ((Byte) ctxt.handleUnexpectedToken(targetType, p)).byteValue(); + } + + protected Byte _parseByte(JsonParser p, DeserializationContext ctxt, + Class targetType) throws IOException + { + final JsonToken t = p.currentToken(); + if (t == JsonToken.VALUE_NUMBER_INT) return p.getByteValue(); + if (t == JsonToken.VALUE_NULL) { + return (Byte) _coerceNullToken(ctxt, false); + } + if (t == JsonToken.VALUE_STRING) { // let's do implicit re-parse + String text = p.getText(); + CoercionAction act = _checkFromStringCoercion(ctxt, text, + LogicalType.Integer, targetType); + if (act == CoercionAction.AsNull) { + return (Byte) getNullValue(ctxt); + } + if (act == CoercionAction.AsEmpty) { + return (Byte) getEmptyValue(ctxt); + } + text = text.trim(); + if (_hasTextualNull(text)) { + return (Byte) _coerceTextualNull(ctxt, false); + } + int value; + try { + value = NumberInput.parseInt(text); + } catch (IllegalArgumentException iae) { + return (Byte) ctxt.handleWeirdStringValue(_valueClass, text, + "not a valid Byte value"); + } + // So far so good: but does it fit? + // as per [JACKSON-804], allow range up to 255, inclusive + if (_byteOverflow(value)) { + return (Byte) ctxt.handleWeirdStringValue(_valueClass, text, + "overflow, value cannot be represented as 8-bit value"); + // fall-through for deferred fails + } + return Byte.valueOf((byte) value); } - return (byte) value; + if (t == JsonToken.VALUE_NUMBER_FLOAT) { + CoercionAction act = _checkFloatToIntCoercion(ctxt, p, targetType); + if (act == CoercionAction.AsNull) { + return (Byte) getNullValue(ctxt); + } + if (act == CoercionAction.AsEmpty) { + return (Byte) getEmptyValue(ctxt); + } + return p.getByteValue(); + } + if (t == JsonToken.START_ARRAY) { // [databind#381] + return (Byte) _deserializeFromArray(p, ctxt); + } + return (Byte) ctxt.handleUnexpectedToken(_valueClass, p); } protected final short _parseShortPrimitive(JsonParser p, DeserializationContext ctxt) throws IOException { + final JsonToken t = p.currentToken(); + if (t == JsonToken.VALUE_NUMBER_INT) return p.getShortValue(); + if (t == JsonToken.VALUE_NULL) { + return (Byte) _coerceNullToken(ctxt, true); + } int value = _parseIntPrimitive(p, ctxt); // So far so good: but does it fit? if (_shortOverflow(value)) { @@ -942,23 +1046,16 @@ protected CoercionAction _checkFromStringCoercion(DeserializationContext ctxt, S if (value.length() == 0) { act = ctxt.findCoercionAction(logicalType, rawTargetType, CoercionInputShape.EmptyString); - if (act == CoercionAction.Fail) { - ctxt.reportInputMismatch(this, -"Cannot coerce empty String (\"\") to %s (but could if enabling coercion using `CoercionConfig`)", -_coercedTypeDesc()); - } + return _checkCoercionActionFail(ctxt, act, "empty String (\"\")"); } else if (_isBlank(value)) { act = ctxt.findCoercionFromBlankString(logicalType, rawTargetType, CoercionAction.Fail); - if (act == CoercionAction.Fail) { - ctxt.reportInputMismatch(this, -"Cannot coerce blank String (all whitespace) to %s (but could if enabling coercion using `CoercionConfig`)", -_coercedTypeDesc()); - } + return _checkCoercionActionFail(ctxt, act, "blank String (all whitespace)"); } else { act = ctxt.findCoercionAction(logicalType, rawTargetType, CoercionInputShape.String); if (act == CoercionAction.Fail) { + // since it MIGHT (but might not), create desc here, do not use helper ctxt.reportInputMismatch(this, -"Cannot coerce String value (\"%s\") to %s (but might if enabling coercion using `CoercionConfig`)", +"Cannot coerce String value (\"%s\") to %s (but might if coercion using `CoercionConfig` was enabled)", value, _coercedTypeDesc()); } } @@ -968,17 +1065,30 @@ protected CoercionAction _checkFromStringCoercion(DeserializationContext ctxt, S /** * @since 2.12 */ - protected Boolean _coerceBooleanFromInt(DeserializationContext ctxt, JsonParser p, + protected CoercionAction _checkFloatToIntCoercion(DeserializationContext ctxt, JsonParser p, Class rawTargetType) throws IOException { + final CoercionAction act = ctxt.findCoercionAction(LogicalType.Integer, + rawTargetType, CoercionInputShape.Float); + if (act == CoercionAction.Fail) { + _checkCoercionActionFail(ctxt, act, "Floating-point value ("+p.getText()+")"); + } + return act; + } - final CoercionAction act = ctxt.findCoercionAction(LogicalType.Boolean, rawTargetType, CoercionInputShape.Integer); + /** + * @since 2.12 + */ + protected Boolean _coerceBooleanFromInt(DeserializationContext ctxt, JsonParser p, + Class rawTargetType) + throws IOException + { + CoercionAction act = ctxt.findCoercionAction(LogicalType.Boolean, rawTargetType, CoercionInputShape.Integer); switch (act) { case Fail: - ctxt.reportInputMismatch(this, -"Cannot coerce Integer value (%s) to %s (but might if enabling coercion using `CoercionConfig`)", -p.getText(), _coercedTypeDesc()); + _checkCoercionActionFail(ctxt, act, "Integer value ("+p.getText()+")"); + break; case AsNull: return null; case AsEmpty: @@ -988,8 +1098,22 @@ protected Boolean _coerceBooleanFromInt(DeserializationContext ctxt, JsonParser // 13-Oct-2016, tatu: As per [databind#1324], need to be careful wrt // degenerate case of huge integers, legal in JSON. // Also note that number tokens can not have WS to trim: - boolean b = !"0".equals(p.getText()); - return b; + if (p.getNumberType() == NumberType.INT) { + // but minor optimization for common case is possible: + return p.getIntValue() != 0; + } + return !"0".equals(p.getText()); + } + + protected CoercionAction _checkCoercionActionFail(DeserializationContext ctxt, + CoercionAction act, String inputDesc) throws IOException + { + if (act == CoercionAction.Fail) { + ctxt.reportInputMismatch(this, +"Cannot coerce %s to %s (but could if coercion was enabled using `CoercionConfig`)", +inputDesc, _coercedTypeDesc()); + } + return act; } /* 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 fbc4751bb3..33117d4aab 100644 --- a/src/test/java/com/fasterxml/jackson/databind/convert/TestArrayConversions.java +++ b/src/test/java/com/fasterxml/jackson/databind/convert/TestArrayConversions.java @@ -12,7 +12,7 @@ public class TestArrayConversions extends com.fasterxml.jackson.databind.BaseMapTest { final static String OVERFLOW_MSG_BYTE = "out of range of Java byte"; - final static String OVERFLOW_MSG = "overflow"; + final static String OVERFLOW_MSG_SHORT = "out of range of Java short"; final static String OVERFLOW_MSG_INT = "out of range of int"; final static String OVERFLOW_MSG_LONG = "out of range of long"; @@ -103,7 +103,7 @@ public void testOverflows() try { MAPPER.convertValue(new int[] { -99999 }, short[].class); } catch (IllegalArgumentException e) { - verifyException(e, OVERFLOW_MSG); + verifyException(e, OVERFLOW_MSG_SHORT); } // Int overflow try { diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/jdk/JDKScalarsTest.java b/src/test/java/com/fasterxml/jackson/databind/deser/jdk/JDKScalarsTest.java index b5b0498723..0d334f2e16 100644 --- a/src/test/java/com/fasterxml/jackson/databind/deser/jdk/JDKScalarsTest.java +++ b/src/test/java/com/fasterxml/jackson/databind/deser/jdk/JDKScalarsTest.java @@ -118,7 +118,11 @@ static class VoidBean { public Void value; } - private final ObjectMapper MAPPER = new ObjectMapper(); + private final ObjectMapper MAPPER = newJsonMapper(); + + final ObjectMapper MAPPER_NO_COERCION =jsonMapperBuilder() + .disable(MapperFeature.ALLOW_COERCION_OF_SCALARS) + .build(); /* /********************************************************** @@ -854,14 +858,14 @@ private void _testNullForPrimitiveArrays(Class cls, Object defValue, final String JSON_WITH_NULL = "[ null ]"; final String SIMPLE_NAME = "`"+cls.getSimpleName()+"`"; final ObjectReader readerCoerceOk = MAPPER.readerFor(cls); - final ObjectReader readerNoCoerce = readerCoerceOk + final ObjectReader readerNoNulls = readerCoerceOk .with(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES); Object ob = readerCoerceOk.forType(cls).readValue(JSON_WITH_NULL); assertEquals(1, Array.getLength(ob)); assertEquals(defValue, Array.get(ob, 0)); try { - readerNoCoerce.readValue(JSON_WITH_NULL); + readerNoNulls.readValue(JSON_WITH_NULL); fail("Should not pass"); } catch (JsonMappingException e) { verifyException(e, "Cannot coerce `null`"); @@ -873,8 +877,9 @@ private void _testNullForPrimitiveArrays(Class cls, Object defValue, assertEquals(1, Array.getLength(ob)); assertEquals(defValue, Array.get(ob, 0)); + final ObjectReader readerNoEmpty = MAPPER_NO_COERCION.readerFor(cls); try { - readerNoCoerce.readValue(EMPTY_STRING_JSON); + readerNoEmpty.readValue(EMPTY_STRING_JSON); fail("Should not pass"); } catch (JsonMappingException e) { // 07-Jun-2020, tatu: during transition, two acceptable alternatives