diff --git a/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpRequest.java index 62023d264380..39dbd08d4d2d 100644 --- a/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpRequest.java @@ -50,6 +50,7 @@ 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 URI getURI() { */ 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); } } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java index fd8fb73524e0..fca387965ca4 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java @@ -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.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 @@ public ServletServerHttpRequest(MultiValueMap headers, HttpServl 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 @@ private static MultiValueMap createDefaultHttpHeaders(HttpServle 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") diff --git a/spring-web/src/test/java/org/springframework/http/server/ServletServerHttpRequestTests.java b/spring-web/src/test/java/org/springframework/http/server/ServletServerHttpRequestTests.java index a021b4c9b9b9..6dad7e7ab2e8 100644 --- a/spring-web/src/test/java/org/springframework/http/server/ServletServerHttpRequestTests.java +++ b/spring-web/src/test/java/org/springframework/http/server/ServletServerHttpRequestTests.java @@ -23,6 +23,8 @@ 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.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 @@ void getUriWithQueryString() { 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