Fix CompatibleFieldSerializer failing to skip removed generic collection/map fields#1287
Open
theigl wants to merge 4 commits into
Open
Fix CompatibleFieldSerializer failing to skip removed generic collection/map fields#1287theigl wants to merge 4 commits into
theigl wants to merge 4 commits into
Conversation
…ion fields (#1098) When readUnknownFieldData=true (the default), the writer pushes a field's declared generic type (e.g. List<String>) before calling CollectionSerializer, which selects a compact binary format with no per-element class info. The reader's unknown-field skip path did not push any generic type before calling kryo.readObject(), so CollectionSerializer chose the self-describing format and misread the compact bytes, causing a KryoException. Fix: store each field's generic type arguments alongside its name in the serialized schema. When skipping an unknown field during read, reconstruct and push a GenericType from the stored arguments so CollectionSerializer uses the same compact format that was written. Note: this changes the schema format for the readUnknownFieldData=true path and is not wire-compatible with data written by older versions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
#907: removing a Map<K,V> field caused the same corrupt-read as #1098 — the two-argument GenericType is now stored in the schema and reconstructed on the read side, so MapSerializer uses the same compact key/value encoding on both paths. #834: the chunked+references variant of the generic-collection-removal bug (the root cause theigl noted as "another issue that I cannot fix at this point") is now covered. The InputChunked.nextChunk fix already recovered from the stream corruption; this test verifies the root cause is gone. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
readFieldTypeArgs now returns null early if any type argument was stored as null (i.e. written from a wildcard or type variable). Without this guard, the caller would push a GenericType with a null slot in its arguments array, causing DefaultGenerics.nextGenericClass() to NPE on arguments[0].resolve(). Returning null suppresses the GenericType push entirely, so the skipping reader falls back to the self-describing format — which matches the writer, because for unresolvable type params CollectionSerializer already chose self-describing format at write time. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…d-serializer-generic-collection-removal # Conflicts: # src/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializer.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1098, also fixes #907 (same root cause).
Root cause
When
readUnknownFieldData=true(the default),CompatibleFieldSerializer.write()callsReflectField.write()which pushes the field's declared generic type (e.g.List<String>) before invoking the serializer.CollectionSerializerandMapSerializerdetect the pushed type vianextGenericClass()/nextGenericTypes()and choose a compact binary format with no per-element class information.On the read side, when the field has been removed from the class, the unknown-field skip path called
kryo.readObject()without pushing anyGenericType. The serializer therefore chose the self-describing format and tried to read class-per-element headers that were never written, corrupting the stream and throwing aKryoException.Fix
When
readUnknownFieldData=true, the serializer now writes each field's generic type arguments alongside its name in the schema section (anumArgsVarInt followed by class registrations for each resolved type argument). On the read side,readFields()loads these stored arguments and the unknown-field skip path reconstructs aGenericTypefrom them before callingreadObject(), ensuringCollectionSerializer/MapSerializeruse the same compact format that was written.A null-arg guard in
readFieldTypeArgs()handles wildcard/type-variable arguments (e.g.List<?>) by suppressing theGenericTypepush entirely — for those fields both writer and reader already use self-describing format, so no push is needed and none is safe (it would cause a NPE insideDefaultGenerics.nextGenericClass()).Trade-off
This is a wire-format breaking change for the
readUnknownFieldData=truepath. Data written by older Kryo versions cannot be read by this version and vice versa. The existing behaviour forchunked=true(catch exception +nextChunk()) already handled stream recovery and remains the recommended approach for users who need to continue reading pre-existing serialized data.Test plan
testRemovedGenericCollectionField— regression for When entity add a field extends Collection, CompatibleFieldSerializer lost compatibility. #1098:List<String>field removed, non-chunked,readUnknownFieldData=truetestRemovedGenericMapField— regression for Deserialization fails with "Unable to read unknown data" after removing a field with Collection type from the class (Issue with backward compatibility) #907:Map<Integer,String>field removedtestRemovedGenericCollectionFieldWithChunkedEncoding— regression for Buffer underflow while deserializing with Collection type field removed (with reproducible test case) #834 root cause:List<String>field removed,chunked=true,references=true🤖 Generated with Claude Code