Add serializable check for pojo (#11431)

AlbumenJ · web-flow · commit ce3b0e285a46 · 2023-02-01T14:03:03.000+08:00
diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/beanutil/JavaBeanSerializeUtil.java b/dubbo-common/src/main/java/org/apache/dubbo/common/beanutil/JavaBeanSerializeUtil.java
@@ -16,12 +16,6 @@
  */
 package org.apache.dubbo.common.beanutil;
 
-import org.apache.dubbo.common.logger.Logger;
-import org.apache.dubbo.common.logger.LoggerFactory;
-import org.apache.dubbo.common.utils.LogHelper;
-import org.apache.dubbo.common.utils.ReflectUtils;
-import org.apache.dubbo.common.utils.SerializeClassChecker;
-
 import java.lang.reflect.Array;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
@@ -32,6 +26,12 @@
 import java.util.IdentityHashMap;
 import java.util.Map;
 
+import org.apache.dubbo.common.logger.Logger;
+import org.apache.dubbo.common.logger.LoggerFactory;
+import org.apache.dubbo.common.utils.LogHelper;
+import org.apache.dubbo.common.utils.ReflectUtils;
+import org.apache.dubbo.common.utils.SerializeClassChecker;
+
 public final class JavaBeanSerializeUtil {
 
     private static final Logger logger = LoggerFactory.getLogger(JavaBeanSerializeUtil.class);
@@ -466,7 +466,9 @@ public static Class<?> name2Class(ClassLoader loader, String name) throws ClassN
             name = name.substring(1, name.length() - 1);
         }
         SerializeClassChecker.getInstance().validateClass(name);
-        return Class.forName(name, false, loader);
+        Class<?> aClass = Class.forName(name, false, loader);
+        SerializeClassChecker.getInstance().validateClass(aClass);
+        return aClass;
     }
 
     private static boolean isArray(String type) {
diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/constants/CommonConstants.java b/dubbo-common/src/main/java/org/apache/dubbo/common/constants/CommonConstants.java
@@ -17,13 +17,13 @@
 
 package org.apache.dubbo.common.constants;
 
-import org.apache.dubbo.common.URL;
-
 import java.net.NetworkInterface;
 import java.util.Properties;
 import java.util.concurrent.ExecutorService;
 import java.util.regex.Pattern;
 
+import org.apache.dubbo.common.URL;
+
 public interface CommonConstants {
     String DUBBO = "dubbo";
 
@@ -412,6 +412,8 @@ public interface CommonConstants {
 
     String CLASS_DESERIALIZE_BLOCKED_LIST = "dubbo.security.serialize.blockedClassList";
 
+    String CLASS_DESERIALIZE_CHECK_SERIALIZABLE = "dubbo.application.check-serializable";
+
     String ENABLE_NATIVE_JAVA_GENERIC_SERIALIZE = "dubbo.security.serialize.generic.native-java-enable";
 
     String SERIALIZE_BLOCKED_LIST_FILE_PATH = "security/serialize.blockedlist";
diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/utils/PojoUtils.java b/dubbo-common/src/main/java/org/apache/dubbo/common/utils/PojoUtils.java
@@ -16,11 +16,6 @@
  */
 package org.apache.dubbo.common.utils;
 
-import org.apache.dubbo.common.config.ConfigurationUtils;
-import org.apache.dubbo.common.constants.CommonConstants;
-import org.apache.dubbo.common.logger.Logger;
-import org.apache.dubbo.common.logger.LoggerFactory;
-
 import java.lang.reflect.Array;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
@@ -55,6 +50,11 @@
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 
+import org.apache.dubbo.common.config.ConfigurationUtils;
+import org.apache.dubbo.common.constants.CommonConstants;
+import org.apache.dubbo.common.logger.Logger;
+import org.apache.dubbo.common.logger.LoggerFactory;
+
 import static org.apache.dubbo.common.utils.ClassUtils.isAssignableFrom;
 
 /**
@@ -410,6 +410,7 @@ private static Object realize0(Object pojo, Class<?> type, Type genericType, fin
                         CLASS_NOT_FOUND_CACHE.put((String) className, NOT_FOUND_VALUE);
                     }
                 }
+                SerializeClassChecker.getInstance().validateClass(type);
             }
 
             // special logic for enum
diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/utils/SerializeClassChecker.java b/dubbo-common/src/main/java/org/apache/dubbo/common/utils/SerializeClassChecker.java
@@ -16,16 +16,19 @@
  */
 package org.apache.dubbo.common.utils;
 
-import org.apache.dubbo.common.beanutil.JavaBeanSerializeUtil;
-import org.apache.dubbo.common.logger.Logger;
-import org.apache.dubbo.common.logger.LoggerFactory;
-
 import java.io.IOException;
+import java.io.Serializable;
 import java.util.Arrays;
 import java.util.Locale;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicLong;
 
+import org.apache.dubbo.common.beanutil.JavaBeanSerializeUtil;
+import org.apache.dubbo.common.config.ConfigurationUtils;
+import org.apache.dubbo.common.constants.CommonConstants;
+import org.apache.dubbo.common.logger.Logger;
+import org.apache.dubbo.common.logger.LoggerFactory;
+
 import static org.apache.dubbo.common.constants.CommonConstants.CLASS_DESERIALIZE_ALLOWED_LIST;
 import static org.apache.dubbo.common.constants.CommonConstants.CLASS_DESERIALIZE_BLOCKED_LIST;
 import static org.apache.dubbo.common.constants.CommonConstants.CLASS_DESERIALIZE_BLOCK_ALL;
@@ -44,6 +47,8 @@ public class SerializeClassChecker {
     private final LFUCache<String, Object> CLASS_ALLOW_LFU_CACHE = new LFUCache<>();
     private final LFUCache<String, Object> CLASS_BLOCK_LFU_CACHE = new LFUCache<>();
 
+    private final boolean checkSerializable;
+
     private final AtomicLong counter = new AtomicLong(0);
 
     private SerializeClassChecker() {
@@ -83,6 +88,7 @@ private SerializeClassChecker() {
             CLASS_DESERIALIZE_BLOCKED_SET.addAll(Arrays.asList(classStrings));
         }
 
+        checkSerializable = Boolean.parseBoolean(ConfigurationUtils.getProperty(CommonConstants.CLASS_DESERIALIZE_CHECK_SERIALIZABLE, "true"));
     }
 
     public static SerializeClassChecker getInstance() {
@@ -137,6 +143,12 @@ public void validateClass(String name) {
         CLASS_ALLOW_LFU_CACHE.put(name, CACHE);
     }
 
+    public void validateClass(Class<?> aClass) {
+        if (checkSerializable && !Serializable.class.isAssignableFrom(aClass)) {
+            error(aClass.getName());
+        }
+    }
+
     private void error(String name) {
         String notice = "Trigger the safety barrier! " +
                 "Catch not allowed serialize class. " +
diff --git a/dubbo-common/src/test/java/org/apache/dubbo/common/beanutil/Bean.java b/dubbo-common/src/test/java/org/apache/dubbo/common/beanutil/Bean.java
@@ -1,12 +1,13 @@
 package org.apache.dubbo.common.beanutil;
 
-import org.apache.dubbo.rpc.model.person.FullAddress;
-import org.apache.dubbo.rpc.model.person.PersonStatus;
-import org.apache.dubbo.rpc.model.person.Phone;
-
+import java.io.Serializable;
 import java.util.Collection;
 import java.util.Date;
 import java.util.Map;
+
+import org.apache.dubbo.rpc.model.person.FullAddress;
+import org.apache.dubbo.rpc.model.person.PersonStatus;
+import org.apache.dubbo.rpc.model.person.Phone;
 /*
  * Licensed to the Apache Software Foundation (ASF) under one or more
  * contributor license agreements.  See the NOTICE file distributed with
@@ -23,7 +24,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-public class Bean {
+public class Bean implements Serializable {
 
     private Class<?> type;
 
diff --git a/dubbo-common/src/test/java/org/apache/dubbo/common/model/Person.java b/dubbo-common/src/test/java/org/apache/dubbo/common/model/Person.java
@@ -16,9 +16,10 @@
  */
 package org.apache.dubbo.common.model;
 
+import java.io.Serializable;
 import java.util.Arrays;
 
-public class Person {
+public class Person implements Serializable {
     byte oneByte = 123;
     private String name = "name1";
     private int age = 11;
diff --git a/dubbo-common/src/test/java/org/apache/dubbo/common/model/User.java b/dubbo-common/src/test/java/org/apache/dubbo/common/model/User.java
@@ -17,12 +17,13 @@
 
 package org.apache.dubbo.common.model;
 
+import java.io.Serializable;
 import java.util.Objects;
 
 /**
  * this class has no nullary constructor and some field is primitive
  */
-public class User {
+public class User implements Serializable {
     private int age;
 
     private String name;
diff --git a/dubbo-common/src/test/java/org/apache/dubbo/common/utils/PojoUtilsTest.java b/dubbo-common/src/test/java/org/apache/dubbo/common/utils/PojoUtilsTest.java
@@ -16,19 +16,7 @@
  */
 package org.apache.dubbo.common.utils;
 
-import org.apache.dubbo.common.model.Person;
-import org.apache.dubbo.common.model.SerializablePerson;
-import org.apache.dubbo.common.model.User;
-import org.apache.dubbo.common.model.person.BigPerson;
-import org.apache.dubbo.common.model.person.FullAddress;
-import org.apache.dubbo.common.model.person.PersonInfo;
-import org.apache.dubbo.common.model.person.PersonStatus;
-import org.apache.dubbo.common.model.person.Phone;
-
-import com.alibaba.fastjson.JSONObject;
-import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.Test;
-
+import java.io.Serializable;
 import java.lang.reflect.Method;
 import java.lang.reflect.Type;
 import java.text.SimpleDateFormat;
@@ -43,6 +31,19 @@
 import java.util.Map;
 import java.util.UUID;
 
+import org.apache.dubbo.common.model.Person;
+import org.apache.dubbo.common.model.SerializablePerson;
+import org.apache.dubbo.common.model.User;
+import org.apache.dubbo.common.model.person.BigPerson;
+import org.apache.dubbo.common.model.person.FullAddress;
+import org.apache.dubbo.common.model.person.PersonInfo;
+import org.apache.dubbo.common.model.person.PersonStatus;
+import org.apache.dubbo.common.model.person.Phone;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import com.alibaba.fastjson.JSONObject;
+
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
@@ -764,7 +765,7 @@ public enum Day {
         SUNDAY, MONDAY, TUESDAY, WEDNESDAY, THURSDAY, FRIDAY, SATURDAY
     }
 
-    public static class BasicTestData {
+    public static class BasicTestData implements Serializable {
 
         public boolean a;
         public char b;
@@ -836,7 +837,7 @@ public boolean equals(Object obj) {
 
     }
 
-    public static class Parent {
+    public static class Parent implements Serializable {
         public String gender;
         public String email;
         String name;
@@ -881,7 +882,7 @@ public void setChild(Child child) {
         }
     }
 
-    public static class Child {
+    public static class Child implements Serializable {
         public String gender;
         public int age;
         String toy;
@@ -921,7 +922,7 @@ public void setParent(Parent parent) {
         }
     }
 
-    public static class TestData {
+    public static class TestData implements Serializable {
         private Map<String, Child> children = new HashMap<String, Child>();
         private List<Child> list = new ArrayList<Child>();
 
@@ -950,7 +951,7 @@ public void addChild(Child child) {
         }
     }
 
-    public static class InnerPojo<T> {
+    public static class InnerPojo<T> implements Serializable {
         private List<T> list;
 
         public List<T> getList() {
@@ -962,7 +963,7 @@ public void setList(List<T> list) {
         }
     }
 
-    public static class ListResult<T> {
+    public static class ListResult<T> implements Serializable {
         List<T> result;
 
         public List<T> getResult() {
diff --git a/dubbo-common/src/test/java/org/apache/dubbo/common/utils/SerializeClassCheckerTest.java b/dubbo-common/src/test/java/org/apache/dubbo/common/utils/SerializeClassCheckerTest.java
@@ -16,18 +16,18 @@
  */
 package org.apache.dubbo.common.utils;
 
-import org.apache.dubbo.common.constants.CommonConstants;
+import java.net.Socket;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Locale;
 
-import javassist.compiler.Javac;
+import org.apache.dubbo.common.constants.CommonConstants;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Test;
 
-import java.net.Socket;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Locale;
+import javassist.compiler.Javac;
 
 public class SerializeClassCheckerTest {
 
@@ -62,6 +62,26 @@ public void testCommon() {
         });
     }
 
+    @Test
+    public void testSerializable1() {
+        SerializeClassChecker serializeClassChecker = SerializeClassChecker.getInstance();
+        Assertions.assertThrows(IllegalArgumentException.class, ()-> serializeClassChecker.validateClass(List.class));
+    }
+
+    @Test
+    public void testSerializable2() {
+        System.setProperty(CommonConstants.CLASS_DESERIALIZE_CHECK_SERIALIZABLE, "false");
+        SerializeClassChecker serializeClassChecker = SerializeClassChecker.getInstance();
+
+        try {
+            serializeClassChecker.validateClass(List.class);
+        } catch (Throwable t) {
+            Assertions.fail(t);
+        }
+
+        System.clearProperty(CommonConstants.CLASS_DESERIALIZE_CHECK_SERIALIZABLE);
+    }
+
     @Test
     public void testAddAllow() {
         System.setProperty(CommonConstants.CLASS_DESERIALIZE_ALLOWED_LIST, Socket.class.getName() + "," + Javac.class.getName());
