fix #4213: Content-Length when entity is unallowed

[bursauxa]

Adds Content-Length: 0 header on responses that do not allow a response body (204, 205 and 304).

I did not add a specific test, as there was already a test for rendering a 304 response - I simply made that test more correct 🙂

Fixes #4213

[lightbend-cla-validator]

At least one commit author (antoine.bursaux@kelkoogroup.com) is not linked to a user. See https://help.github.com/en/articles/why-are-my-commits-linked-to-the-wrong-user#commits-are-not-linked-to-any-user

[johanandren]

Trying to verify this is the right thing I found RFC 9110 (which obsoletes 7231) saying:

https://httpwg.org/specs/rfc9110.html#field.content-length

A server MAY send a Content-Length header field in a 304 (Not Modified) response to a conditional GET request (Section 15.4.5); a server MUST NOT send Content-Length in such a response unless its field value equals the decimal number of octets that would have been sent in the content of a 200 (OK) response to the same request.

And

A server MUST NOT send a Content-Length header field in any response with a status code of 1xx (Informational) or 204 (No Content). A server MUST NOT send a Content-Length header field in any 2xx (Successful) response to a CONNECT request (Section 9.3.6).

So I wonder if a fix can really be this easy, I think it needs to be specifically/selectively emitted based on the status code and request method. It also explicitly contradicts emitting it for 204.

[bursauxa]

Thanks @johanandren for pointing that out. You are right, it will require some case-by-case handling, I will proceed with the changes.

Also I inadvertently added the header to 1xx responses (allowsEntity was set to false in the base class Informational for all codes of that region, not for specific codes like it was for 204, 205 and 304). It will be fixed along the way.

[johanandren]

Wdyt about the explicit MUST NOT emit content length for 204, that seems like it contradicts what the clients expect if it is as you describe in the issue?

[bursauxa]

Wdyt about the explicit MUST NOT emit content length for 204, that seems like it contradicts what the clients expect if it is as you describe in the issue?

I specifically encountered the issue with a 205 code. It was mentioned in the linked issue, but I oversimplified in this pull request. Testing with curl shows that indeed, a 204 code without Content-Length header works fine - the client does not hang.

It turns out to be very specific to the 205 code... The previous condition on status.allowsEntity matches all codes where Content-Length must not be used, with the exception of 205 that does not allow an entity but should still provide a Content-Length.

There are also cases with HEAD and CONNECT methods that I did not encounter, I do not even know to what degree they would realistically cause issues to clients (a response body seems very unlikely for these methods), but might as well align them with the RFC while working on it.

[bursauxa]

Not entirely related, but you made me think @johanandren ... And I stumbled upon these tests with HEAD requests containing bodies (ResponseRendererSpec.scala, around line 150). This is not valid according to the same RFC 9110 you mentioned earlier:

The HEAD method is identical to GET except that the server MUST NOT send content in the response.

[johanandren]

I think we can leave HEAD returning bodies alone for now, as you say it is against spec, but in case anyone is relying on doing that already.

[johanandren]

Some related test failures, and some I'm not entirely sure about, to look into.

Also, failures from MiMa which is our binary incompatibility detection, I think all of them can now safely be filtered as described here: https://github.com/akka/akka-http/blob/main/CONTRIBUTING.md#binary-compatibility

[bursauxa]

Tests were good with previous run, this one should take care of binary incompatibilities.

[johanandren]

LGTM