Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix openapi item with url illegalKey 400 error #4549

Merged
merged 9 commits into from
Sep 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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;
nobodyiam marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -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<ItemDTO> findItemsByNamespace(@PathVariable("appId") String appId,
@PathVariable("clusterName") String clusterName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<String> 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();
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
nobodyiam marked this conversation as resolved.
Show resolved Hide resolved

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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<HttpGet> 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";
Expand Down Expand Up @@ -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<HttpPut> 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";
Expand Down Expand Up @@ -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<HttpDelete> 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";
Expand Down
Original file line number Diff line number Diff line change
@@ -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));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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<OpenItemDTO> findItemsByNamespace(@PathVariable String appId, @PathVariable String env,
@PathVariable String clusterName, @PathVariable String namespaceName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down
Loading