diff --git a/CHANGES.md b/CHANGES.md index 77cca6b1fbd..38c507b0b40 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -29,6 +29,7 @@ Apollo 2.1.0 * [Add a potential json value check feature](https://github.com/apolloconfig/apollo/pull/4519) * [Add index for table ReleaseHistory](https://github.com/apolloconfig/apollo/pull/4550) * [add an option to custom oidc userDisplayName](https://github.com/apolloconfig/apollo/pull/4507) +* [fix openapi item with url illegalKey 400 error](https://github.com/apolloconfig/apollo/pull/4549) ------------------ All issues and pull requests are [here](https://github.com/apolloconfig/apollo/milestone/11?closed=1) diff --git a/apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java b/apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java index 330c89b6b82..758e130be03 100644 --- a/apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java +++ b/apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java @@ -32,13 +32,13 @@ import com.ctrip.framework.apollo.common.exception.NotFoundException; import com.ctrip.framework.apollo.common.utils.BeanUtils; import com.ctrip.framework.apollo.core.utils.StringUtils; - +import java.nio.charset.StandardCharsets; +import java.util.Base64; import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.stream.Collectors; - import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.web.bind.annotation.DeleteMapping; @@ -238,6 +238,14 @@ public ItemDTO get(@PathVariable("appId") String appId, return BeanUtils.transform(ItemDTO.class, item); } + @GetMapping("/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/encodedItems/{key:.+}") + public ItemDTO getByEncodedKey(@PathVariable("appId") String appId, + @PathVariable("clusterName") String clusterName, + @PathVariable("namespaceName") String namespaceName, @PathVariable("key") String key) { + return this.get(appId, clusterName, namespaceName, + new String(Base64.getUrlDecoder().decode(key.getBytes(StandardCharsets.UTF_8)))); + } + @GetMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/items-with-page") public PageDTO findItemsByNamespace(@PathVariable("appId") String appId, @PathVariable("clusterName") String clusterName, diff --git a/apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/client/url/OpenApiPathBuilder.java b/apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/client/url/OpenApiPathBuilder.java index 34017fb45d4..72c94b07116 100644 --- a/apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/client/url/OpenApiPathBuilder.java +++ b/apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/client/url/OpenApiPathBuilder.java @@ -16,12 +16,16 @@ */ package com.ctrip.framework.apollo.openapi.client.url; + +import com.ctrip.framework.apollo.openapi.utils.UrlUtils; import com.google.common.base.Joiner; import com.google.common.base.Strings; import com.google.common.escape.Escaper; import com.google.common.net.UrlEscapers; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; +import java.util.Base64; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -34,11 +38,12 @@ public class OpenApiPathBuilder { private static final String CLUSTERS_PATH = "clusters"; private static final String NAMESPACES_PATH = "namespaces"; private static final String ITEMS_PATH = "items"; + private static final String ENCODED_ITEMS_PATH = "encodedItems"; private static final String RELEASE_PATH = "releases"; private final static List SORTED_PATH_KEYS = Arrays.asList(ENVS_PATH, ENV_PATH, APPS_PATH, CLUSTERS_PATH, - NAMESPACES_PATH, ITEMS_PATH, RELEASE_PATH); + NAMESPACES_PATH, ITEMS_PATH, ENCODED_ITEMS_PATH, RELEASE_PATH); private static final Escaper PATH_ESCAPER = UrlEscapers.urlPathSegmentEscaper(); private static final Escaper QUERY_PARAM_ESCAPER = UrlEscapers.urlFormParameterEscaper(); @@ -84,7 +89,12 @@ public OpenApiPathBuilder namespacesPathVal(String namespaces) { } public OpenApiPathBuilder itemsPathVal(String items) { - pathVariable.put(ITEMS_PATH, escapePath(items)); + if (UrlUtils.hasIllegalChar(items)) { + items = new String(Base64.getEncoder().encode(items.getBytes(StandardCharsets.UTF_8))); + pathVariable.put(ENCODED_ITEMS_PATH, escapePath(items)); + } else { + pathVariable.put(ITEMS_PATH, escapePath(items)); + } return this; } diff --git a/apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/utils/UrlUtils.java b/apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/utils/UrlUtils.java new file mode 100644 index 00000000000..ab93b6d42da --- /dev/null +++ b/apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/utils/UrlUtils.java @@ -0,0 +1,43 @@ +/* + * Copyright 2022 Apollo Authors + * + * Licensed 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 com.ctrip.framework.apollo.openapi.utils; + +import com.google.common.base.Strings; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * @author Abner + * @since 8/31/22 + */ +public final class UrlUtils { + + private static final String ILLEGAL_KEY_REGEX = "[/\\\\]+"; + private static final Pattern ILLEGAL_KEY_PATTERN = Pattern.compile(ILLEGAL_KEY_REGEX, + Pattern.MULTILINE); + + private UrlUtils() { + } + + public static boolean hasIllegalChar(String key) { + if (Strings.isNullOrEmpty(key)) { + return false; + } + Matcher matcher = ILLEGAL_KEY_PATTERN.matcher(key); + return matcher.find(); + } +} diff --git a/apollo-openapi/src/test/java/com/ctrip/framework/apollo/openapi/client/ApolloOpenApiClientTest.java b/apollo-openapi/src/test/java/com/ctrip/framework/apollo/openapi/client/ApolloOpenApiClientTest.java index 98dc923f704..607735404a4 100644 --- a/apollo-openapi/src/test/java/com/ctrip/framework/apollo/openapi/client/ApolloOpenApiClientTest.java +++ b/apollo-openapi/src/test/java/com/ctrip/framework/apollo/openapi/client/ApolloOpenApiClientTest.java @@ -16,8 +16,7 @@ */ package com.ctrip.framework.apollo.openapi.client; -import static org.junit.Assert.*; - +import static org.junit.Assert.assertEquals; import org.junit.Test; public class ApolloOpenApiClientTest { diff --git a/apollo-openapi/src/test/java/com/ctrip/framework/apollo/openapi/client/service/ItemOpenApiServiceTest.java b/apollo-openapi/src/test/java/com/ctrip/framework/apollo/openapi/client/service/ItemOpenApiServiceTest.java index a65f8fff5f2..81a24bc3456 100644 --- a/apollo-openapi/src/test/java/com/ctrip/framework/apollo/openapi/client/service/ItemOpenApiServiceTest.java +++ b/apollo-openapi/src/test/java/com/ctrip/framework/apollo/openapi/client/service/ItemOpenApiServiceTest.java @@ -21,8 +21,9 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; - import com.ctrip.framework.apollo.openapi.dto.OpenItemDTO; +import java.nio.charset.StandardCharsets; +import java.util.Base64; import org.apache.http.client.methods.HttpDelete; import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpPost; @@ -75,6 +76,26 @@ public void testGetItem() throws Exception { someAppId, someCluster, someNamespace, someKey), get.getURI().toString()); } + @Test + public void testGetItemByIllegalKey() throws Exception { + String someKey = "protocol//:host:port"; + + final ArgumentCaptor request = ArgumentCaptor.forClass(HttpGet.class); + + itemOpenApiService.getItem(someAppId, someEnv, someCluster, someNamespace, someKey); + + verify(httpClient, times(1)).execute(request.capture()); + + HttpGet get = request.getValue(); + + assertEquals(String + .format("%s/envs/%s/apps/%s/clusters/%s/namespaces/%s/encodedItems/%s", someBaseUrl, + someEnv, + someAppId, someCluster, someNamespace, + new String(Base64.getEncoder().encode(someKey.getBytes(StandardCharsets.UTF_8)))), + get.getURI().toString()); + } + @Test public void testGetNotExistedItem() throws Exception { String someKey = "someKey"; @@ -153,6 +174,33 @@ public void testUpdateItem() throws Exception { someNamespace, someKey), put.getURI().toString()); } + @Test + public void testUpdateItemByIllegalKey() throws Exception { + String someKey = "hello\\world"; + String someValue = "someValue"; + String someModifiedBy = "someModifiedBy"; + + OpenItemDTO itemDTO = new OpenItemDTO(); + itemDTO.setKey(someKey); + itemDTO.setValue(someValue); + itemDTO.setDataChangeLastModifiedBy(someModifiedBy); + + final ArgumentCaptor request = ArgumentCaptor.forClass(HttpPut.class); + + itemOpenApiService.updateItem(someAppId, someEnv, someCluster, someNamespace, itemDTO); + + verify(httpClient, times(1)).execute(request.capture()); + + HttpPut put = request.getValue(); + + assertEquals(String + .format("%s/envs/%s/apps/%s/clusters/%s/namespaces/%s/encodedItems/%s", someBaseUrl, + someEnv, someAppId, someCluster, + someNamespace, + new String(Base64.getEncoder().encode(someKey.getBytes(StandardCharsets.UTF_8)))), + put.getURI().toString()); + } + @Test(expected = RuntimeException.class) public void testUpdateItemWithError() throws Exception { String someKey = "someKey"; @@ -227,6 +275,27 @@ public void testRemoveItem() throws Exception { someAppId, someCluster, someNamespace, someKey, someOperator), delete.getURI().toString()); } + @Test + public void testRemoveItemByIllegalKey() throws Exception { + String someKey = "protocol//:host:port"; + String someOperator = "someOperator"; + + final ArgumentCaptor request = ArgumentCaptor.forClass(HttpDelete.class); + + itemOpenApiService.removeItem(someAppId, someEnv, someCluster, someNamespace, someKey, + someOperator); + + verify(httpClient, times(1)).execute(request.capture()); + + HttpDelete delete = request.getValue(); + + assertEquals( + String.format("%s/envs/%s/apps/%s/clusters/%s/namespaces/%s/encodedItems/%s?operator=%s", + someBaseUrl, someEnv, someAppId, someCluster, someNamespace, + new String(Base64.getEncoder().encode(someKey.getBytes(StandardCharsets.UTF_8))), + someOperator), delete.getURI().toString()); + } + @Test(expected = RuntimeException.class) public void testRemoveItemWithError() throws Exception { String someKey = "someKey"; diff --git a/apollo-openapi/src/test/java/com/ctrip/framework/apollo/openapi/client/utils/UrlUtilsTest.java b/apollo-openapi/src/test/java/com/ctrip/framework/apollo/openapi/client/utils/UrlUtilsTest.java new file mode 100644 index 00000000000..80dc0690db4 --- /dev/null +++ b/apollo-openapi/src/test/java/com/ctrip/framework/apollo/openapi/client/utils/UrlUtilsTest.java @@ -0,0 +1,55 @@ +/* + * Copyright 2022 Apollo Authors + * + * Licensed 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 com.ctrip.framework.apollo.openapi.client.utils; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import com.ctrip.framework.apollo.openapi.utils.UrlUtils; +import org.junit.Test; + +/** + * @author huanghousheng + * @since 9/7/22 + */ +public class UrlUtilsTest { + + @Test + public void testSlash() { + String someKey = "protocol//:host:port"; + assertTrue(UrlUtils.hasIllegalChar(someKey)); + } + + @Test + public void testBackslash() { + String someKey = "a\\c"; + assertTrue(UrlUtils.hasIllegalChar(someKey)); + } + + @Test + public void testNormalKey() { + String someKey = "someKey"; + assertFalse(UrlUtils.hasIllegalChar(someKey)); + } + + @Test + public void testDot() { + String someKey = "a.b"; + assertFalse(UrlUtils.hasIllegalChar(someKey)); + } + +} diff --git a/apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/ItemController.java b/apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/ItemController.java index 0a6a50895c5..7cdeb108719 100644 --- a/apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/ItemController.java +++ b/apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/ItemController.java @@ -26,6 +26,8 @@ import com.ctrip.framework.apollo.portal.environment.Env; import com.ctrip.framework.apollo.portal.service.ItemService; import com.ctrip.framework.apollo.portal.spi.UserService; +import java.nio.charset.StandardCharsets; +import java.util.Base64; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.validation.annotation.Validated; import org.springframework.web.bind.annotation.DeleteMapping; @@ -67,6 +69,14 @@ public OpenItemDTO getItem(@PathVariable String appId, @PathVariable String env, return this.itemOpenApiService.getItem(appId, env, clusterName, namespaceName, key); } + @GetMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/encodedItems/{key:.+}") + public OpenItemDTO getItemByEncodedKey(@PathVariable String appId, @PathVariable String env, + @PathVariable String clusterName, + @PathVariable String namespaceName, @PathVariable String key) { + return this.getItem(appId, env, clusterName, namespaceName, + new String(Base64.getDecoder().decode(key.getBytes(StandardCharsets.UTF_8)))); + } + @PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#request, #appId, #namespaceName, #env)") @PostMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/items") public OpenItemDTO createItem(@PathVariable String appId, @PathVariable String env, @@ -118,6 +128,17 @@ public void updateItem(@PathVariable String appId, @PathVariable String env, } } + @PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#request, #appId, #namespaceName, #env)") + @PutMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/encodedItems/{key:.+}") + public void updateItemByEncodedKey(@PathVariable String appId, @PathVariable String env, + @PathVariable String clusterName, @PathVariable String namespaceName, + @PathVariable String key, @RequestBody OpenItemDTO item, + @RequestParam(defaultValue = "false") boolean createIfNotExists, HttpServletRequest request) { + this.updateItem(appId, env, clusterName, namespaceName, + new String(Base64.getDecoder().decode(key.getBytes(StandardCharsets.UTF_8))), item, + createIfNotExists, request); + } + @PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#request, #appId, #namespaceName, #env)") @DeleteMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/items/{key:.+}") public void deleteItem(@PathVariable String appId, @PathVariable String env, @@ -137,6 +158,17 @@ public void deleteItem(@PathVariable String appId, @PathVariable String env, this.itemOpenApiService.removeItem(appId, env, clusterName, namespaceName, key, operator); } + @PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#request, #appId, #namespaceName, #env)") + @DeleteMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/encodedItems/{key:.+}") + public void deleteItemByEncodedKey(@PathVariable String appId, @PathVariable String env, + @PathVariable String clusterName, @PathVariable String namespaceName, + @PathVariable String key, @RequestParam String operator, + HttpServletRequest request) { + this.deleteItem(appId, env, clusterName, namespaceName, + new String(Base64.getDecoder().decode(key.getBytes(StandardCharsets.UTF_8))), operator, + request); + } + @GetMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/items") public OpenPageDTO findItemsByNamespace(@PathVariable String appId, @PathVariable String env, @PathVariable String clusterName, @PathVariable String namespaceName, diff --git a/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/api/AdminServiceAPI.java b/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/api/AdminServiceAPI.java index 9a586d557a8..692978d2967 100644 --- a/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/api/AdminServiceAPI.java +++ b/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/api/AdminServiceAPI.java @@ -20,6 +20,8 @@ import com.ctrip.framework.apollo.openapi.dto.OpenItemDTO; import com.ctrip.framework.apollo.portal.environment.Env; import com.google.common.base.Joiner; +import java.nio.charset.StandardCharsets; +import java.util.Base64; import org.springframework.boot.actuate.health.Health; import org.springframework.core.ParameterizedTypeReference; import org.springframework.http.HttpEntity; @@ -30,8 +32,11 @@ import org.springframework.util.CollectionUtils; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; - -import java.util.*; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Set; @Service @@ -187,6 +192,13 @@ public ItemDTO loadItem(Env env, String appId, String clusterName, String namesp ItemDTO.class, appId, clusterName, namespaceName, key); } + public ItemDTO loadItemByEncodeKey(Env env, String appId, String clusterName, String namespaceName, String key) { + return restTemplate.get(env, + "apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/encodedItems/{key}", + ItemDTO.class, appId, clusterName, namespaceName, + new String(Base64.getEncoder().encode(key.getBytes(StandardCharsets.UTF_8)))); + } + public ItemDTO loadItemById(Env env, long itemId) { return restTemplate.get(env, "items/{itemId}", ItemDTO.class, itemId); } diff --git a/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ItemService.java b/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ItemService.java index e125bc11e7d..64cfffa9ede 100644 --- a/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ItemService.java +++ b/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ItemService.java @@ -22,6 +22,7 @@ import com.ctrip.framework.apollo.common.exception.BadRequestException; import com.ctrip.framework.apollo.common.utils.BeanUtils; import com.ctrip.framework.apollo.core.enums.ConfigFileFormat; +import com.ctrip.framework.apollo.openapi.utils.UrlUtils; import com.ctrip.framework.apollo.openapi.dto.OpenItemDTO; import com.ctrip.framework.apollo.portal.api.AdminServiceAPI.ItemAPI; import com.ctrip.framework.apollo.portal.api.AdminServiceAPI.NamespaceAPI; @@ -169,6 +170,9 @@ public List findDeletedItems(String appId, Env env, String clusterName, } public ItemDTO loadItem(Env env, String appId, String clusterName, String namespaceName, String key) { + if (UrlUtils.hasIllegalChar(key)) { + return itemAPI.loadItemByEncodeKey(env, appId, clusterName, namespaceName, key); + } return itemAPI.loadItem(env, appId, clusterName, namespaceName, key); }