From 9819558cba10b831b5a48b3574ac9670621f870b Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 17 Jan 2017 12:41:54 -0500 Subject: [PATCH] Support encoded query string in reactive requests In addition to the updates from PR#1297 this commit adds an integration test and fixes for the resulting failures with RxNetty, Reactor Netty, and Undertow. Also replaced use of URLDecoder which is not for decoding URIs with use of UriUtils for decoding query parameters. Issue: SPR-15140 --- .../RequestMappingIntegrationTests.java | 6 +++++ .../reactive/AbstractServerHttpRequest.java | 26 +++++------------- .../reactive/ReactorServerHttpRequest.java | 19 ++++++------- .../reactive/RxNettyServerHttpRequest.java | 27 ++++++++----------- .../reactive/UndertowServerHttpRequest.java | 16 ++++++++--- .../ServerHttpRequestIntegrationTests.java | 6 ++--- .../reactive/ServerHttpRequestTests.java | 6 ++--- 7 files changed, 49 insertions(+), 57 deletions(-) diff --git a/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingIntegrationTests.java b/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingIntegrationTests.java index 3140a06483..942a5624ee 100644 --- a/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingIntegrationTests.java +++ b/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingIntegrationTests.java @@ -60,6 +60,12 @@ public class RequestMappingIntegrationTests extends AbstractRequestMappingIntegr assertEquals(expected, performGet("/param?name=George", new HttpHeaders(), String.class).getBody()); } + @Test // SPR-15140 + public void handleWithEncodedParam() throws Exception { + String expected = "Hello ++\u00e0!"; + assertEquals(expected, performGet("/param?name=%20%2B+%C3%A0", new HttpHeaders(), String.class).getBody()); + } + @Test public void longStreamResult() throws Exception { String[] expected = {"0", "1", "2", "3", "4"}; diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpRequest.java index 6acf05c0d5..83bc27ca3b 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpRequest.java @@ -16,9 +16,7 @@ package org.springframework.http.server.reactive; -import java.io.UnsupportedEncodingException; import java.net.URI; -import java.net.URLDecoder; import java.nio.charset.StandardCharsets; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -29,6 +27,7 @@ import org.springframework.util.CollectionUtils; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; +import org.springframework.web.util.UriUtils; /** * Common base class for {@link ServerHttpRequest} implementations. @@ -79,22 +78,6 @@ public abstract class AbstractServerHttpRequest implements ServerHttpRequest { return this.queryParams; } - /** - * A method for decoding name and value string in a name-value pair. - *

Note that the plus sign "+" is converted into a space character " ".

- * @param encodedString the string to be decoded - * @return the decoded string - * @see java.net.URLDecoder#decode(String, String) - */ - private static String decodeQueryParam(final String encodedString) { - try { - return URLDecoder.decode(encodedString, StandardCharsets.UTF_8.name()); - } catch (UnsupportedEncodingException e) { - // StandardCharsets are guaranteed to be available on every implementation of the Java platform, so this should never happen. - throw new IllegalStateException(e); - } - } - /** * A method for parsing of the query into name-value pairs. The return * value is turned into an immutable map and cached. @@ -113,13 +96,16 @@ public abstract class AbstractServerHttpRequest implements ServerHttpRequest { String eq = matcher.group(2); String value = matcher.group(3); value = (value != null ? value : (StringUtils.hasLength(eq) ? "" : null)); - queryParams.add(decodeQueryParam(name), - value != null ? decodeQueryParam(value) : null); + queryParams.add(decodeQueryParam(name), decodeQueryParam(value)); } } return queryParams; } + private static String decodeQueryParam(String value) { + return (value != null ? UriUtils.decode(value, StandardCharsets.UTF_8) : null); + } + @Override public MultiValueMap getCookies() { if (this.cookies == null) { diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpRequest.java index eba9e34948..fee98d0af8 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpRequest.java @@ -56,20 +56,17 @@ public class ReactorServerHttpRequest extends AbstractServerHttpRequest { private static URI initUri(HttpServerRequest channel) { Assert.notNull("'channel' must not be null"); + InetSocketAddress address = channel.remoteAddress(); + return (address == null ? URI.create(channel.uri()) : getBaseUrl(address).resolve(channel.uri())); + } + + private static URI getBaseUrl(InetSocketAddress address) { try { - URI uri = new URI(channel.uri()); - InetSocketAddress remoteAddress = channel.remoteAddress(); - return new URI( - uri.getScheme(), - uri.getUserInfo(), - (remoteAddress != null ? remoteAddress.getHostString() : null), - (remoteAddress != null ? remoteAddress.getPort() : -1), - uri.getPath(), - uri.getQuery(), - uri.getFragment()); + return new URI(null, null, address.getHostString(), address.getPort(), null, null, null); } catch (URISyntaxException ex) { - throw new IllegalStateException("Could not get URI: " + ex.getMessage(), ex); + // Should not happen... + throw new IllegalStateException(ex); } } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/RxNettyServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/RxNettyServerHttpRequest.java index 215bc82692..b1b38bc0ea 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/RxNettyServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/RxNettyServerHttpRequest.java @@ -63,25 +63,20 @@ public class RxNettyServerHttpRequest extends AbstractServerHttpRequest { private static URI initUri(HttpServerRequest request) { Assert.notNull("'request', request must not be null"); + return StringUtils.isEmpty(request.getHostHeader()) ? + URI.create(request.getUri()) : getBaseUrl(request).resolve(request.getUri()); + } + + private static URI getBaseUrl(HttpServerRequest request) { + HttpHeaders headers = new HttpHeaders(); + headers.add("Host", request.getHostHeader()); + InetSocketAddress address = headers.getHost(); try { - URI uri = new URI(request.getUri()); - InetSocketAddress remoteAddress = null; - if (!StringUtils.isEmpty(request.getHostHeader())) { - HttpHeaders headers = new HttpHeaders(); - headers.add("Host", request.getHostHeader()); - remoteAddress = headers.getHost(); - } - return new URI( - uri.getScheme(), - uri.getUserInfo(), - (remoteAddress != null ? remoteAddress.getHostString() : null), - (remoteAddress != null ? remoteAddress.getPort() : -1), - uri.getPath(), - uri.getQuery(), - uri.getFragment()); + return new URI(null, null, address.getHostString(), address.getPort(), null, null, null); } catch (URISyntaxException ex) { - throw new IllegalStateException("Could not get URI: " + ex.getMessage(), ex); + // Should not happen... + throw new IllegalStateException(ex); } } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java index 08f67d27bb..bae47f5f3e 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java @@ -63,11 +63,19 @@ public class UndertowServerHttpRequest extends AbstractServerHttpRequest { private static URI initUri(HttpServerExchange exchange) { Assert.notNull(exchange, "HttpServerExchange is required."); + String requestURI = exchange.getRequestURI(); + String query = exchange.getQueryString(); + String requestUriAndQuery = StringUtils.isEmpty(query) ? requestURI : requestURI + "?" + query; + return (exchange.isHostIncludedInRequestURI()) ? + URI.create(requestUriAndQuery) : getBaseUri(exchange).resolve(requestUriAndQuery); + } + + private static URI getBaseUri(HttpServerExchange exchange) { try { - String query = exchange.getQueryString(); - return new URI(exchange.getRequestScheme(), null, - exchange.getHostName(), exchange.getHostPort(), - exchange.getRequestURI(), StringUtils.hasText(query) ? query : null, null); + String scheme = exchange.getRequestScheme(); + String host = exchange.getHostName(); + int port = exchange.getHostPort(); + return new URI(scheme, null, host, port, null, null, null); } catch (URISyntaxException ex) { throw new IllegalStateException("Could not get URI: " + ex.getMessage(), ex); diff --git a/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpRequestIntegrationTests.java b/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpRequestIntegrationTests.java index 3b2569d92b..eed78bd62c 100644 --- a/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpRequestIntegrationTests.java +++ b/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpRequestIntegrationTests.java @@ -36,9 +36,9 @@ public class ServerHttpRequestIntegrationTests extends AbstractHttpHandlerIntegr @Test public void checkUri() throws Exception { - RestTemplate restTemplate = new RestTemplate(); - RequestEntity request = RequestEntity.post(new URI("http://localhost:" + port + "/foo?param=bar")).build(); - ResponseEntity response = restTemplate.exchange(request, Void.class); + URI url = new URI("http://localhost:" + port + "/foo?param=bar"); + RequestEntity request = RequestEntity.post(url).build(); + ResponseEntity response = new RestTemplate().exchange(request, Void.class); assertEquals(HttpStatus.OK, response.getStatusCode()); } diff --git a/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpRequestTests.java b/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpRequestTests.java index 922fb503ec..c79534ae2e 100644 --- a/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpRequestTests.java +++ b/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpRequestTests.java @@ -64,11 +64,11 @@ public class ServerHttpRequestTests { assertEquals(Arrays.asList("1", "2"), params.get("a")); } - @Test - public void queryParamsWithUrlEncodedValue() throws Exception { + @Test // SPR-15140 + public void queryParamsWithEncodedValue() throws Exception { MultiValueMap params = createHttpRequest("/path?a=%20%2B+%C3%A0").getQueryParams(); assertEquals(1, params.size()); - assertEquals(Collections.singletonList(" + \u00e0"), params.get("a")); + assertEquals(Collections.singletonList(" ++\u00e0"), params.get("a")); } @Test