[GitHub] [trafficserver] hnakamur commented on a diff in pull request #9877: Do not add content-length for status 204 cache

["hnakamur (via GitHub)" <gi...@apache.org>]

hnakamur commented on code in PR #9877:
URL: https://github.com/apache/trafficserver/pull/9877#discussion_r1236001947


##########
proxy/http/HttpTransact.cc:
##########
@@ -6748,7 +6748,9 @@ HttpTransact::handle_content_length_header(State *s, HTTPHdr *header, HTTPHdr *b
         change_response_header_because_of_range_request(s, header);
         s->hdr_info.trust_response_cl = true;
       } else {
-        header->set_content_length(cl);
+        if (!(s->source == SOURCE_CACHE && header->status_get() == HTTP_STATUS_NO_CONTENT)) {

Review Comment:
   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.
   https://github.com/hnakamur/atstest-go/commit/aaeccc240c80d78167e3b2b52d0781f2fb05cdc8
   https://github.com/hnakamur/trafficserver/commit/2faa3f9ab72b0b6dbfa02ebb831b98cb8340d033
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org