Null passed in required @RequestParam now fails

[bostandyksoft]

Since 5.3.1 version null, passed for required nullable parameter throws Exception "is not present"

@RequestMapping("/findTree")
public void findTree(@RequestParam("parentId") Long parentId) {
  if (parentId == null) {
    //Root
  } else {
    //Is not root
  }
}

now, i send request "/myurl/findTree?parentId="

Expected behavior - parameter parentId passed to method as null
Current behavior - exception within AbstractNamedValueMethodArgumentResolver

[jhoeller]

This is a consequence of #23939. We've tightened our rules for what required=true means in 5.3: It effectively means non-null for conversion results now. In a scenario like yours, an empty parameter String would still come in as such, but a converted value that led to a null fallback would be rejected as missing value.

Could you simply declare your parameter as required=false or as @Nullable?

[bostandyksoft]

Thanks for quick answer. It is ok for our small project - replace all parameters, that could be nullable, to Optional<?>. I've created this task because dont found any "migration guide" about this moment.

For larger projects this "feature" can be more paintful.

Best regards

[bostandyksoft]

Is better to create some way to toggle this behavior.

Best regards

[rstoyanchev]

There is an item for the UUID issue in #23939 in the migration notes. I think the broader tightening is worth mentioning more explicitly as well. We could also update the reference docs, for example the section on type conversion for method arguments, and/or subsequent sections on specific arguments like @RequestParam.

[rstoyanchev]

I'm turning this into a documentation improvement.

[rstoyanchev]

I've added a paragraph to the Type Conversion section and also made the migration note a bit more general.

[drahkrub]

Wow, this is a massive change - spent some hours searching and debugging until I landed here...

Is better to create some way to toggle this behavior.

That would be really nice. We have a large code base, that would be quite some work otherwise.

[rstoyanchev]

@drahkrub I'm sorry for the experience. I have improved the description in the upgrade notes so it's easier to find if you search for query parameter or @RequestParam.

Can you provide a little more context perhaps? I imagine an argument of some type other than String, which has to be converted to null for an empty String, which in effect means the controller method argument is not required after all and it should be possible and accurate to flag as not required.

[drahkrub]

@rstoyanchev Thanks for your answer!

We are in a long process of moving a quite old and large productive system from Spring 4.3.x to Spring Boot - and last week we jumped from Spring Boot 2.3.x to 2.4.x, not studying the upgrade notes of Spring 5.3 (our fault ;-)).

Can you provide a little more context perhaps? I imagine an argument of some type other than String,

I grepped through our source code and found about 200 usages of @RequestParam like that (with the default required = true). A considerable part was written by me (long time ago), but by far not all. Almost always the parameters are passed on to services which process them further - in this respect it is not immediately visible if some or all of them are handled as nullable.

which has to be converted to null for an empty String, which in effect means the controller method argument is not required after all and it should be possible and accurate to flag as not required.

Yes, that would be possible and I understand your arguments, but

1. it would be quite some work to do, ;-)
2. a typical use case (of the old pre 5.3 behaviour) is to express that a parameter must be provided, only in some exceptional case(s) it may be null.

The old(er) parts of our system still work with JSPs and contain lots of CRUD-Controllers working that way, e.g. (simplified)

@RequestMapping("/edit")
public void get(@RequestParam(value = "book") Book book) {
    if (book == null) {
        book = new Book();
    }
    ...

Here "book" is some identifier which must always be provided and is converted in a book entity. Only if a new book is created "book" may exceptionally be given as an empty string. This way, the JSP providing the view (and the identifier of the book when the form is posted back to the controller) can easily handle the different cases new book / existing book.

And if the URL /edit is called without a parameter "book" a MissingServletRequestParameterException is thrown ('Required Book parameter "book" is not present').
How to achieve that in Spring 5.3 (in a simple way)? All three possibilities

1. @RequestParam(value = "book", required = false) Book book
2. @RequestParam("book") @Nullable Book book
3. @RequestParam("book") Optional<Book> book

have completely different semantics since the URL above can now be called without any exception if the parameter "book" is not given at all.

Apart from that this feels quite strange in the cases 2 and 3 as the parameter "book" is stated there as required (per default).

EDIT: The MissingServletRequestParameterException thrown by RequestParamMethodArgumentResolver in Spring 5.3 (see initial comment of this issue) if some parameter is provided as an empty string also feels plain wrong because the parameter is not missing in the servlet request - it's very well contained in its parameterMap!

[drahkrub]

@rstoyanchev @jhoeller Does it make sense to open a new issue due to my "concerns"?

[rstoyanchev]

Thanks for the extra context.

I sympathize with the effort part, especially since it was possible to accumulate such debt given the previous behavior, but at some point the adjustment had to be made and those 200+ cases you have are currently exposed to null whether the intent is for that to be the case or not.

The possibility of null makes the required flag marginally useful at best. It means code is not protected and must always check for null. The only benefit is knowing a query parameter was present, even if empty, but I'm not sure how useful that is when the end result is a null either way and the code using it must be prepared for it. It makes more sense to keep required as in "guaranteed to not result in null" by default vs requiring a controller to always check for null by default.

As for @Nullable and Optional looking strange next to @RequestParam, I suppose it requires a bit of an adjustment given the mental model acquired for its default behavior, but both @Nullable or Optional are very unambiguous markers that cannot be overlooked and a quick look at the Javadoc confirms their interpretation. To look at it from the opposite perspective, what alternative is there for when we encounter @Nullable or Optional with @RequestParam? We could reject them but why shouldn't it be possible to use them in this way? They are simply more, and more recent ways of expressing the same as required=false.

[drahkrub]

@rstoyanchev Thanks again for your answer!

I will try to be brief:

I sympathize with the effort part, especially since it was possible to accumulate such debt given the previous behavior, but at some point the adjustment had to be made and those 200+ cases you have are currently exposed to null whether the intent is for that to be the case or not.

That's absolutely true, but

1. that was the default behaviour until Spring 5.3 (yes, I saw the word "adjustment" ;-)),
2. the usage of @Nullable kind of reverts to the pre-5.3 behaviour (with all its problems you've described), but at the same time makes the parameter required absolutely meaningless,
3. all problems you've described can be avoided by the usage of Optional - but in this case I still would like to see a MissingServletRequestParameterException if required=true and no parameter is provided at all.

The possibility of null makes the required flag marginally useful at best.

It ensures the awareness of the frontend that a parameter must be provided.

It means code is not protected and must always check for null.

Thanks to Java 8 this can be laveraged by the usage of Optional.

The only benefit is knowing a query parameter was present, even if empty, but I'm not sure how useful that is when the end result is a null either way and the code using it must be prepared for it.

See my last two comments.

It makes more sense to keep required as in "guaranteed to not result in null" by default vs requiring a controller to always check for null by default.

@RequestParam(required = true) means for me "the presence of a request param is required, otherwise an exception is thrown". To express "garanteed to not result in null" I would like to see and use something like @RequestParam(nullable = false).

As for @Nullable and Optional looking strange next to @RequestParam,

Ok, I have expressed myself unclear: @Nullable and Optional do not look strange next to @RequestParam (whereby I see Optional being more useful compared to @Nullable which is only a hint for IDEs or superior human brains ;-)). What does feel strange is that no exception is thrown if no parameter is provided only because of the usage of @Nullable and Optional.

I suppose it requires a bit of an adjustment given the mental model acquired for its default behavior, but both @Nullable or Optional are very unambiguous markers that cannot be overlooked and a quick look at the Javadoc confirms their interpretation.

I agree.

To look at it from the opposite perspective, what alternative is there for when we encounter @Nullable or Optional with @RequestParam? We could reject them but why shouldn't it be possible to use them in this way? They are simply more, and more recent ways of expressing the same as required=false.

As I said, I don't reject them - the opposite is true! But your last statement is presumably the core of our "disagreement": For me @Nullable and Optional are not "more recent ways of expressing the same as required=false", they are "only" recent ways of handling the potential presence of null values when working with @RequestParam - regardless of the value of required! The mere presence of these annotations should not overrule required = true and make it meaningless or even worse change its meaning to required = false, i.e., I would always expect a MissingServletRequestParameterException if no parameter is provided in this case.

And apart from being a breaking change required = true should not enforce the usage of @Nullable (not so useful in my eyes) or Optional if one want's to allow an empty string as a parameter (resulting in null) - that's too much paternalism for my taste, null is not bad per se. The usage of Optional should of cause be possible to avoid missing null checks and so on - regardless of required being true or false.

I really think it would have been better to stick with the pre-5.3 behaviour

• regarding exceptions (thrown and not thrown) and
• regarding the acceptance of null as fullfilling required = true - because required should only address the presence of the parameter not its value (which would be the case with something like nullable=...).

(I just re-read my text and noticed some repetitions - but perhaps these make my point more clear)

[drahkrub]

Hello, me again.

I did some tests and I have to admit that the behavior is not as I would like it to be in 5.2.x either:

1. @RequestParam(value = "id", required = true) Integer id

URL	5.2.13.RELEASE	5.3.4	*
/rest	MissingServletRequestParameterException	MissingServletRequestParameterException
/rest?id=	id == null	MissingServletRequestParameterException	(A)

2. @RequestParam(value = "id", required = true) @Nullable Integer id

URL	5.2.13.RELEASE	5.3.4	*
/rest	id == null	id == null	(B)
/rest?id=	id == null	id == null

3. @RequestParam(value = "id", required = true) Optional<Integer> id

URL	5.2.13.RELEASE	5.3.4	*
/rest	id == Optional.empty()	id == Optional.empty()	(C)
/rest?id=	id == Optional.empty()	id == Optional.empty()

To summarize:

What I would prefer with required = true:

URL	Integer id	@Nullable Integer id	Optional<Integer> id
/rest	Exception	Exception	Exception
/rest?id=	id == null	id == null	id == Optional.empty()

And with required = false:

URL	Integer id	@Nullable Integer id	Optional<Integer> id
/rest	id == null	id == null	id == Optional.empty()
/rest?id=	id == null	id == null	id == Optional.empty()

The actual behaviour in (B) and (C) feels strange to me, but I can live with that much better than with the MissingServletRequestParameterException in (A) - that still feels plain wrong to me.

Thanks for your patience. ;-)

[drahkrub]

One last argument against the new behaviour (A) in 5.3:

Let's say a a request parameter named "bookId" must be provided (so using @Nullable or Optional actually makes no sense) which is converted by some GenericConverter to a book entity like e.g. in

@GetMapping("/edit")
public void get(@RequestParam(value = "bookId", required = true) Book book) {
    ...

or

@GetMapping("/edit/{bookId}")
public void get(@PathVariable(value = "bookId", required = true) Book book) {
    ...

Now, right before calling the URL with some bookId of an existing book entity, this very book is deleted in another session, such that the type conversion for bookId gives book == null, which in turn leads to a MissingServletRequestParameterException respectively a MissingPathVariableException in Spring 5.3 - now it's not an easy task to distinguish this from the case that no parameter was provided at all (to produce senseful error messages like "the book you want to edit does not exist anymore").

And in this case also the usage of @Nullable or Optional can not help because then you get null respectively Optional.empty() and still cannot easily distinguish between the two aformentioned cases.

(in Spring 5.2.x it's easy since you get null or an Exception is thrown)

EDIT: In this sense the change in 5.3 makes e.g. Spring Datas DomainClassConverter more or less useless.

[rstoyanchev]

@drahkrub we do make changes from time to time. Backwards compatibility is a high priority but minor and major releases are opportunities to refine behavior, sometimes through breaking changes.

It ensures the awareness of the frontend that a parameter must be provided.

Doesn't a 400 error ensure that better? By absorbing the lack of a value, as you do currently, you are making it OK for the client to not provide it. Moreover you are exposed to clients with potentially malicious intent submitting an empty parameter to get past the 400 and cause other issues where a null isn't actually expected or protected against in your code.

As for all the cases you laid out, I still think the most intuitive interpretation of "/rest?id=" is that no id was provided. We'll have to agree to disagree on that but one thing is for certain, any further changes to this behavior will cause even longer debate.

[drahkrub]

We'll have to agree to disagree

@rstoyanchev :-) Ok, regarding the handling of "/rest?id=" I'll stop arguing now. ;-)

However, I would appreciate a brief assessment of my last comment (the one with a non blank String parameter, which eventually get's converted to null, making "Spring Datas DomainClassConverter (every Converter implementing GenericConverter) more or less useless" as I claimed). It's hard for me to imagine that this isn't a problem for other users as well (or I'm not smart enough to see an elegant solution/workaround).

[rstoyanchev]

For the DomainClassConverter, it is a more specialized scenario where extracting request input is combined with the loading of an entity. The act of combining those two naturally makes it a little harder to tell the reason for the outcome.

That said if this is on an application controller, you could declare it as optional or nullable, and handle the null inside the controller by returning a status that indicates a missing resource or bad input.

Another alternative would be to apply an @ExceptionHandler to classes that load entities in this way and then check for the actual value of the raw request parameter. Come to think of it, even in 5.2 you wouldn't have been able to tell if a null is due to an empty value id or no entity matching to the provided id.

[drahkrub]

Come to think of it, even in 5.2 you wouldn't have been able to tell if a null is due to an empty value id or no entity matching to the provided id.

@rstoyanchev :-) That point goes to you if you talk about calling /edit?bookId= with a required RequestParam "bookId" (in 5.2).

But what about /edit/{bookId} with a required PathVariable "bookId"? Such an URL cannot be called with an empty value, which means you get a book entity or null in 5.2. In 5.3 you get a book entity or a MissingPathVariableException if some provided (and therefore not missing) PathVariable cannot be converted into an existant book entity.

Another alternative would be to apply an @ExceptionHandler to classes that load entities in this way and then check for the actual value of the raw request parameter.
[...] if this is on an application controller, you could declare it as optional or nullable, and handle the null inside the controller by returning a status that indicates a missing resource or bad input.

Checking the raw request (think of PathVariables!) to distinguish between these two cases is something I wouldn't call elegant. ;-)

Wouldn't it be possible/better to handle this in AbstractNamedValueMethodArgumentResolver, something like

convertedArg = binder.convertIfNecessary(arg, parameter.getParameterType(), parameter);

(instead of arg = binder.convertIfNecessary(arg,[...]);) and taking into account some non nullness of arg if convertedArg is null?