diff --git a/config/src/main/java/com/alibaba/nacos/config/server/remote/ConfigChangeBatchListenRequestHandler.java b/config/src/main/java/com/alibaba/nacos/config/server/remote/ConfigChangeBatchListenRequestHandler.java index f7bc8a55d8a..4583af292b1 100644 --- a/config/src/main/java/com/alibaba/nacos/config/server/remote/ConfigChangeBatchListenRequestHandler.java +++ b/config/src/main/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/config/src/main/java/com/alibaba/nacos/config/server/remote/ConfigChangeClusterSyncRequestHandler.java b/config/src/main/java/com/alibaba/nacos/config/server/remote/ConfigChangeClusterSyncRequestHandler.java index cc284f1db2c..a806ee25621 100644 --- a/config/src/main/java/com/alibaba/nacos/config/server/remote/ConfigChangeClusterSyncRequestHandler.java +++ b/config/src/main/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 index e1f0a93b23d..902da8ee5a7 100644 --- 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/config/src/main/java/com/alibaba/nacos/config/server/service/dump/disk/ConfigRawDiskService.java b/config/src/main/java/com/alibaba/nacos/config/server/service/dump/disk/ConfigRawDiskService.java index a9e34250710..09fa0052a7c 100644 --- a/config/src/main/java/com/alibaba/nacos/config/server/service/dump/disk/ConfigRawDiskService.java +++ b/config/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 index 5d4f633d73f..4a075a02eb3 100644 --- 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/config/src/test/java/com/alibaba/nacos/config/server/service/dump/disk/ConfigRawDiskServiceTest.java b/config/src/test/java/com/alibaba/nacos/config/server/service/dump/disk/ConfigRawDiskServiceTest.java index a0cce0f95f6..cf01eb366c0 100644 --- a/config/src/test/java/com/alibaba/nacos/config/server/service/dump/disk/ConfigRawDiskServiceTest.java +++ b/config/src/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 index 8b9b6c3ae16..0e6991e8f7c 100644 --- 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())); + } + }