Better reporting of REQMOD failures during body processing.
When REQMOD body processing fails, the server side needs to abort the
in-progress transaction with the origin server. Use a new custom error
detail and HTTP 500 (Internal Server Error) instead of 502 (Bad Gateway)
status code for this case.
The HTTP 500 (Internal Server Error) is more appropriate because most
deployments consider adaptation to be a "part of the proxy service"
rather than an "external gateway". And the custom error detail carries
more information than a potentially stale and often irrelevant errno global.
Earlier Squid versions (e.g., v3.0) were returning HTTP 500 (Internal
Server Error) error under similar conditions. Then, after some code
improvements, we started handing these errors better, but the new code
used ERR_READ_ERROR and the 502 (Bad Gateway) codes for the symmetry
with similar error handling code.
Unfortunately, preserving that symmetry was wrong in this case because
we are not dealing with a server-side reading error here. After an
upgrade to v3.2, some users noticed the increased number of 502 (Bad
Gateway) errors, which wrongly implied origin server problems rather
than ICAP server problems.
Thank you,
Alex.
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.
The HTTP 500 (Internal Server Error) is more appropriate because most
deployments consider adaptation to be a "part of the proxy service" rather
than an "external gateway". And the custom error detail carries more
information than a potentially stale and often irrelevant errno global.
=== modified file 'src/err_detail_type.h'
--- src/err_detail_type.h 2010-12-15 17:52:35 +0000
+++ src/err_detail_type.h 2011-03-22 18:14:13 +0000
@@ -1,29 +1,30 @@
#ifndef _SQUID_ERR_DETAIL_H
#define _SQUID_ERR_DETAIL_H
typedef enum {
ERR_DETAIL_NONE,
ERR_DETAIL_START = 100000, // to avoid clashes with most OS error numbers
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
ERR_DETAIL_RESPMOD_BLOCK_EARLY, // RESPMOD denied client access to HTTP response, before any part of the response was sent
ERR_DETAIL_RESPMOD_BLOCK_LATE, // RESPMOD denied client access to HTTP response, after [a part of] the response was sent
ERR_DETAIL_ICAP_XACT_START, // transaction start failure
ERR_DETAIL_ICAP_XACT_BODY_CONSUMER_ABORT, // transaction body consumer gone
ERR_DETAIL_ICAP_INIT_GONE, // initiator gone
ERR_DETAIL_ICAP_XACT_CLOSE, // ICAP connection closed unexpectedly
ERR_DETAIL_ICAP_XACT_OTHER, // other ICAP transaction errors
ERR_DETAIL_EXCEPTION_OTHER, //other errors ( eg std C++ lib errors)
ERR_DETAIL_MAX,
ERR_DETAIL_EXCEPTION_START = 110000 // offset for exception ID details
} err_detail_type;
extern const char *err_detail_type_str[];
inline
const char *errorDetailName(int errDetailId)
{
=== modified file 'src/http.cc'
--- src/http.cc 2011-03-09 17:44:55 +0000
+++ src/http.cc 2011-03-22 18:15:02 +0000
@@ -34,40 +34,41 @@
*/
/*
* Anonymizing patch by [email protected]
* have a look into http-anon.c to get more informations.
*/
#include "squid.h"
#include "acl/FilledChecklist.h"
#if USE_AUTH
#include "auth/UserRequest.h"
#endif
#include "base/AsyncJobCalls.h"
#include "base/TextException.h"
#include "base64.h"
#include "comm/Write.h"
#if USE_DELAY_POOLS
#include "DelayPools.h"
#endif
+#include "err_detail_type.h"
#include "errorpage.h"
#include "http.h"
#include "HttpControlMsg.h"
#include "HttpHdrContRange.h"
#include "HttpHdrSc.h"
#include "HttpHdrScTarget.h"
#include "HttpReply.h"
#include "HttpRequest.h"
#include "MemBuf.h"
#include "MemObject.h"
#include "protos.h"
#include "rfc1738.h"
#include "SquidTime.h"
#include "Store.h"
#define SQUID_ENTER_THROWING_CODE() try {
#define SQUID_EXIT_THROWING_CODE(status) \
status = true; \
} \
@@ -2287,42 +2288,46 @@
debugs(11, 1, "http handleMoreRequestBodyAvailable: Likely proxy abuse detected '" << orig_request->client_addr << "' -> '" << entry->url() << "'" );
if (virginReply()->sline.status == HTTP_INVALID_HEADER) {
comm_close(fd);
return;
}
}
}
HttpStateData::handleMoreRequestBodyAvailable();
}
// premature end of the request body
void
HttpStateData::handleRequestBodyProducerAborted()
{
ServerStateData::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);
}
abortTransaction("request body producer aborted");
}
// called when we wrote request headers(!) or a part of the body
void
HttpStateData::sentRequestBody(const CommIoCbParams &io)
{
if (io.size > 0)
kb_incr(&statCounter.server.http.kbytes_out, io.size);
ServerStateData::sentRequestBody(io);
}
// Quickly abort the transaction
// TODO: destruction should be sufficient as the destructor should cleanup,
// including canceling close handlers
void
