Skip to content

Commit 283fc94

Browse files
sbrannensbrannen
committed
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
1 parent d5b4685 commit 283fc94

File tree

2 files changed

+114
-17
lines changed

2 files changed

+114
-17
lines changed

‎spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilder.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,10 @@ private void params(MockHttpServletRequest request, UriComponents uriComponents)
362362
String name = values.getKey();
363363
for (String value : values.getValue()) {
364364
try {
365-
request.addParameter(name, URLDecoder.decode(value, "UTF-8"));
365+
if (value != null) {
366+
value = URLDecoder.decode(value, "UTF-8");
367+
}
368+
request.addParameter(name, value);
366369
}
367370
catch (UnsupportedEncodingException e) {
368371
throw new RuntimeException(e);

‎spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilderTests.java‎

Lines changed: 110 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818

1919
import java.net.MalformedURLException;
2020
import java.net.URL;
21-
import java.util.ArrayList;
22-
import java.util.Arrays;
2321
import java.util.Collections;
2422
import java.util.HashMap;
2523
import java.util.List;
@@ -47,6 +45,7 @@
4745
import com.gargoylesoftware.htmlunit.WebRequest;
4846
import com.gargoylesoftware.htmlunit.util.NameValuePair;
4947

48+
import static java.util.Arrays.asList;
5049
import static org.hamcrest.Matchers.*;
5150
import static org.junit.Assert.assertThat;
5251
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
@@ -75,7 +74,6 @@ public class HtmlUnitRequestBuilderTests {
7574
public void setUp() throws Exception {
7675
webRequest = new WebRequest(new URL("http://example.com:80/test/this/here"));
7776
webRequest.setHttpMethod(HttpMethod.GET);
78-
webRequest.setRequestParameters(new ArrayList<>());
7977
requestBuilder = new HtmlUnitRequestBuilder(sessions, webClient, webRequest);
8078
}
8179

@@ -314,9 +312,9 @@ public void buildRequestLocaleMulti() {
314312

315313
MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
316314

317-
// FIXME Locale.ENGLISH is due to fact cannot remove it from MockHttpServletRequest
318-
List<Locale> expected = Arrays.asList(new Locale("da"), new Locale("en", "gb", "0.8"), new Locale("en", "",
319-
"0.7"), Locale.ENGLISH);
315+
// Append Locale.ENGLISH since MockHttpServletRequest automatically sets it as the
316+
// preferred locale.
317+
List<Locale> expected = asList(new Locale("da"), new Locale("en", "gb", "0.8"), new Locale("en", "", "0.7"), Locale.ENGLISH);
320318
assertThat(Collections.list(actualRequest.getLocales()), equalTo(expected));
321319
}
322320

@@ -344,25 +342,64 @@ public void buildRequestLocalMissing() throws Exception {
344342

345343
@Test
346344
public void buildRequestMethods() {
347-
HttpMethod[] methods = HttpMethod.values();
348-
349-
for (HttpMethod expectedMethod : methods) {
345+
for (HttpMethod expectedMethod : HttpMethod.values()) {
350346
webRequest.setHttpMethod(expectedMethod);
351347
String actualMethod = requestBuilder.buildRequest(servletContext).getMethod();
352348
assertThat(actualMethod, equalTo(expectedMethod.name()));
353349
}
354350
}
355351

356352
@Test
357-
public void buildRequestParameterMap() throws Exception {
358-
setParameter("name", "value");
353+
public void buildRequestParameterMapViaWebRequestDotSetRequestParametersWithSingleRequestParam() {
354+
webRequest.setRequestParameters(asList(new NameValuePair("name", "value")));
359355

360356
MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
361357

362358
assertThat(actualRequest.getParameterMap().size(), equalTo(1));
363359
assertThat(actualRequest.getParameter("name"), equalTo("value"));
364360
}
365361

362+
@Test
363+
public void buildRequestParameterMapViaWebRequestDotSetRequestParametersWithSingleRequestParamWithNullValue() {
364+
webRequest.setRequestParameters(asList(new NameValuePair("name", null)));
365+
366+
MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
367+
368+
assertThat(actualRequest.getParameterMap().size(), equalTo(1));
369+
assertThat(actualRequest.getParameter("name"), nullValue());
370+
}
371+
372+
@Test
373+
public void buildRequestParameterMapViaWebRequestDotSetRequestParametersWithSingleRequestParamWithEmptyValue() {
374+
webRequest.setRequestParameters(asList(new NameValuePair("name", "")));
375+
376+
MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
377+
378+
assertThat(actualRequest.getParameterMap().size(), equalTo(1));
379+
assertThat(actualRequest.getParameter("name"), equalTo(""));
380+
}
381+
382+
@Test
383+
public void buildRequestParameterMapViaWebRequestDotSetRequestParametersWithSingleRequestParamWithValueSetToSpace() {
384+
webRequest.setRequestParameters(asList(new NameValuePair("name", " ")));
385+
386+
MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
387+
388+
assertThat(actualRequest.getParameterMap().size(), equalTo(1));
389+
assertThat(actualRequest.getParameter("name"), equalTo(" "));
390+
}
391+
392+
@Test
393+
public void buildRequestParameterMapViaWebRequestDotSetRequestParametersWithMultipleRequestParams() {
394+
webRequest.setRequestParameters(asList(new NameValuePair("name1", "value1"), new NameValuePair("name2", "value2")));
395+
396+
MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
397+
398+
assertThat(actualRequest.getParameterMap().size(), equalTo(2));
399+
assertThat(actualRequest.getParameter("name1"), equalTo("value1"));
400+
assertThat(actualRequest.getParameter("name2"), equalTo("value2"));
401+
}
402+
366403
@Test
367404
public void buildRequestParameterMapFromSingleQueryParam() throws Exception {
368405
webRequest.setUrl(new URL("http://example.com/example/?name=value"));
@@ -373,6 +410,36 @@ public void buildRequestParameterMapFromSingleQueryParam() throws Exception {
373410
assertThat(actualRequest.getParameter("name"), equalTo("value"));
374411
}
375412

413+
@Test
414+
public void buildRequestParameterMapFromSingleQueryParamWithoutValueAndWithoutEqualsSign() throws Exception {
415+
webRequest.setUrl(new URL("http://example.com/example/?name"));
416+
417+
MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
418+
419+
assertThat(actualRequest.getParameterMap().size(), equalTo(1));
420+
assertThat(actualRequest.getParameter("name"), nullValue());
421+
}
422+
423+
@Test
424+
public void buildRequestParameterMapFromSingleQueryParamWithoutValueButWithEqualsSign() throws Exception {
425+
webRequest.setUrl(new URL("http://example.com/example/?name="));
426+
427+
MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
428+
429+
assertThat(actualRequest.getParameterMap().size(), equalTo(1));
430+
assertThat(actualRequest.getParameter("name"), equalTo(""));
431+
}
432+
433+
@Test
434+
public void buildRequestParameterMapFromSingleQueryParamWithValueSetToEncodedSpace() throws Exception {
435+
webRequest.setUrl(new URL("http://example.com/example/?name=%20"));
436+
437+
MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
438+
439+
assertThat(actualRequest.getParameterMap().size(), equalTo(1));
440+
assertThat(actualRequest.getParameter("name"), equalTo(" "));
441+
}
442+
376443
@Test
377444
public void buildRequestParameterMapFromMultipleQueryParams() throws Exception {
378445
webRequest.setUrl(new URL("http://example.com/example/?name=value&param2=value+2"));
@@ -428,6 +495,36 @@ public void buildRequestQueryWithSingleQueryParam() throws Exception {
428495
assertThat(actualRequest.getQueryString(), equalTo(expectedQuery));
429496
}
430497

498+
@Test
499+
public void buildRequestQueryWithSingleQueryParamWithoutValueAndWithoutEqualsSign() throws Exception {
500+
String expectedQuery = "param";
501+
webRequest.setUrl(new URL("http://example.com/example?" + expectedQuery));
502+
503+
MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
504+
505+
assertThat(actualRequest.getQueryString(), equalTo(expectedQuery));
506+
}
507+
508+
@Test
509+
public void buildRequestQueryWithSingleQueryParamWithoutValueButWithEqualsSign() throws Exception {
510+
String expectedQuery = "param=";
511+
webRequest.setUrl(new URL("http://example.com/example?" + expectedQuery));
512+
513+
MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
514+
515+
assertThat(actualRequest.getQueryString(), equalTo(expectedQuery));
516+
}
517+
518+
@Test
519+
public void buildRequestQueryWithSingleQueryParamWithValueSetToEncodedSpace() throws Exception {
520+
String expectedQuery = "param=%20";
521+
webRequest.setUrl(new URL("http://example.com/example?" + expectedQuery));
522+
523+
MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
524+
525+
assertThat(actualRequest.getQueryString(), equalTo(expectedQuery));
526+
}
527+
431528
@Test
432529
public void buildRequestQueryWithMultipleQueryParams() throws Exception {
433530
String expectedQuery = "param1=value1&param2=value2";
@@ -744,7 +841,8 @@ public void mergeParameter() throws Exception {
744841
.defaultRequest(get("/").param(paramName, paramValue, paramValue2))
745842
.build();
746843

747-
assertThat(Arrays.asList(mockMvc.perform(requestBuilder).andReturn().getRequest().getParameterValues(paramName)), contains(paramValue, paramValue2));
844+
MockHttpServletRequest performedRequest = mockMvc.perform(requestBuilder).andReturn().getRequest();
845+
assertThat(asList(performedRequest.getParameterValues(paramName)), contains(paramValue, paramValue2));
748846
}
749847

750848
@Test
@@ -785,10 +883,6 @@ private void assertSingleSessionCookie(String expected) {
785883
assertThat("JSESSIONID=" + actual + "; Path=/test; Domain=example.com", equalTo(expected));
786884
}
787885

788-
private void setParameter(String name, String value) {
789-
webRequest.getRequestParameters().add(new NameValuePair(name, value));
790-
}
791-
792886
private String getContextPath() {
793887
return (String) ReflectionTestUtils.getField(requestBuilder, "contextPath");
794888
}

0 commit comments

Comments
 (0)