search

jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more
https://eclipse.dev/jetty
Other
3.83k stars 1.91k forks source link

Response.sendError should not set reason. #3225

Closed gregw closed 5 years ago

gregw commented 5 years ago

Allowing the application to set the response reason string has been deprecated. Jetty-10 should change so that the message passed to sendError is only used in the error page body and not as the reason string.

joakime commented 5 years ago

Oddly, Servlet 4.0.2 doesn't indicate HttpServletResponse.sendError(int, String) as deprecated.

https://github.com/eclipse-ee4j/servlet-api/blob/4.0.2-RELEASE/src/main/java/javax/servlet/http/HttpServletResponse.java#L170-L191

gregw commented 5 years ago

@joakime sendError(int, String) is not deprecated, but the string is not the reason, instead it is described as the basis for the response body:

     * The server defaults to creating
     * the response to look like an HTML-formatted server error page
     * containing the specified message, setting the content type to
     * "text/html"

It is setStatus(int,String) that is deprecated

martinmeeser commented 4 years ago

I am using a custom error handler (extending from ErrorHandler). Since upgrading to 9.4.21, the whole thing is broken, because reason is always null.

What happens to the string that is passed with sendError(code, str)? I can not find it at the response now?

I do not follow the reasoning behind this change?

Update: Note: Within my error handler I very solidly determine the response type, and decide for myself how and where to embed the reason into the response?

That possibility seems now lost?

joakime commented 4 years ago

Reason is deprecated.

It was deprecated in the Servlet spec, and it was deprecated in the HTTP protocol (HTTP/2 dropped it entirely, HTTP/3 also does not have it).

jglick commented 4 years ago

See also #4154.

paulmillar commented 3 years ago

@joakime Could you provide your source for the statement "[Reason] was deprecated in the HTTP protocol" ?

RFC 7231 says:

6.1. Overview of Status Codes

The status codes listed below are defined in this specification, Section 4 of [RFC7232], Section 4 of [RFC7233], and Section 3 of [RFC7235]. The reason phrases listed here are only recommendations -- they can be replaced by local equivalents without affecting the protocol.

This suggests to me that customised reason phrases are compatible with HTTP/1.1, so I am surprised to hear that they are deprecated.

joakime commented 3 years ago

@paulmillar the reason phrase existed for special class of user-agent called interactive http clients (also known as "interactive text client").

Which is a special kind of HTTP user-agent where the user is actually using the protocol directly. (Think of it as a terminal / console tool similar to a SQL tool where the user is entering SQL commands and getting responses)

Examples of interactive textual http clients ...

This also means that no web browser is considered an interactive textual http client.

Over the course of time, the HTTP spec has evolved away from making things easier for interactive textual http clients. Stricter format rules, no more line folding allowed, whitespace rules are now strict, dropping of TEXT rule, delimiters are far more strict, etc ...

The RFC 7231 even tells you about this change in the reason phrase ...

The reason-phrase element exists for the sole purpose of providing a textual description associated with the numeric status code, mostly out of deference to earlier Internet application protocols that were more frequently used with interactive text clients. A client SHOULD ignore the reason-phrase content.

It existed for interactive text clients, and the client should ignore the reason phrase.

RFC 7231 also made the rules for reason phrase stricter over RFC2616 by dropping the TEXT rule. Which pretty much means it only supports 7-bit US-ASCII (with no encoding support). This change to the RFC7230 spec, dropping of the TEXT rule, also made supporting custom reason phrases increasingly impossible.

If you were providing description in the reason phrase for your clients, then you are not probably aware of RFC 7807.

https://tools.ietf.org/html/rfc7807 - Problem Details for HTTP APIs

When the specs for HTTP/2 and HTTP/3 entered the picture, the support this kind of interactive client was dropped entirely. The features present in the HTTP spec specifically for those kinds of clients were dropped. (eg: http version, reason phrase, etc)

gregw commented 3 years ago

@martinmeeser sorry for stupidly late response.... if you are writing a Jetty ErroHandler, then just use the setStatusWithReason method so the reason get's set.