Type safety for readValue() with TypeReference
#2196
Conversation
The ObjectMapper.readValue(String, TypeReference) was not type-safe at
compile time, because the type of parameter valueTypeRef was missing
generic type parameter T. This fact could cause heap pollution when
client would write something like this:
```
List<String> list;
list = mapper.readValue("[33]", new TypeReference<List<Integer>>(){});
String str = list.get(0); // throws ClassCastException
```
cowtowncoder
changed the title
readValue() with TypeReference
|
Ah. Yes, this seems reasonable. My main/only concern is regarding compatibility with existing code. Due to type-erasure, I think change is binary-compatible, so not being problematic for things already compiled. So, this probably should not go in 2.9.x (or earlier) patch release. It would be fine for 3.x ( |
|
@nguyenfilip Thank you again for submitting this -- I ended up manually merging these changes. |
| @@ -1453,7 +1453,7 @@ public JsonParser treeAsTokens(TreeNode n) { | |||
| * expected for result type (or has other mismatch issues) | |||
| */ | |||
| @SuppressWarnings({ "unchecked", "rawtypes" }) | |||
| public <T> T readValue(String content, TypeReference valueTypeRef) | |||
| public <T> T readValue(String content, TypeReference<T> valueTypeRef) | |||
There was a problem hiding this comment.
@cowtowncoder just running in to this during an upgrade from 2.9 to 2.10
The purpose of the types on TypeReference is to supply concrete implementation classes through the reflection API, but often those implementations are not what you would want to return out of a method.
The purpose of on readValue is to define the left hand side of the assignment.
As a concrete example, say you have interface Foo and data class FooImpl. I would need to construct a TypeReference<List<FooImpl>> to supply appropriate type info to the objectMapper, but as the assigned type I would like to use List<Foo>.
In the previous case this worked due to the noted lack of type safety between return type and TypeReference type.
To get around this now I'd have to erase the type off the return value from readValue and manually cast back to List (or some other set of operations that has equally bad type safety to the prior untyped valueTypeRef case).
I don't have a suggestion, just an observation that this adds a bit more friction to the end user..
Maybe someone better at generics can figure out the proper incantation where the return value is type compatible with the valueType but still allows returning a less specific type than what is needed for deserialization. (Maybe that is not possible with java generics in this use case)
There was a problem hiding this comment.
Yes, this is unfortunately problematic. Had I realized problems with inability to vary type parameters independently, I would not have made this change.
There was a problem hiding this comment.
+1 at least post an example how folks can fix this, heck of an issue to run into during an upgrade in a point release.
Jakson make changes FasterXML/jackson-databind#2196
In Java the type safety is enforced at run-time for reifiable types (e.g. arrays that throw ArrayStoreException) but for non-reifiable types such as generic collections, it is important that type safety is enforced at compile time. Its not possible to enforce type safety at runtime, because of the type erasure.
The ObjectMapper.readValue(String, TypeReference) was not type-safe at
compile time, because the type of parameter valueTypeRef was missing
generic type parameter T. This fact could cause heap pollution when
client would write something like this: