From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Wed, 30 Mar 2011 18:14:08 +0000 (-0600)
Subject: Better reporting of REQMOD failures during body processing.
X-Git-Tag: take06~27^2~50
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=8b997339d1616c06ff2a3b5236310e20a1b0b3a7;p=thirdparty%2Fsquid.git

Better reporting of REQMOD failures during body processing.

When REQMOD body processing fails, the server-side needs to abort the
in-progress transaction. Use HTTP 500 (Internal Server Error) instead of 502
(Bad Gateway) status code and a new custom error detail for this case.

There is no perfect status code for ICAP errors because some view ICAP
processing as an integral part of the proxy (closer to the internal
proxy code) and some treat it as an external entity (closer to an
"upstream" web server).

Nevertheless, 502 (Bad Gateway) feels like a worse fit because from web
user and traffic flow point of views ICAP is less of a "gateway" or
"upstream" server than it is an "internally" used auxiliary service.
When both status codes were used, we have received reasonable complaints
about 502 responses but not (IIRC) about 500 responses.

Using custom error detail is better than errno in this context because
errno is often not set for ICAP errors and because by the time we
generate an error, the errno value is likely to be from a different
system call error anyway.
---

diff --git a/src/err_detail_type.h b/src/err_detail_type.h
index 826617d25c..914925be3e 100644
--- a/src/err_detail_type.h
+++ b/src/err_detail_type.h
@@ -7,6 +7,7 @@ typedef enum {
     ERR_DETAIL_CLT_REQMOD_ABORT = ERR_DETAIL_START, // client-side detected transaction abort
     ERR_DETAIL_CLT_REQMOD_REQ_BODY, // client-side detected REQMOD request body adaptation failure
     ERR_DETAIL_CLT_REQMOD_RESP_BODY, // client-side detected REQMOD satisfaction reply body failure
+    ERR_DETAIL_SRV_REQMOD_REQ_BODY, // server-side detected REQMOD request body abort
     ERR_DETAIL_ICAP_RESPMOD_EARLY, // RESPMOD failed w/o store entry
     ERR_DETAIL_ICAP_RESPMOD_LATE,  // RESPMOD failed with a store entry
     ERR_DETAIL_REQMOD_BLOCK, // REQMOD denied client access
diff --git a/src/http.cc b/src/http.cc
index 0130ef57a3..299f53a08c 100644
--- a/src/http.cc
+++ b/src/http.cc
@@ -51,6 +51,7 @@
 #if USE_DELAY_POOLS
 #include "DelayPools.h"
 #endif
+#include "err_detail_type.h"
 #include "errorpage.h"
 #include "http.h"
 #include "HttpControlMsg.h"
@@ -2304,8 +2305,12 @@ HttpStateData::handleRequestBodyProducerAborted()
     if (entry->isEmpty()) {
         debugs(11, 3, "request body aborted: FD " << fd);
         ErrorState *err;
-        err = errorCon(ERR_READ_ERROR, HTTP_BAD_GATEWAY, fwd->request);
-        err->xerrno = errno;
+        // We usually get here when ICAP REQMOD aborts during body processing.
+        // We might also get here if client-side aborts, but then our response
+        // should not matter because either client-side will provide its own or
+        // there will be no response at all (e.g., if the the client has left).
+        err = errorCon(ERR_ICAP_FAILURE, HTTP_INTERNAL_SERVER_ERROR, fwd->request);
+        err->xerrno = ERR_DETAIL_SRV_REQMOD_REQ_BODY;
         fwd->fail(err);
     }