From: Alex Rousskov Date: Mon, 24 Aug 2015 21:07:31 +0000 (-0600) Subject: When a RESPMOD service aborts, mark the body it produced as truncated. X-Git-Tag: SQUID_4_0_1~108 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7224ca5ac5a4eda002c30089f9a461069aeb122d;p=thirdparty%2Fsquid.git When a RESPMOD service aborts, mark the body it produced as truncated. Without these changes, the recipient of the truncated body often cannot tell that the body was actually truncated (e.g., when Squid uses chunked encoding for body delivery). Lying about truncation may result in rather serious user-level problems. --- diff --git a/src/Store.h b/src/Store.h index dabbcee225..28e528bb93 100644 --- a/src/Store.h +++ b/src/Store.h @@ -66,6 +66,8 @@ public: } virtual bool isAccepting() const; virtual size_t bytesWanted(Range const aRange, bool ignoreDelayPool = false) const; + /// flags [truncated or too big] entry with ENTRY_BAD_LENGTH and releases it + void lengthWentBad(const char *reason); virtual void complete(); virtual store_client_t storeClientType() const; virtual char const *getSerialisedMetaData(); diff --git a/src/client_side.cc b/src/client_side.cc index d21fa51e2b..65d0092cba 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -958,6 +958,8 @@ ClientSocketContext::lengthToSend(Range const &available) void ClientSocketContext::noteSentBodyBytes(size_t bytes) { + debugs(33, 7, bytes << " body bytes"); + http->out.offset += bytes; if (!http->request->range) diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc index c777cfecd9..c90f16ba8d 100644 --- a/src/client_side_reply.cc +++ b/src/client_side_reply.cc @@ -1238,6 +1238,11 @@ clientReplyContext::replyStatus() return STREAM_FAILED; } + if (EBIT_TEST(http->storeEntry()->flags, ENTRY_BAD_LENGTH)) { + debugs(88, 5, "clientReplyStatus: truncated response body"); + return STREAM_UNPLANNED_COMPLETE; + } + if (!done) { debugs(88, 5, "clientReplyStatus: closing, !done, but read 0 bytes"); return STREAM_FAILED; diff --git a/src/clients/Client.cc b/src/clients/Client.cc index ca0eb96f0a..abfcff2c99 100644 --- a/src/clients/Client.cc +++ b/src/clients/Client.cc @@ -798,8 +798,22 @@ Client::endAdaptedBodyConsumption() // premature end of the adapted response body void Client::handleAdaptedBodyProducerAborted() { + if (abortOnBadEntry("entry went bad while waiting for the now-aborted adapted body")) + return; + + Must(adaptedBodySource != nullptr); + if (!adaptedBodySource->exhausted()) { + debugs(11,5, "waiting to consume the remainder of the aborted adapted body"); + return; // resumeBodyStorage() should eventually consume the rest + } + stopConsumingFrom(adaptedBodySource); - handleAdaptationAborted(); + + if (handledEarlyAdaptationAbort()) + return; + + entry->lengthWentBad("body adaptation aborted"); + handleAdaptationCompleted(); // the user should get a truncated response } // common part of noteAdaptationAnswer and handleAdaptedBodyProductionEnded @@ -832,18 +846,29 @@ Client::handleAdaptationAborted(bool bypassable) return; // TODO: bypass if possible + if (!handledEarlyAdaptationAbort()) + abortTransaction("adaptation failure with a filled entry"); +} +/// If the store entry is still empty, fully handles adaptation abort, returning +/// true. Otherwise just updates the request error detail and returns false. +bool +Client::handledEarlyAdaptationAbort() +{ if (entry->isEmpty()) { - debugs(11,9, HERE << "creating ICAP error entry after ICAP failure"); + debugs(11,8, "adaptation failure with an empty entry: " << *entry); ErrorState *err = new ErrorState(ERR_ICAP_FAILURE, Http::scInternalServerError, request); err->detailError(ERR_DETAIL_ICAP_RESPMOD_EARLY); fwd->fail(err); fwd->dontRetry(true); - } else if (request) { // update logged info directly - request->detailError(ERR_ICAP_FAILURE, ERR_DETAIL_ICAP_RESPMOD_LATE); + abortTransaction("adaptation failure with an empty entry"); + return true; // handled } - abortTransaction("ICAP failure"); + if (request) // update logged info directly + request->detailError(ERR_ICAP_FAILURE, ERR_DETAIL_ICAP_RESPMOD_LATE); + + return false; // the caller must handle } // adaptation service wants us to deny HTTP client access to this response diff --git a/src/clients/Client.h b/src/clients/Client.h index ba9650d1d8..d276729f18 100644 --- a/src/clients/Client.h +++ b/src/clients/Client.h @@ -124,6 +124,7 @@ protected: void handleAdaptationCompleted(); void handleAdaptationBlocked(const Adaptation::Answer &answer); void handleAdaptationAborted(bool bypassable = false); + bool handledEarlyAdaptationAbort(); /// called by StoreEntry when it has more buffer space available void resumeBodyStorage(); diff --git a/src/servers/Http1Server.cc b/src/servers/Http1Server.cc index be3fdb1061..b3fea716e6 100644 --- a/src/servers/Http1Server.cc +++ b/src/servers/Http1Server.cc @@ -19,6 +19,7 @@ #include "profiler/Profiler.h" #include "servers/Http1Server.h" #include "SquidConfig.h" +#include "Store.h" CBDATA_NAMESPACED_CLASS_INIT(Http1, Server); @@ -250,6 +251,7 @@ Http::One::Server::handleReply(HttpReply *rep, StoreIOBuffer receivedData) // the last-chunk if there was no error, ignoring responseFinishedOrFailed. const bool mustSendLastChunk = http->request->flags.chunkedReply && !http->request->flags.streamError && + !EBIT_TEST(http->storeEntry()->flags, ENTRY_BAD_LENGTH) && !context->startOfOutput(); const bool responseFinishedOrFailed = !rep && !receivedData.data && diff --git a/src/store.cc b/src/store.cc index c0d6c2b5fd..7968453bbd 100644 --- a/src/store.cc +++ b/src/store.cc @@ -1083,6 +1083,14 @@ storeCheckCachableStats(StoreEntry *sentry) store_check_cachable_hist.yes.Default); } +void +StoreEntry::lengthWentBad(const char *reason) +{ + debugs(20, 3, "because " << reason << ": " << *this); + EBIT_SET(flags, ENTRY_BAD_LENGTH); + releaseRequest(); +} + void StoreEntry::complete() { @@ -1107,10 +1115,8 @@ StoreEntry::complete() assert(mem_status == NOT_IN_MEMORY); - if (!validLength()) { - EBIT_SET(flags, ENTRY_BAD_LENGTH); - releaseRequest(); - } + if (!EBIT_TEST(flags, ENTRY_BAD_LENGTH) && !validLength()) + lengthWentBad("!validLength() in complete()"); #if USE_CACHE_DIGESTS if (mem_obj->request)