Skip to content

Commit ffc36de

Browse files
authored
Fix #5281: wrong type used for fallback setter (#5888)
1 parent 6e8dab6 commit ffc36de

4 files changed

Lines changed: 142 additions & 52 deletions

File tree

release-notes/VERSION

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,10 @@ Versions: 3.x (for earlier see VERSION-2.x)
152152
(fix by @cowtowncoder, w/ Claude code)
153153
#4983: `JsonTypeInfo.Id.MINIMAL_CLASS` generates invalid type on sub-package
154154
(reported, fix by Benoit C-L)
155+
#5281: Reading into existing instance uses creator property setup instead
156+
of mutator (setter, field)
157+
(reported by @odrotbohm)
158+
(fix by @cowtowncoder, w/ Claude code)
155159
#5330: `@JsonProperty`on enum values with
156160
`@JsonFormat(shape = JsonFormat.Shape.NUMBER)` is ignored
157161
(reported by Christoffer H)

src/main/java/tools/jackson/databind/deser/CreatorProperty.java

Lines changed: 64 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import tools.jackson.core.JacksonException;
88
import tools.jackson.core.JsonParser;
9+
import tools.jackson.core.JsonToken;
910
import tools.jackson.databind.*;
1011
import tools.jackson.databind.exc.InvalidDefinitionException;
1112
import tools.jackson.databind.introspect.AnnotatedMember;
@@ -17,9 +18,9 @@
1718
/**
1819
* This concrete sub-class implements property that is passed
1920
* via Creator (constructor or static factory method).
20-
* It is not a full-featured implementation in that its set method
21-
* should usually not be called for primary mutation -- instead, value must separately passed --
22-
* but some aspects are still needed (specifically, injection).
21+
* It is not a full-featured implementation in that its set method should
22+
* usually not be called for primary mutation -- instead, value must be passed
23+
* separately -- but some aspects are still needed (specifically, injection).
2324
*<p>
2425
* Note on injectable values: unlike with other mutators, where
2526
* deserializer and injecting are separate, here we treat the two as related
@@ -56,6 +57,17 @@ public class CreatorProperty
5657
*/
5758
protected SettableBeanProperty _fallbackSetter;
5859

60+
/**
61+
* Pre-computed flag that is {@code true} if {@link #_fallbackSetter}'s
62+
* declared type matches this property's (creator parameter) type, so that
63+
* the existing value deserializer can be reused when reading into an
64+
* existing instance. Cached to avoid repeating the type comparison on
65+
* every call to {@link #deserializeAndSet}/{@link #deserializeSetAndReturn}.
66+
*
67+
* @since 3.2
68+
*/
69+
protected boolean _fallbackSetterTypeMatches;
70+
5971
protected final int _creatorIndex;
6072

6173
/**
@@ -111,6 +123,7 @@ protected CreatorProperty(CreatorProperty src, PropertyName newName) {
111123
_annotated = src._annotated;
112124
_injectableValue = src._injectableValue;
113125
_fallbackSetter = src._fallbackSetter;
126+
_fallbackSetterTypeMatches = src._fallbackSetterTypeMatches;
114127
_creatorIndex = src._creatorIndex;
115128
_ignorable = src._ignorable;
116129
}
@@ -121,6 +134,7 @@ protected CreatorProperty(CreatorProperty src, ValueDeserializer<?> deser,
121134
_annotated = src._annotated;
122135
_injectableValue = src._injectableValue;
123136
_fallbackSetter = src._fallbackSetter;
137+
_fallbackSetterTypeMatches = src._fallbackSetterTypeMatches;
124138
_creatorIndex = src._creatorIndex;
125139
_ignorable = src._ignorable;
126140
}
@@ -131,6 +145,7 @@ protected CreatorProperty(CreatorProperty src, TypeDeserializer typeDeser)
131145
_annotated = src._annotated;
132146
_injectableValue = src._injectableValue;
133147
_fallbackSetter = src._fallbackSetter;
148+
_fallbackSetterTypeMatches = src._fallbackSetterTypeMatches;
134149
_creatorIndex = src._creatorIndex;
135150
_ignorable = src._ignorable;
136151
}
@@ -176,6 +191,8 @@ public void fixAccess(DeserializationConfig config) {
176191
*/
177192
public void setFallbackSetter(SettableBeanProperty fallbackSetter) {
178193
_fallbackSetter = fallbackSetter;
194+
_fallbackSetterTypeMatches = (fallbackSetter != null)
195+
&& _type.equals(fallbackSetter.getType());
179196
}
180197

181198
@Override
@@ -219,15 +236,15 @@ public void deserializeAndSet(JsonParser p, DeserializationContext ctxt,
219236
Object instance) throws JacksonException
220237
{
221238
_verifySetter();
222-
_fallbackSetter.set(ctxt, instance, deserialize(p, ctxt));
239+
_fallbackSetter.set(ctxt, instance, _deserializeForSetter(p, ctxt));
223240
}
224241

225242
@Override
226243
public Object deserializeSetAndReturn(JsonParser p,
227244
DeserializationContext ctxt, Object instance) throws JacksonException
228245
{
229246
_verifySetter();
230-
return _fallbackSetter.setAndReturn(ctxt, instance, deserialize(p, ctxt));
247+
return _fallbackSetter.setAndReturn(ctxt, instance, _deserializeForSetter(p, ctxt));
231248
}
232249

233250
@Override
@@ -294,6 +311,48 @@ public boolean isInjectionOnly() {
294311
/**********************************************************************
295312
*/
296313

314+
/**
315+
* Helper method for {@code deserializeAndSet} and {@code deserializeSetAndReturn}:
316+
* deserializes value using the fallback setter's type if it differs from the
317+
* creator parameter type.
318+
*<p>
319+
* [databind#5281]: When updating an existing instance, the creator parameter type
320+
* (e.g. {@code String[]} from varargs) may differ from the setter/field type
321+
* (e.g. {@code Collection<String>}). Must deserialize using the setter's type
322+
* to avoid {@code ClassCastException}.
323+
*
324+
* @since 3.2
325+
*/
326+
private Object _deserializeForSetter(JsonParser p, DeserializationContext ctxt)
327+
throws JacksonException
328+
{
329+
// Common case: types match, use this property's (already resolved) deserializer
330+
if (_fallbackSetterTypeMatches) {
331+
return deserialize(p, ctxt);
332+
}
333+
// Types differ: find deserializer for the fallback setter's type.
334+
// Note: we use `_nullProvider` (this CreatorProperty's) rather than the
335+
// fallback setter's: `BeanPropertyDefinition` merges annotations across
336+
// accessors, so `@JsonSetter(nulls=...)` on the setter is already reflected
337+
// here via `BeanDeserializerBase.resolve()`, whereas the fallback setter
338+
// itself is stored as a raw `MethodProperty` and never contextualized.
339+
if (p.hasToken(JsonToken.VALUE_NULL)) {
340+
return _nullProvider.getNullValue(ctxt);
341+
}
342+
ValueDeserializer<Object> deser = ctxt.findContextualValueDeserializer(
343+
_fallbackSetter.getType(), _fallbackSetter);
344+
// If fallback setter has a TypeDeserializer (polymorphism via @JsonTypeInfo),
345+
// honor it instead of plain deserialize()
346+
final TypeDeserializer typeDeser = _fallbackSetter.getValueTypeDeserializer();
347+
Object value = (typeDeser == null)
348+
? deser.deserialize(p, ctxt)
349+
: deser.deserializeWithType(p, ctxt, typeDeser);
350+
if (value == null) {
351+
value = _nullProvider.getNullValue(ctxt);
352+
}
353+
return value;
354+
}
355+
297356
private final void _verifySetter() throws JacksonException {
298357
if (_fallbackSetter == null) {
299358
_reportMissingSetter(null, null);
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
package tools.jackson.databind.deser.creators;
2+
3+
import java.util.ArrayList;
4+
import java.util.Arrays;
5+
import java.util.Collection;
6+
7+
import org.junit.jupiter.api.Test;
8+
9+
import tools.jackson.databind.ObjectMapper;
10+
import tools.jackson.databind.json.JsonMapper;
11+
import tools.jackson.databind.testutil.DatabindTestUtil;
12+
13+
import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
14+
15+
// [databind#5281] Reading into existing instance uses creator property setup instead
16+
// of accessor (setter, field)
17+
public class CreatorPropertyReaderForUpdating5281Test
18+
extends DatabindTestUtil
19+
{
20+
// Creator param type (String[]) differs from setter type (Collection<String>)
21+
public static class ArrayListHolder {
22+
Collection<String> values;
23+
24+
public ArrayListHolder(String... values) {
25+
this.values = new ArrayList<>();
26+
this.values.addAll(Arrays.asList(values));
27+
}
28+
29+
public void setValues(Collection<String> values) {
30+
this.values = values;
31+
}
32+
}
33+
34+
// Creator param type and setter type match: exercises the
35+
// `_fallbackSetterTypeMatches == true` fast path
36+
public static class MatchingTypeHolder {
37+
Collection<String> values;
38+
39+
public MatchingTypeHolder(Collection<String> values) {
40+
this.values = (values == null) ? new ArrayList<>() : new ArrayList<>(values);
41+
}
42+
43+
public void setValues(Collection<String> values) {
44+
this.values = values;
45+
}
46+
}
47+
48+
// Verify that readerForUpdating uses setter type (Collection<String>),
49+
// not creator parameter type (String[]), avoiding ClassCastException
50+
@Test
51+
public void readerForUpdatingUsesSetterType() throws Exception {
52+
ObjectMapper mapper = JsonMapper.builder().build();
53+
54+
ArrayListHolder holder = mapper.readerForUpdating(new ArrayListHolder("A"))
55+
.readValue("{ \"values\" : [ \"A\", \"B\" ]}");
56+
57+
// Setter replaces values (no merge), so we get the new array only
58+
assertThat(holder.values).hasSize(2)
59+
.containsExactly("A", "B");
60+
}
61+
62+
// Regression: when creator and setter types match, the fast path still works
63+
@Test
64+
public void readerForUpdatingTypeMatchFastPath() throws Exception {
65+
ObjectMapper mapper = JsonMapper.builder().build();
66+
67+
MatchingTypeHolder holder = mapper.readerForUpdating(
68+
new MatchingTypeHolder(Arrays.asList("X")))
69+
.readValue("{ \"values\" : [ \"A\", \"B\" ]}");
70+
71+
assertThat(holder.values).hasSize(2)
72+
.containsExactly("A", "B");
73+
}
74+
}

src/test/java/tools/jackson/databind/tofix/ReaderForUpdating5281Test.java

Lines changed: 0 additions & 47 deletions
This file was deleted.

0 commit comments

Comments
 (0)