This is an automated email from the ASF dual-hosted git repository.
wujimin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git
commit 839a52e27c754cb5ce14f20063902f21065bd26c
Author: liubao <bi...@qq.com>
AuthorDate: Fri Dec 4 17:23:24 2020 +0800
[SCB-2145]fix local yaml unsafe parse problem
---
.../org/apache/servicecomb/config/YAMLUtil.java | 13 ++++-
.../config/archaius/sources/YAMLConfigLoader.java | 7 +--
.../apache/servicecomb/config/TestYAMLUtil.java | 66 ++++++++++++++++++++++
.../servicecomb/router/cache/RouterRuleCache.java | 17 +++---
.../router/custom/RouterInvokeFilter.java | 7 +--
.../servicecomb/router/RouterDistributorTest.java | 60 ++++++++++----------
.../localregistry/LocalRegistryStore.java | 35 ++++++------
.../client/LocalServiceRegistryClientImpl.java | 17 ++++--
8 files changed, 151 insertions(+), 71 deletions(-)
diff --git
a/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/YAMLUtil.java
b/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/YAMLUtil.java
index abcf1fa..e37b4e2 100644
---
a/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/YAMLUtil.java
+++
b/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/YAMLUtil.java
@@ -24,12 +24,17 @@ import java.io.InputStream;
import java.util.LinkedHashMap;
import java.util.Map;
+import org.yaml.snakeyaml.TypeDescription;
import org.yaml.snakeyaml.Yaml;
+import org.yaml.snakeyaml.constructor.Constructor;
+import org.yaml.snakeyaml.constructor.SafeConstructor;
/**
* Created by on 2017/1/5.
*/
public final class YAMLUtil {
+ private static final Yaml SAFE_PARSER = new Yaml(new SafeConstructor());
+
private YAMLUtil() {
}
@@ -45,11 +50,15 @@ public final class YAMLUtil {
@SuppressWarnings("unchecked")
public static Map<String, Object> yaml2Properties(InputStream input) {
Map<String, Object> configurations = new LinkedHashMap<>();
- Yaml yaml = new Yaml();
- yaml.loadAll(input).forEach(data ->
configurations.putAll(retrieveItems("", (Map<String, Object>) data)));
+ SAFE_PARSER.loadAll(input).forEach(data ->
configurations.putAll(retrieveItems("", (Map<String, Object>) data)));
return configurations;
}
+ public static <T> T parserObject(String yamlContent, Class<T> clazz) {
+ Yaml parser = new Yaml(new Constructor(new TypeDescription(clazz, clazz)));
+ return parser.loadAs(yamlContent, clazz);
+ }
+
@SuppressWarnings("unchecked")
public static Map<String, Object> retrieveItems(String prefix, Map<String,
Object> propertieMap) {
Map<String, Object> result = new LinkedHashMap<>();
diff --git
a/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/archaius/sources/YAMLConfigLoader.java
b/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/archaius/sources/YAMLConfigLoader.java
index f0fb453..f50451f 100644
---
a/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/archaius/sources/YAMLConfigLoader.java
+++
b/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/archaius/sources/YAMLConfigLoader.java
@@ -22,16 +22,13 @@ import java.io.InputStream;
import java.net.URL;
import java.util.Map;
-import org.yaml.snakeyaml.Yaml;
+import org.apache.servicecomb.config.YAMLUtil;
public class YAMLConfigLoader extends AbstractConfigLoader {
- @SuppressWarnings("unchecked")
@Override
protected Map<String, Object> loadData(URL url) throws IOException {
- Yaml yaml = new Yaml();
-
try (InputStream inputStream = url.openStream()) {
- return yaml.loadAs(inputStream, Map.class);
+ return YAMLUtil.yaml2Properties(inputStream);
}
}
}
diff --git
a/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/TestYAMLUtil.java
b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/TestYAMLUtil.java
new file mode 100644
index 0000000..7dd950c
--- /dev/null
+++
b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/TestYAMLUtil.java
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.servicecomb.config;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestYAMLUtil {
+ public static class Person {
+ String name;
+
+ public String getName() {
+ return name;
+ }
+
+ public void setName(String name) {
+ this.name = name;
+ }
+ }
+
+ public static class UnsafePerson {
+ String name;
+
+ public String getName() {
+ return name;
+ }
+
+ public void setName(String name) {
+ this.name = name;
+ }
+ }
+
+ @Test
+ public void testSafeParser() {
+ Person person = YAMLUtil.parserObject("name: hello", Person.class);
+ Assert.assertEquals("hello", person.getName());
+
+ person =
YAMLUtil.parserObject("!!org.apache.servicecomb.config.TestYAMLUtil$Person\n"
+ + "name: hello", Person.class);
+ Assert.assertEquals("hello", person.getName());
+
+ person =
YAMLUtil.parserObject("!!org.apache.servicecomb.config.TestYAMLUtil$UnsafePerson\n"
+ + "name: hello", Person.class);
+ Assert.assertEquals("hello", person.getName());
+
+ // using Object.class is not safe, do not used in product code.
+ Object object =
YAMLUtil.parserObject("!!org.apache.servicecomb.config.TestYAMLUtil$UnsafePerson\n"
+ + "name: hello", Object.class);
+ Assert.assertEquals("hello", ((UnsafePerson) object).getName());
+ }
+}
diff --git
a/handlers/handler-router/src/main/java/org/apache/servicecomb/router/cache/RouterRuleCache.java
b/handlers/handler-router/src/main/java/org/apache/servicecomb/router/cache/RouterRuleCache.java
index 0136b08..941a8ac 100644
---
a/handlers/handler-router/src/main/java/org/apache/servicecomb/router/cache/RouterRuleCache.java
+++
b/handlers/handler-router/src/main/java/org/apache/servicecomb/router/cache/RouterRuleCache.java
@@ -16,20 +16,22 @@
*/
package org.apache.servicecomb.router.cache;
-import com.google.common.collect.Interner;
-import com.google.common.collect.Interners;
-import com.netflix.config.DynamicPropertyFactory;
-import com.netflix.config.DynamicStringProperty;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.servicecomb.config.YAMLUtil;
import org.apache.servicecomb.router.model.PolicyRuleItem;
import org.apache.servicecomb.router.model.ServiceInfoCache;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.util.CollectionUtils;
-import org.apache.commons.lang3.StringUtils;
-import org.yaml.snakeyaml.Yaml;
+
+import com.google.common.collect.Interner;
+import com.google.common.collect.Interners;
+import com.netflix.config.DynamicPropertyFactory;
+import com.netflix.config.DynamicStringProperty;
/**
* @Author GuoYl123
@@ -78,11 +80,10 @@ public class RouterRuleCache {
if (StringUtils.isEmpty(ruleStr)) {
return false;
}
- Yaml yaml = new Yaml();
List<PolicyRuleItem> policyRuleItemList;
try {
policyRuleItemList = Arrays
- .asList(yaml.loadAs(ruleStr, PolicyRuleItem[].class));
+ .asList(YAMLUtil.parserObject(ruleStr, PolicyRuleItem[].class));
} catch (Exception e) {
LOGGER.error("route management Serialization failed: {}",
e.getMessage());
return false;
diff --git
a/handlers/handler-router/src/main/java/org/apache/servicecomb/router/custom/RouterInvokeFilter.java
b/handlers/handler-router/src/main/java/org/apache/servicecomb/router/custom/RouterInvokeFilter.java
index 7e345a9..b381a04 100644
---
a/handlers/handler-router/src/main/java/org/apache/servicecomb/router/custom/RouterInvokeFilter.java
+++
b/handlers/handler-router/src/main/java/org/apache/servicecomb/router/custom/RouterInvokeFilter.java
@@ -17,10 +17,12 @@
package org.apache.servicecomb.router.custom;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
import org.apache.servicecomb.common.rest.filter.HttpServerFilter;
import org.apache.servicecomb.core.Invocation;
import org.apache.servicecomb.foundation.common.utils.JsonUtils;
@@ -30,8 +32,6 @@ import org.apache.servicecomb.swagger.invocation.Response;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.util.CollectionUtils;
-import org.apache.commons.lang3.StringUtils;
-import org.yaml.snakeyaml.Yaml;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.netflix.config.DynamicPropertyFactory;
@@ -118,8 +118,7 @@ public class RouterInvokeFilter implements HttpServerFilter
{
}
try {
if (CollectionUtils.isEmpty(allHeader)) {
- Yaml yaml = new Yaml();
- allHeader = yaml.load(str);
+ allHeader = Arrays.asList(str.split(","));
}
} catch (Exception e) {
LOGGER.error("route management Serialization failed: {}",
e.getMessage());
diff --git
a/handlers/handler-router/src/test/java/org/apache/servicecomb/router/RouterDistributorTest.java
b/handlers/handler-router/src/test/java/org/apache/servicecomb/router/RouterDistributorTest.java
index 89e34e1..2979ab2 100644
---
a/handlers/handler-router/src/test/java/org/apache/servicecomb/router/RouterDistributorTest.java
+++
b/handlers/handler-router/src/test/java/org/apache/servicecomb/router/RouterDistributorTest.java
@@ -17,27 +17,29 @@
package org.apache.servicecomb.router;
-import com.netflix.config.DynamicPropertyFactory;
-import com.netflix.config.DynamicStringProperty;
-import com.netflix.loadbalancer.Server;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import mockit.Expectations;
+
import org.apache.servicecomb.router.cache.RouterRuleCache;
import org.apache.servicecomb.router.distribute.AbstractRouterDistributor;
import org.apache.servicecomb.router.distribute.RouterDistributor;
import org.junit.Assert;
import org.junit.Test;
+import com.netflix.config.DynamicPropertyFactory;
+import com.netflix.config.DynamicStringProperty;
+import com.netflix.loadbalancer.Server;
+
+import mockit.Expectations;
+
/**
- * @Author GuoYl123
- * @Date 2019/11/4
+ * @author GuoYl123
**/
public class RouterDistributorTest {
- private static String ruleStr = ""
+ private static final String RULE_STRING = ""
+ " - precedence: 2 #优先级\n"
+ " match: #匹配策略\n"
+ " source: xx #匹配某个服务名\n"
@@ -66,7 +68,7 @@ public class RouterDistributorTest {
+ " version: 1\n"
+ " app: a";
- private static String targetServiceName = "test_server";
+ private static final String TARGET_SERVICE_NAME = "test_server";
@Test
public void testHeaderIsNull() {
@@ -77,13 +79,13 @@ public class RouterDistributorTest {
@Test
public void testVersionNotMatch() {
- Map<String, String> headermap = new HashMap<>();
- headermap.put("userId", "01");
- headermap.put("appId", "01");
- headermap.put("formate", "json");
+ Map<String, String> headerMap = new HashMap<>();
+ headerMap.put("userId", "01");
+ headerMap.put("appId", "01");
+ headerMap.put("format", "json");
List<ServiceIns> list = getMockList();
list.remove(1);
- List<ServiceIns> serverList = mainFilter(list, headermap);
+ List<ServiceIns> serverList = mainFilter(list, headerMap);
Assert.assertEquals(1, serverList.size());
Assert.assertEquals("01", serverList.get(0).getHost());
}
@@ -93,43 +95,45 @@ public class RouterDistributorTest {
Map<String, String> headermap = new HashMap<>();
headermap.put("userId", "01");
headermap.put("appId", "01");
- headermap.put("formate", "json");
+ headermap.put("format", "json");
List<ServiceIns> serverList = mainFilter(getMockList(), headermap);
Assert.assertEquals(1, serverList.size());
Assert.assertEquals("02", serverList.get(0).getHost());
}
private List<ServiceIns> getMockList() {
- List<ServiceIns> serverlist = new ArrayList<>();
+ List<ServiceIns> serverList = new ArrayList<>();
ServiceIns ins1 = new ServiceIns("01");
ins1.setVersion("2.0");
ServiceIns ins2 = new ServiceIns("02");
ins2.addTags("app", "a");
- serverlist.add(ins1);
- serverlist.add(ins2);
- return serverlist;
+ serverList.add(ins1);
+ serverList.add(ins2);
+ return serverList;
}
private List<ServiceIns> mainFilter(List<ServiceIns> serverlist, Map<String,
String> headermap) {
- RouterDistributor<ServiceIns, ServiceIns> testDistributer = new
TestDistributer();
+ RouterDistributor<ServiceIns, ServiceIns> testDistributer = new
TestDistributor();
DynamicPropertyFactory dpf = DynamicPropertyFactory.getInstance();
- DynamicStringProperty strp = new DynamicStringProperty("", ruleStr);
+ DynamicStringProperty rule = new DynamicStringProperty("", RULE_STRING);
new Expectations(dpf) {
{
dpf.getStringProperty(anyString, null, (Runnable) any);
- result = strp;
+ result = rule;
}
};
RouterRuleCache.refresh();
return RouterFilter
- .getFilteredListOfServers(serverlist, targetServiceName, headermap,
+ .getFilteredListOfServers(serverlist, TARGET_SERVICE_NAME, headermap,
testDistributer);
}
- class ServiceIns extends Server {
+ static class ServiceIns extends Server {
String version = "1.1";
- String serverName = targetServiceName;
+
+ String serverName = TARGET_SERVICE_NAME;
+
Map<String, String> tags = new HashMap<>();
public ServiceIns(String id) {
@@ -152,18 +156,14 @@ public class RouterDistributorTest {
this.version = version;
}
- public void setServerName(String serverName) {
- this.serverName = serverName;
- }
-
public void addTags(String key, String v) {
tags.put(key, v);
}
}
- class TestDistributer extends AbstractRouterDistributor<ServiceIns,
ServiceIns> {
+ static class TestDistributor extends AbstractRouterDistributor<ServiceIns,
ServiceIns> {
- public TestDistributer() {
+ public TestDistributor() {
init(a -> a, ServiceIns::getVersion, ServiceIns::getServerName,
ServiceIns::getTags);
}
}
diff --git
a/service-registry/registry-local/src/main/java/org/apache/servicecomb/localregistry/LocalRegistryStore.java
b/service-registry/registry-local/src/main/java/org/apache/servicecomb/localregistry/LocalRegistryStore.java
index f21bcad..98c43f4 100644
---
a/service-registry/registry-local/src/main/java/org/apache/servicecomb/localregistry/LocalRegistryStore.java
+++
b/service-registry/registry-local/src/main/java/org/apache/servicecomb/localregistry/LocalRegistryStore.java
@@ -30,6 +30,7 @@ import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Collectors;
+import org.apache.servicecomb.config.YAMLUtil;
import org.apache.servicecomb.foundation.common.utils.BeanUtils;
import org.apache.servicecomb.foundation.common.utils.JvmUtils;
import org.apache.servicecomb.localregistry.RegistryBean.Instance;
@@ -40,7 +41,6 @@ import
org.apache.servicecomb.registry.api.registry.MicroserviceInstance;
import org.apache.servicecomb.registry.api.registry.MicroserviceInstances;
import org.apache.servicecomb.swagger.SwaggerUtils;
import org.apache.servicecomb.swagger.generator.SwaggerGenerator;
-import org.yaml.snakeyaml.Yaml;
import com.google.common.annotations.VisibleForTesting;
@@ -111,37 +111,38 @@ public class LocalRegistryStore {
private List<RegistryBean> loadYamlBeans() {
List<RegistryBean> beans = new ArrayList<>();
- InputStream is = null;
-
try {
ClassLoader loader = JvmUtils.findClassLoader();
Enumeration<URL> urls = loader.getResources(REGISTRY_FILE_NAME);
+
while (urls.hasMoreElements()) {
URL url = urls.nextElement();
- is = url.openStream();
- if (is != null) {
- beans.addAll(initFromData(is));
+
+ InputStream is = null;
+ try {
+ is = url.openStream();
+ if (is != null) {
+ beans.addAll(initFromData(is));
+ }
+ } finally {
+ if (is != null) {
+ try {
+ is.close();
+ } catch (IOException e) {
+ // nothing to do
+ }
+ }
}
}
} catch (IOException e) {
throw new IllegalStateException(e);
- } finally {
- if (is != null) {
- try {
- is.close();
- } catch (IOException e) {
- // nothing to do
- }
- }
}
return beans;
}
- @SuppressWarnings("unchecked")
private List<RegistryBean> initFromData(InputStream is) {
- Yaml yaml = new Yaml();
- Map<String, Object> data = yaml.loadAs(is, Map.class);
+ Map<String, Object> data = YAMLUtil.yaml2Properties(is);
return initFromData(data);
}
diff --git
a/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/client/LocalServiceRegistryClientImpl.java
b/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/client/LocalServiceRegistryClientImpl.java
index a63e9b2..202e301 100644
---
a/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/client/LocalServiceRegistryClientImpl.java
+++
b/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/client/LocalServiceRegistryClientImpl.java
@@ -17,6 +17,7 @@
package org.apache.servicecomb.serviceregistry.client;
+import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.List;
@@ -30,6 +31,7 @@ import javax.ws.rs.core.Response.Status;
import org.apache.commons.lang3.StringUtils;
import org.apache.servicecomb.config.BootStrapProperties;
+import org.apache.servicecomb.config.YAMLUtil;
import org.apache.servicecomb.foundation.vertx.AsyncResultCallback;
import
org.apache.servicecomb.registry.api.event.MicroserviceInstanceChangedEvent;
import org.apache.servicecomb.registry.api.registry.FindInstancesResponse;
@@ -48,7 +50,6 @@ import
org.apache.servicecomb.serviceregistry.api.response.HeartbeatResponse;
import org.apache.servicecomb.serviceregistry.client.http.Holder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import org.yaml.snakeyaml.Yaml;
import com.google.common.base.Charsets;
import com.google.common.hash.Hashing;
@@ -94,10 +95,16 @@ public class LocalServiceRegistryClientImpl implements
ServiceRegistryClient {
}
private void initFromData(InputStream is) {
- Yaml yaml = new Yaml();
- @SuppressWarnings("unchecked")
- Map<String, Object> data = yaml.loadAs(is, Map.class);
- initFromData(data);
+ try {
+ Map<String, Object> data = YAMLUtil.yaml2Properties(is);
+ initFromData(data);
+ } finally {
+ try {
+ is.close();
+ } catch (IOException e) {
+ LOGGER.error("", e);
+ }
+ }
}
private void initFromData(Map<String, Object> data) {
