Empty or blank required UUID header validation results in 200 response status

[maxdewil]

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.

[sbrannen]

In 92228f0, 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?

[sbrannen]

This issue also inspired #23940.

[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.

[sbrannen]

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

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

[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?

[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.

[hicf]

see pr#23950

[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.

[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)?

[maxdewil]

@DavidZemon actually your question is related to another issue: #11041 (and also this one : #16519)

[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?

[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.

[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!

[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.

[sbrannen]

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