Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#774 Validate read types against field type instead of valueClass #794

Merged
merged 1 commit into from
Nov 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ public T read (Kryo kryo, Input input, Class<? extends T> 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);
Expand Down
1 change: 1 addition & 0 deletions src/com/esotericsoftware/kryo/util/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -230,23 +232,27 @@ public void testChangeFieldTypeWithChunkedEncodingDisabled () {
}

private void testChangeFieldType(int length, boolean chunked) {
CompatibleFieldSerializer<AnotherClass> serializer = new CompatibleFieldSerializer<>(kryo, AnotherClass.class);
CompatibleFieldSerializer<ClassWithStringField> 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<AnotherClass> 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) {
Expand All @@ -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<ClassWithWrapperAndPrimitive> 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
Expand Down Expand Up @@ -456,33 +465,22 @@ public void testClassWithSuperTypeFields() {
// https://github.com/EsotericSoftware/kryo/issues/774
@Test
public void testClassWithObjectField() {
CompatibleFieldSerializer<ClassWithSuperTypeFields> serializer = new CompatibleFieldSerializer<>(kryo,
ClassWithObjectField.class);
CompatibleFieldSerializer<ClassWithObjectField> 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 {
Expand Down Expand Up @@ -621,28 +619,39 @@ 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
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;
}
}

Expand Down Expand Up @@ -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);
}
}
}
2 changes: 2 additions & 0 deletions test/com/esotericsoftware/kryo/util/UtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down