From 9434f515e559ccb744064cfc82da3c41c64248ef Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 9 Dec 2016 16:31:38 -0800 Subject: [PATCH] Fix #357 properly. Finally! --- release-notes/VERSION | 2 + .../ser/std/StdDelegatingSerializer.java | 4 -- .../databind/ser/std/StdSerializer.java | 31 +++++++++------ .../convert/TestConvertingDeserializer.java | 24 +++++++++++- .../failing/ConvertingSerializer357Test.java | 38 ------------------- 5 files changed, 44 insertions(+), 55 deletions(-) delete mode 100644 src/test/java/com/fasterxml/jackson/failing/ConvertingSerializer357Test.java diff --git a/release-notes/VERSION b/release-notes/VERSION index 58af5c6ea0..b81276187f 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -12,6 +12,8 @@ Project: jackson-databind #291: @JsonTypeInfo with As.EXTERNAL_PROPERTY doesn't work if external type property is referenced more than once (reported by Starkom@github) +#357: StackOverflowError with contentConverter that returns array type + (reported by Florian S) #476: Allow "Serialize as POJO" using `@JsonFormat(shape=Shape.OBJECT)` class annotation #507: Support for default `@JsonView` for a class (suggested by Mark W) diff --git a/src/main/java/com/fasterxml/jackson/databind/ser/std/StdDelegatingSerializer.java b/src/main/java/com/fasterxml/jackson/databind/ser/std/StdDelegatingSerializer.java index 203b3fc077..f16210114b 100644 --- a/src/main/java/com/fasterxml/jackson/databind/ser/std/StdDelegatingSerializer.java +++ b/src/main/java/com/fasterxml/jackson/databind/ser/std/StdDelegatingSerializer.java @@ -160,15 +160,11 @@ public void serialize(Object value, JsonGenerator gen, SerializerProvider provid provider.defaultSerializeNull(gen); return; } -System.err.println(" delegating.serialize. value: "+value.getClass()); - // 02-Apr-2015, tatu: As per [databind#731] may need to do dynamic lookup JsonSerializer ser = _delegateSerializer; if (ser == null) { -System.err.println("DEBUG: dynamic lookup for delegator... "); ser = _findSerializer(delegateValue, provider); } -System.err.println(" delegating.serialize with -> "+ser); ser.serialize(delegateValue, gen, provider); } diff --git a/src/main/java/com/fasterxml/jackson/databind/ser/std/StdSerializer.java b/src/main/java/com/fasterxml/jackson/databind/ser/std/StdSerializer.java index 50972e4c86..702cd503ef 100644 --- a/src/main/java/com/fasterxml/jackson/databind/ser/std/StdSerializer.java +++ b/src/main/java/com/fasterxml/jackson/databind/ser/std/StdSerializer.java @@ -3,8 +3,8 @@ import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Type; -import java.util.HashSet; -import java.util.Set; +import java.util.IdentityHashMap; +import java.util.Map; import com.fasterxml.jackson.annotation.JsonFormat; import com.fasterxml.jackson.annotation.JsonInclude; @@ -34,6 +34,14 @@ public abstract class StdSerializer { private static final long serialVersionUID = 1L; + /** + * Key used for storing a lock object to prevent infinite recursion when + * constructing converting serializers. + * + * @since 2.9 + */ + private final static Object KEY_CONTENT_CONVERTER_LOCK = new Object(); + /** * Nominal type supported, usually declared type of * property for which serializer is used. @@ -355,30 +363,29 @@ protected JsonSerializer findContextualConvertingSerializer(SerializerProvide { // 08-Dec-2016, tatu: to fix [databind#357], need to prevent recursive calls for // same property - /* @SuppressWarnings("unchecked") - Set conversions = (Set) provider.getAttribute(CONTENT_CONVERTER_LOCK); + Map conversions = (Map) provider.getAttribute(KEY_CONTENT_CONVERTER_LOCK); if (conversions != null) { - if (conversions.contains(property)) { + Object lock = conversions.get(property); + if (lock != null) { return existingSerializer; } } else { - conversions = new HashSet<>(); - provider.setAttribute(CONTENT_CONVERTER_LOCK, provider); + conversions = new IdentityHashMap<>(); + provider.setAttribute(KEY_CONTENT_CONVERTER_LOCK, conversions); } + conversions.put(property, Boolean.TRUE); try { - } finally { - } - */ JsonSerializer ser = findConvertingContentSerializer(provider, property, existingSerializer); if (ser != null) { return provider.handleSecondaryContextualization(ser, property); } + } finally { + conversions.remove(property); + } return existingSerializer; } -// private final static Object CONTENT_CONVERTER_LOCK = new Object(); - /** * @deprecated Since 2.9 use {link {@link #findContextualConvertingSerializer} instead */ diff --git a/src/test/java/com/fasterxml/jackson/databind/convert/TestConvertingDeserializer.java b/src/test/java/com/fasterxml/jackson/databind/convert/TestConvertingDeserializer.java index d88e39710e..9b0946c950 100644 --- a/src/test/java/com/fasterxml/jackson/databind/convert/TestConvertingDeserializer.java +++ b/src/test/java/com/fasterxml/jackson/databind/convert/TestConvertingDeserializer.java @@ -4,6 +4,7 @@ import java.util.*; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; import com.fasterxml.jackson.databind.util.StdConverter; public class TestConvertingDeserializer @@ -99,6 +100,21 @@ static class LowerCaseTextArray { public String[] texts; } + // [databind#357] + static class Value { } + + static class ListWrapper { + @JsonSerialize(contentConverter = ValueToStringListConverter.class) + public List list = Arrays.asList(new Value()); + } + + static class ValueToStringListConverter extends StdConverter> { + @Override + public List convert(Value value) { + return Arrays.asList("Hello world!"); + } + } + // for [databind#795] static class ToNumberConverter extends StdConverter @@ -114,7 +130,7 @@ static class Issue795Bean @JsonDeserialize(converter=ToNumberConverter.class) public Number value; } - + /* /********************************************************** /* Test methods @@ -198,6 +214,12 @@ public void testPropertyAnnotationForMaps() throws Exception assertEquals(2, p.y); } + // [databind#357] + public void testConverterForList357() throws Exception { + String json = objectWriter().writeValueAsString(new ListWrapper()); + assertEquals("{\"list\":[[\"Hello world!\"]]}", json); + } + // [databind#795] public void testConvertToAbstract() throws Exception { diff --git a/src/test/java/com/fasterxml/jackson/failing/ConvertingSerializer357Test.java b/src/test/java/com/fasterxml/jackson/failing/ConvertingSerializer357Test.java deleted file mode 100644 index c168a5785e..0000000000 --- a/src/test/java/com/fasterxml/jackson/failing/ConvertingSerializer357Test.java +++ /dev/null @@ -1,38 +0,0 @@ -package com.fasterxml.jackson.failing; - -import java.util.*; - -import com.fasterxml.jackson.databind.annotation.JsonSerialize; - -import com.fasterxml.jackson.databind.util.StdConverter; - -public class ConvertingSerializer357Test - extends com.fasterxml.jackson.databind.BaseMapTest -{ - // [databind#357] - static class Value { } - - static class ListWrapper { - @JsonSerialize(contentConverter = ValueToStringListConverter.class) - public List list = Arrays.asList(new Value()); - } - - static class ValueToStringListConverter extends StdConverter> { - @Override - public List convert(Value value) { - return Arrays.asList("Hello world!"); - } - } - - /* - /********************************************************** - /* Test methods - /********************************************************** - */ - - // [databind#357] - public void testConverterForList357() throws Exception { - String json = objectWriter().writeValueAsString(new ListWrapper()); - assertEquals("{\"list\":[[\"Hello world!\"]]}", json); - } -}