From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Fri, 11 Mar 2011 22:22:13 +0000 (-0700)
Subject: Fixed propagation of eCAP transaction meta-information to core Squid
X-Git-Tag: take06~27^2~97
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=aaf0559d4323bd1b990dfa7b589c9fac549fda29;p=thirdparty%2Fsquid.git

Fixed propagation of eCAP transaction meta-information to core Squid
by synchronizing the history of the virgin and eCAP-adapted/cloned request.

If the request history is created after the request got cloned, the cloned
request will have no history unless we explicitly import the newly created
history. Hopefully, it is not possible for the cloned request to get its own,
diverging history before the import (we check and throw if that happens).

This is one more example why a MasterTransaction class (with history) needs
to be extracted and separated from the HttpRequest class.
---

diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc
index f2e42b17ac..0919540b2f 100644
--- a/src/HttpRequest.cc
+++ b/src/HttpRequest.cc
@@ -439,6 +439,17 @@ HttpRequest::adaptLogHistory() const
     return HttpRequest::adaptHistory(loggingNeedsHistory);
 }
 
+void
+HttpRequest::adaptHistoryImport(const HttpRequest &them)
+{
+    if (!adaptHistory_) {
+        adaptHistory_ = them.adaptHistory_; // may be nil
+    } else {
+        // check that histories did not diverge
+        Must(!them.adaptHistory_ || them.adaptHistory_ == adaptHistory_);
+    }
+}
+
 #endif
 
 bool
diff --git a/src/HttpRequest.h b/src/HttpRequest.h
index 6fed46a1a0..898c329a12 100644
--- a/src/HttpRequest.h
+++ b/src/HttpRequest.h
@@ -118,6 +118,8 @@ public:
     Adaptation::History::Pointer adaptLogHistory() const;
     /// Returns possibly nil history, creating it if requested
     Adaptation::History::Pointer adaptHistory(bool createIfNone = false) const;
+    /// Makes their history ours, throwing on conflicts
+    void adaptHistoryImport(const HttpRequest &them);
 #endif
 #if ICAP_CLIENT
     /// Returns possibly nil history, creating it if icap logging is enabled
diff --git a/src/adaptation/Iterator.cc b/src/adaptation/Iterator.cc
index 712a34f140..6982594348 100644
--- a/src/adaptation/Iterator.cc
+++ b/src/adaptation/Iterator.cc
@@ -191,8 +191,10 @@ bool Adaptation::Iterator::updatePlan(bool adopt)
     Must(r);
 
     Adaptation::History::Pointer ah = r->adaptHistory();
-    if (!ah)
+    if (!ah) {
+        debugs(85,9, HERE << "no history to store a service-proposed plan");
         return false; // the feature is not enabled or is not triggered
+    }
 
     String services;
     if (!ah->extractNextServices(services)) { // clears history
diff --git a/src/adaptation/ecap/XactionRep.cc b/src/adaptation/ecap/XactionRep.cc
index 20bb82c318..8349a2d6d3 100644
--- a/src/adaptation/ecap/XactionRep.cc
+++ b/src/adaptation/ecap/XactionRep.cc
@@ -313,7 +313,7 @@ Adaptation::Ecap::XactionRep::useVirgin()
     // check that clone() copies the pipe so that we do not have to
     Must(!theVirginRep.raw().header->body_pipe == !clone->body_pipe);
 
-    updateHistory();
+    updateHistory(clone);
     sendAnswer(Answer::Forward(clone));
     Must(done());
 }
@@ -329,7 +329,7 @@ Adaptation::Ecap::XactionRep::useAdapted(const libecap::shared_ptr<libecap::Mess
     HttpMsg *msg = answer().header;
     if (!theAnswerRep->body()) { // final, bodyless answer
         proxyingAb = opNever;
-        updateHistory();
+        updateHistory(msg);
         sendAnswer(Answer::Forward(msg));
     } else { // got answer headers but need to handle body
         proxyingAb = opOn;
@@ -339,7 +339,7 @@ Adaptation::Ecap::XactionRep::useAdapted(const libecap::shared_ptr<libecap::Mess
         rep->tieBody(this); // sets us as a producer
         Must(msg->body_pipe != NULL); // check tieBody
 
-        updateHistory();
+        updateHistory(msg);
         sendAnswer(Answer::Forward(msg));
 
         debugs(93,4, HERE << "adapter will produce body" << status());
@@ -356,7 +356,7 @@ Adaptation::Ecap::XactionRep::blockVirgin()
 
     sinkVb("blockVirgin");
 
-    updateHistory();
+    updateHistory(NULL);
     sendAnswer(Answer::Block(service().cfg().key));
     Must(done());
 }
@@ -364,7 +364,7 @@ Adaptation::Ecap::XactionRep::blockVirgin()
 /// Called just before sendAnswer() to record adapter meta-information
 /// which may affect answer processing and may be needed for logging.
 void
-Adaptation::Ecap::XactionRep::updateHistory()
+Adaptation::Ecap::XactionRep::updateHistory(HttpMsg *adapted)
 {
     if (!theMaster) // all updates rely on being able to query the adapter
         return;
@@ -406,6 +406,10 @@ Adaptation::Ecap::XactionRep::updateHistory()
         theMaster->visitEachOption(extractor);
         ah->recordMeta(&meta);
     }
+
+    // Add just-created history to the adapted/cloned request that lacks it.
+    if (HttpRequest *adaptedReq = dynamic_cast<HttpRequest*>(adapted))
+        adaptedReq->adaptHistoryImport(*request);
 }
 
 
diff --git a/src/adaptation/ecap/XactionRep.h b/src/adaptation/ecap/XactionRep.h
index d0552f1017..027c921b10 100644
--- a/src/adaptation/ecap/XactionRep.h
+++ b/src/adaptation/ecap/XactionRep.h
@@ -86,7 +86,7 @@ protected:
 
     void moveAbContent();
 
-    void updateHistory();
+    void updateHistory(HttpMsg *adapted);
     void terminateMaster();
     void scheduleStop(const char *reason);