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 9cabcd83e6564db285e9f0d17ba9c0e53709b9f2..47fe985d0d714cf76c77dfb547be829fd6617db3 100644 (file)
--- a/src/Store.h
+++ b/src/Store.h
@@ -73,6 +73,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 8dcefb163f9464265fba5095fb3ecdf673a133ad..3ba425db06900bb9809d9032b58e754b9d37e48d 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -963,6 +963,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 126815819fce62fc902e3dad92541a168a818b84..5f05dcf8e1cbb7d40e54bb3811334a0667e398a7 100644 (file)
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@ -1217,6 +1217,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 10aa17b3c37e4e54c5fc3518bbd319cea1597b58..32f3c3a311664dfc310b84ca5c65e83774708df3 100644 (file)
--- a/src/clients/Client.cc
+++ b/src/clients/Client.cc
@@ -797,8 +797,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
@@ -831,18 +845,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 de55adf1b1e898eb0dce0f17bdaed5681a40c0ac..608568f706bc7853a33e8ae8664da756a44ba5eb 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/HttpServer.cc b/src/servers/HttpServer.cc
index 49c91ce00285a183e88044de5b33c46aad09689f..1665a12e661c138456fc6bdda518cb56a6aa29a1 100644 (file)
--- a/src/servers/HttpServer.cc
+++ b/src/servers/HttpServer.cc
@@ -16,6 +16,7 @@
 #include "profiler/Profiler.h"
 #include "servers/forward.h"
 #include "SquidConfig.h"
+#include "Store.h"
 
 namespace Http
 {
@@ -145,6 +146,7 @@ Http::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 286eeef979e53b685a700020a63110cd9ae8bee9..521c9d08f0eb8902572b90aa12ec7c0c0464f7dd 100644 (file)
--- a/src/store.cc
+++ b/src/store.cc
@@ -1040,6 +1040,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()
 {
@@ -1064,10 +1072,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)