From 283fc945722365eeb4986d3dbc503ff93b45e2b7 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 30 Sep 2015 21:48:05 +0200 Subject: [PATCH] Support null query param values in HtmlUnitRequestBuilder Prior to this commit, HtmlUnitRequestBuilder would incorrectly attempt to decode null values for query parameters (i.e., query parameters such as 'error' in 'http://example.com/login?error') which resulted in a NullPointerException since URLDecoder.decode() does not support null values. This commit fixes this issue by ensuring that HtmlUnitRequestBuilder only attempts to decode non-null query parameter values. Issue: SPR-13524 --- .../htmlunit/HtmlUnitRequestBuilder.java | 5 +- .../htmlunit/HtmlUnitRequestBuilderTests.java | 126 +++++++++++++++--- 2 files changed, 114 insertions(+), 17 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilder.java index 5e27edf14f..e38a9b27c2 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilder.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilder.java @@ -362,7 +362,10 @@ final class HtmlUnitRequestBuilder implements RequestBuilder, Mergeable { String name = values.getKey(); for (String value : values.getValue()) { try { - request.addParameter(name, URLDecoder.decode(value, "UTF-8")); + if (value != null) { + value = URLDecoder.decode(value, "UTF-8"); + } + request.addParameter(name, value); } catch (UnsupportedEncodingException e) { throw new RuntimeException(e); diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilderTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilderTests.java index 330e4ae2ee..61e09c3271 100644 --- a/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilderTests.java +++ b/spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilderTests.java @@ -18,8 +18,6 @@ package org.springframework.test.web.servlet.htmlunit; import java.net.MalformedURLException; import java.net.URL; -import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -47,6 +45,7 @@ import com.gargoylesoftware.htmlunit.WebClient; import com.gargoylesoftware.htmlunit.WebRequest; import com.gargoylesoftware.htmlunit.util.NameValuePair; +import static java.util.Arrays.asList; import static org.hamcrest.Matchers.*; import static org.junit.Assert.assertThat; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; @@ -75,7 +74,6 @@ public class HtmlUnitRequestBuilderTests { public void setUp() throws Exception { webRequest = new WebRequest(new URL("http://example.com:80/test/this/here")); webRequest.setHttpMethod(HttpMethod.GET); - webRequest.setRequestParameters(new ArrayList<>()); requestBuilder = new HtmlUnitRequestBuilder(sessions, webClient, webRequest); } @@ -314,9 +312,9 @@ public class HtmlUnitRequestBuilderTests { MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext); - // FIXME Locale.ENGLISH is due to fact cannot remove it from MockHttpServletRequest - List expected = Arrays.asList(new Locale("da"), new Locale("en", "gb", "0.8"), new Locale("en", "", - "0.7"), Locale.ENGLISH); + // Append Locale.ENGLISH since MockHttpServletRequest automatically sets it as the + // preferred locale. + List expected = asList(new Locale("da"), new Locale("en", "gb", "0.8"), new Locale("en", "", "0.7"), Locale.ENGLISH); assertThat(Collections.list(actualRequest.getLocales()), equalTo(expected)); } @@ -344,9 +342,7 @@ public class HtmlUnitRequestBuilderTests { @Test public void buildRequestMethods() { - HttpMethod[] methods = HttpMethod.values(); - - for (HttpMethod expectedMethod : methods) { + for (HttpMethod expectedMethod : HttpMethod.values()) { webRequest.setHttpMethod(expectedMethod); String actualMethod = requestBuilder.buildRequest(servletContext).getMethod(); assertThat(actualMethod, equalTo(expectedMethod.name())); @@ -354,8 +350,8 @@ public class HtmlUnitRequestBuilderTests { } @Test - public void buildRequestParameterMap() throws Exception { - setParameter("name", "value"); + public void buildRequestParameterMapViaWebRequestDotSetRequestParametersWithSingleRequestParam() { + webRequest.setRequestParameters(asList(new NameValuePair("name", "value"))); MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext); @@ -363,6 +359,47 @@ public class HtmlUnitRequestBuilderTests { assertThat(actualRequest.getParameter("name"), equalTo("value")); } + @Test + public void buildRequestParameterMapViaWebRequestDotSetRequestParametersWithSingleRequestParamWithNullValue() { + webRequest.setRequestParameters(asList(new NameValuePair("name", null))); + + MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext); + + assertThat(actualRequest.getParameterMap().size(), equalTo(1)); + assertThat(actualRequest.getParameter("name"), nullValue()); + } + + @Test + public void buildRequestParameterMapViaWebRequestDotSetRequestParametersWithSingleRequestParamWithEmptyValue() { + webRequest.setRequestParameters(asList(new NameValuePair("name", ""))); + + MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext); + + assertThat(actualRequest.getParameterMap().size(), equalTo(1)); + assertThat(actualRequest.getParameter("name"), equalTo("")); + } + + @Test + public void buildRequestParameterMapViaWebRequestDotSetRequestParametersWithSingleRequestParamWithValueSetToSpace() { + webRequest.setRequestParameters(asList(new NameValuePair("name", " "))); + + MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext); + + assertThat(actualRequest.getParameterMap().size(), equalTo(1)); + assertThat(actualRequest.getParameter("name"), equalTo(" ")); + } + + @Test + public void buildRequestParameterMapViaWebRequestDotSetRequestParametersWithMultipleRequestParams() { + webRequest.setRequestParameters(asList(new NameValuePair("name1", "value1"), new NameValuePair("name2", "value2"))); + + MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext); + + assertThat(actualRequest.getParameterMap().size(), equalTo(2)); + assertThat(actualRequest.getParameter("name1"), equalTo("value1")); + assertThat(actualRequest.getParameter("name2"), equalTo("value2")); + } + @Test public void buildRequestParameterMapFromSingleQueryParam() throws Exception { webRequest.setUrl(new URL("http://example.com/example/?name=value")); @@ -373,6 +410,36 @@ public class HtmlUnitRequestBuilderTests { assertThat(actualRequest.getParameter("name"), equalTo("value")); } + @Test + public void buildRequestParameterMapFromSingleQueryParamWithoutValueAndWithoutEqualsSign() throws Exception { + webRequest.setUrl(new URL("http://example.com/example/?name")); + + MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext); + + assertThat(actualRequest.getParameterMap().size(), equalTo(1)); + assertThat(actualRequest.getParameter("name"), nullValue()); + } + + @Test + public void buildRequestParameterMapFromSingleQueryParamWithoutValueButWithEqualsSign() throws Exception { + webRequest.setUrl(new URL("http://example.com/example/?name=")); + + MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext); + + assertThat(actualRequest.getParameterMap().size(), equalTo(1)); + assertThat(actualRequest.getParameter("name"), equalTo("")); + } + + @Test + public void buildRequestParameterMapFromSingleQueryParamWithValueSetToEncodedSpace() throws Exception { + webRequest.setUrl(new URL("http://example.com/example/?name=%20")); + + MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext); + + assertThat(actualRequest.getParameterMap().size(), equalTo(1)); + assertThat(actualRequest.getParameter("name"), equalTo(" ")); + } + @Test public void buildRequestParameterMapFromMultipleQueryParams() throws Exception { webRequest.setUrl(new URL("http://example.com/example/?name=value¶m2=value+2")); @@ -428,6 +495,36 @@ public class HtmlUnitRequestBuilderTests { assertThat(actualRequest.getQueryString(), equalTo(expectedQuery)); } + @Test + public void buildRequestQueryWithSingleQueryParamWithoutValueAndWithoutEqualsSign() throws Exception { + String expectedQuery = "param"; + webRequest.setUrl(new URL("http://example.com/example?" + expectedQuery)); + + MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext); + + assertThat(actualRequest.getQueryString(), equalTo(expectedQuery)); + } + + @Test + public void buildRequestQueryWithSingleQueryParamWithoutValueButWithEqualsSign() throws Exception { + String expectedQuery = "param="; + webRequest.setUrl(new URL("http://example.com/example?" + expectedQuery)); + + MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext); + + assertThat(actualRequest.getQueryString(), equalTo(expectedQuery)); + } + + @Test + public void buildRequestQueryWithSingleQueryParamWithValueSetToEncodedSpace() throws Exception { + String expectedQuery = "param=%20"; + webRequest.setUrl(new URL("http://example.com/example?" + expectedQuery)); + + MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext); + + assertThat(actualRequest.getQueryString(), equalTo(expectedQuery)); + } + @Test public void buildRequestQueryWithMultipleQueryParams() throws Exception { String expectedQuery = "param1=value1¶m2=value2"; @@ -744,7 +841,8 @@ public class HtmlUnitRequestBuilderTests { .defaultRequest(get("/").param(paramName, paramValue, paramValue2)) .build(); - assertThat(Arrays.asList(mockMvc.perform(requestBuilder).andReturn().getRequest().getParameterValues(paramName)), contains(paramValue, paramValue2)); + MockHttpServletRequest performedRequest = mockMvc.perform(requestBuilder).andReturn().getRequest(); + assertThat(asList(performedRequest.getParameterValues(paramName)), contains(paramValue, paramValue2)); } @Test @@ -785,10 +883,6 @@ public class HtmlUnitRequestBuilderTests { assertThat("JSESSIONID=" + actual + "; Path=/test; Domain=example.com", equalTo(expected)); } - private void setParameter(String name, String value) { - webRequest.getRequestParameters().add(new NameValuePair(name, value)); - } - private String getContextPath() { return (String) ReflectionTestUtils.getField(requestBuilder, "contextPath"); }