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 f2e42b17ac288238053f3ed342374b05dc853ff5..0919540b2f1bd7ca94de8bb89256318ae25ca4b0 100644 (file)
--- 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 6fed46a1a09fc7b53d11d1b6ea6f03db0bb9d400..898c329a121c41fc12817bed525e8dc3b9b7742f 100644 (file)
--- 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 712a34f140030d11f0dda057704d7010ba205964..6982594348e64507b33bdeaf23345f1850b9e12a 100644 (file)
--- 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 20bb82c3188d31383e5369bf9fd04108a50cf9a4..8349a2d6d3fc7c4902105782e5fad2f745ff4827 100644 (file)
--- 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 d0552f10171f3bf7637b288a0d97c3c9b8c9416c..027c921b10647e9b089e291af4e6f4e9d0f090b4 100644 (file)
--- 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);