eclipse-ee4j / jersey

Eclipse Jersey Project - Read our Wiki:
https://github.com/eclipse-ee4j/jersey/wiki
Other
690 stars 352 forks source link

ResponseWriter should use sendError instead of setStatus #5286

Open amarktl opened 1 year ago

amarktl commented 1 year ago

In the ResponseWriter the setStatus(int, String) method is used which is deprecated.

https://github.com/eclipse-ee4j/jersey/blob/6bd50c8130393aaf7806ced1f99020b10be62582/containers/jersey-servlet-core/src/main/java/org/glassfish/jersey/servlet/internal/ResponseWriter.java#L147

If used with Jetty as servlet container the message is dropped by intention and therefore no longer visible on the client.

See change: https://github.com/eclipse/jetty.project/commit/5d9c55be8f281c6c2e49825822413c6d203b1487#diff-bcf0034ba316615035f07846625ce33a55705a292049c63976c0393246d48b50

According to the deprecation hint sendError should be used instead.

jansupol commented 1 year ago

Please refer to the version of Jetty/Jersey.

amarktl commented 1 year ago

Jersey (org.glassfish.jersey.servlet.internal.ResponseWriter): org.glassfish.jersey.core.jersey-common_2.38.0.jar Jetty (org.eclipse.jetty.server.Response): jetty-server-10.0.12.jar

jansupol commented 1 year ago

The Jetty issue fixes the sendError(code, message) so that the message is no longer the reason code. Unfortunately, they also drop the reason code from setStatus(). So we could either use sendError(code, reason_phrase), which would not be correct since the sendError message is not meant to be a reason phrase, or we can use response.setStatus(code) and drop the reason_phrase to avoid using the deprecated method (which we do with Servlet 6 container integration).

Jetty has a new method setStatusWithReason, but it is not available in generic HttpSevletResponse. Please keep in mind the other pre-Servlet 6 containers keep the setStatus(code, deprecation) behavior, unlike Jetty.

jansupol commented 1 year ago

setError does not seem to propagate the user-defined HTTP headers, which is another incompatibility.