From 35248498aeb269d280a63ff4c15e4d845408871a Mon Sep 17 00:00:00 2001
From: Rossen Stoyanchev <rstoyanchev@pivotal.io>
Date: Tue, 1 Aug 2017 12:41:45 +0200
Subject: [PATCH] Fix regression in HttpPutFormContentFilter

Re-arrange the checks so that if there is no form parameter, then
immediately and unconditionally delegate to super.getParameterValues().
Or reversely if there is no super.getParameterValues() then return the
form parameter.

So the only remaining case is when combining values present in both.
In that case we'll take both only if a queryString exists.

One extra fix is to not even wrap the request if we did not parse any
form parameters at all which can happen with HttpHiddenMethodFilter.

Issue: SPR-15828, 15835
---
 .../web/filter/HttpPutFormContentFilter.java  | 24 ++++++++++---------
 .../filter/HttpPutFormContentFilterTests.java | 11 ++++++++-
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/spring-web/src/main/java/org/springframework/web/filter/HttpPutFormContentFilter.java b/spring-web/src/main/java/org/springframework/web/filter/HttpPutFormContentFilter.java
index 5f03905e5f..8a8146cfba 100644
--- a/spring-web/src/main/java/org/springframework/web/filter/HttpPutFormContentFilter.java
+++ b/spring-web/src/main/java/org/springframework/web/filter/HttpPutFormContentFilter.java
@@ -97,13 +97,15 @@ public class HttpPutFormContentFilter extends OncePerRequestFilter {
 					return request.getInputStream();
 				}
 			};
-			MultiValueMap<String, String> formParameters = formConverter.read(null, inputMessage);
-			HttpServletRequest wrapper = new HttpPutFormContentRequestWrapper(request, formParameters);
-			filterChain.doFilter(wrapper, response);
-		}
-		else {
-			filterChain.doFilter(request, response);
+			MultiValueMap<String, String> formParameters = this.formConverter.read(null, inputMessage);
+			if (!formParameters.isEmpty()) {
+				HttpServletRequest wrapper = new HttpPutFormContentRequestWrapper(request, formParameters);
+				filterChain.doFilter(wrapper, response);
+				return;
+			}
 		}
+
+		filterChain.doFilter(request, response);
 	}
 
 	private boolean isFormContentType(HttpServletRequest request) {
@@ -160,17 +162,17 @@ public class HttpPutFormContentFilter extends OncePerRequestFilter {
 
 		@Override
 		public String[] getParameterValues(String name) {
-			String[] queryParam = (super.getQueryString() != null ? super.getParameterValues(name) : null);
+			String[] parameterValues = super.getParameterValues(name);
 			List<String> formParam = this.formParameters.get(name);
 			if (formParam == null) {
-				return queryParam;
+				return parameterValues;
 			}
-			else if (queryParam == null) {
+			if (parameterValues == null || getQueryString() == null) {
 				return formParam.toArray(new String[formParam.size()]);
 			}
 			else {
-				List<String> result = new ArrayList<String>(queryParam.length + formParam.size());
-				result.addAll(Arrays.asList(queryParam));
+				List<String> result = new ArrayList<String>(parameterValues.length + formParam.size());
+				result.addAll(Arrays.asList(parameterValues));
 				result.addAll(formParam);
 				return result.toArray(new String[result.size()]);
 			}
diff --git a/spring-web/src/test/java/org/springframework/web/filter/HttpPutFormContentFilterTests.java b/spring-web/src/test/java/org/springframework/web/filter/HttpPutFormContentFilterTests.java
index 6a5de837dd..07758e96fb 100644
--- a/spring-web/src/test/java/org/springframework/web/filter/HttpPutFormContentFilterTests.java
+++ b/spring-web/src/test/java/org/springframework/web/filter/HttpPutFormContentFilterTests.java
@@ -58,7 +58,7 @@ public class HttpPutFormContentFilterTests {
 
 	@Test
 	public void wrapPutAndPatchOnly() throws Exception {
-		request.setContent("".getBytes("ISO-8859-1"));
+		request.setContent("foo=bar".getBytes("ISO-8859-1"));
 		for (HttpMethod method : HttpMethod.values()) {
 			request.setMethod(method.name());
 			filterChain = new MockFilterChain();
@@ -204,4 +204,13 @@ public class HttpPutFormContentFilterTests {
 		assertArrayEquals(new String[] {"value4"}, parameters.get("name4"));
 	}
 
+	@Test // SPR-15835
+	public void hiddenHttpMethodFilterFollowedByHttpPutFormContentFilter() throws Exception {
+		request.addParameter("_method", "PUT");
+		request.addParameter("hiddenField", "testHidden");
+		filter.doFilter(request, response, filterChain);
+
+		assertArrayEquals(new String[]{"testHidden"}, filterChain.getRequest().getParameterValues("hiddenField"));
+	}
+
 }