From 8c08204c781ecb425912937fafbb25284167b783 Mon Sep 17 00:00:00 2001 From: Can Bezmen Date: Fri, 11 Jun 2021 15:28:04 +0300 Subject: [PATCH 1/2] Issue #549 * Fixed: Form URL Encoded bug fixed. Fixes gh-549. --- .../MatrixVariableParameterProcessor.java | 3 ++- .../openfeign/support/SpringEncoder.java | 24 +++++++++++++------ .../openfeign/support/SpringEncoderTests.java | 13 ++++++++++ .../openfeign/valid/FeignClientTests.java | 17 +++++++++++++ 4 files changed, 49 insertions(+), 8 deletions(-) diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java index f0ee2793..c20b1a4d 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java @@ -76,7 +76,8 @@ public class MatrixVariableParameterProcessor implements AnnotatedParameterProce Map paramMap = (Map) object; return paramMap.keySet().stream().filter(key -> paramMap.get(key) != null) - .map(key -> ";" + key + "=" + paramMap.get(key).toString()).collect(Collectors.joining()); + .map(key -> ";" + key + "=" + paramMap.get(key).toString()) + .collect(Collectors.joining()); } } diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringEncoder.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringEncoder.java index 257ca87c..f71a020e 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringEncoder.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringEncoder.java @@ -99,8 +99,8 @@ public class SpringEncoder implements Encoder { requestContentType = MediaType.valueOf(type); } - if (isMultipartType(requestContentType)) { - this.springFormEncoder.encode(requestBody, bodyType, request); + if (isFormRelatedContentType(requestContentType)) { + springFormEncoder.encode(requestBody, bodyType, request); return; } else { @@ -117,7 +117,7 @@ public class SpringEncoder implements Encoder { private void encodeWithMessageConverter(Object requestBody, Type bodyType, RequestTemplate request, MediaType requestContentType) { - for (HttpMessageConverter messageConverter : this.messageConverters.getObject() + for (HttpMessageConverter messageConverter : messageConverters.getObject() .getConverters()) { FeignOutputMessage outputMessage; try { @@ -223,11 +223,21 @@ public class SpringEncoder implements Encoder { } } + private boolean isFormRelatedContentType(MediaType requestContentType) { + return isMultipartType(requestContentType) + || isFormUrlEncoded(requestContentType); + } + private boolean isMultipartType(MediaType requestContentType) { return Arrays.asList(MediaType.MULTIPART_FORM_DATA, MediaType.MULTIPART_MIXED, MediaType.MULTIPART_RELATED).contains(requestContentType); } + private boolean isFormUrlEncoded(MediaType requestContentType) { + return Arrays.asList(MediaType.APPLICATION_FORM_URLENCODED) + .contains(requestContentType); + } + private boolean binaryContentType(FeignOutputMessage outputMessage) { MediaType contentType = outputMessage.getHeaders().getContentType(); return contentType == null || Stream @@ -244,21 +254,21 @@ public class SpringEncoder implements Encoder { private final HttpHeaders httpHeaders; private FeignOutputMessage(RequestTemplate request) { - this.httpHeaders = getHttpHeaders(request.headers()); + httpHeaders = getHttpHeaders(request.headers()); } @Override public OutputStream getBody() throws IOException { - return this.outputStream; + return outputStream; } @Override public HttpHeaders getHeaders() { - return this.httpHeaders; + return httpHeaders; } public ByteArrayOutputStream getOutputStream() { - return this.outputStream; + return outputStream; } } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringEncoderTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringEncoderTests.java index 58068799..1684580f 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringEncoderTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringEncoderTests.java @@ -60,6 +60,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.http.HttpHeaders.ACCEPT; import static org.springframework.http.HttpHeaders.CONTENT_LENGTH; import static org.springframework.http.HttpHeaders.CONTENT_TYPE; +import static org.springframework.http.MediaType.APPLICATION_FORM_URLENCODED_VALUE; import static org.springframework.http.MediaType.APPLICATION_OCTET_STREAM_VALUE; import static org.springframework.http.MediaType.MULTIPART_FORM_DATA_VALUE; import static org.springframework.http.MediaType.TEXT_PLAIN_VALUE; @@ -192,6 +193,18 @@ public class SpringEncoderTests { .as("Body content cannot be decoded").contains("hi"); } + @Test + public void testFromURLEncodedValue() { + Encoder encoder = context.getInstance("can", Encoder.class); + assertThat(encoder).isNotNull(); + RequestTemplate request = new RequestTemplate(); + request.header(CONTENT_TYPE, APPLICATION_FORM_URLENCODED_VALUE); + String body = "test"; + encoder.encode(body, String.class, request); + assertThat(new String(request.requestBody().asBytes())) + .as("Body content cannot be decoded").contains(body); + } + @Test public void testNoCharsetForBinaryFiles() { Encoder encoder = context.getInstance("test", Encoder.class); diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java index 86ef5f50..11b4f62e 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java @@ -423,6 +423,13 @@ public class FeignClientTests { this.nullHystrixClientWithFallBackFactory.fail(); } + @Test + public void testFormURLEncoded() { + Hello hello = new Hello(HELLO_WORLD_1); + Hello response = testClient.getFormUrlEncoded(hello); + assertThat(response).isEqualTo(hello); + } + @Test public void namedFeignClientWorks() { assertThat(this.namedHystrixClient).as("namedHystrixClient was null").isNotNull(); @@ -615,6 +622,10 @@ public class FeignClientTests { @RequestMapping(method = RequestMethod.GET, path = "/tostring") String getToString(@RequestParam("arg") Arg arg); + @RequestMapping(method = RequestMethod.POST, path = "/form-urlencoded", + consumes = MediaType.APPLICATION_FORM_URLENCODED_VALUE) + Hello getFormUrlEncoded(Hello hello); + @RequestMapping(method = RequestMethod.GET, path = "/tostring2") String getToString(@RequestParam("arg") OtherArg arg); @@ -1154,6 +1165,12 @@ public class FeignClientTests { return result.toString(); } + @RequestMapping(method = RequestMethod.POST, path = "/form-urlencoded", + consumes = MediaType.APPLICATION_FORM_URLENCODED_VALUE) + Hello getFormUrlEncoded(Hello hello) { + return hello; + } + } public static class Hello { From d897b18e160a4e9e5734a9dee396d39e59f87517 Mon Sep 17 00:00:00 2001 From: Olga MaciaszekSharma Date: Mon, 5 Jul 2021 11:26:06 +0200 Subject: [PATCH 2/2] Add missing author tags. Refactor. --- .../cloud/openfeign/support/SpringEncoder.java | 13 +++++++++---- .../openfeign/support/SpringEncoderTests.java | 3 ++- .../cloud/openfeign/valid/FeignClientTests.java | 17 ++++++++++------- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringEncoder.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringEncoder.java index f71a020e..40859889 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringEncoder.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringEncoder.java @@ -24,6 +24,7 @@ import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collection; +import java.util.Objects; import java.util.stream.Stream; import feign.RequestTemplate; @@ -48,6 +49,10 @@ import org.springframework.web.multipart.MultipartFile; import static org.springframework.cloud.openfeign.support.FeignUtils.getHeaders; import static org.springframework.cloud.openfeign.support.FeignUtils.getHttpHeaders; +import static org.springframework.http.MediaType.APPLICATION_FORM_URLENCODED; +import static org.springframework.http.MediaType.MULTIPART_FORM_DATA; +import static org.springframework.http.MediaType.MULTIPART_MIXED; +import static org.springframework.http.MediaType.MULTIPART_RELATED; /** * @author Spencer Gibb @@ -56,6 +61,7 @@ import static org.springframework.cloud.openfeign.support.FeignUtils.getHttpHead * @author Aaron Whiteside * @author Darren Foong * @author Olga Maciaszek-Sharma + * @author Can Bezmen */ @SuppressWarnings("rawtypes") public class SpringEncoder implements Encoder { @@ -229,13 +235,12 @@ public class SpringEncoder implements Encoder { } private boolean isMultipartType(MediaType requestContentType) { - return Arrays.asList(MediaType.MULTIPART_FORM_DATA, MediaType.MULTIPART_MIXED, - MediaType.MULTIPART_RELATED).contains(requestContentType); + return Arrays.asList(MULTIPART_FORM_DATA, MULTIPART_MIXED, MULTIPART_RELATED) + .contains(requestContentType); } private boolean isFormUrlEncoded(MediaType requestContentType) { - return Arrays.asList(MediaType.APPLICATION_FORM_URLENCODED) - .contains(requestContentType); + return Objects.equals(APPLICATION_FORM_URLENCODED, requestContentType); } private boolean binaryContentType(FeignOutputMessage outputMessage) { diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringEncoderTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringEncoderTests.java index 1684580f..61753e8f 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringEncoderTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringEncoderTests.java @@ -69,6 +69,7 @@ import static org.springframework.http.MediaType.TEXT_PLAIN_VALUE; * @author Spencer Gibb * @author Olga Maciaszek-Sharma * @author Ahmad Mozafarnia + * @author Can Bezmen */ @RunWith(SpringJUnit4ClassRunner.class) @SpringBootTest(classes = SpringEncoderTests.Application.class, @@ -195,7 +196,7 @@ public class SpringEncoderTests { @Test public void testFromURLEncodedValue() { - Encoder encoder = context.getInstance("can", Encoder.class); + Encoder encoder = context.getInstance("formUrlEncoded", Encoder.class); assertThat(encoder).isNotNull(); RequestTemplate request = new RequestTemplate(); request.header(CONTENT_TYPE, APPLICATION_FORM_URLENCODED_VALUE); diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java index 11b4f62e..84831252 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java @@ -91,6 +91,7 @@ import org.springframework.http.ResponseEntity; import org.springframework.mock.web.MockMultipartFile; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +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.RequestMapping; @@ -102,6 +103,7 @@ import org.springframework.web.multipart.MultipartFile; import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.core.IsInstanceOf.instanceOf; +import static org.springframework.http.MediaType.APPLICATION_FORM_URLENCODED_VALUE; /** * @author Spencer Gibb @@ -110,6 +112,7 @@ import static org.hamcrest.core.IsInstanceOf.instanceOf; * @author Halvdan Hoem Grelland * @author Aaron Whiteside * @author Darren Foong + * @author Can Bezmen */ @RunWith(SpringJUnit4ClassRunner.class) @SpringBootTest(classes = FeignClientTests.Application.class, @@ -426,7 +429,7 @@ public class FeignClientTests { @Test public void testFormURLEncoded() { Hello hello = new Hello(HELLO_WORLD_1); - Hello response = testClient.getFormUrlEncoded(hello); + Hello response = testClient.postToUrlEncoded(hello); assertThat(response).isEqualTo(hello); } @@ -622,9 +625,9 @@ public class FeignClientTests { @RequestMapping(method = RequestMethod.GET, path = "/tostring") String getToString(@RequestParam("arg") Arg arg); - @RequestMapping(method = RequestMethod.POST, path = "/form-urlencoded", - consumes = MediaType.APPLICATION_FORM_URLENCODED_VALUE) - Hello getFormUrlEncoded(Hello hello); + @PostMapping(path = "/form-urlencoded", + consumes = APPLICATION_FORM_URLENCODED_VALUE) + Hello postToUrlEncoded(Hello hello); @RequestMapping(method = RequestMethod.GET, path = "/tostring2") String getToString(@RequestParam("arg") OtherArg arg); @@ -1165,9 +1168,9 @@ public class FeignClientTests { return result.toString(); } - @RequestMapping(method = RequestMethod.POST, path = "/form-urlencoded", - consumes = MediaType.APPLICATION_FORM_URLENCODED_VALUE) - Hello getFormUrlEncoded(Hello hello) { + @PostMapping(path = "/form-urlencoded", + consumes = APPLICATION_FORM_URLENCODED_VALUE) + Hello postToFormUrlEncoded(Hello hello) { return hello; }