From acbba44ba8ec5e45c636fb08f03bb62c5a81d83e Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 13 Apr 2026 10:17:34 -0700 Subject: [PATCH] Fix #343: improve checking for Afterburner deser optimizations --- .../deser/ABDeserializerModifier.java | 70 ++++++++++--------- .../deser/SuperSonicBeanDeserializer.java | 12 ++++ .../deser/SuperSonicUnrolledDeserializer.java | 12 ++++ .../deser/UnknownProperty343Test.java | 56 +++++++++++++++ release-notes/VERSION | 2 + 5 files changed, 118 insertions(+), 34 deletions(-) create mode 100644 afterburner/src/test/java/tools/jackson/module/afterburner/deser/UnknownProperty343Test.java diff --git a/afterburner/src/main/java/tools/jackson/module/afterburner/deser/ABDeserializerModifier.java b/afterburner/src/main/java/tools/jackson/module/afterburner/deser/ABDeserializerModifier.java index 65413098..e95bd6ae 100644 --- a/afterburner/src/main/java/tools/jackson/module/afterburner/deser/ABDeserializerModifier.java +++ b/afterburner/src/main/java/tools/jackson/module/afterburner/deser/ABDeserializerModifier.java @@ -43,41 +43,43 @@ public BeanDeserializerBuilder updateBuilder(DeserializationConfig config, BeanDescription.Supplier beanDescRef, BeanDeserializerBuilder builder) { final Class beanClass = beanDescRef.getBeanClass(); - // [module-afterburner#21]: Can't force access to sealed packages, or anything within "java." - if (!MyClassLoader.canAddClassInPackageOf(beanClass)) { - return builder; - } - /* Hmmh. Can we access stuff from private classes? - * Possibly, if we can use parent class loader. - * (should probably skip all non-public?) - */ - if (_classLoader != null) { - if (Modifier.isPrivate(beanClass.getModifiers())) { - return builder; - } - } - PropertyMutatorCollector collector = new PropertyMutatorCollector(beanClass); - List> newProps = findOptimizableProperties( - config, collector, builder.getProperties()); - // and if we found any, create mutator proxy, replace property objects - if (!newProps.isEmpty()) { - BeanPropertyMutator baseMutator = collector.buildMutator(_classLoader); - for (OptimizedSettableBeanProperty prop : newProps) { - builder.addOrReplaceProperty(prop.withMutator(baseMutator), true); + // Mutator/creator bytecode injection requires being able to define a class + // in the target's package. [module-afterburner#21] rules out sealed packages + // (which in 3.x also covers every package in a named JPMS module) and core + // "java.*" / "sun.misc" / etc. Also: private classes are unreachable when + // using our own classloader rather than the target's. + // + // Note: the SuperSonic deserializer wrapping below is independent of bytecode + // injection — it reuses whichever `SettableBeanProperty` instances the builder + // holds, optimized or not — so we still apply it even when injection is + // disallowed. This lets JPMS users benefit from the ordered-property fast + // path even though afterburner can't inject mutators into their modules. + final boolean canInject = MyClassLoader.canAddClassInPackageOf(beanClass) + && !(_classLoader != null && Modifier.isPrivate(beanClass.getModifiers())); + + if (canInject) { + PropertyMutatorCollector collector = new PropertyMutatorCollector(beanClass); + List> newProps = findOptimizableProperties( + config, collector, builder.getProperties()); + // and if we found any, create mutator proxy, replace property objects + if (!newProps.isEmpty()) { + BeanPropertyMutator baseMutator = collector.buildMutator(_classLoader); + for (OptimizedSettableBeanProperty prop : newProps) { + builder.addOrReplaceProperty(prop.withMutator(baseMutator), true); + } } - } - // Second thing: see if we could (re)generate Creator(s): - ValueInstantiator inst = builder.getValueInstantiator(); - /* Hmmh. Probably better to require exact default implementation - * and not sub-class; chances are sub-class uses its own - * construction anyway. - */ - if (inst.getClass() == StdValueInstantiator.class) { - // also, only override if using default creator (no-arg ctor, no-arg static factory) - if (inst.canCreateUsingDefault()) { - inst = new CreatorOptimizer(beanClass, _classLoader, (StdValueInstantiator) inst).createOptimized(); - if (inst != null) { - builder.setValueInstantiator(inst); + // Second thing: see if we could (re)generate Creator(s): + ValueInstantiator inst = builder.getValueInstantiator(); + // Hmmh. Probably better to require exact default implementation + // and not sub-class; chances are sub-class uses its own + // construction anyway. + if (inst.getClass() == StdValueInstantiator.class) { + // also, only override if using default creator (no-arg ctor, no-arg static factory) + if (inst.canCreateUsingDefault()) { + inst = new CreatorOptimizer(beanClass, _classLoader, (StdValueInstantiator) inst).createOptimized(); + if (inst != null) { + builder.setValueInstantiator(inst); + } } } } diff --git a/afterburner/src/main/java/tools/jackson/module/afterburner/deser/SuperSonicBeanDeserializer.java b/afterburner/src/main/java/tools/jackson/module/afterburner/deser/SuperSonicBeanDeserializer.java index d33870cf..80a230b0 100644 --- a/afterburner/src/main/java/tools/jackson/module/afterburner/deser/SuperSonicBeanDeserializer.java +++ b/afterburner/src/main/java/tools/jackson/module/afterburner/deser/SuperSonicBeanDeserializer.java @@ -168,6 +168,12 @@ public Object deserialize(JsonParser p, DeserializationContext ctxt) public Object deserialize(JsonParser p, DeserializationContext ctxt, Object bean) throws JacksonException { + // The ordered-property fast path below is only safe when no view filtering, + // non-standard creation or other non-vanilla features are in play; otherwise + // fall back to the parent implementation which handles all of those. + if (!_vanillaProcessing) { + return super.deserialize(p, ctxt, bean); + } // [databind#631]: Assign current value, to be accessible by custom serializers p.assignCurrentValue(bean); if (_injectables != null) { @@ -244,6 +250,12 @@ public Object deserialize(JsonParser p, DeserializationContext ctxt, Object bean public Object deserializeFromObject(JsonParser p, DeserializationContext ctxt) throws JacksonException { + // The ordered-property fast path below is only safe when no view filtering, + // non-standard creation or other non-vanilla features are in play; otherwise + // fall back to the parent implementation which handles all of those. + if (!_vanillaProcessing) { + return super.deserializeFromObject(p, ctxt); + } // See BeanDeserializer.deserializeFromObject [databind#622] // Allow Object Id references to come in as JSON Objects as well... if ((_objectIdReader != null) && _objectIdReader.maySerializeAsObject()) { diff --git a/afterburner/src/main/java/tools/jackson/module/afterburner/deser/SuperSonicUnrolledDeserializer.java b/afterburner/src/main/java/tools/jackson/module/afterburner/deser/SuperSonicUnrolledDeserializer.java index 4c4d076d..886a1168 100644 --- a/afterburner/src/main/java/tools/jackson/module/afterburner/deser/SuperSonicUnrolledDeserializer.java +++ b/afterburner/src/main/java/tools/jackson/module/afterburner/deser/SuperSonicUnrolledDeserializer.java @@ -251,6 +251,12 @@ public Object deserialize(JsonParser p, DeserializationContext ctxt) public Object deserialize(JsonParser p, DeserializationContext ctxt, Object bean) throws JacksonException { + // The ordered-property fast path below is only safe when no view filtering, + // non-standard creation or other non-vanilla features are in play; otherwise + // fall back to the parent implementation which handles all of those. + if (!_vanillaProcessing) { + return super.deserialize(p, ctxt, bean); + } // [databind#631]: Assign current value, to be accessible by custom serializers p.assignCurrentValue(bean); if (_injectables != null) { @@ -345,6 +351,12 @@ public Object deserialize(JsonParser p, DeserializationContext ctxt, Object bean public Object deserializeFromObject(JsonParser p, DeserializationContext ctxt) throws JacksonException { + // The ordered-property fast path below is only safe when no view filtering, + // non-standard creation or other non-vanilla features are in play; otherwise + // fall back to the parent implementation which handles all of those. + if (!_vanillaProcessing) { + return super.deserializeFromObject(p, ctxt); + } // See BeanDeserializer.deserializeFromObject [databind#622] // Allow Object Id references to come in as JSON Objects as well... if ((_objectIdReader != null) && _objectIdReader.maySerializeAsObject()) { diff --git a/afterburner/src/test/java/tools/jackson/module/afterburner/deser/UnknownProperty343Test.java b/afterburner/src/test/java/tools/jackson/module/afterburner/deser/UnknownProperty343Test.java new file mode 100644 index 00000000..bd2b0d6e --- /dev/null +++ b/afterburner/src/test/java/tools/jackson/module/afterburner/deser/UnknownProperty343Test.java @@ -0,0 +1,56 @@ +package tools.jackson.module.afterburner.deser; + +import org.junit.jupiter.api.Test; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonView; + +import tools.jackson.databind.DeserializationFeature; +import tools.jackson.databind.ObjectMapper; +import tools.jackson.module.afterburner.AfterburnerTestBase; + +import static org.junit.jupiter.api.Assertions.*; + +// [modules-base#343]: SuperSonicBeanDeserializer.deserializeFromObject threw on a +// trailing unknown property after all known ordered properties were consumed, even +// with FAIL_ON_UNKNOWN_PROPERTIES disabled. +public class UnknownProperty343Test extends AfterburnerTestBase +{ + // Marker class used purely to force the bean deserializer to operate in + // non-vanilla mode (view processing active), which routes through + // BeanDeserializer.deserializeFromObject rather than the vanilla fast path — + // and thus through SuperSonicBeanDeserializer.deserializeFromObject, which is + // where issue #343 lives. + static class DefaultView { } + + // 8 properties -> SuperSonicBeanDeserializer (threshold is > 6) + @JsonView(DefaultView.class) + public static class POJO343 { + @JsonProperty("a") public String a; + @JsonProperty("b") public String b; + @JsonProperty("c") public String c; + @JsonProperty("d") public String d; + @JsonProperty("e") public String e; + @JsonProperty("f") public String f; + @JsonProperty("g") public String g; + @JsonProperty("h") public String h; + } + + private final ObjectMapper MAPPER = afterburnerMapperBuilder() + .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + .build(); + + @Test + public void testTrailingUnknownProperty() throws Exception + { + String json = "{" + + "\"a\":\"a\",\"b\":\"b\",\"c\":\"c\",\"d\":\"d\"," + + "\"e\":\"e\",\"f\":\"f\",\"g\":\"g\",\"h\":\"h\"," + + "\"i\":\"i\"}"; + POJO343 result = MAPPER.readerWithView(DefaultView.class) + .forType(POJO343.class) + .readValue(json); + assertEquals("a", result.a); + assertEquals("h", result.h); + } +} diff --git a/release-notes/VERSION b/release-notes/VERSION index 11866d48..9691ec62 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -28,6 +28,8 @@ Active maintainers: `java.lang.IllegalStateException`: Multiple definitions of method getBody found (reported by @bxvs888) (fix by @cowtowncoder, w/ Claude code) +#343: Improve Afterburner testing, access checks + (fix by @cowtowncoder, w/ Claude code) - Removed project Android SDK level overrides, use defaults (ASDK 34) (affects `android-records` module's verification; was validating ASDK 26)