Do not add content-length for status 204 cache

[hnakamur]

In master and version 9.2.1, content-length header is added when serving the cache with status 204 No Content.
It seems caused by Make 204 cacheable again by maskit · Pull Request #9333 · apache/trafficserver.

With this pull request, content-length is not added when serving the cache with status 204 No Content.

I made test cases at https://github.com/hnakamur/atstest-go.

• The test log with this fix is at log/atsmasterfix-test.log and there is no content-length in the status 204 response from the cache: log/atsmasterfix-test.log#L35-L41.
• The test log without this fix is at log/atsmaster-test.log and there is content-length in the status 204 response from the cache: log/atsmaster-test.log#L40.

(Note aside: the response headers in above logs are sorted alphabetically so the order is not same as the received response).

I also made the same fix for trafficserver version 9.2.1 at https://github.com/hnakamur/trafficserver/tree/9_2_1_dont_add_content_length_for_status_204_cache

• The test log with this fix is at log/ats921fix-test.log and there is no content-length in the status 204 response from the cache: log/ats921fix-test.log#L35-L41.
• The test log without this fix is at log/ats921-test.log and there is content-length in the status 204 response from the cache: log/ats921-test.log#L40.

https://www.rfc-editor.org/rfc/rfc9110.html#section-8.6-8

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).

[bneradt]

[approve ci rocky]

[maskit]

Can you tell me why s->source == SOURCE_CACHE is needed?

[hnakamur]

I think in general the reverse proxy is supposed to send the reponse from the origin server as is, so I thought it would be better to limit the guard not to add content-length to the case for sending from the status 204 cache.
However, as I'm writing this, I noticed that it would also delete content-length when sending the status 204 cache after receiving a status 204 response having content-length from an origin server. So the code above is modifying the response from an origin server after all.

So, should we avoid to add content-length for both the case when it sends from cache or when it proxies the response from an origin server? If so, is if (header->status_get() != HTTP_STATUS_NO_CONTENT) the correct?

I ran my tests for if (header->status_get() != HTTP_STATUS_NO_CONTENT) and confirmed they pass.
hnakamur/atstest-go@aaeccc2
hnakamur@2faa3f9

[maskit]

Having different behavior for the two sources (origin server and cache) doesn't make sense to me. The document in cache is originally from an origin server and it's just cached. If we removed the header only when we serve a document from cache, a client would see inconsistent responses that depend on the cache status. So, yes, I think we should check only the status code.

[hnakamur]

Thanks for your explanation. I pushed 2faa3f9.

[maskit]

Thanks.

[bryancall]

Cherry picked to the 9.2.x branch