Skip to content

Commit 0929a1c

Browse files
cowtowncodercowtowncoder
authored
Fix #343: improve checking for Afterburner deser optimizations (#346)
1 parent 9377f88 commit 0929a1c

File tree

5 files changed

+118
-34
lines changed

5 files changed

+118
-34
lines changed

‎afterburner/src/main/java/tools/jackson/module/afterburner/deser/ABDeserializerModifier.java‎

Lines changed: 36 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -43,41 +43,43 @@ public BeanDeserializerBuilder updateBuilder(DeserializationConfig config,
4343
BeanDescription.Supplier beanDescRef, BeanDeserializerBuilder builder)
4444
{
4545
final Class<?> beanClass = beanDescRef.getBeanClass();
46-
// [module-afterburner#21]: Can't force access to sealed packages, or anything within "java."
47-
if (!MyClassLoader.canAddClassInPackageOf(beanClass)) {
48-
return builder;
49-
}
50-
/* Hmmh. Can we access stuff from private classes?
51-
* Possibly, if we can use parent class loader.
52-
* (should probably skip all non-public?)
53-
*/
54-
if (_classLoader != null) {
55-
if (Modifier.isPrivate(beanClass.getModifiers())) {
56-
return builder;
57-
}
58-
}
59-
PropertyMutatorCollector collector = new PropertyMutatorCollector(beanClass);
60-
List<OptimizedSettableBeanProperty<?>> newProps = findOptimizableProperties(
61-
config, collector, builder.getProperties());
62-
// and if we found any, create mutator proxy, replace property objects
63-
if (!newProps.isEmpty()) {
64-
BeanPropertyMutator baseMutator = collector.buildMutator(_classLoader);
65-
for (OptimizedSettableBeanProperty<?> prop : newProps) {
66-
builder.addOrReplaceProperty(prop.withMutator(baseMutator), true);
46+
// Mutator/creator bytecode injection requires being able to define a class
47+
// in the target's package. [module-afterburner#21] rules out sealed packages
48+
// (which in 3.x also covers every package in a named JPMS module) and core
49+
// "java.*" / "sun.misc" / etc. Also: private classes are unreachable when
50+
// using our own classloader rather than the target's.
51+
//
52+
// Note: the SuperSonic deserializer wrapping below is independent of bytecode
53+
// injection — it reuses whichever `SettableBeanProperty` instances the builder
54+
// holds, optimized or not — so we still apply it even when injection is
55+
// disallowed. This lets JPMS users benefit from the ordered-property fast
56+
// path even though afterburner can't inject mutators into their modules.
57+
final boolean canInject = MyClassLoader.canAddClassInPackageOf(beanClass)
58+
&& !(_classLoader != null && Modifier.isPrivate(beanClass.getModifiers()));
59+
60+
if (canInject) {
61+
PropertyMutatorCollector collector = new PropertyMutatorCollector(beanClass);
62+
List<OptimizedSettableBeanProperty<?>> newProps = findOptimizableProperties(
63+
config, collector, builder.getProperties());
64+
// and if we found any, create mutator proxy, replace property objects
65+
if (!newProps.isEmpty()) {
66+
BeanPropertyMutator baseMutator = collector.buildMutator(_classLoader);
67+
for (OptimizedSettableBeanProperty<?> prop : newProps) {
68+
builder.addOrReplaceProperty(prop.withMutator(baseMutator), true);
69+
}
6770
}
68-
}
69-
// Second thing: see if we could (re)generate Creator(s):
70-
ValueInstantiator inst = builder.getValueInstantiator();
71-
/* Hmmh. Probably better to require exact default implementation
72-
* and not sub-class; chances are sub-class uses its own
73-
* construction anyway.
74-
*/
75-
if (inst.getClass() == StdValueInstantiator.class) {
76-
// also, only override if using default creator (no-arg ctor, no-arg static factory)
77-
if (inst.canCreateUsingDefault()) {
78-
inst = new CreatorOptimizer(beanClass, _classLoader, (StdValueInstantiator) inst).createOptimized();
79-
if (inst != null) {
80-
builder.setValueInstantiator(inst);
71+
// Second thing: see if we could (re)generate Creator(s):
72+
ValueInstantiator inst = builder.getValueInstantiator();
73+
// Hmmh. Probably better to require exact default implementation
74+
// and not sub-class; chances are sub-class uses its own
75+
// construction anyway.
76+
if (inst.getClass() == StdValueInstantiator.class) {
77+
// also, only override if using default creator (no-arg ctor, no-arg static factory)
78+
if (inst.canCreateUsingDefault()) {
79+
inst = new CreatorOptimizer(beanClass, _classLoader, (StdValueInstantiator) inst).createOptimized();
80+
if (inst != null) {
81+
builder.setValueInstantiator(inst);
82+
}
8183
}
8284
}
8385
}

‎afterburner/src/main/java/tools/jackson/module/afterburner/deser/SuperSonicBeanDeserializer.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,12 @@ public Object deserialize(JsonParser p, DeserializationContext ctxt)
168168
public Object deserialize(JsonParser p, DeserializationContext ctxt, Object bean)
169169
throws JacksonException
170170
{
171+
// The ordered-property fast path below is only safe when no view filtering,
172+
// non-standard creation or other non-vanilla features are in play; otherwise
173+
// fall back to the parent implementation which handles all of those.
174+
if (!_vanillaProcessing) {
175+
return super.deserialize(p, ctxt, bean);
176+
}
171177
// [databind#631]: Assign current value, to be accessible by custom serializers
172178
p.assignCurrentValue(bean);
173179
if (_injectables != null) {
@@ -244,6 +250,12 @@ public Object deserialize(JsonParser p, DeserializationContext ctxt, Object bean
244250
public Object deserializeFromObject(JsonParser p, DeserializationContext ctxt)
245251
throws JacksonException
246252
{
253+
// The ordered-property fast path below is only safe when no view filtering,
254+
// non-standard creation or other non-vanilla features are in play; otherwise
255+
// fall back to the parent implementation which handles all of those.
256+
if (!_vanillaProcessing) {
257+
return super.deserializeFromObject(p, ctxt);
258+
}
247259
// See BeanDeserializer.deserializeFromObject [databind#622]
248260
// Allow Object Id references to come in as JSON Objects as well...
249261
if ((_objectIdReader != null) && _objectIdReader.maySerializeAsObject()) {

‎afterburner/src/main/java/tools/jackson/module/afterburner/deser/SuperSonicUnrolledDeserializer.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,12 @@ public Object deserialize(JsonParser p, DeserializationContext ctxt)
251251
public Object deserialize(JsonParser p, DeserializationContext ctxt, Object bean)
252252
throws JacksonException
253253
{
254+
// The ordered-property fast path below is only safe when no view filtering,
255+
// non-standard creation or other non-vanilla features are in play; otherwise
256+
// fall back to the parent implementation which handles all of those.
257+
if (!_vanillaProcessing) {
258+
return super.deserialize(p, ctxt, bean);
259+
}
254260
// [databind#631]: Assign current value, to be accessible by custom serializers
255261
p.assignCurrentValue(bean);
256262
if (_injectables != null) {
@@ -345,6 +351,12 @@ public Object deserialize(JsonParser p, DeserializationContext ctxt, Object bean
345351
public Object deserializeFromObject(JsonParser p, DeserializationContext ctxt)
346352
throws JacksonException
347353
{
354+
// The ordered-property fast path below is only safe when no view filtering,
355+
// non-standard creation or other non-vanilla features are in play; otherwise
356+
// fall back to the parent implementation which handles all of those.
357+
if (!_vanillaProcessing) {
358+
return super.deserializeFromObject(p, ctxt);
359+
}
348360
// See BeanDeserializer.deserializeFromObject [databind#622]
349361
// Allow Object Id references to come in as JSON Objects as well...
350362
if ((_objectIdReader != null) && _objectIdReader.maySerializeAsObject()) {

‎afterburner/src/test/java/tools/jackson/module/afterburner/deser/UnknownProperty343Test.java‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package tools.jackson.module.afterburner.deser;
2+
3+
import org.junit.jupiter.api.Test;
4+
5+
import com.fasterxml.jackson.annotation.JsonProperty;
6+
import com.fasterxml.jackson.annotation.JsonView;
7+
8+
import tools.jackson.databind.DeserializationFeature;
9+
import tools.jackson.databind.ObjectMapper;
10+
import tools.jackson.module.afterburner.AfterburnerTestBase;
11+
12+
import static org.junit.jupiter.api.Assertions.*;
13+
14+
// [modules-base#343]: SuperSonicBeanDeserializer.deserializeFromObject threw on a
15+
// trailing unknown property after all known ordered properties were consumed, even
16+
// with FAIL_ON_UNKNOWN_PROPERTIES disabled.
17+
public class UnknownProperty343Test extends AfterburnerTestBase
18+
{
19+
// Marker class used purely to force the bean deserializer to operate in
20+
// non-vanilla mode (view processing active), which routes through
21+
// BeanDeserializer.deserializeFromObject rather than the vanilla fast path —
22+
// and thus through SuperSonicBeanDeserializer.deserializeFromObject, which is
23+
// where issue #343 lives.
24+
static class DefaultView { }
25+
26+
// 8 properties -> SuperSonicBeanDeserializer (threshold is > 6)
27+
@JsonView(DefaultView.class)
28+
public static class POJO343 {
29+
@JsonProperty("a") public String a;
30+
@JsonProperty("b") public String b;
31+
@JsonProperty("c") public String c;
32+
@JsonProperty("d") public String d;
33+
@JsonProperty("e") public String e;
34+
@JsonProperty("f") public String f;
35+
@JsonProperty("g") public String g;
36+
@JsonProperty("h") public String h;
37+
}
38+
39+
private final ObjectMapper MAPPER = afterburnerMapperBuilder()
40+
.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
41+
.build();
42+
43+
@Test
44+
public void testTrailingUnknownProperty() throws Exception
45+
{
46+
String json = "{"
47+
+ "\"a\":\"a\",\"b\":\"b\",\"c\":\"c\",\"d\":\"d\","
48+
+ "\"e\":\"e\",\"f\":\"f\",\"g\":\"g\",\"h\":\"h\","
49+
+ "\"i\":\"i\"}";
50+
POJO343 result = MAPPER.readerWithView(DefaultView.class)
51+
.forType(POJO343.class)
52+
.readValue(json);
53+
assertEquals("a", result.a);
54+
assertEquals("h", result.h);
55+
}
56+
}

‎release-notes/VERSION‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ Active maintainers:
2828
`java.lang.IllegalStateException`: Multiple definitions of method getBody found
2929
(reported by @bxvs888)
3030
(fix by @cowtowncoder, w/ Claude code)
31+
#343: Improve Afterburner testing, access checks
32+
(fix by @cowtowncoder, w/ Claude code)
3133
- Removed project Android SDK level overrides, use defaults (ASDK 34)
3234
(affects `android-records` module's verification; was validating ASDK 26)
3335

0 commit comments

Comments
 (0)