Allow removing trailing slashes (#1100)

This commit is contained in:
Olga Maciaszek-Sharma
2024-10-10 15:13:34 +02:00
committed by GitHub
parent b56dc8dbf4
commit 0f55e2d303
9 changed files with 147 additions and 13 deletions

View File

@@ -288,7 +288,12 @@ public class CustomConfiguration {
}
----
TIP: By default, Feign clients do not encode slash `/` characters. You can change this behaviour, by setting the value of `spring.cloud.openfeign.client.decodeSlash` to `false`.
TIP: By default, Feign clients do not encode slash `/` characters. You can change this behaviour, by setting the value of `spring.cloud.openfeign.client.decode-slash` to `false`.
TIP: By default, Feign clients do not remove trailing slash `/` characters from the request path.
You can change this behaviour, by setting the value of `spring.cloud.openfeign.client.remove-trailing-slash` to `true`.
Trailing slash removal from the request path is going to be made the default behaviour in the next major release.
[[springencoder-configuration]]
==== `SpringEncoder` configuration

View File

@@ -73,6 +73,7 @@
|spring.cloud.openfeign.client.default-config | `+++default+++` |
|spring.cloud.openfeign.client.default-to-properties | `+++true+++` |
|spring.cloud.openfeign.client.refresh-enabled | `+++false+++` | Enables options value refresh capability for Feign.
|spring.cloud.openfeign.client.remove-trailing-slash | `+++false+++` | If {@code true}, trailing slashes at the end of request urls will be removed.
|spring.cloud.openfeign.compression.request.content-encoding-types | | The list of content encodings (applicable encodings depend on the used client).
|spring.cloud.openfeign.compression.request.enabled | `+++false+++` | Enables the request sent by Feign to be compressed.
|spring.cloud.openfeign.compression.request.mime-types | `+++[text/xml, application/xml, application/json]+++` | The list of supported mime types.

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2013-2023 the original author or authors.
* Copyright 2013-2024 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.
@@ -61,6 +61,11 @@ public class FeignClientProperties {
*/
private boolean decodeSlash = true;
/**
* If {@code true}, trailing slashes at the end of request urls will be removed.
*/
private boolean removeTrailingSlash;
public boolean isDefaultToProperties() {
return defaultToProperties;
}
@@ -93,6 +98,14 @@ public class FeignClientProperties {
this.decodeSlash = decodeSlash;
}
public boolean isRemoveTrailingSlash() {
return removeTrailingSlash;
}
public void setRemoveTrailingSlash(boolean removeTrailingSlash) {
this.removeTrailingSlash = removeTrailingSlash;
}
@Override
public boolean equals(Object o) {
if (this == o) {
@@ -103,12 +116,13 @@ public class FeignClientProperties {
}
FeignClientProperties that = (FeignClientProperties) o;
return defaultToProperties == that.defaultToProperties && Objects.equals(defaultConfig, that.defaultConfig)
&& Objects.equals(config, that.config) && Objects.equals(decodeSlash, that.decodeSlash);
&& Objects.equals(config, that.config) && Objects.equals(decodeSlash, that.decodeSlash)
&& Objects.equals(removeTrailingSlash, that.removeTrailingSlash);
}
@Override
public int hashCode() {
return Objects.hash(defaultToProperties, defaultConfig, config, decodeSlash);
return Objects.hash(defaultToProperties, defaultConfig, config, decodeSlash, removeTrailingSlash);
}
/**

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2013-2022 the original author or authors.
* Copyright 2013-2024 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.
@@ -145,8 +145,7 @@ public class FeignClientsConfiguration {
@Bean
@ConditionalOnMissingBean
public Contract feignContract(ConversionService feignConversionService) {
boolean decodeSlash = feignClientProperties == null || feignClientProperties.isDecodeSlash();
return new SpringMvcContract(parameterProcessors, feignConversionService, decodeSlash);
return new SpringMvcContract(parameterProcessors, feignConversionService, feignClientProperties);
}
@Bean

View File

@@ -118,6 +118,9 @@ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, ResourceLo
if (!url.contains("://")) {
url = "http://" + url;
}
if (url.endsWith("/")) {
url = url.substring(0, url.length() - 1);
}
try {
new URL(url);
}

View File

@@ -41,6 +41,7 @@ import org.apache.commons.logging.LogFactory;
import org.springframework.cloud.openfeign.AnnotatedParameterProcessor;
import org.springframework.cloud.openfeign.CollectionFormat;
import org.springframework.cloud.openfeign.FeignClientProperties;
import org.springframework.cloud.openfeign.SpringQueryMap;
import org.springframework.cloud.openfeign.annotation.CookieValueParameterProcessor;
import org.springframework.cloud.openfeign.annotation.MatrixVariableParameterProcessor;
@@ -115,6 +116,8 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource
private final boolean decodeSlash;
private final boolean removeTrailingSlash;
public SpringMvcContract() {
this(Collections.emptyList());
}
@@ -128,8 +131,36 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource
this(annotatedParameterProcessors, conversionService, true);
}
/**
* Creates a {@link SpringMvcContract} based on annotatedParameterProcessors,
* conversionService and decodeSlash value.
* @param annotatedParameterProcessors list of {@link AnnotatedParameterProcessor}
* objects used to resolve parameters
* @param conversionService {@link ConversionService} used for type conversion
* @param decodeSlash indicates whether slashes should be decoded
* @deprecated in favour of
* {@link SpringMvcContract#SpringMvcContract(List, ConversionService, FeignClientProperties)}
*/
@Deprecated
public SpringMvcContract(List<AnnotatedParameterProcessor> annotatedParameterProcessors,
ConversionService conversionService, boolean decodeSlash) {
this(annotatedParameterProcessors, conversionService, decodeSlash, false);
}
/**
* Creates a {@link SpringMvcContract} based on annotatedParameterProcessors,
* conversionService and decodeSlash value.
* @param annotatedParameterProcessors list of {@link AnnotatedParameterProcessor}
* objects used to resolve parameters
* @param conversionService {@link ConversionService} used for type conversion
* @param decodeSlash indicates whether slashes should be decoded
* @param removeTrailingSlash indicates whether trailing slashes should be removed
* @deprecated in favour of
* {@link SpringMvcContract#SpringMvcContract(List, ConversionService, FeignClientProperties)}
*/
@Deprecated
public SpringMvcContract(List<AnnotatedParameterProcessor> annotatedParameterProcessors,
ConversionService conversionService, boolean decodeSlash, boolean removeTrailingSlash) {
Assert.notNull(annotatedParameterProcessors, "Parameter processors can not be null.");
Assert.notNull(conversionService, "ConversionService can not be null.");
@@ -140,6 +171,14 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource
this.conversionService = conversionService;
convertingExpanderFactory = new ConvertingExpanderFactory(conversionService);
this.decodeSlash = decodeSlash;
this.removeTrailingSlash = removeTrailingSlash;
}
public SpringMvcContract(List<AnnotatedParameterProcessor> annotatedParameterProcessors,
ConversionService conversionService, FeignClientProperties feignClientProperties) {
this(annotatedParameterProcessors, conversionService,
feignClientProperties == null || feignClientProperties.isDecodeSlash(),
feignClientProperties != null && feignClientProperties.isRemoveTrailingSlash());
}
private static TypeDescriptor createTypeDescriptor(Method method, int paramIndex) {
@@ -229,6 +268,9 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource
if (!pathValue.startsWith("/") && !data.template().path().endsWith("/")) {
pathValue = "/" + pathValue;
}
if (removeTrailingSlash && pathValue.endsWith("/")) {
pathValue = pathValue.substring(0, pathValue.length() - 1);
}
data.template().uri(pathValue, true);
if (data.template().decodeSlash() != decodeSlash) {
data.template().decodeSlash(decodeSlash);

View File

@@ -131,7 +131,7 @@ class FeignClientBuilderTests {
assertFactoryBeanField(builder, "contextId", "TestContext");
// and:
assertFactoryBeanField(builder, "url", "http://Url/");
assertFactoryBeanField(builder, "url", "http://Url");
assertFactoryBeanField(builder, "path", "/Path");
assertFactoryBeanField(builder, "dismiss404", true);
@@ -155,7 +155,7 @@ class FeignClientBuilderTests {
assertFactoryBeanField(builder, "contextId", "TestContext");
// and:
assertFactoryBeanField(builder, "url", "http://Url/");
assertFactoryBeanField(builder, "url", "http://Url");
assertFactoryBeanField(builder, "path", "/Path");
assertFactoryBeanField(builder, "dismiss404", true);
List<FeignBuilderCustomizer> additionalCustomizers = getFactoryBeanField(builder, "additionalCustomizers");

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2013-2022 the original author or authors.
* Copyright 2013-2024 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.
@@ -89,6 +89,12 @@ class FeignClientsRegistrarTests {
return registrar.getName(Collections.singletonMap("name", name));
}
@Test
void testRemoveTrailingSlashFromUrl() {
String url = FeignClientsRegistrar.getUrl("http://localhost/");
assertThat(url).isEqualTo("http://localhost");
}
@Test
void testFallback() {
assertThatExceptionOfType(IllegalArgumentException.class)

View File

@@ -36,6 +36,7 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.cloud.openfeign.CollectionFormat;
import org.springframework.cloud.openfeign.FeignClientProperties;
import org.springframework.cloud.openfeign.SpringQueryMap;
import org.springframework.core.convert.ConversionService;
import org.springframework.data.domain.Page;
@@ -189,8 +190,23 @@ class SpringMvcContractTests {
}
@Test
void testProcessAnnotations_SimplePathIsOnlyASlash() throws Exception {
Method method = TestTemplate_Simple.class.getDeclaredMethod("getSlashPath", String.class);
void testProcessAnnotations_SimplePathIsOnlyASlashWithParam() throws Exception {
Method method = TestTemplate_Simple.class.getDeclaredMethod("getSlashPathWithParam", String.class);
MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
assertThat(data.template().url()).isEqualTo("/?id=" + "{id}");
assertThat(data.template().method()).isEqualTo("GET");
assertThat(data.template().headers().get("Accept").iterator().next())
.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
}
@Test
void testProcessAnnotations_SimplePathIsOnlyASlashWithParamWithTrailingSlashRemoval() throws Exception {
FeignClientProperties properties = new FeignClientProperties();
properties.setRemoveTrailingSlash(true);
contract = new SpringMvcContract(Collections.emptyList(), getConversionService(), properties);
Method method = TestTemplate_Simple.class.getDeclaredMethod("getSlashPathWithParam", String.class);
MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
assertThat(data.template().url()).isEqualTo("/?id=" + "{id}");
@@ -284,6 +300,48 @@ class SpringMvcContractTests {
}
@Test
void testProcessAnnotations_SimplePathIsOnlyASlashWithTrailingSlashRemoval() throws Exception {
FeignClientProperties properties = new FeignClientProperties();
properties.setRemoveTrailingSlash(true);
contract = new SpringMvcContract(Collections.emptyList(), getConversionService(), properties);
Method method = TestTemplate_Simple.class.getDeclaredMethod("getSlashPath");
MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
assertThat(data.template().url()).isEqualTo("/");
assertThat(data.template().method()).isEqualTo("GET");
assertThat(data.template().headers().get("Accept").iterator().next())
.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
}
@Test
void testProcessAnnotations_SimplePathHasTrailingSlash() throws Exception {
Method method = TestTemplate_Simple.class.getDeclaredMethod("getTrailingSlash");
MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
assertThat(data.template().url()).isEqualTo("/test1/test2/");
assertThat(data.template().method()).isEqualTo("GET");
assertThat(data.template().headers().get("Accept").iterator().next())
.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
}
@Test
void testProcessAnnotations_SimplePathHasTrailingSlashWithTrailingSlashRemoval() throws Exception {
FeignClientProperties properties = new FeignClientProperties();
properties.setRemoveTrailingSlash(true);
contract = new SpringMvcContract(Collections.emptyList(), getConversionService(), properties);
Method method = TestTemplate_Simple.class.getDeclaredMethod("getTrailingSlash");
MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
assertThat(data.template().url()).isEqualTo("/test1/test2");
assertThat(data.template().method()).isEqualTo("GET");
assertThat(data.template().headers().get("Accept").iterator().next())
.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
}
@Test
void testProcessAnnotationsOnMethod_Advanced() throws Exception {
Method method = TestTemplate_Advanced.class.getDeclaredMethod("getTest", String.class, String.class,
@@ -738,7 +796,13 @@ class SpringMvcContractTests {
TestObject postMappingTest(@RequestBody TestObject object);
@GetMapping(value = "/", produces = MediaType.APPLICATION_JSON_VALUE)
ResponseEntity<TestObject> getSlashPath(@RequestParam("id") String id);
ResponseEntity<TestObject> getSlashPathWithParam(@RequestParam("id") String id);
@GetMapping(value = "/", produces = MediaType.APPLICATION_JSON_VALUE)
ResponseEntity<TestObject> getSlashPath();
@GetMapping(value = "test1/test2/", produces = MediaType.APPLICATION_JSON_VALUE)
ResponseEntity<TestObject> getTrailingSlash();
@GetMapping(path = "test", produces = MediaType.APPLICATION_JSON_VALUE)
ResponseEntity<TestObject> getTestNoLeadingSlash(@RequestParam("name") String name);