Affects: Spring MVC 5.1.9.RELEASE

Hi,

My REST API takes as input a mandatory X-Request-ID header that is transformed to a UUID object.

So my REST controller method has this parameter: @RequestHeader(value="X-Request-ID", required=true) UUID xRequestID

However, if the X-Request-ID HTTP header: - is absent : 400 Bad Request (OK) - is empty (i.e. the empty string) : 200 OK (NOK, should be 400) - is blank (i.e. a string with whitespace only) : 200 OK (NOK, should be 400) - is not a UUID (for instance "foobar") : 400 Bad Request (OK)

It seems that the problem comes from:

  1. 'is empty' case: StringToUUIDConverter returns null
  2. 'is blank' case: StringToUUIDConverter fails, then Spring MVC defaults to UUIDEditor that returns null.

Comment From: sbrannen

In 92228f0fc0564868afece5e5d15ff4da31234c34, I introduced tests for the status quo.

@maxdewil, is my assumption correct that you would expect argument resolution for an empty or blank UUID header to result in a MethodArgumentConversionNotSupportedException instead of null?

Comment From: sbrannen

This issue also inspired #23940.

Comment From: maxdewil

@maxdewil, is my assumption correct that you would expect argument resolution for an empty or blank UUID header to result in a MethodArgumentConversionNotSupportedException instead of null?

If I pass the header value "foobar", the following exception is thrown : org.springframework.web.method.annotation.MethodArgumentTypeMismatchException: Failed to convert value of type 'java.lang.String' to required type 'java.util.UUID'; nested exception is java.lang.IllegalArgumentException: Invalid UUID string: foobar

I think the behavior should be the same for an empty/blank string.

Comment From: sbrannen

I think the behavior should be the same for an empty/blank string.

OK. We'll see if that's feasible.

Comment From: mdeinum

The required is for checking if the header is present in the request not if it converts to a proper value. If I recall this is the same for all the required properties on the different annotations (like @RequestParam, @CookieValue etc.).

Wouldn't changing it for just @RequestHeader change the contract?

Comment From: sbrannen

@mdeinum, those are very valid points, which I have considered as well.

I'm not sure that it makes sense to change the behavior for only @RequestHeader. We might need/want to change the behavior for all required arguments.

In light of that, we'll discuss amongst the team what (if anything) we want to do in this regard.

Comment From: hicf

see pr#23950

Comment From: sbrannen

Team Decision:

We are considering whether we want to make a change to org.springframework.web.method.annotation.AbstractNamedValueMethodArgumentResolver.resolveArgument(MethodParameter, ModelAndViewContainer, NativeWebRequest, WebDataBinderFactory) that would result in an exception if the arg value is null after the invocation of binder.convertIfNecessary(arg, parameter.getParameterType(), parameter) and the required attribute from the annotation is true.

However, we are not willing to make such a change in a 5.2.x point release and will therefore revisit this topic for 5.3.

Comment From: DavidZemon

I thought I could solve this with @NotBlank, but that doesn't seem to be working. Am I doing something wrong or is that the "expected" behavior (certainly not what I was expecting)?

Comment From: maxdewil

@DavidZemon actually your question is related to another issue: https://github.com/spring-projects/spring-framework/issues/11041 (and also this one : https://github.com/spring-projects/spring-framework/issues/16519)

Comment From: yagyank

Hi everyone, Its my first time contributing to an open source project. I have been working with spring and wanted to contribute as well. For starters can I pick this bug?

Comment From: sbrannen

@yagyank, thanks for volunteering!

This issue is currently labeled with status: pending-design-work which indicates that the core Spring Framework team will investigate what can or should be done.

In light of that, this issue does not fall into the category of ideal-for-contribution. Unfortunately, we don't currently have any open issues labeled as ideal-for-contribution. So feel free to ask on other open issues for which you think you would feel comfortable implementing a fix.

Comment From: armandoalmeida

Hi @maxdewil, I've created a workaround for this problem using an annotation @RequestHeaderNonNull that verifies if the method property is annotated with the @RequestHeader and after that check if the content is null or empty.

https://github.com/armandoalmeida/request-header-required-non-null

I hope it helps you!

Comment From: bclozel

The team discussed this issue. We've come to the conclusion that the StringToUUIDConverter implementation might be odd here (returning null for empty strings).

We don't think that the behavior of the "required" attribute on @RequestHeader is in question here, since the consensus here is that it requires the presence of such header in the request, but not necessarily that it won’t be null.

We first need to review other the existing converter implementations and reconsider this issue after.

Comment From: sbrannen

@jhoeller, please also consider #25878 when working on this issue or reopen #25878 if you deem it appropriate.