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 17b9c606e fix(java): Deserialize nested HashMap subclasses (#3342)
17b9c606e is described below
commit 17b9c606e78b3dc3cda461db2d8e04980f02f523
Author: Sebastian Mandrean <[email protected]>
AuthorDate: Tue Feb 17 03:43:14 2026 +0100
fix(java): Deserialize nested HashMap subclasses (#3342)
## Why?
Deserializing nested HashMap subclasses (e.g. `class MyMap extends
HashMap<String, MyOtherMap>`) throws NPE in both `SCHEMA_CONSISTENT` and
`COMPATIBLE` modes. Two distinct bugs cause this.
## What does this PR do?
**Bug 1: Write/read generics push asymmetry in
`MapLikeSerializer.readJavaChunkGeneric`**
The write path in `writeJavaChunkGeneric` always pushes generic types
onto the `Generics` stack, but the read path conditionally pushed them
based on `hasGenericParameters()`. For nested HashMap subclasses without
own type parameters (only inherited ones), `hasGenericParameters()`
returns false, causing inner maps
to fall into the non-generic read path while bytes were written by the
generic path. This results in a null serializer when
`KEY_DECL_TYPE`/`VALUE_DECL_TYPE` chunk flags are set.
**Fix 1:** Remove the conditional branch and always push/pop generics
symmetrically, matching the write path.
**Bug 2: Meta context index desync in
`ChildContainerSerializers.readAndSkipLayerClassMeta`**
In COMPATIBLE mode, `writeLayerClassMeta` adds layer marker classes to
`metaContext.classMap`, which shares the same index space as
`writeSharedClassMeta`. However, `readAndSkipLayerClassMeta` skipped the
bytes without adding entries to `metaContext.readTypeInfos`. This caused
subsequent `readSharedClassMeta`
reference lookups to use wrong indices.
**Fix 2:** Add a `null` placeholder to `metaContext.readTypeInfos` after
skipping a new layer meta entry.
## Related issues
Fixes #3337
## Does this PR introduce any user-facing change?
- [ ] Does this PR introduce any public API change? No
- [ ] Does this PR introduce any binary protocol compatibility change?
No
Binary compatibility is safe. The fix only changes the read path, it
doesn't change what bytes are written. The write path was already always
pushing generics and writing with KEY_DECL_TYPE/VALUE_DECL_TYPE flags
set. The read path now correctly consumes those same bytes.
For bug 2, the fix only adds a bookkeeping placeholder on the read side,
writing is untouched.
Old writers + new readers: works. New writers + old readers: unchanged
(write path is identical)
## Benchmark
Bug 1 fix removes a conditional branch (one fewer branch per chunk
iteration), so performance should be equal or marginally better.
Bug 2 fix adds a single `ArrayList.add(null)` call per new layer meta
entry, which is negligible.
---
.../collection/ChildContainerSerializers.java | 5 +
.../serializer/collection/MapLikeSerializer.java | 56 ++---
.../collection/ChildContainerSerializersTest.java | 244 +++++++++++++++++++++
3 files changed, 268 insertions(+), 37 deletions(-)
diff --git
a/java/fory-core/src/main/java/org/apache/fory/serializer/collection/ChildContainerSerializers.java
b/java/fory-core/src/main/java/org/apache/fory/serializer/collection/ChildContainerSerializers.java
index 3854ba7fd..837873fd6 100644
---
a/java/fory-core/src/main/java/org/apache/fory/serializer/collection/ChildContainerSerializers.java
+++
b/java/fory-core/src/main/java/org/apache/fory/serializer/collection/ChildContainerSerializers.java
@@ -292,5 +292,10 @@ public class ChildContainerSerializers {
// New type - need to read and skip the TypeDef bytes
long id = buffer.readInt64();
TypeDef.skipTypeDef(buffer, id);
+ // Add a placeholder to keep readTypeInfos indices in sync with the write
side's classMap.
+ // The write side (writeLayerClassMeta) adds layer marker classes to
classMap which shares
+ // the same index space as writeSharedClassMeta. Without this placeholder,
subsequent
+ // readSharedClassMeta reference lookups would use wrong indices.
+ metaContext.readTypeInfos.add(null);
}
}
diff --git
a/java/fory-core/src/main/java/org/apache/fory/serializer/collection/MapLikeSerializer.java
b/java/fory-core/src/main/java/org/apache/fory/serializer/collection/MapLikeSerializer.java
index 0abd12a4c..1e4677c10 100644
---
a/java/fory-core/src/main/java/org/apache/fory/serializer/collection/MapLikeSerializer.java
+++
b/java/fory-core/src/main/java/org/apache/fory/serializer/collection/MapLikeSerializer.java
@@ -853,43 +853,25 @@ public abstract class MapLikeSerializer<T> extends
Serializer<T> {
} else {
valueSerializer = valueGenericType.getSerializer(typeResolver);
}
- if (keyGenericType.hasGenericParameters() ||
valueGenericType.hasGenericParameters()) {
- for (int i = 0; i < chunkSize; i++) {
- generics.pushGenericType(keyGenericType);
- fory.incReadDepth();
- Object key =
- trackKeyRef
- ? binding.readRef(buffer, keySerializer)
- : binding.read(buffer, keySerializer);
- fory.decDepth();
- generics.popGenericType();
- generics.pushGenericType(valueGenericType);
- fory.incReadDepth();
- Object value =
- trackValueRef
- ? binding.readRef(buffer, valueSerializer)
- : binding.read(buffer, valueSerializer);
- fory.decDepth();
- generics.popGenericType();
- map.put(key, value);
- size--;
- }
- } else {
- for (int i = 0; i < chunkSize; i++) {
- // increase depth to avoid read wrong outer generic type
- fory.incReadDepth();
- Object key =
- trackKeyRef
- ? binding.readRef(buffer, keySerializer)
- : binding.read(buffer, keySerializer);
- Object value =
- trackValueRef
- ? binding.readRef(buffer, valueSerializer)
- : binding.read(buffer, valueSerializer);
- fory.decDepth();
- map.put(key, value);
- size--;
- }
+ for (int i = 0; i < chunkSize; i++) {
+ generics.pushGenericType(keyGenericType);
+ fory.incReadDepth();
+ Object key =
+ trackKeyRef
+ ? binding.readRef(buffer, keySerializer)
+ : binding.read(buffer, keySerializer);
+ fory.decDepth();
+ generics.popGenericType();
+ generics.pushGenericType(valueGenericType);
+ fory.incReadDepth();
+ Object value =
+ trackValueRef
+ ? binding.readRef(buffer, valueSerializer)
+ : binding.read(buffer, valueSerializer);
+ fory.decDepth();
+ generics.popGenericType();
+ map.put(key, value);
+ size--;
}
return size > 0 ? (size << 8) | buffer.readUnsignedByte() : 0;
}
diff --git
a/java/fory-core/src/test/java/org/apache/fory/serializer/collection/ChildContainerSerializersTest.java
b/java/fory-core/src/test/java/org/apache/fory/serializer/collection/ChildContainerSerializersTest.java
index 065d68caf..6eb101367 100644
---
a/java/fory-core/src/test/java/org/apache/fory/serializer/collection/ChildContainerSerializersTest.java
+++
b/java/fory-core/src/test/java/org/apache/fory/serializer/collection/ChildContainerSerializersTest.java
@@ -23,18 +23,26 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import java.util.ArrayDeque;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
+import java.util.TreeSet;
import java.util.Vector;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collector;
+import java.util.stream.Collectors;
+import lombok.AllArgsConstructor;
import lombok.Data;
+import lombok.EqualsAndHashCode;
+import lombok.NoArgsConstructor;
import org.apache.fory.Fory;
import org.apache.fory.ForyTestBase;
import org.apache.fory.config.CompatibleMode;
+import org.apache.fory.config.Language;
import org.apache.fory.test.bean.Cyclic;
import org.testng.Assert;
import org.testng.annotations.DataProvider;
@@ -260,6 +268,242 @@ public class ChildContainerSerializersTest extends
ForyTestBase {
serDeMetaShared(fory, outerDO);
}
+ /**
+ * Tests that meta context indices stay synchronized when layer class meta
entries from
+ * readAndSkipLayerClassMeta are interleaved with regular type info entries.
Multiple instances of
+ * the same nested HashMap subclass type force meta context reference
lookups, which would fail if
+ * readAndSkipLayerClassMeta did not add placeholders to readTypeInfos.
+ */
+ @Test(dataProvider = "enableCodegen")
+ public void testMetaContextIndexSyncWithNestedChildMaps(boolean
enableCodegen) {
+ Fory fory =
+ builder()
+ .withCodegen(enableCodegen)
+ .withAsyncCompilation(false)
+ .withRefTracking(false)
+ .withCompatibleMode(CompatibleMode.COMPATIBLE)
+ .build();
+
+ ChildHashMap1 map1a = new ChildHashMap1();
+ map1a.put("k1", "v1");
+
+ ChildHashMap1 map1b = new ChildHashMap1();
+ map1b.put("k2", "v2");
+
+ ChildHashMap2 map2a = new ChildHashMap2();
+ map2a.put("a", map1a);
+
+ ChildHashMap2 map2b = new ChildHashMap2();
+ map2b.put("b", map1b);
+
+ ChildHashMap3 map3a = new ChildHashMap3();
+ map3a.put("x", map2a);
+
+ ChildHashMap3 map3b = new ChildHashMap3();
+ map3b.put("y", map2b);
+
+ ChildHashMap4 map4 = new ChildHashMap4();
+ map4.put("group1", map3a);
+ map4.put("group2", map3b);
+
+ ChildMapHolder holder = new ChildMapHolder("meta-sync-test", map4);
+ ChildMapHolder deserialized = serDe(fory, holder);
+ Assert.assertEquals(deserialized, holder);
+ }
+
+ /* Deeply nested HashMap subclass hierarchy for testing generic propagation
*/
+
+ public static class ChildHashMap1 extends HashMap<String, String> {}
+
+ public static class ChildHashMap2 extends HashMap<String, ChildHashMap1> {}
+
+ public static class ChildHashMap3 extends HashMap<String, ChildHashMap2> {}
+
+ public static class ChildHashMap4 extends HashMap<String, ChildHashMap3> {}
+
+ @Data
+ @NoArgsConstructor
+ @AllArgsConstructor
+ public static class ChildMapHolder {
+ private String id;
+ private ChildHashMap4 nestedMaps;
+ }
+
+ @Test(dataProvider = "enableCodegen")
+ public void testNestedHashMapSubclassSerialization(boolean enableCodegen) {
+ Fory fory =
+ Fory.builder()
+ .withCodegen(enableCodegen)
+ .withRefTracking(false)
+ .requireClassRegistration(false)
+ .withLanguage(Language.JAVA)
+ .build();
+
+ ChildHashMap1 map1a = new ChildHashMap1();
+ map1a.put("k1", "v1");
+ map1a.put("k2", "v2");
+
+ ChildHashMap1 map1b = new ChildHashMap1();
+ map1b.put("k3", "v3");
+ map1b.put("k4", "v4");
+
+ ChildHashMap2 map2a = new ChildHashMap2();
+ map2a.put("a", map1a);
+ map2a.put("b", map1b);
+
+ ChildHashMap2 map2b = new ChildHashMap2();
+ map2b.put("c", map1b);
+
+ ChildHashMap3 map3a = new ChildHashMap3();
+ map3a.put("x", map2a);
+ map3a.put("y", map2b);
+
+ ChildHashMap3 map3b = new ChildHashMap3();
+ map3b.put("z", map2a);
+
+ ChildHashMap4 map4 = new ChildHashMap4();
+ map4.put("group1", map3a);
+ map4.put("group2", map3b);
+
+ ChildMapHolder holder = new ChildMapHolder("doc-123", map4);
+ ChildMapHolder deserialized = serDe(fory, holder);
+ Assert.assertEquals(deserialized, holder);
+ }
+
+ @Test
+ public void testNestedHashMapSubclassWithCompatibleMode() {
+ Fory fory =
+ Fory.builder()
+ .withCodegen(false)
+ .withAsyncCompilation(false)
+ .withRefTracking(false)
+ .requireClassRegistration(false)
+ .withCompatibleMode(CompatibleMode.COMPATIBLE)
+ .withLanguage(Language.JAVA)
+ .build();
+
+ ChildHashMap1 map1a = new ChildHashMap1();
+ map1a.put("k1", "v1");
+ map1a.put("k2", "v2");
+
+ ChildHashMap1 map1b = new ChildHashMap1();
+ map1b.put("k3", "v3");
+ map1b.put("k4", "v4");
+
+ ChildHashMap2 map2a = new ChildHashMap2();
+ map2a.put("a", map1a);
+ map2a.put("b", map1b);
+
+ ChildHashMap2 map2b = new ChildHashMap2();
+ map2b.put("c", map1a);
+
+ ChildHashMap3 map3a = new ChildHashMap3();
+ map3a.put("x", map2a);
+ map3a.put("y", map2b);
+
+ ChildHashMap3 map3b = new ChildHashMap3();
+ map3b.put("z", map2a);
+
+ ChildHashMap4 map4 = new ChildHashMap4();
+ map4.put("group1", map3a);
+ map4.put("group2", map3b);
+
+ ChildMapHolder holder = new ChildMapHolder("config-456", map4);
+ ChildMapHolder deserialized = serDe(fory, holder);
+ Assert.assertEquals(deserialized, holder);
+ }
+
+ /* Mixed collection subclass test (TreeSet + HashMap subclasses) */
+
+ public static class ChildTreeSet extends TreeSet<ChildTreeSetEntry> {
+ public ChildTreeSet() {
+ super();
+ }
+
+ public static ChildTreeSet empty() {
+ return new ChildTreeSet();
+ }
+
+ public static Collector<ChildTreeSetEntry, ?, ChildTreeSet> collector() {
+ return Collectors.collectingAndThen(
+ Collectors.toCollection(TreeSet::new),
+ set -> {
+ ChildTreeSet docs = new ChildTreeSet();
+ docs.addAll(set);
+ return docs;
+ });
+ }
+
+ public static ChildTreeSet of(Collection<ChildTreeSetEntry> multiple) {
+ return multiple.stream().collect(collector());
+ }
+ }
+
+ @Data
+ @NoArgsConstructor
+ @AllArgsConstructor
+ @EqualsAndHashCode
+ public static class ChildTreeSetEntry implements
Comparable<ChildTreeSetEntry> {
+ private String id;
+ private String name;
+
+ @Override
+ public int compareTo(ChildTreeSetEntry o) {
+ return this.id.compareTo(o.id);
+ }
+ }
+
+ @Data
+ @NoArgsConstructor
+ @AllArgsConstructor
+ public static class ChildMixedContainer {
+ private String id;
+ private ChildHashMap4 nestedMaps;
+ private ChildTreeSet entries;
+ private Map<String, ChildTreeSet> entriesByCategory;
+ }
+
+ @Test
+ public void testMixedCollectionSubclassesWithCompatibleMode() {
+ Fory fory =
+ Fory.builder()
+ .withCodegen(false)
+ .withAsyncCompilation(false)
+ .withRefTracking(false)
+ .requireClassRegistration(false)
+ .withCompatibleMode(CompatibleMode.COMPATIBLE)
+ .withLanguage(Language.JAVA)
+ .build();
+
+ ChildHashMap1 map1 = new ChildHashMap1();
+ map1.put("k1", "v1");
+ map1.put("k2", "v2");
+
+ ChildHashMap2 map2 = new ChildHashMap2();
+ map2.put("a", map1);
+
+ ChildHashMap3 map3 = new ChildHashMap3();
+ map3.put("x", map2);
+
+ ChildHashMap4 map4 = new ChildHashMap4();
+ map4.put("group1", map3);
+
+ ChildTreeSet set1 = ChildTreeSet.empty();
+ set1.add(new ChildTreeSetEntry("1", "entry1"));
+ set1.add(new ChildTreeSetEntry("2", "entry2"));
+
+ ChildTreeSet set2 = ChildTreeSet.empty();
+ set2.add(new ChildTreeSetEntry("3", "entry3"));
+
+ Map<String, ChildTreeSet> setsByKey = new HashMap<>();
+ setsByKey.put("category1", set1);
+ setsByKey.put("category2", set2);
+
+ ChildMixedContainer container = new ChildMixedContainer("mixed-789", map4,
set1, setsByKey);
+ ChildMixedContainer deserialized = serDe(fory, container);
+ Assert.assertEquals(deserialized, container);
+ }
+
public static class ChildLinkedListElemList extends
LinkedList<ChildLinkedListElemList> {}
public static class ChildLinkedListElemListStruct {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
