Skip to content

Commit

Permalink
Support strict Type ID handling. (#3854)
Browse files Browse the repository at this point in the history
Add `MapperFeature.REQUIRE_TYPE_ID_FOR_SUBTYPES` (default: true)
  • Loading branch information
stevestorey authored Apr 12, 2023
1 parent ac971e9 commit 5e2c643
Show file tree
Hide file tree
Showing 7 changed files with 153 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,15 @@ public enum MapperFeature implements ConfigFeature
*/
INFER_BUILDER_TYPE_BINDINGS(true),

/**
* Feature that determines what happens when deserializing to a registered sub-type, but no
* type information has been provided. If enabled, then an {@link InvalidTypeIdException}
* will be thrown, if disabled then the deserialization will proceed without the type information.
*
* @since 2.15
*/
REQUIRE_TYPE_ID_FOR_SUBTYPES(true),

/*
/******************************************************
/* View-related features
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ public class AsPropertyTypeDeserializer extends AsArrayTypeDeserializer
protected final As _inclusion;

/**
* Indicates if the current class has a TypeResolver attached or not.
* Indicates that we should be strict about handling missing type information.
*
* @since 2.15
*/
protected final boolean _hasTypeResolver;
protected final boolean _strictTypeIdHandling;

// @since 2.12.2 (see [databind#3055]
protected final String _msgForMissingId = (_property == null)
Expand Down Expand Up @@ -64,25 +64,25 @@ public AsPropertyTypeDeserializer(JavaType bt, TypeIdResolver idRes,
{
super(bt, idRes, typePropertyName, typeIdVisible, defaultImpl);
_inclusion = inclusion;
_hasTypeResolver = true;
_strictTypeIdHandling = false;
}

public AsPropertyTypeDeserializer(AsPropertyTypeDeserializer src, BeanProperty property) {
super(src, property);
_inclusion = src._inclusion;
_hasTypeResolver = src._hasTypeResolver;
_strictTypeIdHandling = src._strictTypeIdHandling;
}

/**
* @since 2.15
*/
public AsPropertyTypeDeserializer(JavaType bt, TypeIdResolver idRes,
String typePropertyName, boolean typeIdVisible, JavaType defaultImpl,
As inclusion, boolean hasTypeResolver)
As inclusion, boolean strictTypeIdHandling)
{
super(bt, idRes, typePropertyName, typeIdVisible, defaultImpl);
_inclusion = inclusion;
_hasTypeResolver = hasTypeResolver;
_strictTypeIdHandling = strictTypeIdHandling;
}

@Override
Expand Down Expand Up @@ -209,8 +209,7 @@ protected Object _deserializeTypedUsingDefaultImpl(JsonParser p,
// genuine, or faked for "dont fail on bad type id")
JsonDeserializer<Object> deser = _findDefaultImplDeserializer(ctxt);
if (deser == null) {
JavaType t = _hasTypeResolver
? _handleMissingTypeId(ctxt, priorFailureMsg) : _baseType;
JavaType t = _strictTypeIdHandling ? _handleMissingTypeId(ctxt, priorFailureMsg): _baseType;

if (t == null) {
// 09-Mar-2017, tatu: Is this the right thing to do?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ public TypeDeserializer buildTypeDeserializer(DeserializationConfig config,
case EXISTING_PROPERTY: // as per [#528] same class as PROPERTY
return new AsPropertyTypeDeserializer(baseType, idRes,
_typeProperty, _typeIdVisible, defaultImpl, _includeAs,
_hasTypeResolver(config, baseType));
_strictTypeIdHandling(config, baseType));
case WRAPPER_OBJECT:
return new AsWrapperTypeDeserializer(baseType, idRes,
_typeProperty, _typeIdVisible, defaultImpl);
Expand Down Expand Up @@ -403,6 +403,28 @@ protected boolean allowPrimitiveTypes(MapperConfig<?> config,
return false;
}

/**
* Determines whether strict type ID handling should be used for this type or not.
* This will be enabled when either the type has type resolver annotations or if
* {@link com.fasterxml.jackson.databind.MapperFeature#REQUIRE_TYPE_ID_FOR_SUBTYPES}
* is enabled.
*
* @param config the deserialization configuration to use
* @param baseType the base type to check for type resolver annotations
*
* @return {@code true} if the class has type resolver annotations, or the strict
* handling feature is enabled, {@code false} otherwise.
*
* @since 2.15
*/
protected boolean _strictTypeIdHandling(DeserializationConfig config, JavaType baseType) {
if (config.isEnabled(MapperFeature.REQUIRE_TYPE_ID_FOR_SUBTYPES)) {
return true;
}
// Otherwise we will be strict if there's a type resolver
return _hasTypeResolver(config, baseType);
}

/**
* Checks whether the given class has annotations indicating some type resolver
* is applied, for example {@link com.fasterxml.jackson.annotation.JsonSubTypes}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.databind.BaseMapTest;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.exc.InvalidDefinitionException;
import com.fasterxml.jackson.databind.exc.InvalidTypeIdException;
import com.fasterxml.jackson.databind.exc.MismatchedInputException;
import com.fasterxml.jackson.databind.json.JsonMapper;

public class JsonTypeInfoIgnored2968Test extends BaseMapTest {
/*
Expand All @@ -15,7 +17,7 @@ public class JsonTypeInfoIgnored2968Test extends BaseMapTest {
/**********************************************************
*/

private static final ObjectMapper MAPPER = new ObjectMapper();
private static final ObjectMapper MAPPER = JsonMapper.builder().disable(MapperFeature.REQUIRE_TYPE_ID_FOR_SUBTYPES).build();

@JsonTypeInfo(
use = JsonTypeInfo.Id.NAME,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package com.fasterxml.jackson.databind.jsontype;

import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.annotation.JsonTypeInfo.Id;
import com.fasterxml.jackson.annotation.JsonTypeName;
import com.fasterxml.jackson.databind.BaseMapTest;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.exc.InvalidTypeIdException;
import com.fasterxml.jackson.databind.json.JsonMapper;

public class StrictJsonTypeInfoHandling3853Test extends BaseMapTest {

@JsonTypeInfo(use = Id.NAME)
interface Command {
}

@JsonTypeName("do-something")
static class DoSomethingCommand implements Command {
}

public void testDefaultHasStrictTypeHandling() throws Exception {
ObjectMapper om = new ObjectMapper();
om.registerSubtypes(DoSomethingCommand.class);

// This should pass in all scenarios
verifyDeserializationWithFullTypeInfo(om);
// and throw an exception if the target was a super-type in all cases
verifyInvalidTypeIdWithSuperclassTarget(om);

// Default is to disallow the deserialization without a type if the target
// is a concrete sub-type
verifyInvalidTypeIdWithConcreteTarget(om);
}

public void testExplicitNonStrictTypeHandling() throws Exception {
ObjectMapper om = JsonMapper.builder().disable(MapperFeature.REQUIRE_TYPE_ID_FOR_SUBTYPES).build();
om.registerSubtypes(DoSomethingCommand.class);

// This should pass in all scenarios
verifyDeserializationWithFullTypeInfo(om);
// and throw an exception if the target was a super-type in all cases
verifyInvalidTypeIdWithSuperclassTarget(om);

// Default is to allow the deserialization without a type if the target
// is a concrete sub-type
verifyDeserializationWithConcreteTarget(om);
}

public void testStrictTypeHandling() throws Exception {
ObjectMapper om = JsonMapper.builder().enable(MapperFeature.REQUIRE_TYPE_ID_FOR_SUBTYPES).build();
om.registerSubtypes(DoSomethingCommand.class);

// This should pass in all scenarios
verifyDeserializationWithFullTypeInfo(om);
// and throw an exception if the target was a super-type in all cases
verifyInvalidTypeIdWithSuperclassTarget(om);

// With strict mode enabled, fail if there's no type information on the
// JSON
verifyInvalidTypeIdWithConcreteTarget(om);

}

private void verifyInvalidTypeIdWithSuperclassTarget(ObjectMapper om) throws Exception {
try {
om.readValue("{}", Command.class);
fail("Should not pass");
} catch (InvalidTypeIdException e) {
verifyException(e, "missing type id property '@type'");
}
}

private void verifyInvalidTypeIdWithConcreteTarget(ObjectMapper om) throws Exception {
try {
om.readValue("{}", DoSomethingCommand.class);
fail("Should not pass");
} catch (InvalidTypeIdException e) {
verifyException(e, "missing type id property '@type'");
}
}

private void verifyDeserializationWithConcreteTarget(ObjectMapper om) throws Exception {
DoSomethingCommand cmd = om.readValue("{}", DoSomethingCommand.class);
assertType(cmd, DoSomethingCommand.class);
}

private void verifyDeserializationWithFullTypeInfo(ObjectMapper om) throws Exception {
Command cmd = om.readValue("{\"@type\":\"do-something\"}", Command.class);
assertType(cmd, DoSomethingCommand.class);
cmd = om.readValue("{\"@type\":\"do-something\"}", DoSomethingCommand.class);
assertType(cmd, DoSomethingCommand.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ static class ChildOfChild extends ChildOfAbstract {
.disable(MapperFeature.USE_BASE_TYPE_AS_DEFAULT_IMPL)
.build();

protected ObjectMapper MAPPER_WITHOUT_BASE_OR_SUBTYPE_ID = jsonMapperBuilder()
.disable(MapperFeature.USE_BASE_TYPE_AS_DEFAULT_IMPL)
.disable(MapperFeature.REQUIRE_TYPE_ID_FOR_SUBTYPES)
.build();

public void testPositiveForParent() throws Exception {
Object o = MAPPER_WITH_BASE.readerFor(Parent.class).readValue("{}");
assertEquals(o.getClass(), Parent.class);
Expand All @@ -60,7 +65,16 @@ public void testNegativeForParent() throws Exception {
}

public void testNegativeForChild() throws Exception {
Child child = MAPPER_WITHOUT_BASE.readerFor(Child.class).readValue("{}");
try {
/*Object o =*/ MAPPER_WITHOUT_BASE.readerFor(Child.class).readValue("{}");
fail("Should not pass");
} catch (InvalidTypeIdException ex) {
assertTrue(ex.getMessage().contains("missing type id property '@class'"));
}
}

public void testNegativeForChildWithoutRequiringTypeId() throws Exception {
Child child = MAPPER_WITHOUT_BASE_OR_SUBTYPE_ID.readerFor(Child.class).readValue("{}");

assertEquals(Child.class, child.getClass());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.databind.BaseMapTest;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.jsontype.BasicPolymorphicTypeValidator;
import com.fasterxml.jackson.databind.jsontype.PolymorphicTypeValidator;
Expand All @@ -28,6 +29,7 @@ public void testDeserializationConcreteClassWithDefaultTyping() throws Exception
ObjectMapper mapper = jsonMapperBuilder()
.activateDefaultTyping(ptv, ObjectMapper.DefaultTyping.NON_FINAL,
JsonTypeInfo.As.PROPERTY)
.disable(MapperFeature.REQUIRE_TYPE_ID_FOR_SUBTYPES)
.build();

final String concreteTypeJson = a2q("{'size': 42}");
Expand Down

0 comments on commit 5e2c643

Please sign in to comment.