[SPR-10578] Revert change for SPR-10402 that allowed treating empty values as missing values Created: 21/May/13  Updated: 15/Jan/19  Resolved: 20/Jun/13

Status: Closed
Project: Spring Framework
Component/s: Web
Affects Version/s: 3.2.3
Fix Version/s: 3.2.4, 4.0 M2

Type: Bug Priority: Blocker
Reporter: Alfred Staflinger Assignee: Rossen Stoyanchev
Resolution: Complete Votes: 6
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
is duplicated by SPR-10584 Optional parameter in web method now ... Resolved
is duplicated by SPR-10592 spring 3.2.3 @RequestParam(value="use... Resolved
Relate
relates to SPR-10402 Request @RequestParam not enforced wi... Closed
Days since last comment: 43 weeks, 6 days ago
Last commented by a User: true
Last updater: Spring Issuemaster

 Description   

Today, I have downloaded Spring Framework 3.2.3.

Now, handleMissingValue is called, even if the @RequestParam attribute required = false!

IMHO, this new behaviour is suboptimal. We have many cases, where an empty string parameter value (which has been converted to null by the property editor) should not be handled as missing parameter (even if required = true)! And I guess that many others who relied on the prior behaviour will be confused, too.



 Comments   
Comment by Rossen Stoyanchev [ 21/May/13 ]

Apologies for the regression.

> Now, handleMissingValue is called, even if the @RequestParam attribute required = false!

That was not intended and can be fixed.

> IMHO, this new behaviour is suboptimal. We have many cases, where an empty string parameter value (which has been converted to null by the property editor) should not be handled as missing parameter (even if required = true)

I can see that semantically an empty string satisfies required=true. However, if then converted to null effectively you don't require it. At least from the controller method's perspective, the logic must accommodate the possibility of null. The distinction that there was a "" is gone. So just trying to understand what is the reason for doing this?

Comment by Alfred Staflinger [ 21/May/13 ]

We have got this @ControllerAdvice ...

import org.springframework.beans.propertyeditors.StringTrimmerEditor;
import org.springframework.web.bind.WebDataBinder;
import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.bind.annotation.InitBinder;

@ControllerAdvice
public class GlobalControllerAdvice {

	@InitBinder
	public void initBinder(WebDataBinder binder) {
		binder.registerCustomEditor(String.class, new StringTrimmerEditor(
				"\u0000\u0001\u0002\u0003\u0004\u0005\u0006\u0007\u0008\u000b\u000c\u000e\u000f\u0010\u0011\u0012\u0013\u0014\u0015\u0016\u0017\u0018\u0019\u001a\u001b\u001c\u001d\u001e\u001f\u007f",
				true));
	}

}

... because we want to have null values instead of empty strings througout the whole application.

For example, we have got two request mappings within the same @Controller:

@RequestMapping(value = "/foo", params = "!param2")
public String method1(@RequestParam String param1)  {
	...
}
@RequestMapping(value = "/foo", params = "param2")
public String method2(@RequestParam String param1, @RequestParam String param2)  {
	...
	if (StringUtils.hasText(param2)) {
		...
	} else {
		...
	}
	...
}

While method1 should handle this request: /foo?param1=value1 (param2 not part of query string),
... method2 should handle /foo?param1=value1&param2= (param2 is empty string)
and /foo?param1=value1&param2=value2 (param2 is non-empty string)

Now, with Spring Framework 3.2.3, we would have to check thousands of methods (like method2 in the example above) because we get HTTP Status 400 (Bad Request) where it has been no problem up to Spring Framework 2.3.2.

Comment by Alfred Staflinger [ 21/May/13 ]

I changed the Priority to Blocker because Spring Framework 3.2.3 is not backward compatible to the previous versions (changed behaviour causes problems: HTTP Response Status 400: Bad Request).

Comment by Rossen Stoyanchev [ 21/May/13 ]

I see. So technically "param2" could be marked required=false but obviously the problem is with the change of behavior. I guess we'll need to roll this back.

Comment by david landis [ 03/Jun/13 ]

No unit tests failed as a result of such a massively breaking change? I've grown accustomed over many years to not seeing such sloppiness from spring source like this regression.

Comment by Rossen Stoyanchev [ 20/Jun/13 ]

The change for SPR-10402 has been reversed with commit 2d8315.

Comment by Spring Issuemaster [ 14/Jan/19 ]

The Spring Framework has migrated to GitHub Issues. This issue corresponds to spring-projects/spring-framework#15207.

Generated at Sun Nov 17 20:59:49 UTC 2019 using Jira 7.13.8#713008-sha1:1606a5c1e7006e1ab135aac81f7a9566b2dbc3a6.