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

Breaking change in MVC: Accept handling is now more strict [SPR-16251] #20798

Closed
spring-issuemaster opened this issue Dec 1, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Dec 1, 2017

Francisco Lozano opened SPR-16251 and commented

Scenario:

  • MVC controller with a ResponseEntity<?> object.
  • Client sends Accept: application/json
  • Server responds with: application/what.ever+json in the ResponseEntity's headers.

This was seemingly OK with 5.0.1 and before. After upgrading to 5.0.2, it forces to return 406, despite the ResponseEntity saying otherwise.

The commit that caused it is:
9a894ab#diff-24571dc5c7743b2a69f7a5df4f2109b2

The RFC says:

If no Accept header field is present, then it is assumed that the client accepts all media types. *If an Accept header field is present, and if the server cannot send a response which is acceptable according to the combined Accept field value, then the server SHOULD send a 406 (not acceptable) response.*

Please note that it says SHOULD, and not MUST:

3. SHOULD   This word, or the adjective "RECOMMENDED", mean that there
   may exist valid reasons in particular circumstances to ignore a
   particular item, but the full implications must be understood and
   carefully weighed before choosing a different course.

So, a patch release of Spring seems to be enforcing a more strict behaviour, disallowing something allowed (albeit not recommended) by RFC, and breaking existing applications.

A workaround is possible by having a nasty servlet filter that manipulates headers, and adds application/*+json when application/json is in Accept... but, considering this is a patch release, I suggest you consider this behaviour change a bug, as it has backward-compatibility implications, and try not to break acceptable flows.


Affects: 5.0.2

Issue Links:

  • #20720 AbstractMessageConverterMethodProcessor ignores HttpEntityMethodProcessor's Content-Type header

Referenced from: commits fda0885

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 1, 2017

Francisco Lozano commented

Caused by #20720

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 2, 2017

Rossen Stoyanchev commented

This should be resolved now. I've made a change to use the media type from the ResponseEntity unconditionally. It was actually the intent of #20720 to allow the controller to specify the content type of the response, but the checks against the requested content type were left in place, but are now removed.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 4, 2017

Francisco Lozano commented

Ah, this means that the behaviour is unintended and that 5.0.3 will behave as 5.0.1? that's good, thanks. I will revert to 5.0.1 and remove the hacky filter workarounds.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 4, 2017

Rossen Stoyanchev commented

Based on your description I believe it should work again but if you could confirm with 5.0.3.BUILD-SNAPSHOT that would be best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.