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
This commit is contained in:
		
							parent
							
								
									ce0bce28da
								
							
						
					
					
						commit
						af83d2332a
					
				| 
						 | 
					@ -98,12 +98,14 @@ public class HttpPutFormContentFilter extends OncePerRequestFilter {
 | 
				
			||||||
				}
 | 
									}
 | 
				
			||||||
			};
 | 
								};
 | 
				
			||||||
			MultiValueMap<String, String> formParameters = this.formConverter.read(null, inputMessage);
 | 
								MultiValueMap<String, String> formParameters = this.formConverter.read(null, inputMessage);
 | 
				
			||||||
			HttpServletRequest wrapper = new HttpPutFormContentRequestWrapper(request, formParameters);
 | 
								if (!formParameters.isEmpty()) {
 | 
				
			||||||
			filterChain.doFilter(wrapper, response);
 | 
									HttpServletRequest wrapper = new HttpPutFormContentRequestWrapper(request, formParameters);
 | 
				
			||||||
		}
 | 
									filterChain.doFilter(wrapper, response);
 | 
				
			||||||
		else {
 | 
									return;
 | 
				
			||||||
			filterChain.doFilter(request, response);
 | 
								}
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
							filterChain.doFilter(request, response);
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	private boolean isFormContentType(HttpServletRequest request) {
 | 
						private boolean isFormContentType(HttpServletRequest request) {
 | 
				
			||||||
| 
						 | 
					@ -162,17 +164,17 @@ public class HttpPutFormContentFilter extends OncePerRequestFilter {
 | 
				
			||||||
		@Override
 | 
							@Override
 | 
				
			||||||
		@Nullable
 | 
							@Nullable
 | 
				
			||||||
		public String[] getParameterValues(String name) {
 | 
							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);
 | 
								List<String> formParam = this.formParameters.get(name);
 | 
				
			||||||
			if (formParam == null) {
 | 
								if (formParam == null) {
 | 
				
			||||||
				return queryParam;
 | 
									return parameterValues;
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
			else if (queryParam == null) {
 | 
								if (parameterValues == null || getQueryString() == null) {
 | 
				
			||||||
				return formParam.toArray(new String[formParam.size()]);
 | 
									return formParam.toArray(new String[formParam.size()]);
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
			else {
 | 
								else {
 | 
				
			||||||
				List<String> result = new ArrayList<>(queryParam.length + formParam.size());
 | 
									List<String> result = new ArrayList<>(parameterValues.length + formParam.size());
 | 
				
			||||||
				result.addAll(Arrays.asList(queryParam));
 | 
									result.addAll(Arrays.asList(parameterValues));
 | 
				
			||||||
				result.addAll(formParam);
 | 
									result.addAll(formParam);
 | 
				
			||||||
				return result.toArray(new String[result.size()]);
 | 
									return result.toArray(new String[result.size()]);
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -58,7 +58,7 @@ public class HttpPutFormContentFilterTests {
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	@Test
 | 
						@Test
 | 
				
			||||||
	public void wrapPutAndPatchOnly() throws Exception {
 | 
						public void wrapPutAndPatchOnly() throws Exception {
 | 
				
			||||||
		request.setContent("".getBytes("ISO-8859-1"));
 | 
							request.setContent("foo=bar".getBytes("ISO-8859-1"));
 | 
				
			||||||
		for (HttpMethod method : HttpMethod.values()) {
 | 
							for (HttpMethod method : HttpMethod.values()) {
 | 
				
			||||||
			request.setMethod(method.name());
 | 
								request.setMethod(method.name());
 | 
				
			||||||
			filterChain = new MockFilterChain();
 | 
								filterChain = new MockFilterChain();
 | 
				
			||||||
| 
						 | 
					@ -204,4 +204,13 @@ public class HttpPutFormContentFilterTests {
 | 
				
			||||||
		assertArrayEquals(new String[] {"value4"}, parameters.get("name4"));
 | 
							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"));
 | 
				
			||||||
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in New Issue