From 5fcef225beca56954c7ce49c67a40aa0d62ce086 Mon Sep 17 00:00:00 2001 From: "nov.lzf" Date: Mon, 1 Apr 2024 14:55:44 +0800 Subject: [PATCH] format check on client labels (#11903) * format check on client labels * log optimize * testcase fix --- .../labels/impl/DefaultLabelsCollector.java | 5 + .../impl/DefaultLabelsCollectorManager.java | 40 ++++++++ .../nacos/common/utils/ConnLabelsUtils.java | 98 ++++++++++--------- .../common/utils/ConnLabelsUtilsTest.java | 10 +- 4 files changed, 100 insertions(+), 53 deletions(-) diff --git a/common/src/main/java/com/alibaba/nacos/common/labels/impl/DefaultLabelsCollector.java b/common/src/main/java/com/alibaba/nacos/common/labels/impl/DefaultLabelsCollector.java index 6c32b975d..f7d2a2dd6 100644 --- a/common/src/main/java/com/alibaba/nacos/common/labels/impl/DefaultLabelsCollector.java +++ b/common/src/main/java/com/alibaba/nacos/common/labels/impl/DefaultLabelsCollector.java @@ -59,6 +59,8 @@ public class DefaultLabelsCollector implements LabelsCollector { public Map collectLabels(Properties properties) { //properties + LOGGER.info("default nacos collect properties raw labels: {}", + properties.getProperty(Constants.APP_CONN_LABELS_KEY)); Map propertiesLabels = ConnLabelsUtils.parseRawLabels( properties.getProperty(Constants.APP_CONN_LABELS_KEY)); if (properties.containsKey(Constants.CONFIG_GRAY_LABEL)) { @@ -67,6 +69,7 @@ public class DefaultLabelsCollector implements LabelsCollector { LOGGER.info("default nacos collect properties labels: {}", propertiesLabels); //jvm + LOGGER.info("default nacos collect jvm raw labels: {}", System.getProperty(Constants.APP_CONN_LABELS_KEY)); Map jvmLabels = ConnLabelsUtils.parseRawLabels( System.getProperty(Constants.APP_CONN_LABELS_KEY)); if (System.getProperty(Constants.CONFIG_GRAY_LABEL) != null) { @@ -75,6 +78,8 @@ public class DefaultLabelsCollector implements LabelsCollector { LOGGER.info("default nacos collect jvm labels: {}", jvmLabels); //env + LOGGER.info("default nacos collect env raw labels: {}", + System.getenv(Constants.APP_CONN_LABELS_KEY.replaceAll(DOT, UNDERSCORE))); Map envLabels = ConnLabelsUtils.parseRawLabels( System.getenv(Constants.APP_CONN_LABELS_KEY.replaceAll(DOT, UNDERSCORE))); if (System.getenv(Constants.CONFIG_GRAY_LABEL.replaceAll(DOT, UNDERSCORE)) != null) { diff --git a/common/src/main/java/com/alibaba/nacos/common/labels/impl/DefaultLabelsCollectorManager.java b/common/src/main/java/com/alibaba/nacos/common/labels/impl/DefaultLabelsCollectorManager.java index 29672c043..61aff81b5 100644 --- a/common/src/main/java/com/alibaba/nacos/common/labels/impl/DefaultLabelsCollectorManager.java +++ b/common/src/main/java/com/alibaba/nacos/common/labels/impl/DefaultLabelsCollectorManager.java @@ -18,6 +18,7 @@ package com.alibaba.nacos.common.labels.impl; import com.alibaba.nacos.common.labels.LabelsCollector; import com.alibaba.nacos.common.labels.LabelsCollectorManager; +import com.alibaba.nacos.common.utils.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -60,6 +61,11 @@ public class DefaultLabelsCollectorManager implements LabelsCollectorManager { LOGGER.info("Process LabelsCollector with [name:{}]", labelsCollector.getName()); for (Map.Entry entry : labelsCollector.collectLabels(properties).entrySet()) { + if (!checkValidLabel(entry.getKey(), entry.getValue())) { + LOGGER.info(" ignore invalid label with [key:{}, value:{}] of collector [name:{}]", entry.getKey(), + entry.getValue(), labelsCollector.getName()); + continue; + } if (innerAddLabel(labels, entry.getKey(), entry.getValue())) { LOGGER.info("pick label with [key:{}, value:{}] of collector [name:{}]", entry.getKey(), entry.getValue(), labelsCollector.getName()); @@ -73,6 +79,40 @@ public class DefaultLabelsCollectorManager implements LabelsCollectorManager { return labels; } + private boolean checkValidLabel(String key, String value) { + return isValid(key) && isValid(value); + } + + private static boolean isValid(String param) { + if (StringUtils.isBlank(param)) { + return false; + } + int length = param.length(); + if (length > maxLength) { + return false; + } + for (int i = 0; i < length; i++) { + char ch = param.charAt(i); + if (!Character.isLetterOrDigit(ch) && !isValidChar(ch)) { + return false; + } + } + return true; + } + + private static char[] validChars = new char[] {'_', '-', '.'}; + + private static int maxLength = 128; + + private static boolean isValidChar(char ch) { + for (char c : validChars) { + if (c == ch) { + return true; + } + } + return false; + } + private ArrayList loadLabelsCollectors() { ServiceLoader labelsCollectors = ServiceLoader.load(LabelsCollector.class); ArrayList labelsCollectorsList = new ArrayList<>(); diff --git a/common/src/main/java/com/alibaba/nacos/common/utils/ConnLabelsUtils.java b/common/src/main/java/com/alibaba/nacos/common/utils/ConnLabelsUtils.java index b2b150d5a..a0f3f5f66 100644 --- a/common/src/main/java/com/alibaba/nacos/common/utils/ConnLabelsUtils.java +++ b/common/src/main/java/com/alibaba/nacos/common/utils/ConnLabelsUtils.java @@ -41,22 +41,22 @@ public class ConnLabelsUtils { public static final int TAG_V2_LABEL_KEY_VALUE_SPLIT_LENGTH = 2; - public static final int TAG_V1_LABEL_KEY_VALUE_SPLIT_LENGTH = 1; - /** - * parse property value to map. - * - * @date 2024/1/29 - * @description will get a key-value map from properties, JVM OPTIONS, ENV by order of properties > JVM OPTIONS > ENV - * which will use the next level value when the current level value isn't setup. - *

eg: if the value of "nacos.app.conn.labels"(properties' key) is "k1=v1,k2=v2"(properties' value), the result will be + * parse property value to map. + * + * @param properties Properties + * @param propertyName which key to get + * @return (String)key-(String)value map + * @date 2024/1/29 + * @description will get a key-value map from properties, JVM OPTIONS, ENV by order of properties > JVM OPTIONS + * > ENV which will use the next level value when the current level value isn't setup. + *

eg: if the value of "nacos.app.conn.labels"(properties' key) is "k1=v1,k2=v2"(properties' value), the result + * will be * a Map with value{k1=v1,k2=v2}.

- * @param properties Properties - * @param propertyName which key to get - * @return (String)key-(String)value map */ public static Map parsePropertyValue2Map(Properties properties, String propertyName) { - String rawLabels = properties.getProperty(propertyName, System.getProperty(propertyName, System.getenv(propertyName))); + String rawLabels = properties.getProperty(propertyName, + System.getProperty(propertyName, System.getenv(propertyName))); if (StringUtils.isBlank(rawLabels)) { LOGGER.info("no value found for property key: {}", propertyName); return new HashMap<>(2); @@ -65,29 +65,24 @@ public class ConnLabelsUtils { } /** - * parse raw json labels into a key-value map. - * - * @date 2024/1/29 - * @description - * @param rawLabels rawLabels to parse - * @return map parsed from rawLabels - */ + * parse raw json labels into a key-value map. + * + * @param rawLabels rawLabels to parse + * @return map parsed from rawLabels + * @date 2024/1/29 + * @description + */ public static Map parseRawLabels(String rawLabels) { if (StringUtils.isBlank(rawLabels)) { return new HashMap<>(2); } HashMap resultMap = new HashMap<>(2); try { - Arrays.stream(rawLabels.split(LABEL_SPLIT_OPERATOR)) - .filter(Objects::nonNull) - .map(String::trim) - .filter(StringUtils::isNotBlank) - .forEach(label -> { + Arrays.stream(rawLabels.split(LABEL_SPLIT_OPERATOR)).filter(Objects::nonNull).map(String::trim) + .filter(StringUtils::isNotBlank).forEach(label -> { String[] kv = label.split(LABEL_EQUALS_OPERATOR); if (kv.length == TAG_V2_LABEL_KEY_VALUE_SPLIT_LENGTH) { resultMap.put(kv[0].trim(), kv[1].trim()); - } else if (kv.length == TAG_V1_LABEL_KEY_VALUE_SPLIT_LENGTH) { - resultMap.put(kv[0].trim(), kv[0].trim()); } else { LOGGER.error("unknown label format: {}", label); } @@ -99,24 +94,32 @@ public class ConnLabelsUtils { } /** - * merge two map into one by using the former value when key is duplicated. - * - * @date 2024/1/29 - * @description merge two map into one preferring using the first one when key is duplicated - * @param preferredMap preferredMap - * @param backwardMap backwardMap - */ + * merge two map into one by using the former value when key is duplicated. + * + * @param preferredMap preferredMap + * @param backwardMap backwardMap + * @date 2024/1/29 + * @description merge two map into one preferring using the first one when key is duplicated + */ public static Map mergeMapByOrder(Map preferredMap, Map backwardMap) { if (preferredMap == null || preferredMap.isEmpty()) { - return new HashMap(8) { { - putAll(backwardMap); } }; + return new HashMap(8) { + { + putAll(backwardMap); + } + }; } if (backwardMap == null || backwardMap.isEmpty()) { - return new HashMap(8) { { - putAll(preferredMap); } }; + return new HashMap(8) { + { + putAll(preferredMap); + } + }; } - HashMap resultMap = new HashMap(8) {{ - putAll(preferredMap); } }; + HashMap resultMap = new HashMap(8) { + { + putAll(preferredMap); + } }; backwardMap.forEach((key, value) -> { if (!resultMap.containsKey(key)) { resultMap.put(key, value); @@ -126,19 +129,18 @@ public class ConnLabelsUtils { } /** - * add prefix for each key in map. - * - * @date 2024/1/29 - * @description add prefix for each key in map - * @param map map to add prefix - * @param prefix prefix - */ + * add prefix for each key in map. + * + * @param map map to add prefix + * @param prefix prefix + * @date 2024/1/29 + * @description add prefix for each key in map + */ public static Map addPrefixForEachKey(Map map, String prefix) { if (map == null || map.isEmpty()) { return map; } - return map.entrySet().stream().filter(Objects::nonNull) - .filter(elem -> !elem.getKey().trim().isEmpty()) + return map.entrySet().stream().filter(Objects::nonNull).filter(elem -> !elem.getKey().trim().isEmpty()) .collect(Collectors.toMap(elem -> prefix + elem.getKey(), Map.Entry::getValue)); } } diff --git a/common/src/test/java/com/alibaba/nacos/common/utils/ConnLabelsUtilsTest.java b/common/src/test/java/com/alibaba/nacos/common/utils/ConnLabelsUtilsTest.java index 0530f25c8..5bfe32565 100644 --- a/common/src/test/java/com/alibaba/nacos/common/utils/ConnLabelsUtilsTest.java +++ b/common/src/test/java/com/alibaba/nacos/common/utils/ConnLabelsUtilsTest.java @@ -39,7 +39,7 @@ public class ConnLabelsUtilsTest { String rawValue = "k1 = v1, k2 = v2"; properties.put(property, rawValue); String property1 = "property2"; - String rawValue1 = "k1, kk2"; + String rawValue1 = "k11=v11, kk2"; properties.put(property1, rawValue1); Map m = ConnLabelsUtils.parsePropertyValue2Map(properties, property); @@ -48,15 +48,15 @@ public class ConnLabelsUtilsTest { assertEquals("v2", m.get("k2")); Map m1 = ConnLabelsUtils.parsePropertyValue2Map(properties, property1); - assertEquals(2, m.size()); - assertEquals("k1", m1.get("k1")); - assertEquals("kk2", m1.get("kk2")); + assertEquals(1, m1.size()); + assertEquals("v11", m1.get("k11")); + assertEquals(null, m1.get("kk2")); m = ConnLabelsUtils.mergeMapByOrder(m, m1); assertEquals(3, m.size()); assertEquals("v1", m.get("k1")); assertEquals("v2", m.get("k2")); - assertEquals("kk2", m.get("kk2")); + assertEquals("v11", m.get("k11")); m = ConnLabelsUtils.addPrefixForEachKey(m, "test_prefix"); assertEquals(3, m.size());