Validate generic type arguments by martinfrancois · Pull Request #6254 · FasterXML/jackson-databind

martinfrancois · 2026-09-29T18:54:12Z

Summary by Gitar

Validation:
- Validate generic type arguments against declared base parameters in DatabindContext
- Add BaseTypeAllowingValidator and integration tests for generic type compatibility

_{This will update automatically on new commits.}

pjfanning

Looks good overall. The new tests fail on 2.18 without this change (4 of 15) and pass with it. The rest of the suite passes too; the only failure, a timezone-name assertion in DateSerializationTest, also fails on the base commit.

Two behaviour changes are probably worth a line in the release notes:

Stricter: Map<String,Integer> read with type id java.util.HashMap<java.lang.String,java.lang.Long> now fails with InvalidTypeIdException. Before, it was accepted and put Long values into the map. That is the correct behaviour, but someone may be relying on the old one.
Looser: with allowIfBaseType(Animal.class), List<Animal> read with type id java.util.ArrayList<Dog> is now accepted; before, the PTV rejected it. This follows from calling validateBaseType on the declared parameter type. It seems fine, since Dog is checked to be an Animal, but it is a change.

(Object type arguments are still accepted as before, e.g. HashMap<Object,Object> into Map<String,Integer>.)

Inline comments below are minor.

pjfanning · 2026-09-29T19:04:43Z

+        // Object is the canonical placeholder for wildcards, unbound parameters,
+        // and runtime-erased values; it cannot itself instantiate an attacker-
+        // selected class either.
+        if (param.isPrimitive() || param.isJavaLangObject()) {


The method javadoc above (L376-395) is out of date now: enums still get the new subtype check and the SubTypeValidator check, so they're only exempt from the PTV. Primitives are now skipped, and the base used is the declared type parameter, not the outer base type.

Rewrote the javadoc. It now says that declaredType is the type parameter the base type declares at the same position (or the unknown type if there is none), that primitives and Object skip all checks, and that enums skip only the PolymorphicTypeValidator while still being checked against the declared type and the built-in deny list. I also renamed the parameter from baseType to declaredType in the helpers so the code reads the way the javadoc describes it.

pjfanning · 2026-09-29T19:04:43Z

        }
-        for (int i = 0, n = param.containedTypeCount(); i < n; ++i) {
-            _validateTypeParameter(baseType, param.containedType(i), ptv, config);
+        if (this instanceof DeserializationContext) {


Minor: an instanceof DeserializationContext check in the base class is a bit of a layering smell. A protected no-op hook in DatabindContext, overridden in DeserializationContext, would be cleaner.

Agreed. DatabindContext now has a protected no-op _validateGenericSubType(JavaType) hook, and DeserializationContext overrides it with the SubTypeValidator call. The instanceof check is gone.

pjfanning · 2026-09-29T19:04:43Z

+        }
+        if (!param.isTypeOrSubTypeOf(baseType.getRawClass())) {
+            throw invalidTypeIdException(baseType, rawName,
+                    "Type parameter is not a subtype of its declared generic base");


Error message: baseType here is the declared type parameter, so users see e.g. "Could not resolve type id 'java.lang.Long' as a subtype of java.lang.Integer". It may not be obvious that this relates to their polymorphic Map property. Could the message also include the outer polymorphic base type?

Changed. The exception now carries the outer polymorphic base type and the complete type id, the same way the container-level checks do, and the extra description names the offending argument. For your example the message is now:

Could not resolve type id 'java.util.HashMap<java.lang.String,java.lang.Long>' as a subtype of `java.util.Map<java.lang.String,java.lang.Integer>`: type parameter `java.lang.Long` (declared as `java.lang.Integer`) is not a subtype of its declared type

PTV denials follow the same shape (... denied resolution of type parameter com.example.Evil(declared asjava.lang.Object)). getTypeId() returns the full id and getBaseType() the polymorphic base. Covered by incompatibleTypeArgumentMessageNamesPolymorphicBaseAndTypeId and the extended allowedOuterBaseDoesNotApproveGenericArguments.

pjfanning · 2026-09-29T19:04:43Z

-        // tree (and array component, if any) and validate each node.
+        // The container itself was already validated above. Validate its type
+        // parameters against the corresponding declared base parameters.
+        _validateTypeParameters(baseType, subType, ptv, config);


The old top-level subType.isArrayType() branch is gone. As far as I can tell, a canonical type id containing < can't parse to a top-level array type, so nothing is lost. Can you confirm that's intended?

Intended, and I checked it against TypeParser rather than assuming. The parser only produces an ArrayType when the class-name token itself is an array class ([Lfoo;, [I), and TypeBindings.create rejects any type argument list for such a class because arrays declare no type variables:

[Ljava.lang.String;<java.lang.Integer> -> IllegalArgumentException: Cannot create TypeBindings for class [Ljava.lang.String; with 1 type parameter: class expects 0 [I<java.lang.Integer> -> same [Ljava.util.List;<> -> IllegalArgumentException: Cannot locate class '>' java.util.List<java.lang.String>[] -> IllegalArgumentException: Unexpected tokens after complete type

So a type id that reaches _resolveAndValidateGeneric (it contains <) can never resolve to a top-level array type, and the branch had no reachable input. Array arguments (ArrayList<Evil[]>) are still unwrapped in _validateTypeParameter.

pjfanning · 2026-09-29T19:04:43Z

+    final void _validateGenericSubType(JavaType type) throws JsonMappingException
+    {
+        SubTypeValidator.instance().validateSubType(this, type,
+                getConfig().introspect(type));


introspect(type) builds a bean description for every type argument, but SubTypeValidator.validateSubType only uses it when it reports a failure. Only generic type ids reach this path, so the cost is small, but the work is wasted in the normal case.

Switched to introspectClassAnnotations(type). SubTypeValidator.validateSubType only needs getBeanClass()/getType() from the description to build the error, so resolving class-level annotations is enough; no property collection happens any more on the success path.

pjfanning · 2026-09-29T19:04:43Z

+    @Override
+    public Validity validateSubClassName(MapperConfig<?> config,
+            JavaType baseType, String subClassName) throws JsonMappingException {
+        if (baseType == _allowedBaseType) {


== works for the StdTypeResolverBuilder → ClassNameIdResolver path, which passes the same instance. A custom TypeResolverBuilder that builds an equal but different JavaType would get a spurious rejection here. _allowedBaseType.equals(baseType) would be more robust at almost no cost (same at L43).

Changed both checks to _allowedBaseType.equals(baseType). Added BaseTypeAllowingValidatorTest, which builds an equal but distinct JavaType through a second TypeFactory cache and checks that it is still approved while a different base type still goes to the delegate.

pjfanning · 2026-09-29T19:07:05Z

@cowtowncoder this change seems too much of a change for a patch - even gitar says it is a High Risk.

martinfrancois · 2026-09-29T20:10:26Z

Thanks for the careful read. All six inline points are addressed in the new commit; details in the threads. On the two behaviour changes: I added a 2.18.12 entry to release-notes/VERSION-2.x that names both (the stricter rejection of HashMap<String,Long> for Map<String,Integer>, and accepting subtypes of an allowed base type as type arguments). Happy to reword it to whatever fits the release.

One more fix came out of re-reviewing the projection logic: when a subtype's projection onto the base contains a self-reference (SelfSupplier<X> implements Supplier<SelfSupplier<X>> projected onto Supplier), the argument X was marked as projected but the ResolvedRecursiveType it sits in reports no type parameters, so X was never validated. _validateTypeParameter now validates the referenced type instead of the placeholder. New test selfReferencingBindingDoesNotHideSubtypeArgument fails without that change.

Verified locally: the polymorphic validation classes (jsontype.vld.*, TestDefaultForObject, the new BaseTypeAllowingValidatorTest) pass; the full suite has the same six pre-existing failures as the untouched 2.18 base on this JDK 25 machine (AccessFixTest Security Manager, DateSerializationTest timezone name, TypeFactoryWithClassLoaderTest Mockito).

…nceof, validate self-referenced arguments - Error messages now carry the polymorphic base type and the complete type id, the same as the container-level checks, and name the offending type argument and the type parameter it was declared as. - Replace the `instanceof DeserializationContext` check with a protected no-op hook in `DatabindContext` that `DeserializationContext` overrides. - Use `introspectClassAnnotations` for the `SubTypeValidator` call; the bean description is only used to report a denial. - Compare the approved base type with `equals` in `BaseTypeAllowingValidator`. - Validate the type a `ResolvedRecursiveType` refers to: the projection of `SelfSupplier<X>` onto `Supplier` yields `Supplier<SelfSupplier<X>>` whose argument is a self-reference with no type parameters of its own, so `X` was never validated. - Bring the `_validateTypeParameter` javadoc up to date and add a release note naming both behaviour changes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

gitar-bot · 2026-09-29T21:26:27Z

CI failed: Fuzzer build failure due to a package namespace mismatch between the fuzzer script and jackson-databind.

Overview

1 log analysis completed, revealing 1 build failure related to OSS-Fuzz compilation where the fuzzer fails to locate the expected jackson-databind package.

Failures

Fuzzer Compilation Error (confidence: high)

Type: build
Affected jobs: 110070677233
Related to change: no
Root cause: The OSS-Fuzz build script attempts to compile ObjectReaderFuzzer.java using tools.jackson.databind.ObjectMapper, but the project is built under a different package namespace (com.fasterxml.jackson.databind), leading to compilation errors.
Suggested fix: Update the OSS-Fuzz integration or fuzzer source code to reference the correct package namespace.

Summary

Change-related failures: 0
Infrastructure/flaky failures: 0
Recommended action: The fuzzer build script in the external OSS-Fuzz configuration needs to be updated to match the package structure of the target branch, as this is unrelated to the changes introduced in the PR.

Code Review ✅ Approved

🔴 High risk · Changes security-sensitive validation of polymorphic generic type arguments.

Adds validation of generic type arguments to prevent invalid type configurations. No issues found.

Review coverage

📋 Rules No rules evaluated

🧪 Functional validation Not enabled · Set up

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

`Auto-apply`	`Compact`
`gitar auto-apply:on`	`gitar display:verbose`

_{Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source}

cowtowncoder · 2026-09-30T19:57:54Z

@martinfrancois Could you create alternative PR for simplified version -- bit easier to read diffs that way.

martinfrancois · 2026-09-30T21:59:52Z

@martinfrancois Could you create alternative PR for simplified version -- bit easier to read diffs that way.

Sure I can @cowtowncoder! Would you prefer it to target the branch from this PR (to better see the differences between this and the alternative) or also against 2.18?

martinfrancois · 2026-09-30T23:32:32Z

@cowtowncoder I opened #6259 against 2.18. If you'd rather have it against the branch of this PR instead, let me know and I'll change it.

Validate generic type arguments

51954db

pjfanning reviewed Sep 29, 2026

View reviewed changes

martinfrancois force-pushed the validate-generic-type-arguments branch from d0bc119 to 455cd79 Compare September 29, 2026 21:24

martinfrancois force-pushed the validate-generic-type-arguments branch from 455cd79 to 74f7f1b Compare September 29, 2026 21:25

cowtowncoder added the cla-received PR already covered by CLA (optional label) label Sep 29, 2026

martinfrancois mentioned this pull request Sep 30, 2026

Validate generic type arguments (simplified alternative to #6254) #6259

Open

Uh oh!

Conversation

martinfrancois commented Sep 29, 2026 • edited by gitar-bot Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Gitar

Uh oh!

pjfanning left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pjfanning Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pjfanning Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pjfanning commented Sep 29, 2026

Uh oh!

martinfrancois commented Sep 29, 2026

Uh oh!

gitar-bot Bot commented Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Failures

Fuzzer Compilation Error (confidence: high)

Summary

Uh oh!

cowtowncoder commented Sep 30, 2026

Uh oh!

martinfrancois commented Sep 30, 2026

Uh oh!

martinfrancois commented Sep 30, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

martinfrancois commented Sep 29, 2026 •

edited by gitar-bot Bot

Loading

pjfanning left a comment •

edited

Loading

pjfanning Sep 29, 2026 •

edited

Loading

pjfanning Sep 29, 2026 •

edited

Loading

gitar-bot Bot commented Sep 29, 2026 •

edited

Loading