SpringCloud/spring-cloud-openfeign
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix for Path variables with / are not url encoded

Browse Source 
# Conflicts:
#	spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientProperties.java
#	spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java
#	spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractIntegrationTests.java
#	spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java
This commit is contained in:
Ram Anaswara
2020-11-15 15:39:57 +00:00
committed by Olga Maciaszek-Sharma
parent 64d3f05e70
commit d419826ca0
7 changed files with 301 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientProperties.java
View File

@@ -36,6 +36,7 @@ import org.springframework.boot.context.properties.ConfigurationProperties;
/**
* @author Eko Kurniawan Khannedy
* @author Ilia Ilinykh
* @author Ram Anaswara
*/
@ConfigurationProperties("feign.client")
public class FeignClientProperties {
@@ -46,6 +47,12 @@ public class FeignClientProperties {
private Map<String, FeignClientConfiguration> config = new HashMap<>();
/**
* Feign clients do not encode slash `/` characters by default. To change this
* behavior, set the `decodeSlash` to `false`.
*/
private boolean decodeSlash = true;
public boolean isDefaultToProperties() {
return this.defaultToProperties;
}
@@ -70,6 +77,14 @@ public class FeignClientProperties {
this.config = config;
}
public boolean isDecodeSlash() {
return decodeSlash;
}
public void setDecodeSlash(boolean decodeSlash) {
this.decodeSlash = decodeSlash;
}
@Override
public boolean equals(Object o) {
if (this == o) {
@@ -81,12 +96,14 @@ public class FeignClientProperties {
FeignClientProperties that = (FeignClientProperties) o;
return this.defaultToProperties == that.defaultToProperties
&& Objects.equals(this.defaultConfig, that.defaultConfig)
&& Objects.equals(this.config, that.config);
&& Objects.equals(this.config, that.config)
&& Objects.equals(this.decodeSlash, that.decodeSlash);
}
@Override
public int hashCode() {
return Objects.hash(this.defaultToProperties, this.defaultConfig, this.config);
return Objects.hash(this.defaultToProperties, this.defaultConfig, this.config,
this.decodeSlash);
}
/**

8
spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java
View File

@@ -82,6 +82,9 @@ public class FeignClientsConfiguration {
@Autowired(required = false)
private SpringDataWebProperties springDataWebProperties;
@Autowired(required = false)
private FeignClientProperties feignClientProperties;
@Bean
@ConditionalOnMissingBean
public Decoder feignDecoder() {
@@ -118,7 +121,10 @@ public class FeignClientsConfiguration {
@Bean
@ConditionalOnMissingBean
public Contract feignContract(ConversionService feignConversionService) {
return new SpringMvcContract(this.parameterProcessors, feignConversionService);
boolean decodeSlash = feignClientProperties == null
|| feignClientProperties.isDecodeSlash();
return new SpringMvcContract(this.parameterProcessors, feignConversionService,
decodeSlash);
}
@Bean

16
spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java
View File

@@ -78,6 +78,7 @@ import static org.springframework.core.annotation.AnnotatedElementUtils.findMerg
* @author Aaron Whiteside
* @author Artyom Romanenko
* @author Darren Foong
* @author Ram Anaswara
*/
public class SpringMvcContract extends Contract.BaseContract
implements ResourceLoaderAware {
@@ -104,6 +105,8 @@ public class SpringMvcContract extends Contract.BaseContract
private ResourceLoader resourceLoader = new DefaultResourceLoader();
private boolean decodeSlash;
public SpringMvcContract() {
this(Collections.emptyList());
}
@@ -116,6 +119,12 @@ public class SpringMvcContract extends Contract.BaseContract
public SpringMvcContract(
List<AnnotatedParameterProcessor> annotatedParameterProcessors,
ConversionService conversionService) {
this(annotatedParameterProcessors, conversionService, true);
}
public SpringMvcContract(
List<AnnotatedParameterProcessor> annotatedParameterProcessors,
ConversionService conversionService, boolean decodeSlash) {
Assert.notNull(annotatedParameterProcessors,
"Parameter processors can not be null.");
Assert.notNull(conversionService, "ConversionService can not be null.");
@@ -126,6 +135,7 @@ public class SpringMvcContract extends Contract.BaseContract
annotatedArgumentProcessors = toAnnotatedArgumentProcessorMap(processors);
this.conversionService = conversionService;
convertingExpanderFactory = new ConvertingExpanderFactory(conversionService);
this.decodeSlash = decodeSlash;
}
private static TypeDescriptor createTypeDescriptor(Method method, int paramIndex) {
@@ -183,6 +193,9 @@ public class SpringMvcContract extends Contract.BaseContract
pathValue = "/" + pathValue;
}
data.template().uri(pathValue);
if (data.template().decodeSlash() != decodeSlash) {
data.template().decodeSlash(decodeSlash);
}
}
}
}
@@ -247,6 +260,9 @@ public class SpringMvcContract extends Contract.BaseContract
pathValue = "/" + pathValue;
}
data.template().uri(pathValue, true);
if (data.template().decodeSlash() != decodeSlash) {
data.template().decodeSlash(decodeSlash);
}
}
}

116
spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/AbstractSpringMvcContractIntegrationTests.java Normal file
View File

@@ -0,0 +1,116 @@
/*
* Copyright 2013-2020 the original author or 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
*
* https://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 org.springframework.cloud.openfeign.support;
import java.nio.charset.Charset;
import feign.Response;
import feign.codec.Decoder;
import feign.codec.Encoder;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.cloud.openfeign.EnableFeignClients;
import org.springframework.cloud.openfeign.FeignClient;
import org.springframework.cloud.openfeign.test.NoSecurityConfiguration;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;
import org.springframework.http.HttpHeaders;
import org.springframework.util.SocketUtils;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestHeader;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
/**
* Abstract class for the integration tests for {@link SpringMvcContract}.
*
* @author Ram Anaswara
*/
public class AbstractSpringMvcContractIntegrationTests {
@BeforeAll
public static void beforeClass() {
System.setProperty("server.port",
String.valueOf(SocketUtils.findAvailableTcpPort()));
}
@AfterAll
public static void afterClass() {
System.clearProperty("server.port");
}
protected String getUrlQueryParam(Response response) {
return response.request().requestTemplate().queries().get("url").stream()
.findFirst().orElseThrow(IllegalStateException::new);
}
@FeignClient(name = "test", url = "http://localhost:${server.port}/",
configuration = NoCodecsFeignConfiguration.class)
interface TestClient {
@PostMapping("/test")
Object sendMessage(@RequestBody String message,
@RequestHeader(HttpHeaders.CONTENT_TYPE) String acceptHeader);
@GetMapping("/get")
Object getMessage(@RequestParam String url);
}
@Configuration(proxyBeanMethods = false)
@EnableFeignClients(clients = TestClient.class)
@EnableAutoConfiguration
@RestController
@Import(NoSecurityConfiguration.class)
protected static class Config {
@PostMapping("/test")
Object sendMessage(@RequestBody String message,
@RequestHeader(HttpHeaders.CONTENT_TYPE) String acceptHeader) {
return message;
}
@GetMapping("/get")
Object getMessage(@RequestParam String url) {
return url;
}
}
// Avoid feign.codec.EncodeException - this feature works for users that override
// Encoder
protected static class NoCodecsFeignConfiguration {
@Bean
public Decoder decoder() {
return (response, type) -> response;
}
@Bean
public Encoder encoder() {
return (object, bodyType, request) -> request
.body(object.toString().getBytes(), Charset.defaultCharset());
}
}
}

84
spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractIntegrationTests.java
View File

@@ -16,101 +16,41 @@
package org.springframework.cloud.openfeign.support;
import java.nio.charset.Charset;
import feign.codec.Decoder;
import feign.codec.Encoder;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import feign.Response;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.cloud.openfeign.EnableFeignClients;
import org.springframework.cloud.openfeign.FeignClient;
import org.springframework.cloud.openfeign.test.NoSecurityConfiguration;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;
import org.springframework.http.HttpHeaders;
import org.springframework.util.SocketUtils;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestHeader;
import org.springframework.web.bind.annotation.RestController;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
/**
* Integration tests for {@link SpringMvcContract}
* Integration tests for {@link SpringMvcContract}.
*
* @author Olga Maciaszek-Sharma
* @author Ram Anaswara
*/
@SpringBootTest(classes = SpringMvcContractIntegrationTests.Config.class,
@SpringBootTest(classes = AbstractSpringMvcContractIntegrationTests.Config.class,
webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT)
public class SpringMvcContractIntegrationTests {
public class SpringMvcContractIntegrationTests
extends AbstractSpringMvcContractIntegrationTests {
@Autowired
private TestClient client;
@BeforeAll
public static void beforeClass() {
System.setProperty("server.port",
String.valueOf(SocketUtils.findAvailableTcpPort()));
}
@AfterAll
public static void afterClass() {
System.clearProperty("server.port");
}
@Test
public void shouldNotThrowInvalidMediaTypeExceptionWhenContentTypeTemplateUsed() {
assertThatCode(() -> client.sendMessage("test", "text/markdown"))
.doesNotThrowAnyException();
}
@FeignClient(name = "test", url = "http://localhost:${server.port}/",
configuration = NoCodecsFeignConfiguration.class)
interface TestClient {
@PostMapping("/test")
Object sendMessage(@RequestBody String message,
@RequestHeader(HttpHeaders.CONTENT_TYPE) String acceptHeader);
}
@Configuration(proxyBeanMethods = false)
@EnableFeignClients(clients = TestClient.class)
@EnableAutoConfiguration
@RestController
@Import(NoSecurityConfiguration.class)
protected static class Config {
@PostMapping("/test")
Object sendMessage(@RequestBody String message,
@RequestHeader(HttpHeaders.CONTENT_TYPE) String acceptHeader) {
return message;
}
}
// avoid feign.codec.EncodeException - this feature works for users that override
// Encoder
protected static class NoCodecsFeignConfiguration {
@Bean
public Decoder decoder() {
return (response, type) -> response;
}
@Bean
public Encoder encoder() {
return (object, bodyType, request) -> request
.body(object.toString().getBytes(), Charset.defaultCharset());
}
@Test
public void feignClientShouldPreserveSlash() {
Response response = (Response) client.getMessage("https://www.google.com");
String urlQueryParam = getUrlQueryParam(response);
assertThat(urlQueryParam).isEqualTo("https%3A//www.google.com");
}
}

49
spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractSlashEncodingIntegrationTests.java Normal file
View File

@@ -0,0 +1,49 @@
/*
* Copyright 2013-2020 the original author or 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
*
* https://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 org.springframework.cloud.openfeign.support;
import feign.Response;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import static org.assertj.core.api.Assertions.assertThat;
/**
* Integration tests for {@link SpringMvcContract}.
*
* @author Ram Anaswara
*/
@SpringBootTest(classes = SpringMvcContractSlashEncodingIntegrationTests.Config.class,
webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT,
properties = { "feign.client.decodeSlash=false" })
public class SpringMvcContractSlashEncodingIntegrationTests
extends AbstractSpringMvcContractIntegrationTests {
@Autowired
private TestClient client;
@Test
public void feignClientShouldNotDecodeEncodedSlash() {
Response response = (Response) client.getMessage("https://www.google.com");
String urlQueryParam = getUrlQueryParam(response);
assertThat(urlQueryParam).isEqualTo("https%3A%2F%2Fwww.google.com");
}
}

87
spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java
View File

@@ -118,11 +118,7 @@ public class SpringMvcContractTests {
@Before
public void setup() {
FormattingConversionServiceFactoryBean conversionServiceFactoryBean = new FormattingConversionServiceFactoryBean();
conversionServiceFactoryBean.afterPropertiesSet();
ConversionService conversionService = conversionServiceFactoryBean.getObject();
contract = new SpringMvcContract(Collections.emptyList(), conversionService);
contract = new SpringMvcContract(Collections.emptyList(), getConversionService());
}
@Test
@@ -136,6 +132,23 @@ public class SpringMvcContractTests {
assertThat(data.template().method()).isEqualTo("GET");
assertThat(data.template().headers().get("Accept").iterator().next())
.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
assertThat(data.template().decodeSlash()).isTrue();
}
@Test
public void testProcessAnnotationOnMethod_Simple_SlashEncoded() throws Exception {
contract = new SpringMvcContract(Collections.emptyList(), getConversionService(),
false);
Method method = TestTemplate_Simple.class.getDeclaredMethod("getTest",
String.class);
MethodMetadata data = contract
.parseAndValidateMetadata(method.getDeclaringClass(), method);
assertThat(data.template().url()).isEqualTo("/test/{id}");
assertThat(data.template().decodeSlash()).isFalse();
}
@Test
@@ -194,6 +207,24 @@ public class SpringMvcContractTests {
assertThat(data.template().method()).isEqualTo("GET");
assertThat(data.indexToName().get(0).iterator().next()).isEqualTo("classId");
assertThat(data.template().decodeSlash()).isTrue();
}
@Test
public void testProcessAnnotations_Class_AnnotationsGetAllTests_EncodeSlash()
throws Exception {
contract = new SpringMvcContract(Collections.emptyList(), getConversionService(),
false);
Method method = TestTemplate_Class_Annotations.class
.getDeclaredMethod("getAllTests", String.class);
MethodMetadata data = contract
.parseAndValidateMetadata(method.getDeclaringClass(), method);
assertThat(data.template().url()).isEqualTo("/prepend/{classId}");
assertThat(data.template().decodeSlash()).isFalse();
}
@Test
@@ -213,6 +244,30 @@ public class SpringMvcContractTests {
assertThat(data.indexToName().get(0).iterator().next())
.isEqualTo(data.indexToName().get(0).iterator().next());
assertThat(data.indexToName().get(0).iterator().next())
.isEqualTo(data.indexToName().get(0).iterator().next());
assertThat(data.template().decodeSlash()).isTrue();
}
@Test
public void testProcessAnnotations_ExtendedInterface_EncodeSlash() throws Exception {
contract = new SpringMvcContract(Collections.emptyList(), getConversionService(),
false);
Method extendedMethod = TestTemplate_Extended.class.getMethod("getAllTests",
String.class);
MethodMetadata extendedData = contract.parseAndValidateMetadata(
extendedMethod.getDeclaringClass(), extendedMethod);
Method method = TestTemplate_Class_Annotations.class
.getDeclaredMethod("getAllTests", String.class);
MethodMetadata data = contract
.parseAndValidateMetadata(method.getDeclaringClass(), method);
assertThat(data.template().url()).isEqualTo(extendedData.template().url());
assertThat(data.template().method()).isEqualTo(extendedData.template().method());
assertThat(data.template().decodeSlash()).isFalse();
}
@Test
@@ -390,6 +445,22 @@ public class SpringMvcContractTests {
assertThat(data.template().method()).isEqualTo("GET");
assertThat(data.template().headers().get("Accept").iterator().next())
.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
assertThat(data.template().decodeSlash()).isTrue();
}
@Test
public void testProcessAnnotations_Advanced3_DecodeSlashFlagNotModified()
throws Exception {
contract = new SpringMvcContract(Collections.emptyList(), getConversionService(),
false);
Method method = TestTemplate_Simple.class.getDeclaredMethod("getTest");
MethodMetadata data = contract
.parseAndValidateMetadata(method.getDeclaringClass(), method);
assertThat(data.template().url()).isEqualTo("/");
assertThat(data.template().decodeSlash()).isTrue();
}
@Test
@@ -608,6 +679,12 @@ public class SpringMvcContractTests {
assertThat(data.formParams()).contains("file", "id");
}
private ConversionService getConversionService() {
FormattingConversionServiceFactoryBean conversionServiceFactoryBean = new FormattingConversionServiceFactoryBean();
conversionServiceFactoryBean.afterPropertiesSet();
return conversionServiceFactoryBean.getObject();
}
public interface TestTemplate_Simple {
@RequestMapping(value = "/test/{id}", method = RequestMethod.GET,