From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Mon, 10 Sep 2012 22:38:09 +0000 (-0600)
Subject: Do not chunk responses carrying a Content-Range header.
X-Git-Tag: sourceformat-review-1~8^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4a7eb77ede09ee73286cb12b0c789364a90087fb;p=thirdparty%2Fsquid.git

Do not chunk responses carrying a Content-Range header.

When Squid forwards a response with a Content-Range header,
ClientSocketContext::socketState() detects the end of the response range(s)
and returns STREAM_*COMPLETE to ClientSocketContext::writeComplete().
The latter thinks that the writing of the response to the client must be
over and calls keepaliveNextRequest() instead of writing the last-chunk
(if any). If the to-client response was chunked, the client gets stuck
waiting for that missing last-chunk.

The multipart Range request case was already excluded from chunking (or it
would probably suffer from the same problem). With this change, no
Content-Range responses will be chunked.

N.B. Some servers send Content-Range responses to basic GET requests
without a Range header, so the problem affects more than just Range requests.

TODO: A proper fix would be to rewrite ClientSocketContext::writeComplete()
and other code so that it does not mix internal ClientStream completion with
[possibly chunk-encoded] writing completion. This should probably be done
along with fixing ClientSocketContext::socketState() and other state-checking
code to ignore to-client persistence (flags.proxy_keepalive), which is not
related to the internal ClientStream state.
---

diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc
index e1d54b7453..40ce0875c3 100644
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@ -1453,7 +1453,11 @@ clientReplyContext::buildReplyHeader()
 
 #endif
 
-    const bool maySendChunkedReply = !request->multipartRangeRequest() &&
+    // XXX: chunking a Content-Range response may not violate specs, but our
+    // ClientSocketContext::writeComplete() confuses the end of ClientStream
+    // with the end of to-client writing and may quit before writing last-chunk
+    const bool maySendChunkedReply = !reply->content_range &&
+                                     !request->multipartRangeRequest() &&
                                      reply->sline.protocol == AnyP::PROTO_HTTP && // response is HTTP
                                      (request->http_ver >= HttpVersion(1, 1));