From e955bacf62fd5417142f7a3128fd272d6cc52d51 Mon Sep 17 00:00:00 2001
From: Jason Song <nobodyiam@gmail.com>
Date: Sat, 11 Jul 2020 14:10:54 +0800
Subject: [PATCH] restrict use of custom yaml types
---
.../apollo/util/yaml/YamlParser.java | 7 +-
.../apollo/util/yaml/YamlParserTest.java | 6 ++
.../src/test/resources/yaml/case9.yaml | 5 ++
apollo-portal/pom.xml | 12 ++-
.../portal/controller/ItemController.java | 18 ++++-
.../portal/controller/ItemControllerTest.java | 76 +++++++++++++++++++
.../src/test/resources/yaml/case1.yaml | 25 ++++++
.../src/test/resources/yaml/case2.yaml | 4 +
.../src/test/resources/yaml/case3.yaml | 5 ++
9 files changed, 151 insertions(+), 7 deletions(-)
create mode 100644 apollo-client/src/test/resources/yaml/case9.yaml
create mode 100644 apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ItemControllerTest.java
create mode 100644 apollo-portal/src/test/resources/yaml/case1.yaml
create mode 100644 apollo-portal/src/test/resources/yaml/case2.yaml
create mode 100644 apollo-portal/src/test/resources/yaml/case3.yaml
diff --git a/apollo-client/src/main/java/com/ctrip/framework/apollo/util/yaml/YamlParser.java b/apollo-client/src/main/java/com/ctrip/framework/apollo/util/yaml/YamlParser.java
index d324da91c1c..373017a406b 100644
--- a/apollo-client/src/main/java/com/ctrip/framework/apollo/util/yaml/YamlParser.java
+++ b/apollo-client/src/main/java/com/ctrip/framework/apollo/util/yaml/YamlParser.java
@@ -13,7 +13,7 @@
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 @@ private interface MatchCallback {
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() {
diff --git a/apollo-client/src/test/java/com/ctrip/framework/apollo/util/yaml/YamlParserTest.java b/apollo-client/src/test/java/com/ctrip/framework/apollo/util/yaml/YamlParserTest.java
index f1adf641856..46262a38ed6 100644
--- a/apollo-client/src/test/java/com/ctrip/framework/apollo/util/yaml/YamlParserTest.java
+++ b/apollo-client/src/test/java/com/ctrip/framework/apollo/util/yaml/YamlParserTest.java
@@ -18,6 +18,7 @@
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 {
@@ -44,6 +45,11 @@ public void testcase8() throws Exception {
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");
diff --git a/apollo-client/src/test/resources/yaml/case9.yaml b/apollo-client/src/test/resources/yaml/case9.yaml
new file mode 100644
index 00000000000..9739252c396
--- /dev/null
+++ b/apollo-client/src/test/resources/yaml/case9.yaml
@@ -0,0 +1,5 @@
+!!javax.script.ScriptEngineManager [
+ !!java.net.URLClassLoader [[
+ !!java.net.URL ["http://localhost"]
+ ]]
+]
\ No newline at end of file
diff --git a/apollo-portal/pom.xml b/apollo-portal/pom.xml
index d9588c83c3d..ee5316ab9d0 100644
--- a/apollo-portal/pom.xml
+++ b/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>
diff --git a/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/ItemController.java b/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/ItemController.java
index b4b0b423c93..0931f1ebc6d 100644
--- a/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/ItemController.java
+++ b/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/ItemController.java
@@ -33,6 +33,11 @@
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;
@@ -208,7 +213,7 @@ public ResponseEntity<Void> syntaxCheckText(@PathVariable String appId, @PathVar
return ResponseEntity.ok().build();
}
- private void doSyntaxCheck(NamespaceTextModel model) {
+ void doSyntaxCheck(NamespaceTextModel model) {
if (StringUtils.isBlank(model.getConfigText())) {
return;
}
@@ -219,7 +224,7 @@ private void doSyntaxCheck(NamespaceTextModel model) {
}
// 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();
@@ -229,5 +234,14 @@ private boolean isValidItem(ItemDTO item) {
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);
+ }
+ }
}
diff --git a/apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ItemControllerTest.java b/apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/ItemControllerTest.java
new file mode 100644
index 00000000000..05f934cb9ca
--- /dev/null
+++ b/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);
+ }
+}
\ No newline at end of file
diff --git a/apollo-portal/src/test/resources/yaml/case1.yaml b/apollo-portal/src/test/resources/yaml/case1.yaml
new file mode 100644
index 00000000000..9153fdc7023
--- /dev/null
+++ b/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'] ]
diff --git a/apollo-portal/src/test/resources/yaml/case2.yaml b/apollo-portal/src/test/resources/yaml/case2.yaml
new file mode 100644
index 00000000000..63b49be99ac
--- /dev/null
+++ b/apollo-portal/src/test/resources/yaml/case2.yaml
@@ -0,0 +1,4 @@
+root:
+ key1: "someValue"
+ key2: 100
+ key1: "anotherValue"
diff --git a/apollo-portal/src/test/resources/yaml/case3.yaml b/apollo-portal/src/test/resources/yaml/case3.yaml
new file mode 100644
index 00000000000..9739252c396
--- /dev/null
+++ b/apollo-portal/src/test/resources/yaml/case3.yaml
@@ -0,0 +1,5 @@
+!!javax.script.ScriptEngineManager [
+ !!java.net.URLClassLoader [[
+ !!java.net.URL ["http://localhost"]
+ ]]
+]
\ No newline at end of file