What harm is there to trim header values in HttpServletResponse setHeader() or addHeader() calls?

[joakime]

With HTTP/2, the Header Value cannot start or end with whitespace.

Code like this ...

@Override
public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException
{
    resp.addHeader("x-example", "foo "); // note the extra space at the end
}

Would fail at the HPACK level on HTTP/2 because of the extra space at the end of the Header value.
Note: this would also fail at the QPACK level on HTTP/3.

This would be trivial to fix at the servlet level to just recommend trimming the incoming value parameter.
But the question is, would this kind of trim of the value cause problems or be against the Servlet spec?

For HTTP/2 we are already lowercasing the header name, as required by the HTTP/2 spec.

See the spec for HTTP/2 ...

RFC9113 Section 8.2.1

A field value MUST NOT start or end with an ASCII whitespace character (ASCII SP or HTAB, 0x20 or 0x09).

A request or response that contains a field that violates any of these conditions MUST be treated as malformed (Section 8.1.1).

RFC9113 Section 8.1.1

Malformed requests or responses that are detected MUST be treated as a stream error (Section 5.4.2) of type PROTOCOL_ERROR.

For malformed requests, a server MAY send an HTTP response prior to closing or resetting the stream. Clients MUST NOT accept a malformed response.

[gregw]

I don't think we should cover up errors like this by default.

However, perhaps this would make a good documentation example to show how to write a simple handler that wraps the response and corrects such issues?

[joakime]

Then perhaps this is a case where the javadoc on the addHeader and setHeader in the spec could be updated to point out these kinds of of cross http version incompatibility.

[joakime]

I don't think we should cover up errors like this by default.

Then would this be a candidate for a new IllegalArgumentException from the servlet spec?

[dsandrade0]

I don't think we should cover up errors like this by default.

Then would this be a candidate for a new IllegalArgumentException from the servlet spec?

I believe this is the better way.

[joakime]

Here's a more detailed argument ...

If we use, in the servlet spec ...

HttpServletResponse.setHeader("x-example", " foo ");

Then we get the following on the HTTP/1.1 protocol.

HTTP/1.1 200 OK\r\n
x-example:  foo \r\n
\r\n

Note the extra spaces around "foo".
In HTTP/1.1 this is always parsed as field name x-example and field value of foo (no spaces).
This is because the HTTP/1.1 parsing of the field value will always drop the OWS before and after the field value.

https://www.rfc-editor.org/rfc/rfc9112.html#name-field-syntax

  field-line   = field-name ":" OWS field-value OWS

So, while we are not technically trimming the value foo when we create an HTTP/1.1 field header.
We are in effect always getting that benefit due to how the HTTP/1.1 protocol works.

However, when someone moves to a new HTTP spec, like HTTP/2, then this protocol feature is no longer present.

So, I argue, why not standardize this in the Servlet spec?
This will be a change that can only benefit users, leaving it as is harms users moving to a new HTTP protocol.

[markt-asf]

We certainly need to update the Javadoc for the headers methods since it still refers to RFC 2047.
RFC 9110 (HTTP Semantics) states that values do not include leading or trailing whitespace.
My preference is for an IllegalArgumentException if the header value (or name) is invalid. Note the names are case insensitive so providing one that uses upper case is not an error.