restrict use of custom yaml types

apollo-client/src/main/java/com/ctrip/framework/apollo/util/yaml/YamlParser.java

@@ -13,7 +13,7 @@ import java.util.Set;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.yaml.snakeyaml.Yaml;
-import org.yaml.snakeyaml.constructor.Constructor;
+import org.yaml.snakeyaml.constructor.SafeConstructor;
 import org.yaml.snakeyaml.nodes.MappingNode;
 import org.yaml.snakeyaml.parser.ParserException;
 
@@ -147,7 +147,10 @@ public class YamlParser {
     void process(Properties properties, Map<String, Object> map);
   }
 
-  private static class StrictMapAppenderConstructor extends Constructor {
+  /**
+   * A specialized {@link SafeConstructor} that checks for duplicate keys.
+   */
+  private static class StrictMapAppenderConstructor extends SafeConstructor {
 
     // Declared as public for use in subclasses
     StrictMapAppenderConstructor() {

apollo-client/src/test/java/com/ctrip/framework/apollo/util/yaml/YamlParserTest.java

@@ -21,6 +21,7 @@ import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 import org.springframework.beans.factory.config.YamlPropertiesFactoryBean;
 import org.springframework.core.io.ByteArrayResource;
+import org.yaml.snakeyaml.constructor.ConstructorException;
 import org.yaml.snakeyaml.parser.ParserException;
 
 public class YamlParserTest {
@@ -57,6 +58,11 @@ public class YamlParserTest {
     testInvalid("case8.yaml");
   }
 
+  @Test(expected = ConstructorException.class)
+  public void testcase9() throws Exception {
+    testInvalid("case9.yaml");
+  }
+
   @Test
   public void testOrderProperties() throws IOException {
     String yamlContent = loadYaml("orderedcase.yaml");

apollo-client/src/test/resources/yaml/case9.yaml

@@ -0,0 +1,5 @@
+!!javax.script.ScriptEngineManager [
+  !!java.net.URLClassLoader [[
+    !!java.net.URL ["http://localhost"]
+  ]]
+]

apollo-portal/pom.xml

@@ -27,11 +27,12 @@
 			<groupId>com.ctrip.framework.apollo</groupId>
 			<artifactId>apollo-openapi</artifactId>
 		</dependency>
+		<!-- yml processing -->
 		<dependency>
-			<groupId>com.h2database</groupId>
-			<artifactId>h2</artifactId>
-			<scope>test</scope>
+			<groupId>org.yaml</groupId>
+			<artifactId>snakeyaml</artifactId>
 		</dependency>
+		<!-- end of yml processing -->
 		<!-- JDK 1.8+ -->
 		<dependency>
 			<groupId>javax.xml.bind</groupId>
@@ -58,6 +59,11 @@
 		<!-- end of JDK 11+ -->
 
 		<!-- test -->
+		<dependency>
+			<groupId>com.h2database</groupId>
+			<artifactId>h2</artifactId>
+			<scope>test</scope>
+		</dependency>
 		<dependency>
 			<groupId>org.eclipse.jetty</groupId>
 			<artifactId>jetty-server</artifactId>

apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/ItemController.java

@@ -33,6 +33,11 @@ import org.springframework.web.bind.annotation.RestController;
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
+import org.yaml.snakeyaml.DumperOptions;
+import org.yaml.snakeyaml.LoaderOptions;
+import org.yaml.snakeyaml.Yaml;
+import org.yaml.snakeyaml.constructor.SafeConstructor;
+import org.yaml.snakeyaml.representer.Representer;
 
 import static com.ctrip.framework.apollo.common.utils.RequestPrecondition.checkModel;
 
@@ -214,7 +219,8 @@ public class ItemController {
       @PathVariable String namespaceName) {
     configService.revokeItem(appId, Env.valueOf(env), clusterName, namespaceName);
   }
-  private void doSyntaxCheck(NamespaceTextModel model) {
+
+  void doSyntaxCheck(NamespaceTextModel model) {
     if (StringUtils.isBlank(model.getConfigText())) {
       return;
     }
@@ -225,7 +231,7 @@ public class ItemController {
     }
 
     // use YamlPropertiesFactoryBean to check the yaml syntax
-    YamlPropertiesFactoryBean yamlPropertiesFactoryBean = new YamlPropertiesFactoryBean();
+    TypeLimitedYamlPropertiesFactoryBean yamlPropertiesFactoryBean = new TypeLimitedYamlPropertiesFactoryBean();
     yamlPropertiesFactoryBean.setResources(new ByteArrayResource(model.getConfigText().getBytes()));
     // this call converts yaml to properties and will throw exception if the conversion fails
     yamlPropertiesFactoryBean.getObject();
@@ -235,5 +241,14 @@ public class ItemController {
     return Objects.nonNull(item) && !StringUtils.isContainEmpty(item.getKey());
   }
 
+  private static class TypeLimitedYamlPropertiesFactoryBean extends YamlPropertiesFactoryBean {
+    @Override
+    protected Yaml createYaml() {
+      LoaderOptions loaderOptions = new LoaderOptions();
+      loaderOptions.setAllowDuplicateKeys(false);
+      return new Yaml(new SafeConstructor(), new Representer(),
+          new DumperOptions(), loaderOptions);
+    }
+  }
 
 }

apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ItemControllerTest.java

@@ -0,0 +1,76 @@
+package com.ctrip.framework.apollo.portal.controller;
+
+import com.ctrip.framework.apollo.core.enums.ConfigFileFormat;
+import com.ctrip.framework.apollo.portal.component.PermissionValidator;
+import com.ctrip.framework.apollo.portal.entity.model.NamespaceTextModel;
+import com.ctrip.framework.apollo.portal.service.ItemService;
+import com.ctrip.framework.apollo.portal.service.NamespaceService;
+import com.ctrip.framework.apollo.portal.spi.UserInfoHolder;
+import com.google.common.base.Charsets;
+import com.google.common.io.Files;
+import java.io.File;
+import java.io.IOException;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+import org.yaml.snakeyaml.constructor.ConstructorException;
+
+@RunWith(MockitoJUnitRunner.class)
+public class ItemControllerTest {
+
+  @Mock
+  private ItemService configService;
+  @Mock
+  private NamespaceService namespaceService;
+  @Mock
+  private UserInfoHolder userInfoHolder;
+  @Mock
+  private PermissionValidator permissionValidator;
+
+  @InjectMocks
+  private ItemController itemController;
+
+  @Before
+  public void setUp() throws Exception {
+    itemController = new ItemController(configService, userInfoHolder, permissionValidator,
+        namespaceService);
+  }
+
+  @Test
+  public void yamlSyntaxCheckOK() throws Exception {
+    String yaml = loadYaml("case1.yaml");
+
+    itemController.doSyntaxCheck(assemble(ConfigFileFormat.YAML.getValue(), yaml));
+  }
+
+  @Test(expected = IllegalStateException.class)
+  public void yamlSyntaxCheckWithDuplicatedValue() throws Exception {
+    String yaml = loadYaml("case2.yaml");
+
+    itemController.doSyntaxCheck(assemble(ConfigFileFormat.YAML.getValue(), yaml));
+  }
+
+  @Test(expected = ConstructorException.class)
+  public void yamlSyntaxCheckWithUnsupportedType() throws Exception {
+    String yaml = loadYaml("case3.yaml");
+
+    itemController.doSyntaxCheck(assemble(ConfigFileFormat.YAML.getValue(), yaml));
+  }
+
+  private NamespaceTextModel assemble(String format, String content) {
+    NamespaceTextModel model = new NamespaceTextModel();
+    model.setFormat(format);
+    model.setConfigText(content);
+
+    return model;
+  }
+
+  private String loadYaml(String caseName) throws IOException {
+    File file = new File("src/test/resources/yaml/" + caseName);
+
+    return Files.toString(file, Charsets.UTF_8);
+  }
+}

apollo-portal/src/test/resources/yaml/case1.yaml

@@ -0,0 +1,25 @@
+root:
+  key1: "someValue"
+  key2: 100
+  key3:
+    key4:
+      key5: '(%sender%) %message%'
+      key6: '* %sender% %message%'
+#  commented: "xxx"
+  list:
+  - 'item 1'
+  - 'item 2'
+  intList:
+    - 100
+    - 200
+  listOfMap:
+  - key: '#mychannel'
+    value: ''
+  - key: '#myprivatechannel'
+    value: 'mypassword'
+  listOfList:
+  - - 'a1'
+    - 'a2'
+  - - 'b1'
+    - 'b2'
+  listOfList2: [ ['a1', 'a2'], ['b1', 'b2'] ]

apollo-portal/src/test/resources/yaml/case2.yaml

@@ -0,0 +1,4 @@
+root:
+  key1: "someValue"
+  key2: 100
+  key1: "anotherValue"

apollo-portal/src/test/resources/yaml/case3.yaml

@@ -0,0 +1,5 @@
+!!javax.script.ScriptEngineManager [
+  !!java.net.URLClassLoader [[
+    !!java.net.URL ["http://localhost"]
+  ]]
+]