Skip to content

Commit

Permalink
GH-3711: Fix HTTP handler for content type header
Browse files Browse the repository at this point in the history 
Fixes #3711

The `contentType` header may come with parameter in its media type.

* Fix `AbstractHttpRequestExecutingMessageHandler` to use `equalsTypeAndSubtype()`
ignoring params
* Some other code clean up in the `AbstractHttpRequestExecutingMessageHandler`
* Ensure in the `HttpRequestExecutingMessageHandlerTests.simpleStringKeyStringValueFormData()`
that provided `contentType` header is handled properly
* Fix `HttpProxyScenarioTests.testHttpMultipartProxyScenario()` for mislead multi-part form
handling

**Cherry-pick to `5.5.x`**
  • Loading branch information
@artembilan @garyrussell
artembilan authored and garyrussell committed Feb 3, 2022
1 parent 64d5b25 commit 4799c3b
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 72 deletions.
17 changes: 8 additions & 9 deletions ...springframework/integration/http/outbound/AbstractHttpRequestExecutingMessageHandler.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2017-2021 the original author or authors.
* Copyright 2017-2022 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.
Expand Down Expand Up @@ -118,8 +118,8 @@ public AbstractHttpRequestExecutingMessageHandler(Expression uriExpression) {

/**
* Set the encoding mode to use.
* By default this is set to {@link DefaultUriBuilderFactory.EncodingMode#TEMPLATE_AND_VALUES}.
* For more complicated scenarios consider to configure an {@link org.springframework.web.util.UriTemplateHandler}
* By default, this is set to {@link DefaultUriBuilderFactory.EncodingMode#TEMPLATE_AND_VALUES}.
* For more complicated scenarios consider configuring an {@link org.springframework.web.util.UriTemplateHandler}
* on an externally provided {@link org.springframework.web.client.RestTemplate}.
* @param encodingMode the mode to use for uri encoding
* @since 5.3
Expand Down Expand Up @@ -151,7 +151,7 @@ public void setHttpMethod(HttpMethod httpMethod) {
/**
* Specify whether the outbound message's payload should be extracted
* when preparing the request body.
* Otherwise the Message instance itself is serialized.
* Otherwise, the Message instance itself is serialized.
* The default value is {@code true}.
* @param extractPayload true if the payload should be extracted.
*/
Expand Down Expand Up @@ -189,7 +189,7 @@ public void setExpectReply(boolean expectReply) {

/**
* Specify the expected response type for the REST request.
* Otherwise it is null and an empty {@link ResponseEntity} is returned from HTTP client.
* Otherwise, it is null and an empty {@link ResponseEntity} is returned from HTTP client.
* To take advantage of the HttpMessageConverters
* registered on this adapter, provide a different type).
* @param expectedResponseType The expected type.
Expand Down Expand Up @@ -378,8 +378,8 @@ private HttpEntity<?> createHttpEntityFromPayload(Message<?> message, HttpMethod
: resolveContentType(payload);
httpHeaders.setContentType(contentType);
}
if ((MediaType.APPLICATION_FORM_URLENCODED.equals(httpHeaders.getContentType()) ||
MediaType.MULTIPART_FORM_DATA.equals(httpHeaders.getContentType()))
if ((MediaType.APPLICATION_FORM_URLENCODED.equalsTypeAndSubtype(httpHeaders.getContentType()) ||
MediaType.MULTIPART_FORM_DATA.equalsTypeAndSubtype(httpHeaders.getContentType()))
&& !(payload instanceof MultiValueMap)) {

Assert.isInstanceOf(Map.class, payload,
Expand Down Expand Up @@ -471,8 +471,7 @@ private boolean isMultipart(Map<String, ?> map) {
if (value.getClass().isArray()) {
value = CollectionUtils.arrayToList(value);
}
if (value instanceof Collection) {
Collection<?> cValues = (Collection<?>) value;
if (value instanceof Collection<?> cValues) {
for (Object cValue : cValues) {
if (cValue != null && !(cValue instanceof String)) {
return true;
Expand Down
31 changes: 14 additions & 17 deletions ...ttp/src/test/java/org/springframework/integration/http/HttpProxyScenarioTests-context.xml
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
<beans:beans
xmlns="http://www.springframework.org/schema/integration/http"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:beans="http://www.springframework.org/schema/beans"
xmlns:int="http://www.springframework.org/schema/integration"
xmlns:util="http://www.springframework.org/schema/util"
xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans.xsd
xmlns="http://www.springframework.org/schema/integration/http"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:beans="http://www.springframework.org/schema/beans"
xmlns:int="http://www.springframework.org/schema/integration"
xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans.xsd
http://www.springframework.org/schema/integration/http https://www.springframework.org/schema/integration/http/spring-integration-http.xsd
http://www.springframework.org/schema/integration https://www.springframework.org/schema/integration/spring-integration.xsd
http://www.springframework.org/schema/util https://www.springframework.org/schema/util/spring-util.xsd">
http://www.springframework.org/schema/integration https://www.springframework.org/schema/integration/spring-integration.xsd">

<inbound-gateway path="/test" request-channel="testChannel"
payload-expression="T(org.springframework.web.context.request.RequestContextHolder).requestAttributes.request.queryString"/>
Expand All @@ -25,15 +23,14 @@
</int:channel>

<inbound-gateway path="/testmp" request-channel="testChannelmp"
merge-with-default-converters="false"
message-converters="converters"
request-payload-type="byte[]" />

<util:list id="converters">
<beans:bean class="org.springframework.http.converter.ByteArrayHttpMessageConverter" />
<beans:bean class="org.springframework.http.converter.StringHttpMessageConverter" />
<beans:bean class="org.springframework.http.converter.json.MappingJackson2HttpMessageConverter" />
</util:list>
message-converters="formHttpMessageConverter"/>

<beans:bean id="formHttpMessageConverter"
class="org.springframework.integration.http.converter.MultipartAwareFormHttpMessageConverter">
<beans:property name="multipartFileReader">
<beans:bean class="org.springframework.integration.http.multipart.SimpleMultipartFileReader"/>
</beans:property>
</beans:bean>

<int:channel id="testChannelmp"/>

Expand Down
76 changes: 40 additions & 36 deletions ...ation-http/src/test/java/org/springframework/integration/http/HttpProxyScenarioTests.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013-2020 the original author or authors.
* Copyright 2013-2022 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.
Expand Down Expand Up @@ -46,6 +46,7 @@
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.web.client.RestTemplate;
Expand Down Expand Up @@ -120,19 +121,19 @@ public void testHttpProxyScenario() throws Exception {
final String contentDispositionValue = "attachment; filename=\"test.txt\"";

Mockito.doAnswer(invocation -> {
String uri = invocation.getArgument(0);
assertThat(uri).isEqualTo("http://testServer/test?foo=bar&FOO=BAR");
HttpEntity<?> httpEntity = (HttpEntity<?>) invocation.getArguments()[2];
HttpHeaders httpHeaders = httpEntity.getHeaders();
assertThat(httpHeaders.getIfModifiedSince()).isEqualTo(ifModifiedSince);
assertThat(httpHeaders.getFirst("If-Unmodified-Since")).isEqualTo(ifUnmodifiedSinceValue);
assertThat(httpHeaders.getFirst("Connection")).isEqualTo("Keep-Alive");

MultiValueMap<String, String> responseHeaders = new LinkedMultiValueMap<>(httpHeaders);
responseHeaders.set("Connection", "close");
responseHeaders.set("Content-Disposition", contentDispositionValue);
return new ResponseEntity<>(responseHeaders, HttpStatus.OK);
}).when(template)
String uri = invocation.getArgument(0);
assertThat(uri).isEqualTo("http://testServer/test?foo=bar&FOO=BAR");
HttpEntity<?> httpEntity = (HttpEntity<?>) invocation.getArguments()[2];
HttpHeaders httpHeaders = httpEntity.getHeaders();
assertThat(httpHeaders.getIfModifiedSince()).isEqualTo(ifModifiedSince);
assertThat(httpHeaders.getFirst("If-Unmodified-Since")).isEqualTo(ifUnmodifiedSinceValue);
assertThat(httpHeaders.getFirst("Connection")).isEqualTo("Keep-Alive");

MultiValueMap<String, String> responseHeaders = new LinkedMultiValueMap<>(httpHeaders);
responseHeaders.set("Connection", "close");
responseHeaders.set("Content-Disposition", contentDispositionValue);
return new ResponseEntity<>(responseHeaders, HttpStatus.OK);
}).when(template)
.exchange(Mockito.anyString(), Mockito.any(HttpMethod.class),
Mockito.any(HttpEntity.class), (Class<?>) isNull(), Mockito.anyMap());

Expand Down Expand Up @@ -160,12 +161,14 @@ public void testHttpProxyScenario() throws Exception {
}

@Test
@SuppressWarnings("unchecked")
public void testHttpMultipartProxyScenario() throws Exception {
MockHttpServletRequest request = new MockHttpServletRequest("POST", "/testmp");

request.addHeader("Connection", "Keep-Alive");
request.setContentType("multipart/form-data;boundary=----WebKitFormBoundarywABD2xqC1FLBijlQ");
request.setContent("foo".getBytes());
MockHttpServletRequest request =
MockMvcRequestBuilders.multipart("/testmp")
.file("foo", "foo".getBytes())
.contentType("multipart/form-data;boundary=----WebKitFormBoundarywABD2xqC1FLBijlQ")
.header("Connection", "Keep-Alive")
.buildRequest(null);

Object handler = this.handlerMapping.getHandler(request).getHandler();
assertThat(handler).isNotNull();
Expand All @@ -174,23 +177,24 @@ public void testHttpMultipartProxyScenario() throws Exception {

RestTemplate template = Mockito.spy(new RestTemplate());
Mockito.doAnswer(invocation -> {
String uri = invocation.getArgument(0);
assertThat(uri).isEqualTo("http://testServer/testmp");
HttpEntity<?> httpEntity = (HttpEntity<?>) invocation.getArguments()[2];
HttpHeaders httpHeaders = httpEntity.getHeaders();
assertThat(httpHeaders.getFirst("Connection")).isEqualTo("Keep-Alive");
assertThat(httpHeaders.getContentType().toString())
.isEqualTo("multipart/form-data;boundary=----WebKitFormBoundarywABD2xqC1FLBijlQ");

HttpEntity<?> entity = (HttpEntity<?>) invocation.getArguments()[2];
assertThat(entity.getBody()).isInstanceOf(byte[].class);
assertThat(new String((byte[]) entity.getBody())).isEqualTo("foo");

MultiValueMap<String, String> responseHeaders = new LinkedMultiValueMap<>(httpHeaders);
responseHeaders.set("Connection", "close");
responseHeaders.set("Content-Type", "text/plain");
return new ResponseEntity<>(responseHeaders, HttpStatus.OK);
}).when(template)
String uri = invocation.getArgument(0);
assertThat(uri).isEqualTo("http://testServer/testmp");
HttpEntity<?> httpEntity = (HttpEntity<?>) invocation.getArguments()[2];
HttpHeaders httpHeaders = httpEntity.getHeaders();
assertThat(httpHeaders.getFirst("Connection")).isEqualTo("Keep-Alive");
assertThat(httpHeaders.getContentType().toString())
.isEqualTo("multipart/form-data;boundary=----WebKitFormBoundarywABD2xqC1FLBijlQ");

HttpEntity<?> entity = (HttpEntity<?>) invocation.getArguments()[2];
assertThat(entity.getBody()).isInstanceOf(MultiValueMap.class);
assertThat(((MultiValueMap<String, ?>) entity.getBody()).getFirst("foo"))
.isEqualTo("foo".getBytes());

MultiValueMap<String, String> responseHeaders = new LinkedMultiValueMap<>(httpHeaders);
responseHeaders.set("Connection", "close");
responseHeaders.set("Content-Type", "text/plain");
return new ResponseEntity<>(responseHeaders, HttpStatus.OK);
}).when(template)
.exchange(Mockito.anyString(), Mockito.any(HttpMethod.class),
Mockito.any(HttpEntity.class), (Class<?>) isNull(), Mockito.anyMap());

Expand Down
29 changes: 19 additions & 10 deletions ...rg/springframework/integration/http/outbound/HttpRequestExecutingMessageHandlerTests.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2021 the original author or authors.
* Copyright 2002-2022 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.
Expand Down Expand Up @@ -61,6 +61,7 @@
import org.springframework.lang.Nullable;
import org.springframework.messaging.Message;
import org.springframework.messaging.MessageChannel;
import org.springframework.messaging.MessageHeaders;
import org.springframework.messaging.PollableChannel;
import org.springframework.messaging.support.GenericMessage;
import org.springframework.mock.http.client.MockClientHttpResponse;
Expand All @@ -85,10 +86,12 @@
*/
public class HttpRequestExecutingMessageHandlerTests {

// Used in the HttpOutboundWithinChainTests-context.xml
public static ParameterizedTypeReference<List<String>> testParameterizedTypeReference() {
return new ParameterizedTypeReference<List<String>>() {
return new ParameterizedTypeReference<>() {

};

}

@Test
Expand All @@ -104,7 +107,11 @@ public void simpleStringKeyStringValueFormData() {
form.put("a", "1");
form.put("b", "2");
form.put("c", "3");
Message<?> message = MessageBuilder.withPayload(form).build();
Message<?> message =
MessageBuilder.withPayload(form)
.setHeader(MessageHeaders.CONTENT_TYPE,
MediaType.APPLICATION_FORM_URLENCODED_VALUE + ";charset=UTF-8")
.build();
QueueChannel replyChannel = new QueueChannel();
handler.setOutputChannel(replyChannel);

Expand All @@ -115,12 +122,14 @@ public void simpleStringKeyStringValueFormData() {
HttpEntity<?> request = template.lastRequestEntity.get();
Object body = request.getBody();
assertThat(request.getHeaders().getContentType()).isNotNull();
assertThat(body instanceof MultiValueMap<?, ?>).isTrue();
assertThat(body).isInstanceOf(MultiValueMap.class);
MultiValueMap<?, ?> map = (MultiValueMap<?, ?>) body;
assertThat(map.get("a").iterator().next()).isEqualTo("1");
assertThat(map.get("b").iterator().next()).isEqualTo("2");
assertThat(map.get("c").iterator().next()).isEqualTo("3");
assertThat(request.getHeaders().getContentType()).isEqualTo(MediaType.APPLICATION_FORM_URLENCODED);
assertThat(request.getHeaders().getContentType()).isNotEqualTo(MediaType.APPLICATION_FORM_URLENCODED);
assertThat(request.getHeaders().getContentType().equalsTypeAndSubtype(MediaType.APPLICATION_FORM_URLENCODED))
.isTrue();
}

@Test
Expand Down Expand Up @@ -541,7 +550,7 @@ public void contentAsByteArray() {

HttpEntity<?> request = template.lastRequestEntity.get();
Object body = request.getBody();
assertThat(body instanceof byte[]).isTrue();
assertThat(body).isInstanceOf(byte[].class);
assertThat(new String(bytes)).isEqualTo("Hello World");
assertThat(request.getHeaders().getContentType()).isEqualTo(MediaType.APPLICATION_OCTET_STREAM);
}
Expand All @@ -564,7 +573,7 @@ public void contentAsXmlSource() {

HttpEntity<?> request = template.lastRequestEntity.get();
Object body = request.getBody();
assertThat(body instanceof Source).isTrue();
assertThat(body).isInstanceOf(Source.class);
assertThat(request.getHeaders().getContentType()).isEqualTo(MediaType.TEXT_XML);
}

Expand Down Expand Up @@ -784,7 +793,7 @@ public void acceptHeaderForSerializableResponse() throws IOException {
assertThat(requestHeaders.getAccept()).isNotNull();
assertThat(requestHeaders.getAccept().size() > 0).isTrue();
List<MediaType> accept = requestHeaders.getAccept();
assertThat(accept.size() > 0).isTrue();
assertThat(accept).hasSizeGreaterThan(0);
assertThat(accept.get(0).getType()).isEqualTo("application");
assertThat(accept.get(0).getSubtype()).isEqualTo("x-java-serialized-object");
}
Expand Down Expand Up @@ -815,13 +824,13 @@ public void acceptHeaderForSerializableResponseMessageExchange() throws IOExcept
assertThat(requestHeaders.getAccept()).isNotNull();
assertThat(requestHeaders.getAccept().size() > 0).isTrue();
List<MediaType> accept = requestHeaders.getAccept();
assertThat(accept.size() > 0).isTrue();
assertThat(accept).hasSizeGreaterThan(0);
assertThat(accept.get(0).getType()).isEqualTo("application");
assertThat(accept.get(0).getSubtype()).isEqualTo("x-java-serialized-object");
}

@Test
public void testNoContentTypeAndSmartConverter() throws IOException {
public void testNoContentTypeAndSmartConverter() {
Sinks.One<HttpHeaders> httpHeadersSink = Sinks.one();
RestTemplate testRestTemplate = new RestTemplate() {
@Nullable
Expand Down

0 comments on commit 4799c3b

Please sign in to comment.