Skip to content

Commit d254818

Browse files
authored
fix(java): configure type checker during build (#3550)
## Why? ## What does this PR do? - add `ForyBuilder.withTypeChecker` so type checkers can be installed before build - wire builder-configured checkers into `Fory` creation and centralize `AllowListChecker` listener setup - update the Java type-registration docs and tests to use the supported allow-list flow ## Related issues Closes #3530. ## AI Contribution Checklist - [ ] Substantial AI assistance was used in this PR: `yes` / `no` - [ ] If `yes`, I included a completed [AI Contribution Checklist](https://github.com/apache/fory/blob/main/AI_POLICY.md#9-contributor-checklist-for-ai-assisted-prs) in this PR description and the required `AI Usage Disclosure`. - [ ] If `yes`, my PR description includes the required `ai_review` summary and screenshot evidence of the final clean AI review results from both fresh reviewers on the current PR diff or current HEAD after the latest code changes. ## Does this PR introduce any user-facing change? - [ ] Does this PR introduce any public API change? - [ ] Does this PR introduce any binary protocol compatibility change?
1 parent 5e4f275 commit d254818

6 files changed

Lines changed: 103 additions & 32 deletions

File tree

docs/guide/java/type-registration.md

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,15 @@ If there are no duplicate names for types, `namespace` can be left as empty to r
6060

6161
### Type Checker
6262

63-
If you invoke `ForyBuilder#requireClassRegistration(false)` to disable class registration check, you can set `org.apache.fory.resolver.TypeChecker` by `TypeResolver#setTypeChecker` to control which classes are allowed for serialization.
63+
If you invoke `ForyBuilder#requireClassRegistration(false)` to disable class registration check, you can configure `org.apache.fory.resolver.TypeChecker` by `ForyBuilder#withTypeChecker` or `TypeResolver#setTypeChecker` to control which classes are allowed for serialization.
6464

6565
For example, you can allow classes started with `org.example.*`:
6666

6767
```java
68-
Fory fory = xxx;
69-
fory.getTypeResolver().setTypeChecker(
70-
(typeResolver, className) -> className.startsWith("org.example."));
68+
Fory fory = Fory.builder()
69+
.requireClassRegistration(false)
70+
.withTypeChecker((typeResolver, className) -> className.startsWith("org.example."))
71+
.build();
7172
```
7273

7374
### AllowListChecker
@@ -76,12 +77,17 @@ Fory provides a `org.apache.fory.resolver.AllowListChecker` which is an allowed/
7677

7778
```java
7879
AllowListChecker checker = new AllowListChecker(AllowListChecker.CheckLevel.STRICT);
79-
ThreadSafeFory fory = Fory.builder().requireClassRegistration(false).buildThreadSafeFory();
80-
fory.setTypeChecker(checker);
8180
checker.allowClass("org.example.*");
81+
ThreadSafeFory fory = Fory.builder()
82+
.requireClassRegistration(false)
83+
.withTypeChecker(checker)
84+
.buildThreadSafeFory();
8285
```
8386

84-
You can use this checker or implement a more sophisticated checker by yourself.
87+
`withTypeChecker` installs the checker on every created runtime immediately, which also avoids the
88+
generic startup warning emitted when class registration is disabled without any checker. You can
89+
still use `TypeResolver#setTypeChecker` or `ThreadSafeFory#setTypeChecker` later if you need to
90+
replace the checker after build time.
8591

8692
## Limit Max Deserialization Depth
8793

java/fory-core/src/main/java/org/apache/fory/AbstractThreadSafeFory.java

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
package org.apache.fory;
2121

2222
import java.util.function.Function;
23-
import org.apache.fory.resolver.AllowListChecker;
24-
import org.apache.fory.resolver.ClassResolver;
2523
import org.apache.fory.resolver.TypeChecker;
2624
import org.apache.fory.resolver.TypeResolver;
2725
import org.apache.fory.serializer.Serializer;
@@ -121,14 +119,7 @@ public TypeResolver getTypeResolver() {
121119

122120
@Override
123121
public void setTypeChecker(TypeChecker typeChecker) {
124-
registerCallback(
125-
fory -> {
126-
TypeResolver typeResolver = fory.getTypeResolver();
127-
typeResolver.setTypeChecker(typeChecker);
128-
if (typeChecker instanceof AllowListChecker && typeResolver instanceof ClassResolver) {
129-
((AllowListChecker) typeChecker).addListener((ClassResolver) typeResolver);
130-
}
131-
});
122+
registerCallback(fory -> fory.getTypeResolver().setTypeChecker(typeChecker));
132123
}
133124

134125
@Override

java/fory-core/src/main/java/org/apache/fory/Fory.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import org.apache.fory.memory.MemoryUtils;
5353
import org.apache.fory.resolver.ClassResolver;
5454
import org.apache.fory.resolver.SharedRegistry;
55+
import org.apache.fory.resolver.TypeChecker;
5556
import org.apache.fory.resolver.TypeInfo;
5657
import org.apache.fory.resolver.TypeResolver;
5758
import org.apache.fory.resolver.XtypeResolver;
@@ -133,6 +134,10 @@ public Fory(ForyBuilder builder, ClassLoader classLoader, SharedRegistry sharedR
133134
? new XtypeResolver(config, classLoader, sharedRegistry, jitContext)
134135
: new ClassResolver(config, classLoader, sharedRegistry, jitContext);
135136
typeResolver.initialize();
137+
TypeChecker configuredTypeChecker = builder.getTypeChecker();
138+
if (configuredTypeChecker != null) {
139+
typeResolver.setTypeChecker(configuredTypeChecker);
140+
}
136141
MetaStringWriter metaStringWriter = new MetaStringWriter();
137142
MetaStringReader metaStringReader = new MetaStringReader(sharedRegistry);
138143
writeContext =

java/fory-core/src/main/java/org/apache/fory/config/ForyBuilder.java

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.apache.fory.pool.ThreadPoolFory;
3636
import org.apache.fory.reflect.ReflectionUtils;
3737
import org.apache.fory.resolver.SharedRegistry;
38+
import org.apache.fory.resolver.TypeChecker;
3839
import org.apache.fory.serializer.JavaSerializer;
3940
import org.apache.fory.serializer.ObjectStreamSerializer;
4041
import org.apache.fory.serializer.Serializer;
@@ -97,6 +98,7 @@ public final class ForyBuilder {
9798
int maxDepth = 50;
9899
float mapRefLoadFactor = 0.51f;
99100
boolean forVirtualThread = false;
101+
TypeChecker typeChecker;
100102
private List<Consumer<ForyBuilder>> actions = new ArrayList<>();
101103
private boolean replayingActions = false;
102104

@@ -349,16 +351,31 @@ public ForyBuilder registerGuavaTypes(boolean register) {
349351
* attack if the classes `constructor`/`equals`/`hashCode` method contain malicious code. Do not
350352
* disable class registration if you can't ensure your environment are *indeed secure*. We are not
351353
* responsible for security risks if you disable this option. If you disable this option, you can
352-
* configure {@link org.apache.fory.resolver.TypeChecker} by {@link
353-
* org.apache.fory.resolver.TypeResolver#setTypeChecker} to control which classes are allowed
354-
* being serialized.
354+
* configure {@link org.apache.fory.resolver.TypeChecker} by {@link #withTypeChecker(TypeChecker)}
355+
* or {@link org.apache.fory.resolver.TypeResolver#setTypeChecker} to control which classes are
356+
* allowed being serialized.
355357
*/
356358
public ForyBuilder requireClassRegistration(boolean requireClassRegistration) {
357359
this.requireClassRegistration = requireClassRegistration;
358360
recordAction(b -> b.requireClassRegistration(requireClassRegistration));
359361
return this;
360362
}
361363

364+
/**
365+
* Configure a {@link TypeChecker} during build time so it is installed on every created runtime.
366+
* This checker is only consulted for unknown class names when class registration checks are
367+
* disabled.
368+
*/
369+
public ForyBuilder withTypeChecker(TypeChecker typeChecker) {
370+
this.typeChecker = typeChecker;
371+
recordAction(b -> b.withTypeChecker(typeChecker));
372+
return this;
373+
}
374+
375+
public TypeChecker getTypeChecker() {
376+
return typeChecker;
377+
}
378+
362379
/**
363380
* Whether suppress class registration warnings. The warnings can be used for security audit, but
364381
* may be annoying, this suppression will be enabled by default.
@@ -572,11 +589,13 @@ private void finish() {
572589
}
573590
}
574591
if (!requireClassRegistration) {
575-
LOG.warn(
576-
"Class registration isn't forced, unknown classes can be deserialized. "
577-
+ "If the environment isn't secure, please enable class registration by "
578-
+ "`ForyBuilder#requireClassRegistration(true)` or configure TypeChecker by "
579-
+ "`TypeResolver#setTypeChecker`");
592+
if (typeChecker == null) {
593+
LOG.warn(
594+
"Class registration isn't forced, unknown classes can be deserialized. "
595+
+ "If the environment isn't secure, please enable class registration by "
596+
+ "`ForyBuilder#requireClassRegistration(true)` or configure TypeChecker by "
597+
+ "`ForyBuilder#withTypeChecker` or `TypeResolver#setTypeChecker`");
598+
}
580599
}
581600
if (GraalvmSupport.IN_GRAALVM_NATIVE_IMAGE && asyncCompilationEnabled) {
582601
LOG.info("Use sync compilation for graalvm native image since it doesn't support JIT.");

java/fory-core/src/main/java/org/apache/fory/resolver/TypeResolver.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1580,7 +1580,10 @@ private void buildGenericMap(Map<String, GenericType> map, GenericType genericTy
15801580
}
15811581

15821582
public void setTypeChecker(TypeChecker typeChecker) {
1583-
extRegistry.typeChecker = typeChecker;
1583+
extRegistry.typeChecker = typeChecker == null ? DEFAULT_TYPE_CHECKER : typeChecker;
1584+
if (extRegistry.typeChecker instanceof AllowListChecker && this instanceof ClassResolver) {
1585+
((AllowListChecker) extRegistry.typeChecker).addListener((ClassResolver) this);
1586+
}
15841587
}
15851588

15861589
public void setSerializerFactory(SerializerFactory serializerFactory) {

java/fory-core/src/test/java/org/apache/fory/resolver/AllowListCheckerTest.java

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,14 @@
2121

2222
import static org.testng.Assert.*;
2323

24+
import java.io.ByteArrayOutputStream;
25+
import java.io.PrintStream;
26+
import java.nio.charset.StandardCharsets;
2427
import org.apache.fory.Fory;
2528
import org.apache.fory.ThreadSafeFory;
2629
import org.apache.fory.exception.InsecureException;
30+
import org.apache.fory.logging.LogLevel;
31+
import org.apache.fory.logging.LoggerFactory;
2732
import org.testng.annotations.Test;
2833

2934
public class AllowListCheckerTest {
@@ -37,7 +42,6 @@ public void testCheckClass() {
3742
assertThrows(InsecureException.class, () -> fory.serialize(new AllowListCheckerTest()));
3843
checker.allowClass(AllowListCheckerTest.class.getName());
3944
byte[] bytes = fory.serialize(new AllowListCheckerTest());
40-
checker.addListener((ClassResolver) fory.getTypeResolver());
4145
checker.disallowClass(AllowListCheckerTest.class.getName());
4246
assertThrows(InsecureException.class, () -> fory.serialize(new AllowListCheckerTest()));
4347
assertThrows(InsecureException.class, () -> fory.deserialize(bytes));
@@ -46,7 +50,6 @@ public void testCheckClass() {
4650
Fory fory = Fory.builder().requireClassRegistration(false).build();
4751
AllowListChecker checker = new AllowListChecker(AllowListChecker.CheckLevel.WARN);
4852
fory.getTypeResolver().setTypeChecker(checker);
49-
checker.addListener((ClassResolver) fory.getTypeResolver());
5053
byte[] bytes = fory.serialize(new AllowListCheckerTest());
5154
checker.disallowClass(AllowListCheckerTest.class.getName());
5255
assertThrows(InsecureException.class, () -> fory.serialize(new AllowListCheckerTest()));
@@ -60,7 +63,6 @@ public void testCheckClassWildcard() {
6063
Fory fory = Fory.builder().requireClassRegistration(false).build();
6164
AllowListChecker checker = new AllowListChecker(AllowListChecker.CheckLevel.STRICT);
6265
fory.getTypeResolver().setTypeChecker(checker);
63-
checker.addListener((ClassResolver) fory.getTypeResolver());
6466
assertThrows(InsecureException.class, () -> fory.serialize(new AllowListCheckerTest()));
6567
checker.allowClass("org.apache.fory.*");
6668
byte[] bytes = fory.serialize(new AllowListCheckerTest());
@@ -72,19 +74,64 @@ public void testCheckClassWildcard() {
7274
Fory fory = Fory.builder().requireClassRegistration(false).build();
7375
AllowListChecker checker = new AllowListChecker(AllowListChecker.CheckLevel.WARN);
7476
fory.getTypeResolver().setTypeChecker(checker);
75-
checker.addListener((ClassResolver) fory.getTypeResolver());
7677
byte[] bytes = fory.serialize(new AllowListCheckerTest());
7778
checker.disallowClass("org.apache.fory.*");
7879
assertThrows(InsecureException.class, () -> fory.serialize(new AllowListCheckerTest()));
7980
assertThrows(InsecureException.class, () -> fory.deserialize(bytes));
8081
}
8182
}
8283

84+
@Test
85+
public void testBuilderConfiguredChecker() {
86+
AllowListChecker checker = new AllowListChecker(AllowListChecker.CheckLevel.STRICT);
87+
checker.allowClass("org.apache.fory.*");
88+
Fory fory = Fory.builder().requireClassRegistration(false).withTypeChecker(checker).build();
89+
byte[] bytes = fory.serialize(new AllowListCheckerTest());
90+
checker.disallowClass("org.apache.fory.*");
91+
assertThrows(InsecureException.class, () -> fory.serialize(new AllowListCheckerTest()));
92+
assertThrows(InsecureException.class, () -> fory.deserialize(bytes));
93+
}
94+
95+
@Test
96+
public void testBuilderConfiguredCheckerSuppressesStartupWarning() {
97+
synchronized (AllowListCheckerTest.class) {
98+
PrintStream previousOut = System.out;
99+
int previousLogLevel = LoggerFactory.getLogLevel();
100+
ByteArrayOutputStream output = new ByteArrayOutputStream();
101+
try (PrintStream capture = new PrintStream(output, true, StandardCharsets.UTF_8.name())) {
102+
System.setOut(capture);
103+
LoggerFactory.setLogLevel(LogLevel.WARN_LEVEL);
104+
105+
Fory.builder().requireClassRegistration(false).build();
106+
assertTrue(
107+
new String(output.toByteArray(), StandardCharsets.UTF_8)
108+
.contains("Class registration isn't forced"));
109+
110+
output.reset();
111+
Fory.builder()
112+
.requireClassRegistration(false)
113+
.withTypeChecker((resolver, className) -> true)
114+
.build();
115+
assertFalse(
116+
new String(output.toByteArray(), StandardCharsets.UTF_8)
117+
.contains("Class registration isn't forced"));
118+
} catch (Exception e) {
119+
throw new AssertionError(e);
120+
} finally {
121+
LoggerFactory.setLogLevel(previousLogLevel);
122+
System.setOut(previousOut);
123+
}
124+
}
125+
}
126+
83127
@Test
84128
public void testThreadSafeFory() {
85129
AllowListChecker checker = new AllowListChecker(AllowListChecker.CheckLevel.STRICT);
86-
ThreadSafeFory fory = Fory.builder().requireClassRegistration(false).buildThreadSafeFory();
87-
fory.setTypeChecker(checker);
130+
ThreadSafeFory fory =
131+
Fory.builder()
132+
.requireClassRegistration(false)
133+
.withTypeChecker(checker)
134+
.buildThreadSafeFory();
88135
checker.allowClass("org.apache.fory.*");
89136
byte[] bytes = fory.serialize(new AllowListCheckerTest());
90137
checker.disallowClass("org.apache.fory.*");

0 commit comments

Comments
 (0)