This is an automated email from the ASF dual-hosted git repository.
chaokunyang pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/fory.git
The following commit(s) were added to refs/heads/main by this push:
new 6ddc4145d fix(java): cache accepted type checker classes (#3773)
6ddc4145d is described below
commit 6ddc4145d99c048900767aa87317a544155ef4d0
Author: Shawn Yang <[email protected]>
AuthorDate: Sun Jun 21 10:53:08 2026 +0530
fix(java): cache accepted type checker classes (#3773)
## Why?
## What does this PR do?
TypeChecker is invoked in multiple places for security concerns, this pr
cache the result to only invoke once
## Related issues
Closes #3769
## 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?
## Benchmark
---
.../org/apache/fory/resolver/AllowListChecker.java | 16 ++++-
.../org/apache/fory/resolver/ClassResolver.java | 2 +-
.../org/apache/fory/resolver/SharedRegistry.java | 32 +++++++++
.../org/apache/fory/resolver/TypeResolver.java | 30 +++++++-
.../apache/fory/resolver/AllowListCheckerTest.java | 84 ++++++++++++++++++++++
5 files changed, 161 insertions(+), 3 deletions(-)
diff --git
a/java/fory-core/src/main/java/org/apache/fory/resolver/AllowListChecker.java
b/java/fory-core/src/main/java/org/apache/fory/resolver/AllowListChecker.java
index 4a3fa0f2a..704e6bd7f 100644
---
a/java/fory-core/src/main/java/org/apache/fory/resolver/AllowListChecker.java
+++
b/java/fory-core/src/main/java/org/apache/fory/resolver/AllowListChecker.java
@@ -77,7 +77,13 @@ public class AllowListChecker implements TypeChecker {
}
public void setCheckLevel(CheckLevel checkLevel) {
- this.checkLevel = checkLevel;
+ try {
+ lock.writeLock().lock();
+ this.checkLevel = checkLevel;
+ clearCheckerCache();
+ } finally {
+ lock.writeLock().unlock();
+ }
}
@Override
@@ -209,6 +215,7 @@ public class AllowListChecker implements TypeChecker {
String prefix = classNameOrPrefix.substring(0,
classNameOrPrefix.length() - 1);
disallowListPrefix.add(prefix);
for (ClassResolver classResolver : listeners.keySet()) {
+ classResolver.clearCheckerCacheForPrefix(prefix);
try {
classResolver.getJITContext().lock();
// clear serializer may throw NullPointerException for field
serialization.
@@ -220,6 +227,7 @@ public class AllowListChecker implements TypeChecker {
} else {
disallowList.add(classNameOrPrefix);
for (ClassResolver classResolver : listeners.keySet()) {
+ classResolver.clearCheckerCacheForClass(classNameOrPrefix);
try {
classResolver.getJITContext().lock();
// clear serializer may throw NullPointerException for field
serialization.
@@ -244,6 +252,12 @@ public class AllowListChecker implements TypeChecker {
}
}
+ private void clearCheckerCache() {
+ for (ClassResolver classResolver : listeners.keySet()) {
+ classResolver.clearCheckerCache();
+ }
+ }
+
@SuppressWarnings({"rawtypes", "unchecked"})
private static class DisallowSerializer extends Serializer {
diff --git
a/java/fory-core/src/main/java/org/apache/fory/resolver/ClassResolver.java
b/java/fory-core/src/main/java/org/apache/fory/resolver/ClassResolver.java
index 4104d023f..253c14b50 100644
--- a/java/fory-core/src/main/java/org/apache/fory/resolver/ClassResolver.java
+++ b/java/fory-core/src/main/java/org/apache/fory/resolver/ClassResolver.java
@@ -1952,7 +1952,7 @@ public class ClassResolver extends TypeResolver {
|| extRegistry.registeredClassIdMap.get(cls) != null
|| shimDispatcher.contains(cls);
} else {
- return extRegistry.typeChecker.checkType(this, cls.getName());
+ return checkType(cls.getName());
}
}
diff --git
a/java/fory-core/src/main/java/org/apache/fory/resolver/SharedRegistry.java
b/java/fory-core/src/main/java/org/apache/fory/resolver/SharedRegistry.java
index fa72d2f1f..d4ba499db 100644
--- a/java/fory-core/src/main/java/org/apache/fory/resolver/SharedRegistry.java
+++ b/java/fory-core/src/main/java/org/apache/fory/resolver/SharedRegistry.java
@@ -58,6 +58,7 @@ import org.apache.fory.type.DescriptorGrouper;
public final class SharedRegistry {
private static final int MAX_CACHED_ENCODED_META_STRINGS = 32768;
private static final int MAX_CACHED_ENCODED_META_STRING_LENGTH = 2048;
+ private static final int MAX_CACHED_TYPE_CHECKER_CLASSES = 8192;
private static final int MIN_REMOTE_TYPE_DEF_LIMIT = 8192;
final ConcurrentIdentityMap<Class<?>, TypeDef> typeDefMap = new
ConcurrentIdentityMap<>();
@@ -85,6 +86,14 @@ public final class SharedRegistry {
new ConcurrentIdentityMap<>();
final ConcurrentIdentityMap<Class<?>, Serializer<?>>
registeredSerializerCache =
new ConcurrentIdentityMap<>();
+ // Positive TypeChecker decisions are shared by class name across this
registry's equivalent Fory
+ // instances. Do not include TypeResolver or TypeChecker identity in the
key; SharedRegistry is
+ // already the sharing boundary for equivalent type-checking policy. Mutable
checker owners must
+ // clear these entries when a policy change can make a previously accepted
class forbidden. This
+ // cache is intentionally bounded; after it fills, duplicate TypeChecker
calls are safer than
+ // retaining attacker-controlled class names without limit.
+ private final ConcurrentHashMap<String, Boolean> acceptedTypeCheckerClasses =
+ new ConcurrentHashMap<>();
private final ConcurrentHashMap<Class<?>, ObjectInstantiator<?>>
objectInstantiatorCache =
new ConcurrentHashMap<>();
private final ConcurrentHashMap<Class<?>, ObjectInstantiator<?>>
objectStreamInstantiatorCache =
@@ -182,6 +191,29 @@ public final class SharedRegistry {
return existing == null ? typeInfo : existing;
}
+ boolean isTypeAccepted(String className) {
+ return acceptedTypeCheckerClasses.containsKey(className);
+ }
+
+ void markTypeAccepted(String className) {
+ if (acceptedTypeCheckerClasses.size() >= MAX_CACHED_TYPE_CHECKER_CLASSES) {
+ return;
+ }
+ acceptedTypeCheckerClasses.putIfAbsent(className, true);
+ }
+
+ void clearCheckerCache() {
+ acceptedTypeCheckerClasses.clear();
+ }
+
+ void clearCheckerCacheForClass(String className) {
+ acceptedTypeCheckerClasses.remove(className);
+ }
+
+ void clearCheckerCacheForPrefix(String prefix) {
+ acceptedTypeCheckerClasses.keySet().removeIf(className ->
className.startsWith(prefix));
+ }
+
TypeDef getOrCreateTypeDef(TypeDef typeDef) {
long id = typeDef.getId();
TypeDef existing = typeDefById.get(id);
diff --git
a/java/fory-core/src/main/java/org/apache/fory/resolver/TypeResolver.java
b/java/fory-core/src/main/java/org/apache/fory/resolver/TypeResolver.java
index 87a18426e..b10498552 100644
--- a/java/fory-core/src/main/java/org/apache/fory/resolver/TypeResolver.java
+++ b/java/fory-core/src/main/java/org/apache/fory/resolver/TypeResolver.java
@@ -1345,7 +1345,7 @@ public abstract class TypeResolver {
// Remote TypeDef/TypeMeta paths reach class materialization through this
owner. Keep
// name-level checks before Class.forName so rejected metadata cannot
force arbitrary class
// loading.
- if (!extRegistry.typeChecker.checkType(this, className)) {
+ if (!checkType(className)) {
throw new InsecureException(
String.format("Class %s is forbidden for serialization.",
className));
}
@@ -2154,12 +2154,40 @@ public abstract class TypeResolver {
}
public void setTypeChecker(TypeChecker typeChecker) {
+ sharedRegistry.clearCheckerCache();
extRegistry.typeChecker = typeChecker == null ? DEFAULT_TYPE_CHECKER :
typeChecker;
if (extRegistry.typeChecker instanceof AllowListChecker && this instanceof
ClassResolver) {
((AllowListChecker) extRegistry.typeChecker).addListener((ClassResolver)
this);
}
}
+ final boolean checkType(String className) {
+ TypeChecker typeChecker = extRegistry.typeChecker;
+ if (typeChecker == DEFAULT_TYPE_CHECKER) {
+ return true;
+ }
+ if (sharedRegistry.isTypeAccepted(className)) {
+ return true;
+ }
+ if (!typeChecker.checkType(this, className)) {
+ return false;
+ }
+ sharedRegistry.markTypeAccepted(className);
+ return true;
+ }
+
+ final void clearCheckerCache() {
+ sharedRegistry.clearCheckerCache();
+ }
+
+ final void clearCheckerCacheForClass(String className) {
+ sharedRegistry.clearCheckerCacheForClass(className);
+ }
+
+ final void clearCheckerCacheForPrefix(String prefix) {
+ sharedRegistry.clearCheckerCacheForPrefix(prefix);
+ }
+
public void registerSerializerFactory(SerializerFactory serializerFactory) {
extRegistry.serializerFactories.add(Preconditions.checkNotNull(serializerFactory));
}
diff --git
a/java/fory-core/src/test/java/org/apache/fory/resolver/AllowListCheckerTest.java
b/java/fory-core/src/test/java/org/apache/fory/resolver/AllowListCheckerTest.java
index 5d9b9b35e..cdd929344 100644
---
a/java/fory-core/src/test/java/org/apache/fory/resolver/AllowListCheckerTest.java
+++
b/java/fory-core/src/test/java/org/apache/fory/resolver/AllowListCheckerTest.java
@@ -24,8 +24,10 @@ import static org.testng.Assert.*;
import java.io.ByteArrayOutputStream;
import java.io.PrintStream;
import java.nio.charset.StandardCharsets;
+import java.util.concurrent.atomic.AtomicInteger;
import org.apache.fory.Fory;
import org.apache.fory.ThreadSafeFory;
+import org.apache.fory.config.Language;
import org.apache.fory.exception.InsecureException;
import org.apache.fory.logging.LogLevel;
import org.apache.fory.logging.LoggerFactory;
@@ -156,6 +158,84 @@ public class AllowListCheckerTest {
}
}
+ @Test
+ public void testTypeCheckerSingleCheck() {
+ Fory fory =
Fory.builder().withLanguage(Language.JAVA).requireClassRegistration(false).build();
+ AtomicInteger checks = new AtomicInteger();
+ fory.getTypeResolver()
+ .setTypeChecker(
+ (resolver, className) -> {
+ if (className.equals(CheckedType.class.getName())) {
+ checks.incrementAndGet();
+ }
+ return true;
+ });
+
+ byte[] bytes = fory.serialize(new CheckedType());
+ CheckedType result = (CheckedType) fory.deserialize(bytes);
+
+ assertEquals(result.value, "test");
+ assertEquals(checks.get(), 1);
+ }
+
+ @Test
+ public void testTypeCheckerCacheClearedByDisallow() {
+ Fory fory =
Fory.builder().withLanguage(Language.JAVA).requireClassRegistration(false).build();
+ AllowListChecker checker = new
AllowListChecker(AllowListChecker.CheckLevel.WARN);
+ fory.getTypeResolver().setTypeChecker(checker);
+
+ byte[] bytes = fory.serialize(new CheckedType());
+ checker.disallowClass(CheckedType.class.getName());
+
+ assertThrows(InsecureException.class, () -> fory.deserialize(bytes));
+ }
+
+ @Test
+ public void testTypeCheckerCacheSharedByRegistry() {
+ SharedRegistry sharedRegistry = new SharedRegistry();
+ Fory writer =
+ Fory.builder()
+ .withLanguage(Language.JAVA)
+ .requireClassRegistration(false)
+ .withSharedRegistry(sharedRegistry)
+ .build();
+ Fory reader =
+ Fory.builder()
+ .withLanguage(Language.JAVA)
+ .requireClassRegistration(false)
+ .withSharedRegistry(sharedRegistry)
+ .build();
+ AtomicInteger checks = new AtomicInteger();
+ TypeChecker checker =
+ (resolver, className) -> {
+ if (className.equals(CheckedType.class.getName())) {
+ checks.incrementAndGet();
+ }
+ return true;
+ };
+ writer.getTypeResolver().setTypeChecker(checker);
+ reader.getTypeResolver().setTypeChecker(checker);
+
+ byte[] bytes = writer.serialize(new CheckedType());
+ CheckedType result = (CheckedType) reader.deserialize(bytes);
+
+ assertEquals(result.value, "test");
+ assertEquals(checks.get(), 1);
+ }
+
+ @Test
+ public void testTypeCheckerCacheLimit() {
+ SharedRegistry sharedRegistry = new SharedRegistry();
+ for (int i = 0; i < 8192; i++) {
+ String className = "test.C" + i;
+ sharedRegistry.markTypeAccepted(className);
+ assertTrue(sharedRegistry.isTypeAccepted(className));
+ }
+ sharedRegistry.markTypeAccepted("test.Overflow");
+
+ assertFalse(sharedRegistry.isTypeAccepted("test.Overflow"));
+ }
+
@Test
public void testThreadSafeFory() {
AllowListChecker checker = new
AllowListChecker(AllowListChecker.CheckLevel.STRICT);
@@ -172,4 +252,8 @@ public class AllowListCheckerTest {
assertThrows(InsecureException.class, () -> fory.serialize(new
AllowListCheckerTest()));
assertThrows(InsecureException.class, () -> fory.deserialize(bytes));
}
+
+ public static class CheckedType {
+ public String value = "test";
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
