mandrean commented on code in PR #3342:
URL: https://github.com/apache/fory/pull/3342#discussion_r2811892574


##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/MapLikeSerializer.java:
##########
@@ -853,43 +853,25 @@ private long readJavaChunkGeneric(
     } 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--;
-      }

Review Comment:
   I think the else branch was a premature read-side optimization that had no 
corresponding branch on the write side (`writeJavaChunkGeneric` always pushes 
generics unconditionally). This asymmetry caused the read path to skip generic 
type propagation for `HashMap` subclasses whose type parameters are inherited 
rather than declared directly (e.g. class `MyMap extends HashMap<String, 
String>`), leading to NPE when the reader encountered chunk flags 
(`KEY_DECL_TYPE`/`VALUE_DECL_TYPE`) that were written assuming generics were on 
the stack.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to