Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@RequestBody required parameter is ignored allowing null payloads [SPR-13490] #18068

Closed
spring-projects-issues opened this issue Sep 23, 2015 · 7 comments
Assignees
Labels
status: duplicate A duplicate of another issue

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Sep 23, 2015

Emanuel Hategan opened SPR-13490 and commented

When handling a POST request in a @RestController, the payload parameter annotated with @RequestBody can be null, regardless of the value of the required attribute.

Payload is JSON (content-type header is included) and deserialization is handled by Jackson.

In case it matters, the project is inheriting from spring-boot-starter-parent version 1.2.5.RELEASE.


Affects: 4.1.7

Reference URL: http://stackoverflow.com/questions/32222099/spring-4-reject-null-requestbody-for-all-endpoints

Issue Links:

Referenced from: commits spring-attic/spring-framework-issues@5bf23a0, spring-attic/spring-framework-issues@c93c892, spring-attic/spring-framework-issues@0319caf

@spring-projects-issues
Copy link
Collaborator Author

Brian Clozel commented

Hi Emanuel Hategan

I couldn't reproduce the issue you're referring to here, using Boot or plain Spring Framework.

Here is a repro project that demonstrates that:

  • a empty body will be rejected by @RequestBody since it's marked as required by default
  • an empty JSON object "{}" will be rejected by the Validator

Could you take a look at this repro project and your app?

Thanks,

@spring-projects-issues
Copy link
Collaborator Author

Emanuel Hategan commented

I've made a pull request with updated tests: spring-attic/spring-framework-issues#111

What I'm thinking is that it should reject payloads that are "null" not only empty "{}" since the null string is deserialized as null payload.

@spring-projects-issues
Copy link
Collaborator Author

Brian Clozel commented

I don't think this behavior is a bug, since:

  • @RequestBody(required=true) states that the body should be injected here and should not be null
  • @Valid states that the method argument should be processed by validators

Now for each case:

  • "{}" is deserialized as an empty Payload object, which will be rejected by the validator that rejects empty messages
  • "" is rejected by @RequestBody(required=true) because the request body is empty
  • "null" is deserialized by Jackson as null, and the Validator is never called on such object

In my opinion, there are several ways to deal with this - manually checking for null values, using @NotNull or even using a custom HandlerMethodArgumentResolver.

One more thing: your SO question was spot on (sorry if it took me a while to understand the whole context) and I understand your concerns - even if there may be "no problem to solve" strictly in the framework, I'd like to find the best answer for this.

Is this "null" request payload a common use case? This looks to me as if you've JSON.stringify(null) on the client side...

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jan 4, 2016

Emanuel Hategan commented

Hi Brian, thank you for looking into this.

You are saying "null" is deserialized by Jackson as null, and the Validator is never called on such object and you are right validator is not called in this case.

However, I still believe "null" values should be rejected earlier, because of the @RequestBody(required=true) according to its documentation: "leading to an exception thrown in case there is no body content. Switch this to {@code false} if you prefer {@code null} to be passed when the body content is {@code null}."

Interestingly enough, in version 4.2.3.RELEASE the issue is fixed: just replace 4.1.7.RELEASE with 4.2.3.RELEASE in the pom and all tests pass.
The fix seems to be in this commit: 3272a3b which is related to #17768

@spring-projects-issues
Copy link
Collaborator Author

Brian Clozel commented

Indeed, this is now fixed.
We're now checking if the resolved arg is null after it's been deserialized, taking care of this use case.

Thanks for the report!

@fveerden
Copy link

fveerden commented May 1, 2023

Issue seems to reappear in springboot 3

@bclozel
Copy link
Member

bclozel commented May 1, 2023

@fveerden commenting on a 7 year old issue won't help here unfortunately. Could you create a new issue with a sample application showing the problem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

3 participants