Author: Philip Allison <philip.allison@smoothwall.net>

Bug 2614 fix: Potential loss of adapted body data from eCAP adapters

It was possible for Squid to stop reading buffered adapted body data before it
has all been sent to the browser.

Squid treated a call to noteAbContentDone by an adapter as a signal to stop
consuming and sending adapted body data. The correct behaviour is to use
noteAbContentDone to record the fact that the adapter has stopped producing
new adapted body data, but continue to consume and send data until all
buffered ab content is consumed and sent (i.e., abContent returns an empty
Area).

Links to relevant discussions:

https://answers.launchpad.net/ecap/+question/63068
https://answers.launchpad.net/ecap/+question/63147

diff --git a/src/adaptation/ecap/XactionRep.cc b/src/adaptation/ecap/XactionRep.cc
index e2b6b0c1ff9ae02f903c47d9be2b69321f9bb8e3..2a96ae6e782ed52dc68c06efe2e622b9f3d36ddf 100644 (file)
--- a/src/adaptation/ecap/XactionRep.cc
+++ b/src/adaptation/ecap/XactionRep.cc
@@ -16,7 +16,8 @@ Adaptation::Ecap::XactionRep::XactionRep(Adaptation::Initiator *anInitiator,
         AsyncJob("Adaptation::Ecap::XactionRep"),
         Adaptation::Initiate("Adaptation::Ecap::XactionRep", anInitiator, aService),
         theVirginRep(virginHeader), theCauseRep(NULL),
-        proxyingVb(opUndecided), proxyingAb(opUndecided), canAccessVb(false)
+        proxyingVb(opUndecided), proxyingAb(opUndecided), canAccessVb(false),
+        abProductionFinished(false), abProductionAtEnd(false)
 {
     if (virginCause)
         theCauseRep = new MessageRep(virginCause);
@@ -275,15 +276,17 @@ Adaptation::Ecap::XactionRep::vbContentShift(libecap::size_type n)
 void
 Adaptation::Ecap::XactionRep::noteAbContentDone(bool atEnd)
 {
-    Must(proxyingAb == opOn);
-    stopProducingFor(answer().body_pipe, atEnd);
-    proxyingAb = opComplete;
+    Must(proxyingAb == opOn && !abProductionFinished);
+    abProductionFinished = true;
+    abProductionAtEnd = atEnd; // store until ready to stop producing ourselves
+    debugs(93,5, HERE << "adapted body production ended");
+    moveAbContent();
 }
 
 void
 Adaptation::Ecap::XactionRep::noteAbContentAvailable()
 {
-    Must(proxyingAb == opOn);
+    Must(proxyingAb == opOn && !abProductionFinished);
     moveAbContent();
 }
 
@@ -374,9 +377,16 @@ Adaptation::Ecap::XactionRep::moveAbContent()
 {
     Must(proxyingAb == opOn);
     const libecap::Area c = theMaster->abContent(0, libecap::nsize);
-    debugs(93,5, HERE << " up to " << c.size << " bytes");
-    if (const size_t used = answer().body_pipe->putMoreData(c.start, c.size))
-        theMaster->abContentShift(used);
+    debugs(93,5, HERE << "up to " << c.size << " bytes");
+    if (c.size == 0 && abProductionFinished) { // no ab now and in the future
+        stopProducingFor(answer().body_pipe, abProductionAtEnd);
+        proxyingAb = opComplete;
+        debugs(93,5, HERE << "last adapted body data retrieved");
+    } else
+    if (c.size > 0) {
+        if (const size_t used = answer().body_pipe->putMoreData(c.start, c.size))
+            theMaster->abContentShift(used);
+    }
 }
 
 const char *

diff --git a/src/adaptation/ecap/XactionRep.h b/src/adaptation/ecap/XactionRep.h
index af9b7ba784082331c94211858df555d329b7d78d..3ca30172273c401cfc243ea1fc130b0becaaf3bd 100644 (file)
--- a/src/adaptation/ecap/XactionRep.h
+++ b/src/adaptation/ecap/XactionRep.h
@@ -93,6 +93,8 @@ private:
     OperationState proxyingVb; // delivering virgin body from core to adapter
     OperationState proxyingAb; // delivering adapted body from adapter to core
     bool canAccessVb;          // virgin BodyPipe content is accessible
+    bool abProductionFinished; // whether adapter has finished producing ab
+    bool abProductionAtEnd;    // whether adapter produced a complete ab
 
     CBDATA_CLASS2(XactionRep);
 };