From b966d1cb428a5e36a88973e61dd96bacb4ccc879 Mon Sep 17 00:00:00 2001 From: Thomas Heigl Date: Tue, 24 Nov 2020 10:31:25 +0100 Subject: [PATCH] #774 Validate read types against `field.getType()` instead of `valueClass` --- .../CompatibleFieldSerializer.java | 2 +- src/com/esotericsoftware/kryo/util/Util.java | 1 + .../CompatibleFieldSerializerTest.java | 145 +++++++++++++----- .../esotericsoftware/kryo/util/UtilTest.java | 2 + 4 files changed, 109 insertions(+), 41 deletions(-) diff --git a/src/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializer.java b/src/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializer.java index 2e9a4f0e2..6d9c4c81a 100644 --- a/src/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializer.java +++ b/src/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializer.java @@ -164,7 +164,7 @@ public T read (Kryo kryo, Input input, Class type) { } // Ensure the type in the data is compatible with the field type. - if (cachedField.valueClass != null && !Util.isAssignableTo(valueClass, cachedField.valueClass)) { + if (cachedField.valueClass != null && !Util.isAssignableTo(valueClass, cachedField.field.getType())) { String message = "Read type is incompatible with the field type: " + className(valueClass) + " -> " + className(cachedField.valueClass) + " (" + getType().getName() + "#" + cachedField + ")"; if (!chunked) throw new KryoException(message); diff --git a/src/com/esotericsoftware/kryo/util/Util.java b/src/com/esotericsoftware/kryo/util/Util.java index 37e95cf23..1848b7c28 100644 --- a/src/com/esotericsoftware/kryo/util/Util.java +++ b/src/com/esotericsoftware/kryo/util/Util.java @@ -230,6 +230,7 @@ public static Class getElementClass (Class arrayClass) { } public static boolean isAssignableTo (Class from, Class to) { + if (to == Object.class) return true; if (to.isAssignableFrom(from)) return true; if (from.isPrimitive()) return isPrimitiveWrapperOf(to, from); if (to.isPrimitive()) return isPrimitiveWrapperOf(from, to); diff --git a/test/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializerTest.java b/test/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializerTest.java index a0a5a59ea..8796ed822 100644 --- a/test/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializerTest.java +++ b/test/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializerTest.java @@ -25,6 +25,8 @@ import com.esotericsoftware.kryo.KryoException; import com.esotericsoftware.kryo.KryoTestCase; import com.esotericsoftware.kryo.SerializerFactory.CompatibleFieldSerializerFactory; +import com.esotericsoftware.kryo.io.Input; +import com.esotericsoftware.kryo.io.Output; import java.io.Serializable; import java.util.Arrays; @@ -230,23 +232,27 @@ public void testChangeFieldTypeWithChunkedEncodingDisabled () { } private void testChangeFieldType(int length, boolean chunked) { - CompatibleFieldSerializer serializer = new CompatibleFieldSerializer<>(kryo, AnotherClass.class); + CompatibleFieldSerializer serializer = new CompatibleFieldSerializer<>(kryo, ClassWithStringField.class); serializer.getCompatibleFieldSerializerConfig().setChunkedEncoding(chunked); kryo.setReferences(false); - kryo.register(AnotherClass.class, serializer); + kryo.register(ClassWithStringField.class, serializer); - roundTrip(length, new AnotherClass("Hacker")); + roundTrip(length, new ClassWithStringField("Hacker")); - serializer.getField("value").setValueClass(Long.class); + final Kryo otherKryo = new Kryo(); + CompatibleFieldSerializer otherSerializer = new CompatibleFieldSerializer<>(kryo, ClassWithLongField.class); + otherSerializer.getCompatibleFieldSerializerConfig().setChunkedEncoding(chunked); + otherKryo.setReferences(false); + otherKryo.register(ClassWithLongField.class, otherSerializer); - final AnotherClass o = (AnotherClass)kryo.readClassAndObject(input); + final ClassWithLongField o = (ClassWithLongField) otherKryo.readClassAndObject(input); assertNull(o.value); } @Test public void testChangePrimitiveAndWrapperFieldTypes () { - testChangePrimitiveAndWrapperFieldTypes(26, true); - testChangePrimitiveAndWrapperFieldTypes(22, false); + testChangePrimitiveAndWrapperFieldTypes(22, true); + testChangePrimitiveAndWrapperFieldTypes(18, false); } private void testChangePrimitiveAndWrapperFieldTypes (int length, boolean chunked) { @@ -257,12 +263,15 @@ private void testChangePrimitiveAndWrapperFieldTypes (int length, boolean chunke roundTrip(length, new ClassWithPrimitiveAndWrapper(1, 1L)); - serializer.getField("primitive").setValueClass(Long.class); - serializer.getField("wrapper").setValueClass(long.class); + final Kryo otherKryo = new Kryo(); + CompatibleFieldSerializer otherSerializer = new CompatibleFieldSerializer<>(kryo, ClassWithWrapperAndPrimitive.class); + otherSerializer.getCompatibleFieldSerializerConfig().setChunkedEncoding(chunked); + otherKryo.setReferences(false); + otherKryo.register(ClassWithWrapperAndPrimitive.class, otherSerializer); - ClassWithPrimitiveAndWrapper o = (ClassWithPrimitiveAndWrapper)kryo.readClassAndObject(input); - assertEquals(1, o.primitive); - assertEquals(1L, o.wrapper, 0); + ClassWithWrapperAndPrimitive o = (ClassWithWrapperAndPrimitive) otherKryo.readClassAndObject(input); + assertEquals(1L, o.value1, 0); + assertEquals(1, o.value2); } @Test @@ -456,33 +465,22 @@ public void testClassWithSuperTypeFields() { // https://github.com/EsotericSoftware/kryo/issues/774 @Test public void testClassWithObjectField() { - CompatibleFieldSerializer serializer = new CompatibleFieldSerializer<>(kryo, - ClassWithObjectField.class); + CompatibleFieldSerializer serializer = new CompatibleFieldSerializer<>(kryo, ClassWithObjectField.class); CompatibleFieldSerializer.CompatibleFieldSerializerConfig config = serializer.getCompatibleFieldSerializerConfig(); config.setChunkedEncoding(true); config.setReadUnknownFieldData(true); kryo.register(ClassWithObjectField.class, serializer); - roundTrip(12, new ClassWithObjectField(123)); - roundTrip(13, new ClassWithObjectField("foo")); - } - - public static class ClassWithObjectField { - Object value; + Output output1 = new Output(4096, Integer.MAX_VALUE); + final ClassWithObjectField o1 = new ClassWithObjectField(123); + kryo.writeClassAndObject(output1, o1); - public ClassWithObjectField() { } + Output output2 = new Output(4096, Integer.MAX_VALUE); + final ClassWithObjectField o2 = new ClassWithObjectField("foo"); + kryo.writeClassAndObject(output2, o2); - public ClassWithObjectField(Object value) { - this.value = value; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - ClassWithObjectField wrapper = (ClassWithObjectField) o; - return Objects.equals(value, wrapper.value); - } + assertEquals(o1, kryo.readClassAndObject(new Input(output1.getBuffer()))); + assertEquals(o2, kryo.readClassAndObject(new Input(output2.getBuffer()))); } public static class TestClass { @@ -621,15 +619,15 @@ public boolean equals (Object obj) { } public static class ClassWithPrimitiveAndWrapper { - long primitive; - Long wrapper; + long value1; + Long value2; public ClassWithPrimitiveAndWrapper () { } - public ClassWithPrimitiveAndWrapper (long primitive, Long wrapper) { - this.primitive = primitive; - this.wrapper = wrapper; + public ClassWithPrimitiveAndWrapper (long value1, Long value2) { + this.value1 = value1; + this.value2 = value2; } @Override @@ -637,12 +635,23 @@ public boolean equals (Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; final ClassWithPrimitiveAndWrapper that = (ClassWithPrimitiveAndWrapper)o; - return primitive == that.primitive && Objects.equals(wrapper, that.wrapper); + return value1 == that.value1 && Objects.equals(value2, that.value2); + } + } + + public static class ClassWithWrapperAndPrimitive { + Long value1; + long value2; + + public ClassWithWrapperAndPrimitive() { } @Override - public int hashCode () { - return Objects.hash(primitive, wrapper); + public boolean equals (Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + final ClassWithWrapperAndPrimitive that = (ClassWithWrapperAndPrimitive)o; + return value1.equals(that.value1) && value2 == that.value2; } } @@ -674,4 +683,60 @@ public int hashCode () { return Objects.hash(value, list, serializable); } } + + public static class ClassWithObjectField { + Object value; + + public ClassWithObjectField() { } + + public ClassWithObjectField(Object value) { + this.value = value; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ClassWithObjectField wrapper = (ClassWithObjectField) o; + return Objects.equals(value, wrapper.value); + } + } + + public static class ClassWithStringField { + String value; + + public ClassWithStringField() { + } + + public ClassWithStringField(String value) { + this.value = value; + } + + @Override + public boolean equals (Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + final ClassWithStringField that = (ClassWithStringField)o; + return Objects.equals(value, that.value); + } + } + + public static class ClassWithLongField { + Long value; + + public ClassWithLongField() { + } + + public ClassWithLongField(Long value) { + this.value = value; + } + + @Override + public boolean equals (Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + final ClassWithLongField that = (ClassWithLongField)o; + return Objects.equals(value, that.value); + } + } } diff --git a/test/com/esotericsoftware/kryo/util/UtilTest.java b/test/com/esotericsoftware/kryo/util/UtilTest.java index 7e9224850..b614d9508 100644 --- a/test/com/esotericsoftware/kryo/util/UtilTest.java +++ b/test/com/esotericsoftware/kryo/util/UtilTest.java @@ -28,6 +28,8 @@ public void testIsAssignableTo() { assertTrue(Util.isAssignableTo(long.class, Long.class)); assertTrue(Util.isAssignableTo(Long.class, Long.class)); assertTrue(Util.isAssignableTo(long.class, long.class)); + assertTrue(Util.isAssignableTo(Long.class, Object.class)); + assertTrue(Util.isAssignableTo(long.class, Object.class)); assertFalse(Util.isAssignableTo(String.class, Long.class)); assertFalse(Util.isAssignableTo(String.class, long.class));