Fix #2128

FasterXML · Sep 6, 2018 · f6cf181 · f6cf181
1 parent 323cd0b
commit f6cf181
Show file tree

Hide file tree

Showing 20 changed files with 82 additions and 37 deletions.
diff --git a/pom.xml b/pom.xml
@@ -5,7 +5,7 @@
   <parent>
     <groupId>com.fasterxml.jackson</groupId>
     <artifactId>jackson-base</artifactId>
-    <version>2.9.6</version>
+    <version>2.9.7-SNAPSHOT</version>
   </parent>
 
   <groupId>com.fasterxml.jackson.core</groupId>

diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x
@@ -23,6 +23,7 @@ Project: jackson-databind
   - CVE-2018-14721)
 #2109: Canonical string for reference type is built incorrectly
  (reported by svarzee@github)
+#2128: Location information included twice for some `JsonMappingException`s
 
 2.9.6 (12-Jun-2018)
 

diff --git a/src/main/java/com/fasterxml/jackson/databind/DatabindContext.java b/src/main/java/com/fasterxml/jackson/databind/DatabindContext.java
@@ -191,7 +191,8 @@ public JavaType resolveSubType(JavaType baseType, String subClass)
             } catch (Exception e) {
                 throw invalidTypeIdException(baseType, subClass, String.format(
                         "problem: (%s) %s",
-                        e.getClass().getName(), e.getMessage()));
+                        e.getClass().getName(),
+                        ClassUtil.exceptionMessage(e)));
             }
             if (baseType.isTypeOrSuperTypeOf(cls)) {
                 return getTypeFactory().constructSpecializedType(baseType, cls);

diff --git a/src/main/java/com/fasterxml/jackson/databind/DeserializationContext.java b/src/main/java/com/fasterxml/jackson/databind/DeserializationContext.java
@@ -710,7 +710,8 @@ public Date parseDate(String dateStr) throws IllegalArgumentException
             return df.parse(dateStr);
         } catch (ParseException e) {
             throw new IllegalArgumentException(String.format(
-                    "Failed to parse Date value '%s': %s", dateStr, e.getMessage()));
+                    "Failed to parse Date value '%s': %s", dateStr,
+                    ClassUtil.exceptionMessage(e)));
         }
     }
 
@@ -1599,7 +1600,7 @@ public JsonMappingException instantiationException(Class<?> instClass, Throwable
         String excMsg;
         if (cause == null) {
             excMsg = "N/A";
-        } else if ((excMsg = cause.getMessage()) == null) {
+        } else if ((excMsg = ClassUtil.exceptionMessage(cause)) == null) {
             excMsg = ClassUtil.nameOf(cause.getClass());
         }
         String msg = String.format("Cannot construct instance of %s, problem: %s",

diff --git a/src/main/java/com/fasterxml/jackson/databind/JsonMappingException.java b/src/main/java/com/fasterxml/jackson/databind/JsonMappingException.java
@@ -7,6 +7,7 @@
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.core.*;
+import com.fasterxml.jackson.databind.util.ClassUtil;
 
 /**
  * Checked exception used to signal fatal problems with mapping of
@@ -335,7 +336,8 @@ public static JsonMappingException from(SerializerProvider ctxt, String msg, Thr
     public static JsonMappingException fromUnexpectedIOE(IOException src) {
         return new JsonMappingException(null,
                 String.format("Unexpected IOException (of type %s): %s",
-                        src.getClass().getName(), src.getMessage()));
+                        src.getClass().getName(),
+                        ClassUtil.exceptionMessage(src)));
     }
 
     /**
@@ -375,7 +377,8 @@ public static JsonMappingException wrapWithPath(Throwable src, Reference ref)
         if (src instanceof JsonMappingException) {
             jme = (JsonMappingException) src;
         } else {
-            String msg = src.getMessage();
+            // [databind#2128]: try to avoid duplication
+            String msg = ClassUtil.exceptionMessage(src);
             // Let's use a more meaningful placeholder if all we have is null
             if (msg == null || msg.length() == 0) {
                 msg = "(was "+src.getClass().getName()+")";
@@ -486,9 +489,7 @@ public String getMessage() {
 
     protected String _buildMessage()
     {
-        /* First: if we have no path info, let's just use parent's
-         * definition as is
-         */
+        // First: if we have no path info, let's just use parent's definition as is
         String msg = super.getMessage();
         if (_path == null) {
             return msg;

diff --git a/src/main/java/com/fasterxml/jackson/databind/SerializerProvider.java b/src/main/java/com/fasterxml/jackson/databind/SerializerProvider.java
@@ -1335,11 +1335,10 @@ protected JsonSerializer<Object> _createAndCacheUntypedSerializer(Class<?> rawTy
         try {
             ser = _createUntypedSerializer(fullType);
         } catch (IllegalArgumentException iae) {
-            /* We better only expose checked exceptions, since those
-             * are what caller is expected to handle
-             */
+            // We better only expose checked exceptions, since those
+            // are what caller is expected to handle
             ser = null; // doesn't matter but compiler whines otherwise
-            reportMappingProblem(iae, iae.getMessage());
+            reportMappingProblem(iae, ClassUtil.exceptionMessage(iae));
         }
 
         if (ser != null) {
@@ -1359,7 +1358,7 @@ protected JsonSerializer<Object> _createAndCacheUntypedSerializer(JavaType type)
             // We better only expose checked exceptions, since those
             // are what caller is expected to handle
             ser = null;
-            reportMappingProblem(iae, iae.getMessage());
+            reportMappingProblem(iae, ClassUtil.exceptionMessage(iae));
         }
 
         if (ser != null) {

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java
@@ -1586,7 +1586,7 @@ public TypeDeserializer findTypeDeserializer(DeserializationConfig config,
             return b.buildTypeDeserializer(config, baseType, subtypes);
         } catch (IllegalArgumentException e0) {
             InvalidDefinitionException e = InvalidDefinitionException.from((JsonParser) null,
-                    e0.getMessage(), baseType);
+                    ClassUtil.exceptionMessage(e0), baseType);
             e.initCause(e0);
             throw e;
         }

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java
@@ -218,7 +218,8 @@ public JsonDeserializer<Object> buildBeanDeserializer(DeserializationContext ctx
             // 05-Apr-2017, tatu: Although it might appear cleaner to require collector
             //   to throw proper exception, it doesn't actually have reference to this
             //   instance so...
-            throw InvalidDefinitionException.from(ctxt.getParser(), e.getMessage(),
+            throw InvalidDefinitionException.from(ctxt.getParser(),
+                    ClassUtil.exceptionMessage(e),
                     beanDesc, null);
         }
         BeanDeserializerBuilder builder = constructBeanDeserializerBuilder(ctxt, beanDesc);
@@ -276,7 +277,8 @@ protected JsonDeserializer<Object> buildBuilderBasedDeserializer(
             // 05-Apr-2017, tatu: Although it might appear cleaner to require collector
             //   to throw proper exception, it doesn't actually have reference to this
             //   instance so...
-            throw InvalidDefinitionException.from(ctxt.getParser(), e.getMessage(),
+            throw InvalidDefinitionException.from(ctxt.getParser(),
+                    ClassUtil.exceptionMessage(e),
                     builderDesc, null);
         }
         final DeserializationConfig config = ctxt.getConfig();

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java b/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java
@@ -265,7 +265,7 @@ protected JsonDeserializer<Object> _createAndCache2(DeserializationContext ctxt,
         } catch (IllegalArgumentException iae) {
             // We better only expose checked exceptions, since those
             // are what caller is expected to handle
-            throw JsonMappingException.from(ctxt, iae.getMessage(), iae);
+            throw JsonMappingException.from(ctxt, ClassUtil.exceptionMessage(iae), iae);
         }
         if (deser == null) {
             return null;

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/SettableAnyProperty.java b/src/main/java/com/fasterxml/jackson/databind/deser/SettableAnyProperty.java
@@ -199,7 +199,7 @@ protected void _throwAsIOE(Exception e, Object propName, Object value)
             StringBuilder msg = new StringBuilder("Problem deserializing \"any\" property '").append(propName);
             msg.append("' of class "+getClassName()+" (expected type: ").append(_type);
             msg.append("; actual type: ").append(actType).append(")");
-            String origMsg = e.getMessage();
+            String origMsg = ClassUtil.exceptionMessage(e);
             if (origMsg != null) {
                 msg.append(", problem: ").append(origMsg);
             } else {
@@ -211,7 +211,7 @@ protected void _throwAsIOE(Exception e, Object propName, Object value)
         ClassUtil.throwIfRTE(e);
         // let's wrap the innermost problem
         Throwable t = ClassUtil.getRootCause(e);
-        throw new JsonMappingException(null, t.getMessage(), t);
+        throw new JsonMappingException(null, ClassUtil.exceptionMessage(t), t);
     }
 
     private String getClassName() { return _setter.getDeclaringClass().getName(); }

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/SettableBeanProperty.java b/src/main/java/com/fasterxml/jackson/databind/deser/SettableBeanProperty.java
@@ -587,7 +587,7 @@ protected void _throwAsIOE(JsonParser p, Exception e, Object value) throws IOExc
                     .append(getType())
                     .append("; actual type: ")
                     .append(actType).append(")");
-            String origMsg = e.getMessage();
+            String origMsg = ClassUtil.exceptionMessage(e);
             if (origMsg != null) {
                 msg.append(", problem: ")
                     .append(origMsg);
@@ -608,7 +608,7 @@ protected IOException _throwAsIOE(JsonParser p, Exception e) throws IOException
         ClassUtil.throwIfRTE(e);
         // let's wrap the innermost problem
         Throwable th = ClassUtil.getRootCause(e);
-        throw JsonMappingException.from(p, th.getMessage(), th);
+        throw JsonMappingException.from(p, ClassUtil.exceptionMessage(th), th);
     }
 
     @Deprecated // since 2.7

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
@@ -522,7 +522,8 @@ protected java.util.Date _parseDate(String value, DeserializationContext ctxt)
             return ctxt.parseDate(value);
         } catch (IllegalArgumentException iae) {
             return (java.util.Date) ctxt.handleWeirdStringValue(_valueClass, value,
-                    "not a valid representation (error: %s)", iae.getMessage());
+                    "not a valid representation (error: %s)",
+                    ClassUtil.exceptionMessage(iae));
         }
     }
 

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/StdKeyDeserializer.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/StdKeyDeserializer.java
@@ -131,7 +131,8 @@ public Object deserializeKey(String key, DeserializationContext ctxt)
             }
         } catch (Exception re) {
             return ctxt.handleWeirdKey(_keyClass, key, "not a valid representation, problem: (%s) %s",
-                    re.getClass().getName(), re.getMessage());
+                    re.getClass().getName(),
+                    ClassUtil.exceptionMessage(re));
         }
         if (_keyClass.isEnum() && ctxt.getConfig().isEnabled(DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_AS_NULL)) {
             return null;
@@ -257,7 +258,8 @@ protected double _parseDouble(String key) throws IllegalArgumentException {
 
     // @since 2.9
     protected Object _weirdKey(DeserializationContext ctxt, String key, Exception e) throws IOException {
-        return ctxt.handleWeirdKey(_keyClass, key, "problem: %s", e.getMessage());
+        return ctxt.handleWeirdKey(_keyClass, key, "problem: %s",
+                ClassUtil.exceptionMessage(e));
     }
 
     /*

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/StdValueInstantiator.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/StdValueInstantiator.java
@@ -453,7 +453,7 @@ protected JsonMappingException wrapException(Throwable t)
             }
         }
         return new JsonMappingException(null,
-                "Instantiation of "+getValueTypeDesc()+" value failed: "+t.getMessage(), t);
+                "Instantiation of "+getValueTypeDesc()+" value failed: "+ClassUtil.exceptionMessage(t), t);
     }
 
     /**

diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/BasicBeanDescription.java b/src/main/java/com/fasterxml/jackson/databind/introspect/BasicBeanDescription.java
@@ -359,7 +359,9 @@ public Object instantiateBean(boolean fixAccess) {
             }
             ClassUtil.throwIfError(t);
             ClassUtil.throwIfRTE(t);
-            throw new IllegalArgumentException("Failed to instantiate bean of type "+_classInfo.getAnnotated().getName()+": ("+t.getClass().getName()+") "+t.getMessage(), t);
+            throw new IllegalArgumentException("Failed to instantiate bean of type "
+                    +_classInfo.getAnnotated().getName()+": ("+t.getClass().getName()+") "
+                    +ClassUtil.exceptionMessage(t), t);
         }
     }
 

diff --git a/src/main/java/com/fasterxml/jackson/databind/ser/DefaultSerializerProvider.java b/src/main/java/com/fasterxml/jackson/databind/ser/DefaultSerializerProvider.java
@@ -168,7 +168,7 @@ public boolean includeFilterSuppressNulls(Object filter) throws JsonMappingExcep
         } catch (Throwable t) {
             String msg = String.format(
 "Problem determining whether filter of type '%s' should filter out `null` values: (%s) %s",
-filter.getClass().getName(), t.getClass().getName(), t.getMessage());
+filter.getClass().getName(), t.getClass().getName(), ClassUtil.exceptionMessage(t));
             reportBadDefinition(filter.getClass(), msg, t);
             return false; // never gets here
         }
@@ -502,7 +502,7 @@ private IOException _wrapAsIOE(JsonGenerator g, Exception e) {
         if (e instanceof IOException) {
             return (IOException) e;
         }
-        String msg = e.getMessage();
+        String msg = ClassUtil.exceptionMessage(e);
         if (msg == null) {
             msg = "[no message for "+e.getClass().getName()+"]";
         }

diff --git a/src/main/java/com/fasterxml/jackson/databind/ser/PropertyBuilder.java b/src/main/java/com/fasterxml/jackson/databind/ser/PropertyBuilder.java
@@ -100,9 +100,9 @@ protected BeanPropertyWriter buildWriter(SerializerProvider prov,
             serializationType = findSerializationType(am, defaultUseStaticTyping, declaredType);
         } catch (JsonMappingException e) {
             if (propDef == null) {
-                return prov.reportBadDefinition(declaredType, e.getMessage());
+                return prov.reportBadDefinition(declaredType, ClassUtil.exceptionMessage(e));
             }
-            return prov.reportBadPropertyDefinition(_beanDesc, propDef, e.getMessage());
+            return prov.reportBadPropertyDefinition(_beanDesc, propDef, ClassUtil.exceptionMessage(e));
         }
 
         // Container types can have separate type serializers for content (value / element) type

diff --git a/src/main/java/com/fasterxml/jackson/databind/util/ClassUtil.java b/src/main/java/com/fasterxml/jackson/databind/util/ClassUtil.java
@@ -7,6 +7,7 @@
 import java.util.*;
 
 import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.DeserializationContext;
 import com.fasterxml.jackson.databind.JavaType;
 import com.fasterxml.jackson.databind.JsonMappingException;
@@ -722,6 +723,12 @@ public static String nameOf(Named named) {
         return backticked(named.getName());
     }
 
+    /*
+    /**********************************************************
+    /* Other escaping, description acces
+    /**********************************************************
+     */
+
     /**
      * Returns either `text` or [null].
      *
@@ -734,6 +741,22 @@ public static String backticked(String text) {
         return new StringBuilder(text.length()+2).append('`').append(text).append('`').toString();
     }
 
+    /**
+     * Helper method that returns {@link Throwable#getMessage()} for all other exceptions
+     * except for {@link JsonProcessingException}, for which {@code getOriginalMessage()} is
+     * returned instead.
+     * Method is used to avoid accidentally including trailing location information twice
+     * in message when wrapping exceptions.
+     *
+     * @since 2.9.7
+     */
+    public static String exceptionMessage(Throwable t) {
+        if (t instanceof JsonProcessingException) {
+            return ((JsonProcessingException) t).getOriginalMessage();
+        }
+        return t.getMessage();
+    }
+
     /*
     /**********************************************************
     /* Primitive type support

diff --git a/src/test/java/com/fasterxml/jackson/databind/exc/BasicExceptionTest.java b/src/test/java/com/fasterxml/jackson/databind/exc/BasicExceptionTest.java
@@ -3,9 +3,10 @@
 import java.io.StringWriter;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Map;
 
 import com.fasterxml.jackson.core.*;
-
+import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.*;
 import com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition;
 import com.fasterxml.jackson.databind.type.TypeFactory;
@@ -111,4 +112,20 @@ public void testUnrecognizedProperty() throws Exception
         assertEquals(getClass(), e.getReferringClass());
         p.close();
     }
+
+    // [databind#2128]: ensure Location added once and only once
+    public void testLocationAddition() throws Exception
+    {
+        try {
+            /*Map<?,?> map =*/ MAPPER.readValue("{\"value\":\"foo\"}",
+                    new TypeReference<Map<ABC, Integer>>() { });
+            fail("Should not pass");
+        } catch (MismatchedInputException e) {
+            String msg = e.getMessage();
+            String[] str = msg.split(" at \\[");
+            if (str.length != 2) {
+                fail("Should only get one 'at [' marker, got "+(str.length-1)+", source: "+msg);
+            }
+        }
+    }
 }
diff --git a/src/test/java/com/fasterxml/jackson/databind/exc/ExceptionPathTest.java b/src/test/java/com/fasterxml/jackson/databind/exc/ExceptionPathTest.java
@@ -38,9 +38,4 @@ public void testReferenceChainForInnerClass() throws Exception
                     reference.toString());
         }
     }
-
-    public static void main(String[] args)
-    {
-        System.err.println("Int, full: "+Integer.TYPE.getName());
-    }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -453,7 +453,7 @@ protected JsonMappingException wrapException(Throwable t) @@
                 }
             }
             return new JsonMappingException(null,
-                    "Instantiation of "+getValueTypeDesc()+" value failed: "+t.getMessage(), t);
+                    "Instantiation of "+getValueTypeDesc()+" value failed: "+ClassUtil.exceptionMessage(t), t);
         }
         /**
@@ Expand Down @@