Affects: spring-web 5.3.22

Expectation

Given an endpoint with an Integer parameter named limit with a default value of 10, when a user makes a request with limit=%20 then the system should either return a 400 - Bad Request or have the default value applied.

Actual

Given an endpoint with an Integer parameter named limit with a default value of 10, when a user makes a request with limit=%20 then the system accepts the request and the limit variable has a null value.

This happens because when checking for the default value, Spring will not trim the string, so " " will be a value and will not trigger the default value.

But when binding the value to the variable, this line will resolve to true (because allowEmpty is set to true and !StringUtils.hasText(" ") resolves to true), so null will be set (instead of generating an exception).

I have no idea how to fix it, because this does not seem to be a bug in the code, but a mismatch of expectations.

Steps to reproduce

  1. Clone the reproducer https://github.com/oscarfh/spring-reproducer
  2. Start the application
  3. Call in your terminal: curl "http://localhost:8080/endpoint?limit=
  4. "50" will be returned, this is the default value set for the limit endpoint, meaning that the default value was applied.
  5. Call curl "http://localhost:8080/endpoint?limit=a"
  6. Note that a 400 is returned, meaning that limit does not support string values.
  7. Call curl "http://localhost:8080/endpoint?limit=%20"
  8. Notice how "null" is returned, meaning that the limit variable was assigned the null value.

This is unexpected because, due to the fact that this variable is an integer with a default value. You either expect it to be the integer supplied by the user or the default value. You do not expect it to be null. This null will then very likely cause a null pointer exception in your code.

Comment From: rstoyanchev

Thanks for the analysis and sample.

There was a similar case in #23939 where the request parameter was required and becomes null after conversion. We added handling in 5.3 to recognize it as a missing value. I think we have a variation of the same case here. The initial decisions around default/missing value are confounded due to the presence of an empty string, but conversion results in null, and a default value could be applied still, instead of leaving it as null. What do you think @jhoeller?

Backwards compatibility concerns are also similar. We cannot apply this in a maintenance release, so this would have to be a potential 6.1 target. In the meantime you can customize the behavior of CustomNumberEditor with an @InitBinder method either in the controller or in an @ControllerAdvice:

@InitBinder
void initBinder(WebDataBinder binder) {
    binder.registerCustomEditor(Number.class, new CustomNumberEditor(Number.class, false));
}

Comment From: RemusRD

Hey can I work on this one? :)

Comment From: rstoyanchev

@RemusRD this one is not labelled with "status: ideal-for-contribution".

Comment From: Nheyll

@rstoyanchev

this one is not labelled with "status: ideal-for-contribution".

None of the issues are labelled with "status: ideal-for-contribution". There were 3 issues with this label since 2020 : https://github.com/spring-projects/spring-framework/labels/status%3A%20ideal-for-contribution