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 dabbcee2253c3bc51398424690be4e515037e016..28e528bb9307a1788cd6a5af633c12d8e902c905 100644 (file)
--- a/src/Store.h
+++ b/src/Store.h
@@ -66,6 +66,8 @@ public:
     }
     virtual bool isAccepting() const;
     virtual size_t bytesWanted(Range<size_t> 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 d21fa51e2b90e13f358702d03aa5ff78ae94375a..65d0092cba7137019e6610eb13f12314c45e66aa 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -958,6 +958,8 @@ ClientSocketContext::lengthToSend(Range<int64_t> 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 c777cfecd95b7696f20f08b0010148d6d0ae5157..c90f16ba8d794b28758fd4acdce5c9f75f8d290e 100644 (file)
--- 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 ca0eb96f0a190e90ae51bbc73816b73e2a4c709c..abfcff2c999b925f81e0731091456068834c35ee 100644 (file)
--- 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 ba9650d1d8000db97f11a8472554ed1f64482985..d276729f18235d69a33f0cd2189af7ff871270a7 100644 (file)
--- 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 be3fdb10619deda28003f2623da526362aa72829..b3fea716e6793476e14989ea9fbad36b8e647dde 100644 (file)
--- 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 c0d6c2b5fd748af172eb74ad688315ed696cbbd6..7968453bbd85d3eed26fbcbeb7a21ebd593a9f88 100644 (file)
--- 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)