activateDefaultTyping(PolymorphicTypeValidator, ...) applies to Default Typing use case, but not to @JsonTypeInfo

[artem-smotrakov]

There are two ways to enable polymorphic type handling:

1. By using @JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, ...) annotation
2. By calling one of the activateDefaultTyping() methods or deprecated unsafe enableDefaultTyping()

Jackson 2.10 now allows specifying a validator for the classes during deserialization. There are also two ways to set a validator:

1. By calling setPolymorphicTypeValidator() method on ObjectMapper or its builder.
2. By specifying the validator in one of the activateDefaultTyping() methods

I noticed that specifying a validator in one of the activateDefaultTyping() methods doesn't necessary mean that the validator is going to be called always.
In particular, if @JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, ...) annotation is used but the validator is specified only via activateDefaultTyping(),
then deserialization is going to work but the validator won't be called.

        PolymorphicTypeValidator ptv =
            BasicPolymorphicTypeValidator.builder()
                .allowIfSubType("com.sap.phosphor.jackson")
                .build();
        ObjectMapper mapper = JsonMapper.builder()
            .activateDefaultTyping(ptv)
            .build();

To turn the validation on in this case, the validator has to be additionally specified via setPolymorphicTypeValidator() method:

        PolymorphicTypeValidator ptv =
            BasicPolymorphicTypeValidator.builder()
                .allowIfSubType("com.sap.phosphor.jackson")
                .build();
        ObjectMapper mapper = JsonMapper.builder()
            .polymorphicTypeValidator(ptv)
            .activateDefaultTyping(ptv)       
            .build();

If I understand the javadoc for ObjectMapper correctly

• using @JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, ...) means "explicit polymorphic types"
• calling activateDefaultTyping() means "enabling automatic inclusion of type information, needed for proper deserialization of polymorphic types"
  It makes me think, these two cases are considered relatively independent.

However, polymorphic type handling is enabled in both cases.

I am wondering if activateDefaultTyping() should also set the specified PolymorphicTypeValidator.
Otherwise, someone may get confused by calling only activateDefaultTyping(PolymorphicTypeValidator, ...) and thinking this is safe enough.

The following patch addresses it:

diff --git a/src/main/java/com/fasterxml/jackson/databind/ObjectMapper.java b/src/main/java/com/fasterxml/jackson/databind/ObjectMapper.java
index bb7ccc716..53d15a00b 100644
--- a/src/main/java/com/fasterxml/jackson/databind/ObjectMapper.java
+++ b/src/main/java/com/fasterxml/jackson/databind/ObjectMapper.java
@@ -1677,6 +1677,7 @@ public class ObjectMapper
         // we'll always use full class name, when using defaulting
         typer = typer.init(JsonTypeInfo.Id.CLASS, null);
         typer = typer.inclusion(includeAs);
+        setPolymorphicTypeValidator(ptv);
         return setDefaultTyping(typer);
     }
 
@@ -1709,6 +1710,7 @@ public class ObjectMapper
         typer = typer.init(JsonTypeInfo.Id.CLASS, null);
         typer = typer.inclusion(JsonTypeInfo.As.PROPERTY);
         typer = typer.typeProperty(propertyName);
+        setPolymorphicTypeValidator(ptv);
         return setDefaultTyping(typer);
     }

[cowtowncoder]

This was conscious decisions based on there being 2 different ways to enable Default Typing.
I didn't (and don't) like the idea of coupling things: in this case both setting validator for both cases and enabling Default Typing. This is why there are 2 separate methods.
I don't think this would necessary make big change, either, since it seems to me that use of Default Typing is usually alternative not use of explicit @JsonTypeInfo (and vice versa): so it would seem unlikely that both would be in use. So users who use @JsonTypeInfo (with class name, "unsafe" base type like Object or Serializable) would regardless need to call setPolymorphicTypeValidator() as they do not enable/activate default typin.

I do think that documentation improvements would make sense, to outline that validator is only used for Default Typing, and not for use via @JsonTypeInfo, at very least.

That said, I am definitely open to ideas for further reducing likelihood on unsafe usage.

Also: this is something that is probably better discussed on jackson-dev mailing list: not all/many developers necessarily follow issue tracker updates:

https://groups.google.com/group/jackson-dev

[vladrich]

I think this is an interesting issue.

@cowtowncoder, I read from your comment that you would like enabling default typing and setting a validator to be orthogonal. One can certainly see it like that. At the same time, the method for activating default typing now requires a mandatory validator. How can we make these two things consistent?

[cowtowncoder]

I struggled with the @JsonTypeInfo part as unlike Default Typing, there wasn't any specific way to enable anything. But further, usage can be perfectly safe without further ado, for example

@JsonTypeInfo(use = Id.CLASS)
public class MyBaseType { .... }

defining that only subtypes are legal; or, on property

public class Menagerie {
   @JsonTypeInfo(use = Id.CLASS)
   public Animal pet;
}

because in both cases types that are possible to deserialize must be subtypes of a type user controls.

The unsafe cases (with base type of Object, Serializable) are the problems.

For 3.0, this can be solved by simply defaulting to restrictive PolymorphicTypeValidator that simply denies all resolution (or, perhaps only ones with base type of Object, Serializable).
But for 2.x we have the backwards-compatibility problem: especially since failures in these cases must be runtime-only ... they may or may not break unit tests users have, but even more importantly due to Jackson being common transitive dependency, may really just start failing code with seemingly unrelated upgrade of underlying framework or a library in use.

[cowtowncoder]

Ok; so, I think addition of #2587 (in 2.11) should help here, to enforce use of safe PTV for @JsonTypeInfo. Methods for configuring the two validators also exist; and for 3.0 default validators are both non-permissive.

I guess that while I could be convinced to make polymorphicTypeValidator(ptv) apply to both cases (that is, avoid need to pass PTV for activateDefaultTyping()), at this point I am not yet sure of benefits: it seems like either:

1. There would still be 2 variants of activateDefaultTyping() (one that takes PTV as override; other that relies on now global default), or
2. Change so activateDefaultTyping() never takes PTV, and now relies on state of configuration set earlier

So I guess I can see that this is a bit confusing, but I am not sure how it could be made less so.

[cowtowncoder]

Will close this issue due to additions in 2.x: I am still open to improvements of course, most specifically ideas for bigger changes in 3.0. But if and when so, please file a new issue.