[Java] fix jdk compatible serialization for inheritance (#1030)

fix jdk compatible serialization for inheritance
apache · Oct 28, 2023 · 0d6d50b · 0d6d50b
1 parent 56b5a15
commit 0d6d50b
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 10 deletions.
diff --git a/java/fury-core/src/main/java/io/fury/serializer/ReplaceResolveSerializer.java b/java/fury-core/src/main/java/io/fury/serializer/ReplaceResolveSerializer.java
@@ -115,7 +115,7 @@ public void setObjectSerializer(Serializer objectSerializer) {
   }
 
   static JDKReplaceResolveMethodInfoCache newJDKMethodInfoCache(Class<?> cls, Fury fury) {
-    Method writeReplaceMethod, readResolveMethod, writeObjectMethod, readObjectMethod;
+    Method writeReplaceMethod, readResolveMethod;
     // In JDK17, set private jdk method accessible will fail by default, use ObjectStreamClass
     // instead, since it set accessible.
     if (Serializable.class.isAssignableFrom(cls)) {
@@ -124,18 +124,12 @@ static JDKReplaceResolveMethodInfoCache newJDKMethodInfoCache(Class<?> cls, Fury
           (Method) ReflectionUtils.getObjectFieldValue(objectStreamClass, "writeReplaceMethod");
       readResolveMethod =
           (Method) ReflectionUtils.getObjectFieldValue(objectStreamClass, "readResolveMethod");
-      writeObjectMethod =
-          (Method) ReflectionUtils.getObjectFieldValue(objectStreamClass, "writeObjectMethod");
-      readObjectMethod =
-          (Method) ReflectionUtils.getObjectFieldValue(objectStreamClass, "readObjectMethod");
     } else {
       // FIXME class with `writeReplace` method defined should be Serializable,
       //  but hessian ignores this check and many existing system are using hessian,
       //  so we just warn it to keep compatibility with most applications.
       writeReplaceMethod = JavaSerializer.getWriteReplaceMethod(cls);
       readResolveMethod = JavaSerializer.getReadResolveMethod(cls);
-      writeObjectMethod = JavaSerializer.getWriteObjectMethod(cls);
-      readObjectMethod = JavaSerializer.getReadObjectMethod(cls);
       if (writeReplaceMethod != null) {
         LOG.warn(
             "{} doesn't implement {}, but defined writeReplace method {}",
@@ -153,10 +147,9 @@ static JDKReplaceResolveMethodInfoCache newJDKMethodInfoCache(Class<?> cls, Fury
     }
     JDKReplaceResolveMethodInfoCache methodInfoCache =
         new JDKReplaceResolveMethodInfoCache(writeReplaceMethod, readResolveMethod, null);
-    boolean hasJDKWriteObjectMethod = writeObjectMethod != null;
-    boolean hasJDKReadObjectMethod = readObjectMethod != null;
     Class<? extends Serializer> serializerClass;
-    if (!hasJDKWriteObjectMethod && !hasJDKReadObjectMethod) {
+    if (JavaSerializer.getReadObjectMethod(cls, true) == null
+        && JavaSerializer.getWriteObjectMethod(cls, true) == null) {
       serializerClass =
           fury.getClassResolver()
               .getObjectSerializerClass(

diff --git a/java/fury-core/src/test/java/io/fury/serializer/ReplaceResolveSerializerTest.java b/java/fury-core/src/test/java/io/fury/serializer/ReplaceResolveSerializerTest.java
@@ -21,12 +21,15 @@
 import static org.testng.Assert.assertThrows;
 import static org.testng.Assert.assertTrue;
 
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import io.fury.Fury;
 import io.fury.FuryTestBase;
 import io.fury.config.Language;
 import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
 import java.io.Serializable;
 import java.util.List;
 import java.util.Map;
@@ -433,4 +436,52 @@ public void testImmutableMapResolve() {
     roundCheck(fury1, fury2, ImmutableMap.of(1, 2));
     roundCheck(fury1, fury2, new SimpleMapTest(ImmutableMap.of("k", 2), ImmutableMap.of(1, 2)));
   }
+
+  public static class InheritanceTestClass {
+    private byte f1;
+
+    public InheritanceTestClass(byte f1) {
+      this.f1 = f1;
+    }
+
+    public Object writeReplace() {
+      return new InheritanceTestClassProxy(f1);
+    }
+  }
+
+  public static class InheritanceTestClassProxyBase implements Serializable {
+    // Mark as transient to make object serializer unable to work, then only
+    // `writeObject/readObject` can be used for serialization.
+    transient byte[] data;
+
+    private void writeObject(ObjectOutputStream stream) throws IOException {
+      stream.write(data.length);
+      stream.write(data);
+    }
+
+    private void readObject(ObjectInputStream stream) throws IOException {
+      int size = stream.read();
+      data = new byte[size];
+      int read = stream.read(data);
+      Preconditions.checkArgument(read == size);
+    }
+  }
+
+  public static class InheritanceTestClassProxy extends InheritanceTestClassProxyBase {
+    public InheritanceTestClassProxy(byte f1) {
+      data = new byte[] {f1};
+    }
+
+    public Object readResolve() {
+      return new InheritanceTestClass(data[0]);
+    }
+  }
+
+  @Test
+  public void testInheritance() {
+    Fury fury = Fury.builder().requireClassRegistration(false).build();
+    byte[] bytes = fury.serialize(new InheritanceTestClass((byte) 10));
+    InheritanceTestClass o = (InheritanceTestClass) fury.deserialize(bytes);
+    assertEquals(o.f1, 10);
+  }
 }