From 8db6d1d8e1793bbcb5b8d69ae8cb7fa0de2b25fa Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 5 Jan 2023 19:54:02 -0800 Subject: [PATCH] Fix #3690: improve error message for to-string coercion (#3717) --- release-notes/CREDITS-2.x | 2 + release-notes/VERSION-2.x | 2 + .../databind/deser/std/StdDeserializer.java | 34 ++++++++++++++--- .../convert/CoerceBoolToStringTest.java | 2 +- .../convert/CoerceFloatToIntTest.java | 16 ++++---- .../convert/CoerceFloatToStringTest.java | 3 +- .../convert/CoerceIntToFloatTest.java | 4 +- .../convert/CoerceIntToStringTest.java | 2 +- .../convert/DisableCoercions3690Test.java | 38 +++++++++++++++++++ 9 files changed, 84 insertions(+), 19 deletions(-) create mode 100644 src/test/java/com/fasterxml/jackson/databind/convert/DisableCoercions3690Test.java diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index d8c80ea2a3..7972aeacb5 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -1039,6 +1039,8 @@ João Guerra (joca-bt@github) (2.11.1) * Reported #3227: Content `null` handling not working for root values (2.13.0) + * Reported #3690: Incorrect target type for arrays when disabling coercion + (2.15.0) Ryan Bohn (bohnman@github) * Reported #2475: `StringCollectionSerializer` calls `JsonGenerator.setCurrentValue(value)`, diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 430f46026b..5f0b1034eb 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -12,6 +12,8 @@ Project: jackson-databind #3680: Timestamp in classes inside jar showing 02/01/1980 (fix contributed by Hervé B) #3682: Transient `Field`s are not ignored as Mutators if there is visible Getter +#3690: Incorrect target type for arrays when disabling coercion + (reported by João G) #3708: Seems like `java.nio.file.Path` is safe for Android API level 26 (contributed by @pjfanning) 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 2d77d9bcbe..92ddcb2f93 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 @@ -1401,6 +1401,9 @@ protected final String _parseString(JsonParser p, DeserializationContext ctxt, throws IOException { CoercionAction act = CoercionAction.TryConvert; + // 05-Jan-2022, tatu: We are usually coercing into `String` as element, + // or such; `_valueClass` would be wrong choice here + final Class rawTargetType = String.class; switch (p.currentTokenId()) { case JsonTokenId.ID_STRING: @@ -1420,17 +1423,20 @@ protected final String _parseString(JsonParser p, DeserializationContext ctxt, case JsonTokenId.ID_START_OBJECT: return ctxt.extractScalarFromObject(p, this, _valueClass); case JsonTokenId.ID_NUMBER_INT: - act = _checkIntToStringCoercion(p, ctxt, _valueClass); + act = _checkIntToStringCoercion(p, ctxt, rawTargetType); break; case JsonTokenId.ID_NUMBER_FLOAT: - act = _checkFloatToStringCoercion(p, ctxt, _valueClass); + act = _checkFloatToStringCoercion(p, ctxt, rawTargetType); break; case JsonTokenId.ID_TRUE: case JsonTokenId.ID_FALSE: - act = _checkBooleanToStringCoercion(p, ctxt, _valueClass); + act = _checkBooleanToStringCoercion(p, ctxt, rawTargetType); break; } + // !!! 05-Jan-2022, tatu: This might work in practice, but in theory + // we should not use "nullProvider" of this deserializer as this + // unlikely to be what we want (it's for containing type, not String) if (act == CoercionAction.AsNull) { return (String) nullProvider.getNullValue(ctxt); } @@ -1655,7 +1661,7 @@ protected CoercionAction _checkCoercionFail(DeserializationContext ctxt, if (act == CoercionAction.Fail) { ctxt.reportBadCoercion(this, targetType, inputValue, "Cannot coerce %s to %s (but could if coercion was enabled using `CoercionConfig`)", -inputDesc, _coercedTypeDesc()); +inputDesc, _coercedTypeDesc(targetType)); } return act; } @@ -1777,8 +1783,7 @@ protected String _coercedTypeDesc() { typeDesc = ClassUtil.getTypeDescription(t); } else { Class cls = handledType(); - structured = cls.isArray() || Collection.class.isAssignableFrom(cls) - || Map.class.isAssignableFrom(cls); + structured = ClassUtil.isCollectionMapOrArray(cls); typeDesc = ClassUtil.getClassDescription(cls); } if (structured) { @@ -1787,6 +1792,23 @@ protected String _coercedTypeDesc() { return typeDesc+" value"; } + /** + * Helper method called to get a description of type into which a scalar value coercion + * is being applied, to be used for constructing exception messages + * on coerce failure. + * + * @return Message with backtick-enclosed name of target type + * + * @since 2.15 + */ + protected String _coercedTypeDesc(Class rawTargetType) { + String typeDesc = ClassUtil.getClassDescription(rawTargetType); + if (ClassUtil.isCollectionMapOrArray(rawTargetType)) { + return "element of "+typeDesc; + } + return typeDesc+" value"; + } + /* /********************************************************************** /* Helper methods for sub-classes, coercions, older (pre-2.12), deprecated diff --git a/src/test/java/com/fasterxml/jackson/databind/convert/CoerceBoolToStringTest.java b/src/test/java/com/fasterxml/jackson/databind/convert/CoerceBoolToStringTest.java index dc8ac69acc..dd7a73f3da 100644 --- a/src/test/java/com/fasterxml/jackson/databind/convert/CoerceBoolToStringTest.java +++ b/src/test/java/com/fasterxml/jackson/databind/convert/CoerceBoolToStringTest.java @@ -67,7 +67,7 @@ public void testCoerceConfigToFail() throws JsonProcessingException { _verifyCoerceFail(MAPPER_TO_FAIL, String.class, "true"); _verifyCoerceFail(MAPPER_TO_FAIL, StringWrapper.class, "{\"str\": false}", "string"); - _verifyCoerceFail(MAPPER_TO_FAIL, String[].class, "[ true ]", "element of `java.lang.String[]`"); + _verifyCoerceFail(MAPPER_TO_FAIL, String[].class, "[ true ]", "to `java.lang.String` value"); } /* diff --git a/src/test/java/com/fasterxml/jackson/databind/convert/CoerceFloatToIntTest.java b/src/test/java/com/fasterxml/jackson/databind/convert/CoerceFloatToIntTest.java index 8bbf95d3c1..f289799f2d 100644 --- a/src/test/java/com/fasterxml/jackson/databind/convert/CoerceFloatToIntTest.java +++ b/src/test/java/com/fasterxml/jackson/databind/convert/CoerceFloatToIntTest.java @@ -77,7 +77,7 @@ public void testLegacyFailDoubleToInt() throws Exception _verifyCoerceFail(READER_LEGACY_FAIL, Integer.class, "1.5", "java.lang.Integer"); _verifyCoerceFail(READER_LEGACY_FAIL, Integer.TYPE, "1.5", "int"); _verifyCoerceFail(READER_LEGACY_FAIL, IntWrapper.class, "{\"i\":-2.25 }", "int"); - _verifyCoerceFail(READER_LEGACY_FAIL, int[].class, "[ 2.5 ]", "element of `int[]`"); + _verifyCoerceFail(READER_LEGACY_FAIL, int[].class, "[ 2.5 ]", "to `int` value"); } public void testLegacyFailDoubleToLong() throws Exception @@ -85,18 +85,18 @@ public void testLegacyFailDoubleToLong() throws Exception _verifyCoerceFail(READER_LEGACY_FAIL, Long.class, "0.5"); _verifyCoerceFail(READER_LEGACY_FAIL, Long.TYPE, "-2.5"); _verifyCoerceFail(READER_LEGACY_FAIL, LongWrapper.class, "{\"l\": 7.7 }"); - _verifyCoerceFail(READER_LEGACY_FAIL, long[].class, "[ -1.35 ]", "element of `long[]`"); + _verifyCoerceFail(READER_LEGACY_FAIL, long[].class, "[ -1.35 ]", "to `long` value"); } public void testLegacyFailDoubleToOther() throws Exception { _verifyCoerceFail(READER_LEGACY_FAIL, Short.class, "0.5"); _verifyCoerceFail(READER_LEGACY_FAIL, Short.TYPE, "-2.5"); - _verifyCoerceFail(READER_LEGACY_FAIL, short[].class, "[ -1.35 ]", "element of `short[]`"); + _verifyCoerceFail(READER_LEGACY_FAIL, short[].class, "[ -1.35 ]", "to `short` value"); _verifyCoerceFail(READER_LEGACY_FAIL, Byte.class, "0.5"); _verifyCoerceFail(READER_LEGACY_FAIL, Byte.TYPE, "-2.5"); - _verifyCoerceFail(READER_LEGACY_FAIL, byte[].class, "[ -1.35 ]", "element of `byte[]`"); + _verifyCoerceFail(READER_LEGACY_FAIL, byte[].class, "[ -1.35 ]", "to `byte` value"); _verifyCoerceFail(READER_LEGACY_FAIL, BigInteger.class, "25236.256"); @@ -280,20 +280,20 @@ public void testCoerceConfigFailFromFloat() throws Exception _verifyCoerceFail(MAPPER_TO_FAIL, Integer.class, "1.5"); _verifyCoerceFail(MAPPER_TO_FAIL, Integer.TYPE, "1.5"); _verifyCoerceFail(MAPPER_TO_FAIL, IntWrapper.class, "{\"i\":-2.25 }", "int"); - _verifyCoerceFail(MAPPER_TO_FAIL, int[].class, "[ 2.5 ]", "element of `int[]`"); + _verifyCoerceFail(MAPPER_TO_FAIL, int[].class, "[ 2.5 ]", "to `int` value"); _verifyCoerceFail(MAPPER_TO_FAIL, Long.class, "0.5"); _verifyCoerceFail(MAPPER_TO_FAIL, Long.TYPE, "-2.5"); _verifyCoerceFail(MAPPER_TO_FAIL, LongWrapper.class, "{\"l\": 7.7 }"); - _verifyCoerceFail(MAPPER_TO_FAIL, long[].class, "[ -1.35 ]", "element of `long[]`"); + _verifyCoerceFail(MAPPER_TO_FAIL, long[].class, "[ -1.35 ]", "to `long` value"); _verifyCoerceFail(MAPPER_TO_FAIL, Short.class, "0.5"); _verifyCoerceFail(MAPPER_TO_FAIL, Short.TYPE, "-2.5"); - _verifyCoerceFail(MAPPER_TO_FAIL, short[].class, "[ -1.35 ]", "element of `short[]`"); + _verifyCoerceFail(MAPPER_TO_FAIL, short[].class, "[ -1.35 ]", "to `short` value"); _verifyCoerceFail(MAPPER_TO_FAIL, Byte.class, "0.5"); _verifyCoerceFail(MAPPER_TO_FAIL, Byte.TYPE, "-2.5"); - _verifyCoerceFail(MAPPER_TO_FAIL, byte[].class, "[ -1.35 ]", "element of `byte[]`"); + _verifyCoerceFail(MAPPER_TO_FAIL, byte[].class, "[ -1.35 ]", "to `byte` value"); _verifyCoerceFail(MAPPER_TO_FAIL, BigInteger.class, "25236.256"); } diff --git a/src/test/java/com/fasterxml/jackson/databind/convert/CoerceFloatToStringTest.java b/src/test/java/com/fasterxml/jackson/databind/convert/CoerceFloatToStringTest.java index 5611d285e9..8f492eea5a 100644 --- a/src/test/java/com/fasterxml/jackson/databind/convert/CoerceFloatToStringTest.java +++ b/src/test/java/com/fasterxml/jackson/databind/convert/CoerceFloatToStringTest.java @@ -68,7 +68,8 @@ public void testCoerceConfigToFail() throws JsonProcessingException { _verifyCoerceFail(MAPPER_TO_FAIL, String.class, "3.5"); _verifyCoerceFail(MAPPER_TO_FAIL, StringWrapper.class, "{\"str\": -5.3}", "string"); - _verifyCoerceFail(MAPPER_TO_FAIL, String[].class, "[ 2.1 ]", "element of `java.lang.String[]`"); + _verifyCoerceFail(MAPPER_TO_FAIL, String[].class, "[ 2.1 ]", + "to `java.lang.String` value"); } /* diff --git a/src/test/java/com/fasterxml/jackson/databind/convert/CoerceIntToFloatTest.java b/src/test/java/com/fasterxml/jackson/databind/convert/CoerceIntToFloatTest.java index e08f5d41ef..c1f1c23085 100644 --- a/src/test/java/com/fasterxml/jackson/databind/convert/CoerceIntToFloatTest.java +++ b/src/test/java/com/fasterxml/jackson/databind/convert/CoerceIntToFloatTest.java @@ -110,12 +110,12 @@ public void testCoerceConfigToFail() throws JsonProcessingException _verifyCoerceFail(MAPPER_TO_FAIL, Float.class, "3"); _verifyCoerceFail(MAPPER_TO_FAIL, Float.TYPE, "-2"); _verifyCoerceFail(MAPPER_TO_FAIL, FloatWrapper.class, "{\"f\": -5}", "float"); - _verifyCoerceFail(MAPPER_TO_FAIL, float[].class, "[ 2 ]", "element of `float[]`"); + _verifyCoerceFail(MAPPER_TO_FAIL, float[].class, "[ 2 ]", "to `float` value"); _verifyCoerceFail(MAPPER_TO_FAIL, Double.class, "-1"); _verifyCoerceFail(MAPPER_TO_FAIL, Double.TYPE, "4"); _verifyCoerceFail(MAPPER_TO_FAIL, DoubleWrapper.class, "{\"d\": 2}", "double"); - _verifyCoerceFail(MAPPER_TO_FAIL, double[].class, "[ -2 ]", "element of `double[]`"); + _verifyCoerceFail(MAPPER_TO_FAIL, double[].class, "[ -2 ]", "to `double` value"); _verifyCoerceFail(MAPPER_TO_FAIL, BigDecimal.class, "73455342"); } diff --git a/src/test/java/com/fasterxml/jackson/databind/convert/CoerceIntToStringTest.java b/src/test/java/com/fasterxml/jackson/databind/convert/CoerceIntToStringTest.java index e186a7777d..1ee22b4e7f 100644 --- a/src/test/java/com/fasterxml/jackson/databind/convert/CoerceIntToStringTest.java +++ b/src/test/java/com/fasterxml/jackson/databind/convert/CoerceIntToStringTest.java @@ -68,7 +68,7 @@ public void testCoerceConfigToFail() throws JsonProcessingException { _verifyCoerceFail(MAPPER_TO_FAIL, String.class, "3"); _verifyCoerceFail(MAPPER_TO_FAIL, StringWrapper.class, "{\"str\": -5}", "string"); - _verifyCoerceFail(MAPPER_TO_FAIL, String[].class, "[ 2 ]", "element of `java.lang.String[]`"); + _verifyCoerceFail(MAPPER_TO_FAIL, String[].class, "[ 2 ]", "to `java.lang.String` value"); } /* diff --git a/src/test/java/com/fasterxml/jackson/databind/convert/DisableCoercions3690Test.java b/src/test/java/com/fasterxml/jackson/databind/convert/DisableCoercions3690Test.java new file mode 100644 index 0000000000..eedd79f71d --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/databind/convert/DisableCoercions3690Test.java @@ -0,0 +1,38 @@ +package com.fasterxml.jackson.databind.convert; + +import java.util.List; + +import com.fasterxml.jackson.databind.*; +import com.fasterxml.jackson.databind.cfg.CoercionAction; +import com.fasterxml.jackson.databind.cfg.CoercionInputShape; +import com.fasterxml.jackson.databind.exc.InvalidFormatException; + +public class DisableCoercions3690Test extends BaseMapTest +{ + static class Input3690 { + public List field; + } + + // [databind#3690] + public void testFailMessage3690() throws Exception + { + ObjectMapper mapper = jsonMapperBuilder() + .withCoercionConfigDefaults(config -> { + config.setCoercion(CoercionInputShape.Boolean, CoercionAction.Fail) + .setCoercion(CoercionInputShape.Integer, CoercionAction.Fail) + .setCoercion(CoercionInputShape.Float, CoercionAction.Fail) + .setCoercion(CoercionInputShape.String, CoercionAction.Fail) + .setCoercion(CoercionInputShape.Array, CoercionAction.Fail) + .setCoercion(CoercionInputShape.Object, CoercionAction.Fail); + }) + .build(); + String json = "{ \"field\": [ 1 ] }"; + try { + mapper.readValue(json, Input3690.class); + fail("Should not pass"); + } catch (InvalidFormatException e) { + verifyException(e, "Cannot coerce Integer value (1)"); + verifyException(e, "to `java.lang.String` value"); + } + } +}