ServletServerHttpRequest.getURI() ignores malformed query string
The resolved URI instance is also being cached now. This should not make a difference in a real Servlet environment but does affect tests which assumed they could modify an HttpServletRequest path behind a pre-created ServletServerHttpRequest instance. Our WebSocket test base class has been revised accordingly, re-creating the ServletServerHttpRequest in such a case.
Issue: SPR-16414
(cherry picked from commit 0e6f8df)
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright 2002-2015 the original author or authors.
|
||||
* Copyright 2002-2018 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.
|
||||
@@ -35,7 +35,8 @@ public interface HttpRequest extends HttpMessage {
|
||||
HttpMethod getMethod();
|
||||
|
||||
/**
|
||||
* Return the URI of the request.
|
||||
* Return the URI of the request (including a query string if any,
|
||||
* but only if it is well-formed for a URI representation).
|
||||
* @return the URI of the request (never {@code null})
|
||||
*/
|
||||
URI getURI();
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright 2002-2017 the original author or authors.
|
||||
* Copyright 2002-2018 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.
|
||||
@@ -59,6 +59,8 @@ public class ServletServerHttpRequest implements ServerHttpRequest {
|
||||
|
||||
private final HttpServletRequest servletRequest;
|
||||
|
||||
private URI uri;
|
||||
|
||||
private HttpHeaders headers;
|
||||
|
||||
private ServerHttpAsyncRequestControl asyncRequestControl;
|
||||
@@ -88,17 +90,36 @@ public class ServletServerHttpRequest implements ServerHttpRequest {
|
||||
|
||||
@Override
|
||||
public URI getURI() {
|
||||
try {
|
||||
StringBuffer url = this.servletRequest.getRequestURL();
|
||||
String query = this.servletRequest.getQueryString();
|
||||
if (StringUtils.hasText(query)) {
|
||||
url.append('?').append(query);
|
||||
if (this.uri == null) {
|
||||
String urlString = null;
|
||||
boolean hasQuery = false;
|
||||
try {
|
||||
StringBuffer url = this.servletRequest.getRequestURL();
|
||||
String query = this.servletRequest.getQueryString();
|
||||
hasQuery = StringUtils.hasText(query);
|
||||
if (hasQuery) {
|
||||
url.append('?').append(query);
|
||||
}
|
||||
urlString = url.toString();
|
||||
this.uri = new URI(urlString);
|
||||
}
|
||||
catch (URISyntaxException ex) {
|
||||
if (!hasQuery) {
|
||||
throw new IllegalStateException(
|
||||
"Could not resolve HttpServletRequest as URI: " + urlString, ex);
|
||||
}
|
||||
// Maybe a malformed query string... try plain request URL
|
||||
try {
|
||||
urlString = this.servletRequest.getRequestURL().toString();
|
||||
this.uri = new URI(urlString);
|
||||
}
|
||||
catch (URISyntaxException ex2) {
|
||||
throw new IllegalStateException(
|
||||
"Could not resolve HttpServletRequest as URI: " + urlString, ex2);
|
||||
}
|
||||
}
|
||||
return new URI(url.toString());
|
||||
}
|
||||
catch (URISyntaxException ex) {
|
||||
throw new IllegalStateException("Could not get HttpServletRequest URI: " + ex.getMessage(), ex);
|
||||
}
|
||||
return this.uri;
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -106,8 +127,8 @@ public class ServletServerHttpRequest implements ServerHttpRequest {
|
||||
if (this.headers == null) {
|
||||
this.headers = new HttpHeaders();
|
||||
|
||||
for (Enumeration<?> headerNames = this.servletRequest.getHeaderNames(); headerNames.hasMoreElements();) {
|
||||
String headerName = (String) headerNames.nextElement();
|
||||
for (Enumeration<?> names = this.servletRequest.getHeaderNames(); names.hasMoreElements();) {
|
||||
String headerName = (String) names.nextElement();
|
||||
for (Enumeration<?> headerValues = this.servletRequest.getHeaders(headerName);
|
||||
headerValues.hasMoreElements();) {
|
||||
String headerValue = (String) headerValues.nextElement();
|
||||
@@ -115,7 +136,8 @@ public class ServletServerHttpRequest implements ServerHttpRequest {
|
||||
}
|
||||
}
|
||||
|
||||
// HttpServletRequest exposes some headers as properties: we should include those if not already present
|
||||
// HttpServletRequest exposes some headers as properties:
|
||||
// we should include those if not already present
|
||||
try {
|
||||
MediaType contentType = this.headers.getContentType();
|
||||
if (contentType == null) {
|
||||
@@ -132,8 +154,8 @@ public class ServletServerHttpRequest implements ServerHttpRequest {
|
||||
Map<String, String> params = new LinkedCaseInsensitiveMap<String>();
|
||||
params.putAll(contentType.getParameters());
|
||||
params.put("charset", charSet.toString());
|
||||
MediaType newContentType = new MediaType(contentType.getType(), contentType.getSubtype(), params);
|
||||
this.headers.setContentType(newContentType);
|
||||
MediaType mediaType = new MediaType(contentType.getType(), contentType.getSubtype(), params);
|
||||
this.headers.setContentType(mediaType);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -181,7 +203,8 @@ public class ServletServerHttpRequest implements ServerHttpRequest {
|
||||
public ServerHttpAsyncRequestControl getAsyncRequestControl(ServerHttpResponse response) {
|
||||
if (this.asyncRequestControl == null) {
|
||||
if (!ServletServerHttpResponse.class.isInstance(response)) {
|
||||
throw new IllegalArgumentException("Response must be a ServletServerHttpResponse: " + response.getClass());
|
||||
throw new IllegalArgumentException(
|
||||
"Response must be a ServletServerHttpResponse: " + response.getClass());
|
||||
}
|
||||
ServletServerHttpResponse servletServerResponse = (ServletServerHttpResponse) response;
|
||||
this.asyncRequestControl = new ServletServerHttpAsyncRequestControl(this, servletServerResponse);
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright 2002-2016 the original author or authors.
|
||||
* Copyright 2002-2018 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.
|
||||
@@ -16,8 +16,10 @@
|
||||
|
||||
package org.springframework.http.server;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.net.URI;
|
||||
import java.nio.charset.Charset;
|
||||
import java.net.URISyntaxException;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.util.List;
|
||||
|
||||
import org.junit.Before;
|
||||
@@ -33,6 +35,7 @@ import static org.junit.Assert.*;
|
||||
|
||||
/**
|
||||
* @author Arjen Poutsma
|
||||
* @author Juergen Hoeller
|
||||
*/
|
||||
public class ServletServerHttpRequestTests {
|
||||
|
||||
@@ -42,30 +45,56 @@ public class ServletServerHttpRequestTests {
|
||||
|
||||
|
||||
@Before
|
||||
public void create() throws Exception {
|
||||
public void create() {
|
||||
mockRequest = new MockHttpServletRequest();
|
||||
request = new ServletServerHttpRequest(mockRequest);
|
||||
}
|
||||
|
||||
|
||||
@Test
|
||||
public void getMethod() throws Exception {
|
||||
public void getMethod() {
|
||||
mockRequest.setMethod("POST");
|
||||
assertEquals("Invalid method", HttpMethod.POST, request.getMethod());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void getURI() throws Exception {
|
||||
public void getUriForSimplePath() throws URISyntaxException {
|
||||
URI uri = new URI("http://example.com/path");
|
||||
mockRequest.setServerName(uri.getHost());
|
||||
mockRequest.setServerPort(uri.getPort());
|
||||
mockRequest.setRequestURI(uri.getPath());
|
||||
mockRequest.setQueryString(uri.getQuery());
|
||||
assertEquals(uri, request.getURI());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void getUriWithQueryString() throws URISyntaxException {
|
||||
URI uri = new URI("http://example.com/path?query");
|
||||
mockRequest.setServerName(uri.getHost());
|
||||
mockRequest.setServerPort(uri.getPort());
|
||||
mockRequest.setRequestURI(uri.getPath());
|
||||
mockRequest.setQueryString(uri.getQuery());
|
||||
assertEquals("Invalid uri", uri, request.getURI());
|
||||
assertEquals(uri, request.getURI());
|
||||
}
|
||||
|
||||
@Test // SPR-16414
|
||||
public void getUriWithQueryParam() throws URISyntaxException {
|
||||
mockRequest.setServerName("example.com");
|
||||
mockRequest.setRequestURI("/path");
|
||||
mockRequest.setQueryString("query=foo");
|
||||
assertEquals(new URI("http://example.com/path?query=foo"), request.getURI());
|
||||
}
|
||||
|
||||
@Test // SPR-16414
|
||||
public void getUriWithMalformedQueryParam() throws URISyntaxException {
|
||||
mockRequest.setServerName("example.com");
|
||||
mockRequest.setRequestURI("/path");
|
||||
mockRequest.setQueryString("query=foo%%x");
|
||||
assertEquals(new URI("http://example.com/path"), request.getURI());
|
||||
}
|
||||
|
||||
@Test // SPR-13876
|
||||
public void getUriWithEncoding() throws Exception {
|
||||
public void getUriWithEncoding() throws URISyntaxException {
|
||||
URI uri = new URI("https://example.com/%E4%B8%AD%E6%96%87" +
|
||||
"?redirect=https%3A%2F%2Fgithub.com%2Fspring-projects%2Fspring-framework");
|
||||
mockRequest.setScheme(uri.getScheme());
|
||||
@@ -73,11 +102,11 @@ public class ServletServerHttpRequestTests {
|
||||
mockRequest.setServerPort(uri.getPort());
|
||||
mockRequest.setRequestURI(uri.getRawPath());
|
||||
mockRequest.setQueryString(uri.getRawQuery());
|
||||
assertEquals("Invalid uri", uri, request.getURI());
|
||||
assertEquals(uri, request.getURI());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void getHeaders() throws Exception {
|
||||
public void getHeaders() {
|
||||
String headerName = "MyHeader";
|
||||
String headerValue1 = "value1";
|
||||
String headerValue2 = "value2";
|
||||
@@ -93,12 +122,12 @@ public class ServletServerHttpRequestTests {
|
||||
assertEquals("Invalid header values returned", 2, headerValues.size());
|
||||
assertTrue("Invalid header values returned", headerValues.contains(headerValue1));
|
||||
assertTrue("Invalid header values returned", headerValues.contains(headerValue2));
|
||||
assertEquals("Invalid Content-Type", new MediaType("text", "plain", Charset.forName("UTF-8")),
|
||||
assertEquals("Invalid Content-Type", new MediaType("text", "plain", StandardCharsets.UTF_8),
|
||||
headers.getContentType());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void getHeadersWithEmptyContentTypeAndEncoding() throws Exception {
|
||||
public void getHeadersWithEmptyContentTypeAndEncoding() {
|
||||
String headerName = "MyHeader";
|
||||
String headerValue1 = "value1";
|
||||
String headerValue2 = "value2";
|
||||
@@ -118,7 +147,7 @@ public class ServletServerHttpRequestTests {
|
||||
}
|
||||
|
||||
@Test
|
||||
public void getBody() throws Exception {
|
||||
public void getBody() throws IOException {
|
||||
byte[] content = "Hello World".getBytes("UTF-8");
|
||||
mockRequest.setContent(content);
|
||||
|
||||
@@ -127,7 +156,7 @@ public class ServletServerHttpRequestTests {
|
||||
}
|
||||
|
||||
@Test
|
||||
public void getFormBody() throws Exception {
|
||||
public void getFormBody() throws IOException {
|
||||
// Charset (SPR-8676)
|
||||
mockRequest.setContentType("application/x-www-form-urlencoded; charset=UTF-8");
|
||||
mockRequest.setMethod("POST");
|
||||
|
||||
Reference in New Issue
Block a user