Addition fix alibaba#12563, Add parameter check for tag.

KomachiSion · Sep 2, 2024 · 9778110 · 9778110
1 parent 5ded85f
commit 9778110
Show file tree

Hide file tree

Showing 7 changed files with 109 additions and 21 deletions.
diff --git a/...in/java/com/alibaba/nacos/config/server/remote/ConfigChangeBatchListenRequestHandler.java b/...in/java/com/alibaba/nacos/config/server/remote/ConfigChangeBatchListenRequestHandler.java
@@ -24,6 +24,7 @@
 import com.alibaba.nacos.auth.annotation.Secured;
 import com.alibaba.nacos.config.server.service.ConfigCacheService;
 import com.alibaba.nacos.config.server.utils.GroupKey2;
+import com.alibaba.nacos.config.server.utils.ParamUtils;
 import com.alibaba.nacos.core.paramcheck.ExtractorManager;
 import com.alibaba.nacos.core.paramcheck.impl.ConfigBatchListenRequestParamExtractor;
 import com.alibaba.nacos.core.remote.RequestHandler;
@@ -55,7 +56,7 @@ public ConfigChangeBatchListenResponse handle(ConfigBatchListenRequest configCha
             throws NacosException {
         String connectionId = StringPool.get(meta.getConnectionId());
         String tag = configChangeListenRequest.getHeader(Constants.VIPSERVER_TAG);
-
+        ParamUtils.checkParam(tag);
         ConfigChangeBatchListenResponse configChangeBatchListenResponse = new ConfigChangeBatchListenResponse();
         for (ConfigBatchListenRequest.ConfigListenContext listenContext : configChangeListenRequest
                 .getConfigListenContexts()) {

diff --git a/...in/java/com/alibaba/nacos/config/server/remote/ConfigChangeClusterSyncRequestHandler.java b/...in/java/com/alibaba/nacos/config/server/remote/ConfigChangeClusterSyncRequestHandler.java
@@ -22,6 +22,7 @@
 import com.alibaba.nacos.api.remote.request.RequestMeta;
 import com.alibaba.nacos.config.server.service.dump.DumpRequest;
 import com.alibaba.nacos.config.server.service.dump.DumpService;
+import com.alibaba.nacos.config.server.utils.ParamUtils;
 import com.alibaba.nacos.core.paramcheck.ExtractorManager;
 import com.alibaba.nacos.core.paramcheck.impl.ConfigRequestParamExtractor;
 import com.alibaba.nacos.core.remote.RequestHandler;
@@ -49,7 +50,7 @@ public ConfigChangeClusterSyncRequestHandler(DumpService dumpService) {
     @ExtractorManager.Extractor(rpcExtractor = ConfigRequestParamExtractor.class)
     public ConfigChangeClusterSyncResponse handle(ConfigChangeClusterSyncRequest configChangeSyncRequest,
             RequestMeta meta) throws NacosException {
-
+        ParamUtils.checkParam(configChangeSyncRequest.getTag());
         DumpRequest dumpRequest = DumpRequest.create(configChangeSyncRequest.getDataId(),
                 configChangeSyncRequest.getGroup(), configChangeSyncRequest.getTenant(),
                 configChangeSyncRequest.getLastModified(), meta.getClientIp());

diff --git a/config/src/main/java/com/alibaba/nacos/config/server/remote/ConfigQueryRequestHandler.java b/config/src/main/java/com/alibaba/nacos/config/server/remote/ConfigQueryRequestHandler.java
@@ -29,6 +29,7 @@
 import com.alibaba.nacos.config.server.service.trace.ConfigTraceService;
 import com.alibaba.nacos.config.server.utils.GroupKey2;
 import com.alibaba.nacos.config.server.utils.LogUtil;
+import com.alibaba.nacos.config.server.utils.ParamUtils;
 import com.alibaba.nacos.config.server.utils.TimeUtils;
 import com.alibaba.nacos.core.control.TpsControl;
 import com.alibaba.nacos.core.paramcheck.ExtractorManager;
@@ -83,7 +84,8 @@ private ConfigQueryResponse getContext(ConfigQueryRequest configQueryRequest, Re
         String autoTag = configQueryRequest.getHeader(com.alibaba.nacos.api.common.Constants.VIPSERVER_TAG);
         String requestIpApp = meta.getLabels().get(CLIENT_APPNAME_HEADER);
         String acceptCharset = ENCODE_UTF8;
-
+        ParamUtils.checkParam(tag);
+        ParamUtils.checkParam(autoTag);
         int lockResult = ConfigCacheService.tryConfigReadLock(groupKey);
         String pullEvent = ConfigTraceService.PULL_EVENT;
         String pullType = ConfigTraceService.PULL_TYPE_OK;

diff --git a/...src/main/java/com/alibaba/nacos/config/server/service/dump/disk/ConfigRawDiskService.java b/...src/main/java/com/alibaba/nacos/config/server/service/dump/disk/ConfigRawDiskService.java
@@ -16,10 +16,13 @@
 
 package com.alibaba.nacos.config.server.service.dump.disk;
 
+import com.alibaba.nacos.api.exception.NacosException;
+import com.alibaba.nacos.api.exception.runtime.NacosRuntimeException;
 import com.alibaba.nacos.api.utils.StringUtils;
 import com.alibaba.nacos.common.pathencoder.PathEncoderManager;
 import com.alibaba.nacos.common.utils.IoUtils;
 import com.alibaba.nacos.config.server.utils.LogUtil;
+import com.alibaba.nacos.config.server.utils.ParamUtils;
 import com.alibaba.nacos.sys.env.EnvUtil;
 import org.apache.commons.io.FileUtils;
 
@@ -65,7 +68,12 @@ public void saveToDisk(String dataId, String group, String tenant, String conten
     /**
      * Returns the path of the server cache file.
      */
-    private static File targetFile(String dataId, String group, String tenant) {
+    static File targetFile(String dataId, String group, String tenant) {
+        try {
+            ParamUtils.checkParam(dataId, group, tenant);
+        } catch (Exception e) {
+            throw new NacosRuntimeException(NacosException.CLIENT_INVALID_PARAM, "parameter is invalid.");
+        }
         // fix https://github.com/alibaba/nacos/issues/10067
         dataId = PathEncoderManager.getInstance().encode(dataId);
         group = PathEncoderManager.getInstance().encode(group);
@@ -86,6 +94,11 @@ private static File targetFile(String dataId, String group, String tenant) {
      * Returns the path of cache file in server.
      */
     private static File targetBetaFile(String dataId, String group, String tenant) {
+        try {
+            ParamUtils.checkParam(dataId, group, tenant);
+        } catch (Exception e) {
+            throw new NacosRuntimeException(NacosException.CLIENT_INVALID_PARAM, "parameter is invalid.");
+        }
         // fix https://github.com/alibaba/nacos/issues/10067
         dataId = PathEncoderManager.getInstance().encode(dataId);
         group = PathEncoderManager.getInstance().encode(group);
@@ -106,6 +119,12 @@ private static File targetBetaFile(String dataId, String group, String tenant) {
      * Returns the path of the tag cache file in server.
      */
     private static File targetTagFile(String dataId, String group, String tenant, String tag) {
+        try {
+            ParamUtils.checkParam(tag);
+            ParamUtils.checkParam(dataId, group, tenant);
+        } catch (Exception e) {
+            throw new NacosRuntimeException(NacosException.CLIENT_INVALID_PARAM, "parameter is invalid.");
+        }
         // fix https://github.com/alibaba/nacos/issues/10067
         dataId = PathEncoderManager.getInstance().encode(dataId);
         group = PathEncoderManager.getInstance().encode(group);

diff --git a/config/src/main/java/com/alibaba/nacos/config/server/utils/ParamUtils.java b/config/src/main/java/com/alibaba/nacos/config/server/utils/ParamUtils.java
@@ -16,14 +16,14 @@
 
 package com.alibaba.nacos.config.server.utils;
 
-import java.util.Map;
-
 import com.alibaba.nacos.api.exception.NacosException;
 import com.alibaba.nacos.api.exception.api.NacosApiException;
 import com.alibaba.nacos.api.model.v2.ErrorCode;
 import com.alibaba.nacos.common.utils.StringUtils;
 import org.springframework.http.HttpStatus;
 
+import java.util.Map;
+
 /**
  * Parameter validity check util.
  *
@@ -100,6 +100,25 @@ public static void checkParam(String dataId, String group, String datumId, Strin
         }
     }
 
+    /**
+     * Check Config basic Parameters.
+     *
+     * @param dataId data Id
+     * @param group  group name
+     * @param namespaceId namespace Id
+     */
+    public static void checkParam(String dataId, String group, String namespaceId) throws NacosApiException {
+        if (StringUtils.isBlank(dataId) || !isValid(dataId.trim())) {
+            throw new NacosApiException(HttpStatus.BAD_REQUEST.value(), ErrorCode.PARAMETER_VALIDATE_ERROR,
+                    "invalid dataId : " + dataId);
+        }
+        if (StringUtils.isBlank(group) || !isValid(group)) {
+            throw new NacosApiException(HttpStatus.BAD_REQUEST.value(), ErrorCode.PARAMETER_VALIDATE_ERROR,
+                    "invalid group : " + group);
+        }
+        checkTenantV2(namespaceId);
+    }
+
     /**
      * Check the tag for [v1].
      */

diff --git a/...test/java/com/alibaba/nacos/config/server/service/dump/disk/ConfigRawDiskServiceTest.java b/...test/java/com/alibaba/nacos/config/server/service/dump/disk/ConfigRawDiskServiceTest.java
@@ -16,6 +16,7 @@
 
 package com.alibaba.nacos.config.server.service.dump.disk;
 
+import com.alibaba.nacos.api.exception.runtime.NacosRuntimeException;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -26,6 +27,7 @@
 import java.nio.file.Paths;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 class ConfigRawDiskServiceTest {
 
@@ -45,9 +47,10 @@ private boolean isWindows() {
      */
     @Test
     void testTargetFile() throws NoSuchMethodException, IllegalAccessException, InvocationTargetException {
-        Method method = ConfigRawDiskService.class.getDeclaredMethod("targetFile", String.class, String.class, String.class);
+        Method method = ConfigRawDiskService.class.getDeclaredMethod("targetFile", String.class, String.class,
+                String.class);
         method.setAccessible(true);
-        File result = (File) method.invoke(null, "aaaa?dsaknkf", "aaaa*dsaknkf", "aaaa:dsaknkf");
+        File result = (File) method.invoke(null, "aaaa-dsaknkf", "aaaa.dsaknkf", "aaaa:dsaknkf");
         // 分解路径
         Path path = Paths.get(result.getPath());
         Path parent = path.getParent();
@@ -56,19 +59,27 @@ void testTargetFile() throws NoSuchMethodException, IllegalAccessException, Invo
         String lastSegment = path.getFileName().toString();
         String secondLastSegment = parent.getFileName().toString();
         String thirdLastSegment = grandParent.getFileName().toString();
-        assertEquals(isWindows() ? "aaaa%A3%dsaknkf" : thirdLastSegment, thirdLastSegment);
-        assertEquals(isWindows() ? "aaaa%A4%dsaknkf" : secondLastSegment, secondLastSegment);
+        assertEquals(isWindows() ? "aaaa-dsaknkf" : thirdLastSegment, thirdLastSegment);
+        assertEquals(isWindows() ? "aaaa.dsaknkf" : secondLastSegment, secondLastSegment);
         assertEquals(isWindows() ? "aaaa%A5%dsaknkf" : lastSegment, lastSegment);
     }
 
+    @Test
+    void testTargetFileWithInvalidParam() {
+        assertThrows(NacosRuntimeException.class, () -> ConfigRawDiskService.targetFile("../aaa", "testG", "testNS"));
+        assertThrows(NacosRuntimeException.class, () -> ConfigRawDiskService.targetFile("testD", "../aaa", "testNS"));
+        assertThrows(NacosRuntimeException.class, () -> ConfigRawDiskService.targetFile("testD", "testG", "../aaa"));
+    }
+
     /**
      * 测试获取beta文件路径.
      */
     @Test
     void testTargetBetaFile() throws NoSuchMethodException, IllegalAccessException, InvocationTargetException {
-        Method method = ConfigRawDiskService.class.getDeclaredMethod("targetBetaFile", String.class, String.class, String.class);
+        Method method = ConfigRawDiskService.class.getDeclaredMethod("targetBetaFile", String.class, String.class,
+                String.class);
         method.setAccessible(true);
-        File result = (File) method.invoke(null, "aaaa?dsaknkf", "aaaa*dsaknkf", "aaaa:dsaknkf");
+        File result = (File) method.invoke(null, "aaaa-dsaknkf", "aaaa.dsaknkf", "aaaa:dsaknkf");
         // 分解路径
         Path path = Paths.get(result.getPath());
         Path parent = path.getParent();
@@ -77,10 +88,19 @@ void testTargetBetaFile() throws NoSuchMethodException, IllegalAccessException,
         String lastSegment = path.getFileName().toString();
         String secondLastSegment = parent.getFileName().toString();
         String thirdLastSegment = grandParent.getFileName().toString();
-        assertEquals(isWindows() ? "aaaa%A3%dsaknkf" : thirdLastSegment, thirdLastSegment);
-        assertEquals(isWindows() ? "aaaa%A4%dsaknkf" : secondLastSegment, secondLastSegment);
+        assertEquals(isWindows() ? "aaaa-dsaknkf" : thirdLastSegment, thirdLastSegment);
+        assertEquals(isWindows() ? "aaaa.dsaknkf" : secondLastSegment, secondLastSegment);
         assertEquals(isWindows() ? "aaaa%A5%dsaknkf" : lastSegment, lastSegment);
-
+    }
+
+    @Test
+    void testTargetBetaFileWithInvalidParam() throws NoSuchMethodException {
+        Method method = ConfigRawDiskService.class.getDeclaredMethod("targetBetaFile", String.class, String.class,
+                String.class);
+        method.setAccessible(true);
+        assertThrows(InvocationTargetException.class, () -> method.invoke(null, "../aaa", "testG", "testNS"));
+        assertThrows(InvocationTargetException.class, () -> method.invoke(null, "testD", "../aaa", "testNS"));
+        assertThrows(InvocationTargetException.class, () -> method.invoke(null, "testD", "testG", "../aaa"));
     }
 
     /**
@@ -92,10 +112,10 @@ void testTargetBetaFile() throws NoSuchMethodException, IllegalAccessException,
      */
     @Test
     void testTargetTagFile() throws NoSuchMethodException, IllegalAccessException, InvocationTargetException {
-        Method method = ConfigRawDiskService.class.getDeclaredMethod("targetTagFile", String.class, String.class, String.class,
-                String.class);
+        Method method = ConfigRawDiskService.class.getDeclaredMethod("targetTagFile", String.class, String.class,
+                String.class, String.class);
         method.setAccessible(true);
-        File result = (File) method.invoke(null, "aaaa?dsaknkf", "aaaa*dsaknkf", "aaaa:dsaknkf", "aaaadsaknkf");
+        File result = (File) method.invoke(null, "aaaa-dsaknkf", "aaaa.dsaknkf", "aaaa:dsaknkf", "aaaa_dsaknkf");
         // 分解路径
         Path path = Paths.get(result.getPath());
         Path parent = path.getParent();
@@ -105,10 +125,23 @@ void testTargetTagFile() throws NoSuchMethodException, IllegalAccessException, I
         String secondLastSegment = parent.getFileName().toString();
         String thirdLastSegment = grandParent.getFileName().toString();
         String fourthLastSegment = greatGrandParent.getFileName().toString();
-        assertEquals(isWindows() ? "aaaa%A3%dsaknkf" : fourthLastSegment, fourthLastSegment);
-        assertEquals(isWindows() ? "aaaa%A4%dsaknkf" : thirdLastSegment, thirdLastSegment);
+        assertEquals(isWindows() ? "aaaa-dsaknkf" : fourthLastSegment, fourthLastSegment);
+        assertEquals(isWindows() ? "aaaa.dsaknkf" : thirdLastSegment, thirdLastSegment);
         assertEquals(isWindows() ? "aaaa%A5%dsaknkf" : secondLastSegment, secondLastSegment);
         String lastSegment = path.getFileName().toString();
-        assertEquals("aaaadsaknkf", lastSegment);
+        assertEquals("aaaa_dsaknkf", lastSegment);
+    }
+
+    @Test
+    void testTargetTagFileWithInvalidParam() throws NoSuchMethodException {
+        Method method = ConfigRawDiskService.class.getDeclaredMethod("targetTagFile", String.class, String.class,
+                String.class, String.class);
+        method.setAccessible(true);
+        assertThrows(InvocationTargetException.class,
+                () -> method.invoke(null, "../aaa", "testG", "testNS", "testTag"));
+        assertThrows(InvocationTargetException.class,
+                () -> method.invoke(null, "testD", "../aaa", "testNS", "testTag"));
+        assertThrows(InvocationTargetException.class, () -> method.invoke(null, "testD", "testG", "../aaa", "testTag"));
+        assertThrows(InvocationTargetException.class, () -> method.invoke(null, "testD", "testG", "testNS", "../aaa"));
     }
 }
diff --git a/config/src/test/java/com/alibaba/nacos/config/server/utils/ParamUtilsTest.java b/config/src/test/java/com/alibaba/nacos/config/server/utils/ParamUtilsTest.java
@@ -17,12 +17,16 @@
 package com.alibaba.nacos.config.server.utils;
 
 import com.alibaba.nacos.api.exception.NacosException;
+import com.alibaba.nacos.api.exception.api.NacosApiException;
 import org.junit.jupiter.api.Test;
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.UUID;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
@@ -289,4 +293,13 @@ void testCheckTenant() {
         }
     }
 
+    @Test
+    void testCheckParamWithNamespaceGroupDataId() {
+        assertThrows(NacosApiException.class, () -> ParamUtils.checkParam("../", "group", ""));
+        assertThrows(NacosApiException.class, () -> ParamUtils.checkParam("dataId", "../", ""));
+        assertThrows(NacosApiException.class, () -> ParamUtils.checkParam("dataId", "group", "../"));
+        assertDoesNotThrow(() -> ParamUtils.checkParam("dataId", "group", ""));
+        assertDoesNotThrow(() -> ParamUtils.checkParam("dataId", "group", UUID.randomUUID().toString()));
+    }
+
 }