diff --git a/src/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializer.java b/src/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializer.java index c3708c36c..17b752370 100644 --- a/src/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializer.java +++ b/src/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializer.java @@ -29,9 +29,13 @@ import com.esotericsoftware.kryo.io.InputChunked; import com.esotericsoftware.kryo.io.Output; import com.esotericsoftware.kryo.io.OutputChunked; +import com.esotericsoftware.kryo.util.Generics.GenericType; import com.esotericsoftware.kryo.util.ObjectMap; import com.esotericsoftware.kryo.util.Util; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; + /** Serializes objects using direct field assignment, providing both forward and backward compatibility. This means fields can be * added or removed without invalidating previously serialized bytes. Renaming or changing the type of a field is not supported. * Like {@link FieldSerializer}, it can serialize most classes without needing annotations. @@ -48,6 +52,8 @@ public class CompatibleFieldSerializer extends FieldSerializer { private static final int binarySearchThreshold = 32; private final CompatibleFieldSerializerConfig config; + // Key used to store per-field generic type arguments in the graph context alongside the CachedField[]. + private final Object typeArgsKey = new Object(); public CompatibleFieldSerializer (Kryo kryo, Class type) { this(kryo, type, new CompatibleFieldSerializerConfig()); @@ -62,6 +68,7 @@ public void write (Kryo kryo, Output output, T object) { int pop = pushTypeVariables(); CachedField[] fields = cachedFields.fields; + boolean chunked = config.chunked, readUnknownTagData = config.readUnknownFieldData; ObjectMap context = kryo.getGraphContext(); if (!context.containsKey(this)) { if (TRACE) trace("kryo", "Write fields for class: " + type.getName()); @@ -70,10 +77,11 @@ public void write (Kryo kryo, Output output, T object) { for (int i = 0, n = fields.length; i < n; i++) { if (TRACE) trace("kryo", "Write field name: " + fields[i].name + pos(output.position())); output.writeString(fields[i].name); + // When readUnknownTagData is enabled, also write the field's generic type arguments so that + // removed fields with parameterized types (e.g. List) can be read back correctly during skip. + if (readUnknownTagData) writeFieldTypeArgs(kryo, output, fields[i]); } } - - boolean chunked = config.chunked, readUnknownTagData = config.readUnknownFieldData; Output fieldOutput; OutputChunked outputChunked = null; if (chunked) @@ -121,6 +129,7 @@ public T read (Kryo kryo, Input input, Class type) { if (fields == null) fields = readFields(kryo, input); boolean chunked = config.chunked, readUnknownTagData = config.readUnknownFieldData; + GenericType[][] schemaTypeArgs = readUnknownTagData ? (GenericType[][])kryo.getGraphContext().get(typeArgsKey) : null; Input fieldInput; InputChunked inputChunked = null; if (chunked) @@ -149,6 +158,11 @@ public T read (Kryo kryo, Input input, Class type) { if (cachedField == null) { // Read unknown data in case it is a reference. if (TRACE) trace("kryo", "Read unknown data, type: " + className(valueClass) + pos(input.position())); + // Reconstruct the generics context from the schema so that serializers like + // CollectionSerializer use the same compact format that was chosen during write. + GenericType fieldGenericType = schemaTypeArgs != null && schemaTypeArgs[i] != null + ? new GenericType(valueClass, schemaTypeArgs[i]) : null; + if (fieldGenericType != null) kryo.getGenerics().pushGenericType(fieldGenericType); try { kryo.readObject(fieldInput, valueClass); } catch (KryoException ex) { @@ -156,6 +170,8 @@ public T read (Kryo kryo, Input input, Class type) { + "#" + cachedField + ")"; if (!chunked) throw new KryoException(message, ex); if (DEBUG) debug("kryo", message, ex); + } finally { + if (fieldGenericType != null) kryo.getGenerics().popGenericType(); } if (chunked) inputChunked.nextChunk(); continue; @@ -193,11 +209,14 @@ public T read (Kryo kryo, Input input, Class type) { private CachedField[] readFields (Kryo kryo, Input input) { if (TRACE) trace("kryo", "Read fields for class: " + type.getName()); + boolean readUnknownTagData = config.readUnknownFieldData; int length = input.validateArrayLength(input.readVarInt(true)); String[] names = new String[length]; + GenericType[][] schemaTypeArgs = readUnknownTagData ? new GenericType[length][] : null; for (int i = 0; i < length; i++) { names[i] = input.readString(); if (TRACE) trace("kryo", "Read field name: " + names[i]); + if (readUnknownTagData) schemaTypeArgs[i] = readFieldTypeArgs(kryo, input); } CachedField[] fields = new CachedField[length]; @@ -239,9 +258,39 @@ else if (compare > 0) } kryo.getGraphContext().put(this, fields); + if (schemaTypeArgs != null) kryo.getGraphContext().put(typeArgsKey, schemaTypeArgs); return fields; } + /** Writes the generic type arguments of a field's declared type to the schema. Stored alongside the field name so that + * removed parameterized fields (e.g. {@code List}) can be correctly read and discarded during deserialization. */ + private void writeFieldTypeArgs (Kryo kryo, Output output, CachedField cachedField) { + Type genericType = cachedField.field.getGenericType(); + if (genericType instanceof ParameterizedType) { + Type[] typeArgs = ((ParameterizedType)genericType).getActualTypeArguments(); + output.writeVarInt(typeArgs.length, true); + for (Type typeArg : typeArgs) + kryo.writeClass(output, typeArg instanceof Class ? (Class)typeArg : null); + } else { + output.writeVarInt(0, true); + } + } + + /** Reads generic type arguments from the schema (written by {@link #writeFieldTypeArgs}). Returns null when there are none or + * when any argument could not be resolved (e.g. a wildcard or type variable written as null), so the caller does not push an + * incomplete GenericType that would cause a NullPointerException inside {@link DefaultGenerics#nextGenericClass()}. */ + private GenericType[] readFieldTypeArgs (Kryo kryo, Input input) { + int numArgs = input.readVarInt(true); + if (numArgs == 0) return null; + GenericType[] args = new GenericType[numArgs]; + for (int i = 0; i < numArgs; i++) { + Registration reg = kryo.readClass(input); + if (reg == null) return null; // Wildcard or type variable — omit GenericType push entirely. + args[i] = new GenericType(reg.getType(), null); + } + return args; + } + public CompatibleFieldSerializerConfig getCompatibleFieldSerializerConfig () { return config; } diff --git a/src/com/esotericsoftware/kryo/util/Generics.java b/src/com/esotericsoftware/kryo/util/Generics.java index 31d396bf5..2f4a2fa32 100644 --- a/src/com/esotericsoftware/kryo/util/Generics.java +++ b/src/com/esotericsoftware/kryo/util/Generics.java @@ -183,6 +183,13 @@ public GenericType (Class fromClass, Class toClass, Type context) { initialize(fromClass, toClass, context); } + /** Creates a GenericType with an explicit type and type-argument list. Used to reconstruct generics context from stored + * schema data when reading removed fields. */ + public GenericType (Type type, GenericType[] arguments) { + this.type = type; + this.arguments = arguments; + } + private void initialize (Class fromClass, Class toClass, Type context) { if (context instanceof ParameterizedType) { // Type with a type parameter, eg ArrayList. diff --git a/test/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializerTest.java b/test/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializerTest.java index f99f05019..888c7788d 100644 --- a/test/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializerTest.java +++ b/test/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializerTest.java @@ -44,10 +44,10 @@ class CompatibleFieldSerializerTest extends KryoTestCase { @Test void testCompatibleFieldSerializer () { - testCompatibleFieldSerializer(83, false, false); - testCompatibleFieldSerializer(116, false, true); - testCompatibleFieldSerializer(80, true, false); - testCompatibleFieldSerializer(113, true, true); + testCompatibleFieldSerializer(90, false, false); + testCompatibleFieldSerializer(123, false, true); + testCompatibleFieldSerializer(87, true, false); + testCompatibleFieldSerializer(120, true, true); } private void testCompatibleFieldSerializer (int length, boolean references, final boolean chunked) { @@ -72,10 +72,10 @@ public CompatibleFieldSerializer newSerializer (Kryo kryo, Class type) { @Test void testAddedField () { - testAddedField(59, false, false); - testAddedField(87, false, true); - testAddedField(63, true, false); - testAddedField(91, true, true); + testAddedField(65, false, false); + testAddedField(93, false, true); + testAddedField(69, true, false); + testAddedField(97, true, true); } private void testAddedField (int length, boolean references, boolean chunked) { @@ -104,16 +104,16 @@ private void testAddedField (int length, boolean references, boolean chunked) { @Test void testAddedFieldToClassWithManyFields () { - testAddedFieldToClassWithManyFields(189, false, false, true); + testAddedFieldToClassWithManyFields(226, false, false, true); testAddedFieldToClassWithManyFields(152, false, false, false); - testAddedFieldToClassWithManyFields(263, false, true, true); + testAddedFieldToClassWithManyFields(300, false, true, true); testAddedFieldToClassWithManyFields(226, false, true, false); - testAddedFieldToClassWithManyFields(227, true, false, true); + testAddedFieldToClassWithManyFields(264, true, false, true); testAddedFieldToClassWithManyFields(190, true, false, false); - testAddedFieldToClassWithManyFields(301, true, true, true); + testAddedFieldToClassWithManyFields(338, true, true, true); testAddedFieldToClassWithManyFields(264, true, true, false); } @@ -177,10 +177,10 @@ private void testAddedFieldToClassWithManyFields (int length, boolean references @Test void testRemovedField () { - testRemovedField(92, false, false); - testRemovedField(125, false, true); - testRemovedField(87, true, false); - testRemovedField(120, true, true); + testRemovedField(99, false, false); + testRemovedField(132, false, true); + testRemovedField(94, true, false); + testRemovedField(127, true, true); } private void testRemovedField (int length, boolean references, boolean chunked) { @@ -215,12 +215,12 @@ private void testRemovedField (int length, boolean references, boolean chunked) @Test void testChangeFieldTypeWithChunkedEncodingEnabled () { - testChangeFieldType(16, true); + testChangeFieldType(17, true); } @Test void testChangeFieldTypeWithChunkedEncodingDisabled () { - assertThrows(KryoException.class, () -> testChangeFieldType(14, false), + assertThrows(KryoException.class, () -> testChangeFieldType(15, false), "Read type is incompatible with the field type: String -> Long"); } @@ -244,8 +244,8 @@ private void testChangeFieldType(int length, boolean chunked) { @Test void testChangePrimitiveAndWrapperFieldTypes () { - testChangePrimitiveAndWrapperFieldTypes(22, true); - testChangePrimitiveAndWrapperFieldTypes(18, false); + testChangePrimitiveAndWrapperFieldTypes(24, true); + testChangePrimitiveAndWrapperFieldTypes(20, false); } private void testChangePrimitiveAndWrapperFieldTypes (int length, boolean chunked) { @@ -269,16 +269,16 @@ private void testChangePrimitiveAndWrapperFieldTypes (int length, boolean chunke @Test void testRemovedFieldFromClassWithManyFields () { - testRemovedFieldFromClassWithManyFields(198, false, false, true); + testRemovedFieldFromClassWithManyFields(236, false, false, true); // testRemovedFieldFromClassWithManyFields(0, false, false, false); // Doesn't support remove. - testRemovedFieldFromClassWithManyFields(274, false, true, true); + testRemovedFieldFromClassWithManyFields(312, false, true, true); testRemovedFieldFromClassWithManyFields(236, false, true, false); - testRemovedFieldFromClassWithManyFields(237, true, false, true); + testRemovedFieldFromClassWithManyFields(275, true, false, true); // testRemovedFieldFromClassWithManyFields(0, true, false, false); // Doesn't support remove. - testRemovedFieldFromClassWithManyFields(313, true, true, true); + testRemovedFieldFromClassWithManyFields(351, true, true, true); testRemovedFieldFromClassWithManyFields(275, true, true, false); } @@ -346,16 +346,16 @@ private void testRemovedFieldFromClassWithManyFields (int length, boolean refere @Test void testRemovedMultipleFieldsFromClassWithManyFields () { - testRemovedMultipleFieldsFromClassWithManyFields(170, false, false, true); + testRemovedMultipleFieldsFromClassWithManyFields(208, false, false, true); // testRemovedMultipleFieldsFromClassWithManyFields(0, false, false, false); // Doesn't support remove. - testRemovedMultipleFieldsFromClassWithManyFields(246, false, true, true); + testRemovedMultipleFieldsFromClassWithManyFields(284, false, true, true); testRemovedMultipleFieldsFromClassWithManyFields(220, false, true, false); - testRemovedMultipleFieldsFromClassWithManyFields(197, true, false, true); + testRemovedMultipleFieldsFromClassWithManyFields(235, true, false, true); // testRemovedMultipleFieldsFromClassWithManyFields(0, true, false, false); // Doesn't support remove. - testRemovedMultipleFieldsFromClassWithManyFields(273, true, true, true); + testRemovedMultipleFieldsFromClassWithManyFields(311, true, true, true); testRemovedMultipleFieldsFromClassWithManyFields(247, true, true, false); } @@ -421,7 +421,7 @@ void testRemoveAllFieldsFromClassWithManyFields () { kryo.register(ClassWithManyFields.class, serializer); ClassWithManyFields object1 = new ClassWithManyFields(); - roundTrip(118, object1); + roundTrip(156, object1); for (FieldSerializer.CachedField field : serializer.getFields()) { serializer.removeField(field.getName()); @@ -433,10 +433,10 @@ void testRemoveAllFieldsFromClassWithManyFields () { @Test void testExtendedClass () { - testExtendedClass(270, false, false); - testExtendedClass(294, false, true); - testExtendedClass(273, true, false); - testExtendedClass(297, true, true); + testExtendedClass(282, false, false); + testExtendedClass(306, false, true); + testExtendedClass(285, true, false); + testExtendedClass(309, true, true); } private void testExtendedClass (int length, boolean references, boolean chunked) { @@ -468,7 +468,7 @@ void testClassWithSuperTypeFields() { config.setReadUnknownFieldData(true); kryo.register(ClassWithSuperTypeFields.class, serializer); - roundTrip(71, new ClassWithSuperTypeFields("foo", Arrays.asList("bar"), "baz")); + roundTrip(75, new ClassWithSuperTypeFields("foo", Arrays.asList("bar"), "baz")); } // https://github.com/EsotericSoftware/kryo/issues/774 @@ -500,7 +500,85 @@ void testClassWithLambdaField () { kryo.register(ClassWithLambdaField.class); kryo.register(ClosureSerializer.Closure.class, new ClosureSerializer()); - roundTrip(236, new ClassWithLambdaField()); + roundTrip(237, new ClassWithLambdaField()); + } + + // https://github.com/EsotericSoftware/kryo/issues/1098 + @Test + void testRemovedGenericCollectionField () { + // Removing a List field caused a deserialization exception because CollectionSerializer uses a compact + // binary format (no per-element class) when the element type is known via generics. Without the fix, the writer + // pushed List's generic type so String was the element type, but the reader skipped unknown fields + // without pushing any generic type, causing CollectionSerializer to use the self-describing format and + // misread the compact bytes. + CompatibleFieldSerializer serializer = new CompatibleFieldSerializer(kryo, ClassWithListField.class); + kryo.register(java.util.ArrayList.class); + kryo.register(ClassWithListField.class, serializer); + + List list = new java.util.ArrayList<>(Arrays.asList("foo", "bar")); + ClassWithListField object1 = new ClassWithListField("hello", list); + Output out = new Output(4096, Integer.MAX_VALUE); + kryo.writeClassAndObject(out, object1); + byte[] bytes = out.toBytes(); + + // Verify basic roundtrip with the full class. + ClassWithListField read1 = (ClassWithListField)kryo.readClassAndObject(new Input(bytes)); + assertEquals(object1, read1); + + // Re-register the serializer with the list field removed, then read the previously-written bytes. + serializer.removeField("list"); + ClassWithListField read2 = (ClassWithListField)kryo.readClassAndObject(new Input(bytes)); + assertEquals("hello", read2.str); + assertNull(read2.list); + } + + // https://github.com/EsotericSoftware/kryo/issues/907 + @Test + void testRemovedGenericMapField () { + kryo.register(java.util.LinkedHashMap.class); + CompatibleFieldSerializer serializer = new CompatibleFieldSerializer(kryo, ClassWithMapField.class); + kryo.register(ClassWithMapField.class, serializer); + + ClassWithMapField object1 = new ClassWithMapField(); + object1.value = "some string"; + object1.map = new java.util.LinkedHashMap<>(); + object1.map.put(1111, "some string"); + object1.map.put(2222, "some string"); + + Output out = new Output(4096, Integer.MAX_VALUE); + kryo.writeClassAndObject(out, object1); + byte[] bytes = out.toBytes(); + + ClassWithMapField read1 = (ClassWithMapField)kryo.readClassAndObject(new Input(bytes)); + assertEquals(object1, read1); + + serializer.removeField("map"); + ClassWithMapField read2 = (ClassWithMapField)kryo.readClassAndObject(new Input(bytes)); + assertEquals("some string", read2.value); + assertNull(read2.map); + } + + // https://github.com/EsotericSoftware/kryo/issues/834 (root cause) + @Test + void testRemovedGenericCollectionFieldWithChunkedEncoding () { + kryo.setReferences(true); + kryo.register(java.util.ArrayList.class); + CompatibleFieldSerializer serializer = new CompatibleFieldSerializer(kryo, ClassWithListField.class); + serializer.getCompatibleFieldSerializerConfig().setChunkedEncoding(true); + kryo.register(ClassWithListField.class, serializer); + + ClassWithListField object1 = new ClassWithListField("hello", new java.util.ArrayList<>(Arrays.asList("foo", "bar"))); + Output out = new Output(4096, Integer.MAX_VALUE); + kryo.writeClassAndObject(out, object1); + byte[] bytes = out.toBytes(); + + ClassWithListField read1 = (ClassWithListField)kryo.readClassAndObject(new Input(bytes)); + assertEquals(object1, read1); + + serializer.removeField("list"); + ClassWithListField read2 = (ClassWithListField)kryo.readClassAndObject(new Input(bytes)); + assertEquals("hello", read2.str); + assertNull(read2.list); } // https://github.com/EsotericSoftware/kryo/issues/840 @@ -510,7 +588,7 @@ void testClassWithGenericField () { kryo.setDefaultSerializer(new CompatibleFieldSerializerFactory(config)); kryo.register(ClassWithGenericField.class); - roundTrip(9, new ClassWithGenericField<>(1)); + roundTrip(10, new ClassWithGenericField<>(1)); } public static class TestClass { @@ -809,4 +887,36 @@ public boolean equals (Object o) { return Objects.equals(value, that.value); } } + + public static class ClassWithMapField { + public String value; + public java.util.Map map; + + public boolean equals (Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ClassWithMapField that = (ClassWithMapField)o; + return Objects.equals(value, that.value) && Objects.equals(map, that.map); + } + } + + public static class ClassWithListField { + public String str; + public List list; + + public ClassWithListField () { + } + + public ClassWithListField (String str, List list) { + this.str = str; + this.list = list; + } + + public boolean equals (Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ClassWithListField that = (ClassWithListField)o; + return Objects.equals(str, that.str) && Objects.equals(list, that.list); + } + } }