From af83d2332a0011d83a4a775d57e2d28b9eb53bee Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Mon, 31 Jul 2017 17:30:22 +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 | 22 ++++++++++--------- .../filter/HttpPutFormContentFilterTests.java | 11 +++++++++- 2 files changed, 22 insertions(+), 11 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 0768f05dc69..c6bd492ef24 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 @@ -98,12 +98,14 @@ public class HttpPutFormContentFilter extends OncePerRequestFilter { } }; MultiValueMap formParameters = this.formConverter.read(null, inputMessage); - HttpServletRequest wrapper = new HttpPutFormContentRequestWrapper(request, formParameters); - filterChain.doFilter(wrapper, response); - } - else { - filterChain.doFilter(request, response); + if (!formParameters.isEmpty()) { + HttpServletRequest wrapper = new HttpPutFormContentRequestWrapper(request, formParameters); + filterChain.doFilter(wrapper, response); + return; + } } + + filterChain.doFilter(request, response); } private boolean isFormContentType(HttpServletRequest request) { @@ -162,17 +164,17 @@ public class HttpPutFormContentFilter extends OncePerRequestFilter { @Override @Nullable public String[] getParameterValues(String name) { - String[] queryParam = (super.getQueryString() != null ? super.getParameterValues(name) : null); + String[] parameterValues = super.getParameterValues(name); List 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 result = new ArrayList<>(queryParam.length + formParam.size()); - result.addAll(Arrays.asList(queryParam)); + List result = new ArrayList<>(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 6a5de837ddd..07758e96fb4 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")); + } + }