HtmlUnitRequestBuilder decodes request parameter names (backport)

Issue: SPR-14177

Author: jhoeller

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

@@ -105,8 +105,8 @@ public MockHttpServletRequest buildRequest(ServletContext servletContext) {
 		String httpMethod = this.webRequest.getHttpMethod().name();
 		UriComponents uriComponents = uriComponents();
 
-		MockHttpServletRequest request = new HtmlUnitMockHttpServletRequest(servletContext, httpMethod,
-				uriComponents.getPath());
+		MockHttpServletRequest request = new HtmlUnitMockHttpServletRequest(
+				servletContext, httpMethod, uriComponents.getPath());
 		parent(request, this.parentBuilder);
 		request.setServerName(uriComponents.getHost()); // needs to be first for additional headers
 		authType(request);
@@ -123,7 +123,7 @@ public MockHttpServletRequest buildRequest(ServletContext servletContext) {
 		request.setProtocol("HTTP/1.1");
 		request.setQueryString(uriComponents.getQuery());
 		request.setScheme(uriComponents.getScheme());
-		pathInfo(uriComponents,request);
+		request.setPathInfo(null);
 
 		return postProcess(request);
 	}
@@ -223,14 +223,14 @@ private void content(MockHttpServletRequest request, String charset) {
 		try {
 			request.setContent(requestBody.getBytes(charset));
 		}
-		catch (UnsupportedEncodingException e) {
-			throw new RuntimeException(e);
+		catch (UnsupportedEncodingException ex) {
+			throw new IllegalStateException(ex);
 		}
 	}
 
 	private void contentType(MockHttpServletRequest request) {
 		String contentType = header("Content-Type");
-		request.setContentType(contentType == null ? MediaType.ALL_VALUE.toString() : contentType);
+		request.setContentType(contentType != null ? contentType : MediaType.ALL_VALUE);
 	}
 
 	private void contextPath(MockHttpServletRequest request, UriComponents uriComponents) {
@@ -245,8 +245,8 @@ private void contextPath(MockHttpServletRequest request, UriComponents uriCompon
 		}
 		else {
 			if (!uriComponents.getPath().startsWith(this.contextPath)) {
-				throw new IllegalArgumentException(uriComponents.getPath() + " should start with contextPath "
-						+ this.contextPath);
+				throw new IllegalArgumentException(uriComponents.getPath() + " should start with contextPath " +
+						this.contextPath);
 			}
 			request.setContextPath(this.contextPath);
 		}
@@ -360,21 +360,26 @@ private void locales(MockHttpServletRequest request) {
 	private void params(MockHttpServletRequest request, UriComponents uriComponents) {
 		for (Entry<String, List<String>> entry : uriComponents.getQueryParams().entrySet()) {
 			String name = entry.getKey();
+			String urlDecodedName = urlDecode(name);
 			for (String value : entry.getValue()) {
-				try {
-					value = (value != null ? URLDecoder.decode(value, "UTF-8") : "");
-					request.addParameter(name, value);
-				}
-				catch (UnsupportedEncodingException e) {
-					throw new RuntimeException(e);
-				}
+				value = (value != null ? urlDecode(value) : "");
+				request.addParameter(urlDecodedName, value);
 			}
 		}
 		for (NameValuePair param : this.webRequest.getRequestParameters()) {
 			request.addParameter(param.getName(), param.getValue());
 		}
 	}
 
+	private String urlDecode(String value) {
+		try {
+			return URLDecoder.decode(value, "UTF-8");
+		}
+		catch (UnsupportedEncodingException ex) {
+			throw new IllegalStateException(ex);
+		}
+	}
+
 	private Locale parseLocale(String locale) {
 		Matcher matcher = LOCALE_PATTERN.matcher(locale);
 		if (!matcher.matches()) {
@@ -392,10 +397,6 @@ private Locale parseLocale(String locale) {
 		return new Locale(language, country, qualifier);
 	}
 
-	private void pathInfo(UriComponents uriComponents, MockHttpServletRequest request) {
-		request.setPathInfo(null);
-	}
-
 	private void servletPath(MockHttpServletRequest request, String requestPath) {
 		String servletPath = requestPath.substring(request.getContextPath().length());
 		if ("".equals(servletPath)) {
@@ -426,8 +427,7 @@ private void ports(UriComponents uriComponents, MockHttpServletRequest request)
 
 	private UriComponents uriComponents() {
 		URL url = this.webRequest.getUrl();
-		UriComponentsBuilder uriBldr = UriComponentsBuilder.fromUriString(url.toExternalForm());
-		return uriBldr.build();
+		return UriComponentsBuilder.fromUriString(url.toExternalForm()).build();
 	}
 
 	@Override
@@ -450,14 +450,18 @@ public Object merge(Object parent) {
 		return this;
 	}
 
+	private CookieManager getCookieManager() {
+		return this.webClient.getCookieManager();
+	}
+
 
 	/**
-	 * An extension to {@link MockHttpServletRequest} that ensures that
-	 * when a new {@link HttpSession} is created, it is added to the managed sessions.
+	 * An extension to {@link MockHttpServletRequest} that ensures that when a
+	 * new {@link HttpSession} is created, it is added to the managed sessions.
 	 */
 	private final class HtmlUnitMockHttpServletRequest extends MockHttpServletRequest {
 
-		private HtmlUnitMockHttpServletRequest(ServletContext servletContext, String method, String requestURI) {
+		public HtmlUnitMockHttpServletRequest(ServletContext servletContext, String method, String requestURI) {
 			super(servletContext, method, requestURI);
 		}
 
@@ -486,16 +490,17 @@ public void setSession(HttpSession session) {
 		}
 	}
 
+
 	/**
 	 * An extension to {@link MockHttpSession} that ensures when
-	 * {@link #invalidate()} is called that the {@link HttpSession} is
-	 * removed from the managed sessions.
+	 * {@link #invalidate()} is called that the {@link HttpSession}
+	 * is removed from the managed sessions.
 	 */
 	private final class HtmlUnitMockHttpSession extends MockHttpSession {
 
 		private final MockHttpServletRequest request;
 
-		private HtmlUnitMockHttpSession(MockHttpServletRequest request) {
+		public HtmlUnitMockHttpSession(MockHttpServletRequest request) {
 			super(request.getServletContext());
 			this.request = request;
 		}
@@ -514,8 +519,4 @@ public void invalidate() {
 		}
 	}
 
-	private CookieManager getCookieManager() {
-		return this.webClient.getCookieManager();
-	}
-
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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,6 +16,7 @@
 
 package org.springframework.test.web.servlet.htmlunit;
 
+import java.io.InputStreamReader;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.Collections;
@@ -27,9 +28,11 @@
 import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpSession;
 
-import org.apache.commons.io.IOUtils;
+import com.gargoylesoftware.htmlunit.HttpMethod;
+import com.gargoylesoftware.htmlunit.WebClient;
+import com.gargoylesoftware.htmlunit.WebRequest;
+import com.gargoylesoftware.htmlunit.util.NameValuePair;
 import org.apache.http.auth.UsernamePasswordCredentials;
-
 import org.junit.Before;
 import org.junit.Test;
 
@@ -39,16 +42,12 @@
 import org.springframework.test.util.ReflectionTestUtils;
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.setup.MockMvcBuilders;
+import org.springframework.util.FileCopyUtils;
 
-import com.gargoylesoftware.htmlunit.HttpMethod;
-import com.gargoylesoftware.htmlunit.WebClient;
-import com.gargoylesoftware.htmlunit.WebRequest;
-import com.gargoylesoftware.htmlunit.util.NameValuePair;
-
-import static java.util.Arrays.asList;
+import static java.util.Arrays.*;
 import static org.hamcrest.Matchers.*;
-import static org.junit.Assert.assertThat;
-import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
+import static org.junit.Assert.*;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
 
 /**
  * Unit tests for {@link HtmlUnitRequestBuilder}.
@@ -77,7 +76,6 @@ public void setUp() throws Exception {
 		requestBuilder = new HtmlUnitRequestBuilder(sessions, webClient, webRequest);
 	}
 
-	// --- constructor
 
 	@Test(expected = IllegalArgumentException.class)
 	public void constructorNullSessions() {
@@ -94,8 +92,6 @@ public void constructorNullWebRequest() {
 		new HtmlUnitRequestBuilder(sessions, webClient, null);
 	}
 
-	// --- buildRequest
-
 	@Test
 	@SuppressWarnings("deprecation")
 	public void buildRequestBasicAuth() {
@@ -245,7 +241,8 @@ public void buildRequestInputStream() throws Exception {
 
 		MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
 
-		assertThat(IOUtils.toString(actualRequest.getInputStream()), equalTo(content));
+		assertThat(FileCopyUtils.copyToString(new InputStreamReader(actualRequest.getInputStream(), "ISO-8859-1")),
+				equalTo(content));
 	}
 
 	@Test
@@ -411,6 +408,26 @@ public void buildRequestParameterMapFromSingleQueryParam() throws Exception {
 		assertThat(actualRequest.getParameter("name"), equalTo("value"));
 	}
 
+	@Test  // SPR-14177
+	public void buildRequestParameterMapDecodesParameterName() throws Exception {
+		webRequest.setUrl(new URL("http://example.com/example/?row%5B0%5D=value"));
+
+		MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
+
+		assertThat(actualRequest.getParameterMap().size(), equalTo(1));
+		assertThat(actualRequest.getParameter("row[0]"), equalTo("value"));
+	}
+
+	@Test
+	public void buildRequestParameterMapDecodesParameterValue() throws Exception {
+		webRequest.setUrl(new URL("http://example.com/example/?name=row%5B0%5D"));
+
+		MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
+
+		assertThat(actualRequest.getParameterMap().size(), equalTo(1));
+		assertThat(actualRequest.getParameter("name"), equalTo("row[0]"));
+	}
+
 	@Test
 	public void buildRequestParameterMapFromSingleQueryParamWithoutValueAndWithoutEqualsSign() throws Exception {
 		webRequest.setUrl(new URL("http://example.com/example/?name"));
@@ -544,7 +561,7 @@ public void buildRequestReader() throws Exception {
 
 		MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
 
-		assertThat(IOUtils.toString(actualRequest.getReader()), equalTo(expectedBody));
+		assertThat(FileCopyUtils.copyToString(actualRequest.getReader()), equalTo(expectedBody));
 	}
 
 	@Test