Hello,

    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.

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. Those changes are too big and too potentially
disruptive to be included in this fix though. Patches welcome.


Thank you,

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

=== modified file 'src/client_side_reply.cc'
--- src/client_side_reply.cc	2012-08-14 11:53:07 +0000
+++ src/client_side_reply.cc	2012-08-24 03:49:33 +0000
@@ -1448,9 +1448,13 @@
                       getMyHostname(), getMyPort());
 
 #endif
 
+    // 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 = !request->multipartRangeRequest() &&
+                                     !reply->content_range &&
                                      reply->sline.protocol == AnyP::PROTO_HTTP && // response is HTTP
                                      (request->http_ver >= HttpVersion(1, 1));
 
     /* Check whether we should send keep-alive */

Reply via email to