Hello,
I saw bogus ERR_ZERO_SIZE_OBJECT responses while testing Squid v3.1,
but the same problem ought to be present in v3.2 as well.
A compliant proxy may retry PUTs, but Squid lacks the [rather
complicated] code required to protect the PUT request body from being
nibbled during the first try, when pconn races are possible. Thus, Squid
cannot safely retry some PUTs today, and FwdState::checkRetriable() must
return false for all PUTs, to avoid bogus ERR_ZERO_SIZE_OBJECT errors
(especially for clients that did not reuse a pconn and, hence, may not
be ready to handle/retry an error response).
In theory, requests with safe or idempotent methods other than PUT might
have bodies so the patch applies the same logic to them as well.
Thank you,
Alex.
Do not reuse persistent connections for PUTs to avoid ERR_ZERO_SIZE_OBJECT.
A compliant proxy may retry PUTs, but Squid lacks the [rather complicated]
code required to protect the PUT request body from being nibbled during the
first try. Thus, Squid cannot safely retry some PUTs today, and
FwdState::checkRetriable() must return false for all PUTs, to avoid
bogus ERR_ZERO_SIZE_OBJECT errors (especially for clients that did not
reuse a pconn and, hence, may not be ready to handle/retry an error response).
In theory, requests with safe or idempotent methods other than PUT might have
bodies so we apply the same logic to them as well.
=== modified file 'src/forward.cc'
--- src/forward.cc 2012-08-23 00:01:00 +0000
+++ src/forward.cc 2012-08-29 18:10:42 +0000
@@ -459,60 +459,66 @@
* Return TRUE if this is the kind of request that can be retried
* after a failure. If the request is not retriable then we don't
* want to risk sending it on a persistent connection. Instead we'll
* force it to go on a new HTTP connection.
*/
bool
FwdState::checkRetriable()
{
/* RFC2616 9.1 Safe and Idempotent Methods */
switch (request->method.id()) {
/* 9.1.1 Safe Methods */
case METHOD_GET:
case METHOD_HEAD:
/* 9.1.2 Idempotent Methods */
case METHOD_PUT:
case METHOD_DELETE:
case METHOD_OPTIONS:
case METHOD_TRACE:
break;
default:
return false;
}
+ // Optimize: A compliant proxy may retry PUTs, but Squid lacks the [rather
+ // complicated] code required to protect the PUT request body from being
+ // nibbled during the first try. Thus, Squid cannot retry some PUTs today.
+ if (request->body_pipe != NULL)
+ return false;
+
return true;
}
void
FwdState::serverClosed(int fd)
{
debugs(17, 2, "fwdServerClosed: FD " << fd << " " << entry->url());
assert(server_fd == fd);
server_fd = -1;
retryOrBail();
}
void
FwdState::retryOrBail()
{
if (checkRetry()) {
int originserver = (servers->_peer == NULL);
debugs(17, 3, "fwdServerClosed: re-forwarding (" << n_tries << " tries, " << (squid_curtime - start_t) << " secs)");
if (servers->next) {
/* use next, or cycle if origin server isn't last */
FwdServer *fs = servers;
FwdServer **T, *T2 = NULL;
servers = fs->next;
for (T = &servers; *T; T2 = *T, T = &(*T)->next);
if (T2 && T2->_peer) {
/* cycle */
*T = fs;
