Skip to content

Commit

Permalink
Reverse SPR-10402 change that caused 3.2.3 regression
Browse files Browse the repository at this point in the history
SPR-10402 in Spring Framework 3.2.3 treated empty request parameter
values as missing values, if the empty value was turned into a null
by a PropertyEditor or Converter. This caused the regression.

Issue: SPR-10578, SPR-10402, SPR-10584
  • Loading branch information
rstoyanchev committed Jun 20, 2013
1 parent 5feac07 commit 2d8315f
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 15 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2012 the original author or authors.
* Copyright 2002-2013 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.
Expand Down Expand Up @@ -97,17 +97,11 @@ else if ("".equals(arg) && (namedValueInfo.defaultValue != null)) {
arg = resolveDefaultValue(namedValueInfo.defaultValue);
}

boolean emptyArgValue = "".equals(arg);

if (binderFactory != null) {
WebDataBinder binder = binderFactory.createBinder(webRequest, null, namedValueInfo.name);
arg = binder.convertIfNecessary(arg, paramType, parameter);
}

if (emptyArgValue && (arg == null)) {
handleMissingValue(namedValueInfo.name, parameter);
}

handleResolvedValue(arg, namedValueInfo.name, parameter, mavContainer, webRequest);

return arg;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2012 the original author or authors.
* Copyright 2002-2013 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.
Expand Down Expand Up @@ -49,6 +49,7 @@
import static org.mockito.BDDMockito.*;
import static org.mockito.Mockito.*;


/**
* Test fixture with {@link org.springframework.web.method.annotation.RequestParamMethodArgumentResolver}.
*
Expand All @@ -70,6 +71,7 @@ public class RequestParamMethodArgumentResolverTests {
private MethodParameter paramServlet30Part;
private MethodParameter paramRequestPartAnnot;
private MethodParameter paramRequired;
private MethodParameter paramNotRequired;

private NativeWebRequest webRequest;

Expand All @@ -81,8 +83,10 @@ public void setUp() throws Exception {

ParameterNameDiscoverer paramNameDiscoverer = new LocalVariableTableParameterNameDiscoverer();

Method method = getClass().getMethod("params", String.class, String[].class, Map.class, MultipartFile.class,
Map.class, String.class, MultipartFile.class, List.class, Part.class, MultipartFile.class, String.class);
Method method = getClass().getMethod("params", String.class, String[].class,
Map.class, MultipartFile.class, Map.class, String.class,
MultipartFile.class, List.class, Part.class, MultipartFile.class,
String.class, String.class);

paramNamedDefaultValueString = new MethodParameter(method, 0);
paramNamedStringArray = new MethodParameter(method, 1);
Expand All @@ -99,6 +103,7 @@ public void setUp() throws Exception {
paramServlet30Part.initParameterNameDiscovery(paramNameDiscoverer);
paramRequestPartAnnot = new MethodParameter(method, 9);
paramRequired = new MethodParameter(method, 10);
paramNotRequired = new MethodParameter(method, 11);

request = new MockHttpServletRequest();
webRequest = new ServletWebRequest(request, new MockHttpServletResponse());
Expand Down Expand Up @@ -243,9 +248,9 @@ public void missingRequestParam() throws Exception {
fail("Expected exception");
}

// SPR-10402
// SPR-10578

@Test(expected = MissingServletRequestParameterException.class)
@Test
public void missingRequestParamEmptyValueConvertedToNull() throws Exception {

WebDataBinder binder = new WebRequestDataBinder(null);
Expand All @@ -256,8 +261,25 @@ public void missingRequestParamEmptyValueConvertedToNull() throws Exception {

this.request.addParameter("stringNotAnnot", "");

resolver.resolveArgument(paramStringNotAnnot, null, webRequest, binderFactory);
fail("Expected exception");
Object arg = resolver.resolveArgument(paramStringNotAnnot, null, webRequest, binderFactory);

assertNull(arg);
}

@Test
public void missingRequestParamEmptyValueNotRequired() throws Exception {

WebDataBinder binder = new WebRequestDataBinder(null);
binder.registerCustomEditor(String.class, new StringTrimmerEditor(true));

WebDataBinderFactory binderFactory = mock(WebDataBinderFactory.class);
given(binderFactory.createBinder(webRequest, null, "name")).willReturn(binder);

this.request.addParameter("name", "");

Object arg = resolver.resolveArgument(paramNotRequired, null, webRequest, binderFactory);

assertNull(arg);
}

@Test
Expand Down Expand Up @@ -311,7 +333,8 @@ public void params(@RequestParam(value = "name", defaultValue = "bar") String pa
List<MultipartFile> multipartFileList,
Part servlet30Part,
@RequestPart MultipartFile requestPartAnnot,
@RequestParam(value = "name") String paramRequired) {
@RequestParam(value = "name") String paramRequired,
@RequestParam(value = "name", required=false) String paramNotRequired) {
}

}

0 comments on commit 2d8315f

Please sign in to comment.