------------------------------------------------------------ revno: 13901 revision-id: squid3@treenet.co.nz-20150829201119-gdxtxfv3romi6j80 parent: squid3@treenet.co.nz-20150829185119-ovu27x2a22e763do author: Alex Rousskov committer: Amos Jeffries branch nick: 3.5 timestamp: Sat 2015-08-29 13:11:19 -0700 message: 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. ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: squid3@treenet.co.nz-20150829201119-gdxtxfv3romi6j80 # target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 # testament_sha1: 3320a1ed799eb32a65da18df0c934d7542977ea4 # timestamp: 2015-08-29 20:24:07 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 # base_revision_id: squid3@treenet.co.nz-20150829185119-\ # ovu27x2a22e763do # # Begin patch === modified file 'src/Store.h' --- src/Store.h 2015-01-13 09:13:49 +0000 +++ src/Store.h 2015-08-29 20:11:19 +0000 @@ -73,6 +73,8 @@ } 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(); === modified file 'src/client_side.cc' --- src/client_side.cc 2015-08-28 13:22:38 +0000 +++ src/client_side.cc 2015-08-29 20:11:19 +0000 @@ -963,6 +963,8 @@ void ClientSocketContext::noteSentBodyBytes(size_t bytes) { + debugs(33, 7, bytes << " body bytes"); + http->out.offset += bytes; if (!http->request->range) === modified file 'src/client_side_reply.cc' --- src/client_side_reply.cc 2015-05-27 23:28:09 +0000 +++ src/client_side_reply.cc 2015-08-29 20:11:19 +0000 @@ -1217,6 +1217,11 @@ 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; === modified file 'src/clients/Client.cc' --- src/clients/Client.cc 2015-01-13 09:13:49 +0000 +++ src/clients/Client.cc 2015-08-29 20:11:19 +0000 @@ -797,8 +797,22 @@ // 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 @@ -831,18 +845,29 @@ 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 + abortTransaction("adaptation failure with an empty entry"); + return true; // handled + } + + if (request) // update logged info directly request->detailError(ERR_ICAP_FAILURE, ERR_DETAIL_ICAP_RESPMOD_LATE); - } - abortTransaction("ICAP failure"); + return false; // the caller must handle } // adaptation service wants us to deny HTTP client access to this response === modified file 'src/clients/Client.h' --- src/clients/Client.h 2015-01-13 09:13:49 +0000 +++ src/clients/Client.h 2015-08-29 20:11:19 +0000 @@ -124,6 +124,7 @@ 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(); === modified file 'src/servers/HttpServer.cc' --- src/servers/HttpServer.cc 2015-01-13 09:13:49 +0000 +++ src/servers/HttpServer.cc 2015-08-29 20:11:19 +0000 @@ -16,6 +16,7 @@ #include "profiler/Profiler.h" #include "servers/forward.h" #include "SquidConfig.h" +#include "Store.h" namespace Http { @@ -145,6 +146,7 @@ // 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 && === modified file 'src/store.cc' --- src/store.cc 2015-01-19 11:50:22 +0000 +++ src/store.cc 2015-08-29 20:11:19 +0000 @@ -1041,6 +1041,14 @@ } void +StoreEntry::lengthWentBad(const char *reason) +{ + debugs(20, 3, "because " << reason << ": " << *this); + EBIT_SET(flags, ENTRY_BAD_LENGTH); + releaseRequest(); +} + +void StoreEntry::complete() { debugs(20, 3, "storeComplete: '" << getMD5Text() << "'"); @@ -1064,10 +1072,8 @@ 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)