Fixed #1513

Author: cowtowncoder

release-notes/VERSION

@@ -7,8 +7,12 @@ Project: jackson-databind
 
 #935: `@JsonProperty(access = Access.READ_ONLY)` - unexpected behaviour
 #1317: '@JsonIgnore' annotation not working with creator properties, serialization
+#1367: No Object Id found for an instance when using `@ConstructorProperties`
 #1505: @JsonEnumDefaultValue should take precedence over FAIL_ON_NUMBERS_FOR_ENUMS
  (suggested by Stephan S)
+#1506: Missing `KeyDeserializer` for `CharSequence`
+#1513: `MapSerializer._orderEntries()` throws NPE when operating on `ConcurrentHashMap`
+ (reported by Sovietaced@github)
 - Simplified processing of class annotations (for `AnnotatedClass`) to try to
   solve rare concurrency problems with "root name" annotations.
 

src/main/java/com/fasterxml/jackson/databind/ser/std/MapSerializer.java

@@ -952,7 +952,9 @@ protected Map<?,?> _orderEntries(Map<?,?> input, JsonGenerator gen,
         }
         // [databind#1411]: TreeMap does not like null key... (although note that
         //   check above should prevent this code from being called in that case)
-        if (input.containsKey(null)) {
+        // [databind#153]: but, apparently, some custom Maps do manage hit this
+        //   problem.
+        if (_hasNullKey(input)) {
             TreeMap<Object,Object> result = new TreeMap<Object,Object>();
             for (Map.Entry<?,?> entry : input.entrySet()) {
                 Object key = entry.getKey();
@@ -967,6 +969,22 @@ protected Map<?,?> _orderEntries(Map<?,?> input, JsonGenerator gen,
         return new TreeMap<Object,Object>(input);
     }
 
+    /**
+     * @since 2.8.7
+     */
+    protected boolean _hasNullKey(Map<?,?> input) {
+        // 19-Feb-2017, tatu: As per [databind#1513] there are many cases where `null`
+        //   keys are not allowed, and even attempt to check for presence can cause
+        //   problems. Without resorting to external sorting (and internal API change),
+        //   or custom sortable Map implementation (more code) we can try black- or
+        //   white-listing (that is; either skip known problem cases; or only apply for
+        //   known good cases).
+        //   While my first instinct was to do black-listing (remove Hashtable and ConcurrentHashMap),
+        //   all in all it is probably better to just white list `HashMap` (and its sub-classes).
+        
+        return (input instanceof HashMap) && input.containsKey(null);
+    }
+    
     protected void _writeNullKeyedEntry(JsonGenerator gen, SerializerProvider provider,
             Object suppressableValue, Object value) throws IOException
     {

src/test/java/com/fasterxml/jackson/databind/ser/TestMapSerialization.java

@@ -287,14 +287,27 @@ public void testNullJsonInTypedMap691() throws Exception {
     }
 
     // [databind#1513]
-    public void testConcurrentSkipListMap() throws Exception
+    public void testConcurrentMaps() throws Exception
     {
+        final ObjectWriter w = MAPPER.writer(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS);
+
         Map<String,String> input = new ConcurrentSkipListMap<String,String>();
+        input.put("x", "y");
         input.put("a", "b");
+        String json = w.writeValueAsString(input);
+        assertEquals(aposToQuotes("{'a':'b','x':'y'}"), json);
+
+        input = new ConcurrentHashMap<String,String>();
         input.put("x", "y");
-        String json = MAPPER
-                .writer().with(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS)
-                .writeValueAsString(input);
+        input.put("a", "b");
+        json = w.writeValueAsString(input);
+        assertEquals(aposToQuotes("{'a':'b','x':'y'}"), json);
+
+        // One more: while not technically concurrent map at all, exhibits same issue
+        input = new Hashtable<String,String>();
+        input.put("x", "y");
+        input.put("a", "b");
+        json = w.writeValueAsString(input);
         assertEquals(aposToQuotes("{'a':'b','x':'y'}"), json);
     }
 }