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

ResponseEntity should allow the usage of unassigned HTTP status codes [SPR-14205] #18779

Closed
spring-issuemaster opened this issue Apr 22, 2016 · 12 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Apr 22, 2016

Robert Winkler opened SPR-14205 and commented

Hello,

the current implementation of the ResponseEntity does not allow the usage of unassigned HTTP status codes, because of the HttpStatus enum which contains only IANA registered HTTP Status codes.

But the HTTP specification allows the usage of new/extension/unassigned status codes. See: http://tools.ietf.org/html/rfc7231#section-6 and http://tools.ietf.org/html/rfc7231#section-8.2.2
Unfortunately, custom status codes are sometimes used by RESTful APIs.

It would be nice if the ResponseEntity would allow custom status codes. The ResponseEntity needs a constructor or builder method which excepts custom status codes.

The IANA list shows the codes which are unassigned:
http://www.iana.org/assignments/http-status-codes/http-status-codes.xhtml


Affects: 4.2.5

Issue Links:

  • #18350 Regression: HttpEntityMethodProcessor does not allow other Http methods than defined in the HttpMethod Enum
  • #18642 Support for HTTP Vary configuration (e.g. in reaction to locale-based rendering)
  • #16374 Provide builders for HttpEntity and ResponseEntity
  • #20529 RestTemplate doesn't consistently tolerate unknown HTTP status codes
  • #20622 Returning non-standard HTTP status code causes exceptions on WebFlux
  • #20913 ServerResponse should allow the usage of unassigned HTTP status codes

Referenced from: commits d06188e

0 votes, 6 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 22, 2016

Juergen Hoeller commented

Since 4.1, there is already a status(int) build method on ResponseEntity... We should allow for carrying unknown status values through. This should be doable for 4.3 still.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 24, 2016

Robert Winkler commented

Hi,
maybe by creating an Interface which is implemented by the HttpStatus enum and can be implemented by users to provide their own HttpStatus Enum implementations? Would be the less intrusive change if there is not another obstacle in some other code?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 26, 2016

Juergen Hoeller commented

I've implemented this behind ResponseEntity.status(int), carrying the HTTP status code as an Integer in that case, and making it transparently accessible through a new getStatusCodeValue() method (extracting either HttpStatus.value() or the Integer value) which all affect MVC return value handlers are using for passing the specified value straight to the HttpServletResponse now, never adapting it to a constrained HttpStatus in that code path.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 27, 2016

Robert Winkler commented

Thank you very much for your work and fast feedback!
This new feature has a high priority for us (Deutsche Telekom) and is currently a blocker for us :(
Which spring-framework release is going to be released earlier? Version 4.2.6 or 4.3?
If its version 4.2.6, do you think you could merge this also into 4.2.6?

We are using Spring Boot 1.3.3 and the Spring IO Platform 2.0.3.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 27, 2016

Juergen Hoeller commented

We're going to co-release 4.3 RC2 and 4.2.6 next week, with 4.2.6 essentially just containing bug fixes and log refinements. 4.3 GA is scheduled for June 1st, so not far away either. BTW, the JIRA roadmap panel is always up to date with respect to our current target dates.

From my perspective, this kind of change is at home in 4.3, going through a release candidate and then GA in about a month anyway. After all, there could be subtle regressions such as somebody expecting an exception from trying to set an unknown status code, which is ok for a 4.3 but arguably not for a 4.2.x release.

So please give the upcoming 4.3 RC2 a try in your integration tests. If any issues remain (in particular with respect to status code handling in the various code paths in Spring MVC), let me know ASAP. Let's make sure that 4.3 GA fully works for you then.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 27, 2016

Robert Winkler commented

Yes, you are right. Its a non backwards-compatible change and should not be part of 4.2.6.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 27, 2016

Syed Wasim Ali commented

Say, an implementation needs to respond with a 438 "Invalid Identity Header". ResponseEntity.status(int) takes care of setting the http status code. What is the expected string representation for the supposed custom status code? Is there a way to influence the string representation outcome?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 27, 2016

Juergen Hoeller commented

Through the Servlet API, all we can do is to delegate to HttpServletResponse.setStatus(int)... You may of course also set headers and in particular the body of the response.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 27, 2016

Robert Winkler commented

@Syed: You mean you would like to have a

ResponseEntity.status(int, String)

method which results in a

HttpServletResponse.sendError(int, String)

?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 27, 2016

Juergen Hoeller commented

HttpServletResponse.sendError is a beast though: It typically triggers rendering of the error page, with the specified message ending up as text in an HTML body. We intentionally do not use it for HTTP status handling.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 27, 2016

Robert Winkler commented

True

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 27, 2016

Syed Wasim Ali commented

@Robert, yes. Working on implementing the ietf draft. At some time in the near future there will be a call to supplement the IANA registry for the status codes. Can live with the status(int) til then. Ideally, we should be able to use the named constants and optionally add to them with all the essential attributes.thanks for looking into this.

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