From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Sat, 27 Jun 2009 01:14:58 +0000 (-0600)
Subject: Author: Philip Allison <philip.allison@smoothwall.net>
X-Git-Tag: SQUID_3_2_0_1~926
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7477a3435368a04788ec166945d97f409a5e77f0;p=thirdparty%2Fsquid.git

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 e2b6b0c1ff..2a96ae6e78 100644
--- 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 af9b7ba784..3ca3017227 100644
--- 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);
 };