Lenient handling of malformed query in ServletServerHttpRequest

Closes gh-30489
This commit is contained in:
rstoyanchev 2024-10-08 16:00:31 +01:00
parent af85d1997b
commit 1f4743af54
3 changed files with 78 additions and 35 deletions

View File

@ -50,6 +50,7 @@ import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.LinkedCaseInsensitiveMap;
import org.springframework.util.StringUtils;
import org.springframework.web.util.UriComponentsBuilder;
/**
* {@link ServerHttpRequest} implementation that is based on a {@link HttpServletRequest}.
@ -119,31 +120,37 @@ public class ServletServerHttpRequest implements ServerHttpRequest {
*/
public static URI initURI(HttpServletRequest servletRequest) {
String urlString = null;
String query = null;
boolean hasQuery = false;
try {
StringBuffer url = servletRequest.getRequestURL();
String query = servletRequest.getQueryString();
StringBuffer requestURL = servletRequest.getRequestURL();
query = servletRequest.getQueryString();
hasQuery = StringUtils.hasText(query);
if (hasQuery) {
url.append('?').append(query);
requestURL.append('?').append(query);
}
urlString = url.toString();
urlString = requestURL.toString();
return 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 = servletRequest.getRequestURL().toString();
return new URI(urlString);
}
catch (URISyntaxException ex2) {
throw new IllegalStateException(
"Could not resolve HttpServletRequest as URI: " + urlString, ex2);
if (hasQuery) {
try {
// Maybe malformed query, try to parse and encode it
query = UriComponentsBuilder.fromUriString("?" + query).build().toUri().getRawQuery();
return new URI(servletRequest.getRequestURL().toString() + "?" + query);
}
catch (URISyntaxException ex2) {
try {
// Try leaving it out
return new URI(servletRequest.getRequestURL().toString());
}
catch (URISyntaxException ex3) {
// ignore
}
}
}
throw new IllegalStateException(
"Could not resolve HttpServletRequest as URI: " + urlString, ex);
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* Copyright 2002-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.
@ -51,6 +51,7 @@ import org.springframework.util.LinkedCaseInsensitiveMap;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.util.StringUtils;
import org.springframework.web.util.UriComponentsBuilder;
/**
* Adapt {@link ServerHttpRequest} to the Servlet {@link HttpServletRequest}.
@ -90,8 +91,8 @@ class ServletServerHttpRequest extends AbstractServerHttpRequest {
AsyncContext asyncContext, String servletPath, DataBufferFactory bufferFactory, int bufferSize)
throws IOException, URISyntaxException {
super(HttpMethod.valueOf(request.getMethod()), initUri(request), request.getContextPath() + servletPath,
initHeaders(headers, request));
super(HttpMethod.valueOf(request.getMethod()), initUri(request),
request.getContextPath() + servletPath, initHeaders(headers, request));
Assert.notNull(bufferFactory, "'bufferFactory' must not be null");
Assert.isTrue(bufferSize > 0, "'bufferSize' must be greater than 0");
@ -121,14 +122,42 @@ class ServletServerHttpRequest extends AbstractServerHttpRequest {
return headers;
}
private static URI initUri(HttpServletRequest request) throws URISyntaxException {
Assert.notNull(request, "'request' must not be null");
StringBuffer url = request.getRequestURL();
String query = request.getQueryString();
if (StringUtils.hasText(query)) {
url.append('?').append(query);
@SuppressWarnings("JavaExistingMethodCanBeUsed")
private static URI initUri(HttpServletRequest servletRequest) {
Assert.notNull(servletRequest, "'request' must not be null");
String urlString = null;
String query = null;
boolean hasQuery = false;
try {
StringBuffer requestURL = servletRequest.getRequestURL();
query = servletRequest.getQueryString();
hasQuery = StringUtils.hasText(query);
if (hasQuery) {
requestURL.append('?').append(query);
}
urlString = requestURL.toString();
return new URI(urlString);
}
catch (URISyntaxException ex) {
if (hasQuery) {
try {
// Maybe malformed query, try to parse and encode it
query = UriComponentsBuilder.fromUriString("?" + query).build().toUri().getRawQuery();
return new URI(servletRequest.getRequestURL().toString() + "?" + query);
}
catch (URISyntaxException ex2) {
try {
// Try leaving it out
return new URI(servletRequest.getRequestURL().toString());
}
catch (URISyntaxException ex3) {
// ignore
}
}
}
throw new IllegalStateException(
"Could not resolve HttpServletRequest as URI: " + urlString, ex);
}
return new URI(url.toString());
}
@SuppressWarnings("NullAway")

View File

@ -23,6 +23,8 @@ import java.util.List;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
@ -31,6 +33,7 @@ import org.springframework.util.FileCopyUtils;
import org.springframework.web.testfixture.servlet.MockHttpServletRequest;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
/**
* @author Arjen Poutsma
@ -78,24 +81,28 @@ class ServletServerHttpRequestTests {
assertThat(request.getURI()).isEqualTo(uri);
}
@Test // SPR-16414
void getUriWithQueryParam() {
// gh-20960
@ParameterizedTest(name = "{displayName}({arguments})")
@CsvSource(delimiter='|', value = {
"query=foo | ?query=foo",
"query=foo%%x | ?query=foo%25%25x"
})
void getUriWithMalformedQueryParam(String inputQuery, String expectedQuery) {
mockRequest.setScheme("https");
mockRequest.setServerPort(443);
mockRequest.setServerName("example.com");
mockRequest.setRequestURI("/path");
mockRequest.setQueryString("query=foo");
assertThat(request.getURI()).isEqualTo(URI.create("https://example.com/path?query=foo"));
mockRequest.setQueryString(inputQuery);
assertThat(request.getURI()).isEqualTo(URI.create("https://example.com/path" + expectedQuery));
}
@Test // SPR-16414
void getUriWithMalformedQueryParam() {
@Test
void getUriWithMalformedPath() {
mockRequest.setScheme("https");
mockRequest.setServerPort(443);
mockRequest.setServerName("example.com");
mockRequest.setRequestURI("/path");
mockRequest.setQueryString("query=foo%%x");
assertThat(request.getURI()).isEqualTo(URI.create("https://example.com/path"));
mockRequest.setRequestURI("/p%th");
assertThatIllegalStateException().isThrownBy(() -> request.getURI());
}
@Test // SPR-13876