Hello,

    The attached patch allows eCAP and ICAP services to set StoreID
values, just like many existing store_id_program helpers do. To avoid
surprises from rogue services, the optional behavior must be enabled in
squid.conf by adding "store-id=1" to the e/icap_service directive.
Adaptation services are executed before the StoreID helpers but they may
co-exist. The latest StoreID value wins.

The patch also merged eCAP and ICAP code that updates adaptation
history, resolving an old TODO.

Disclaimer: After these changes, options returned by broken eCAP
adapters that implemented adapter::Xaction::option() without also
implementing adapter::Xaction::visitEachOption() will stop working.
Similarly, options returned by broken eCAP adapters that implemented
visitEachOption() without also implementing option() may start working.
I do not think this is a big deal because the loss of backward
compatibility is limited to broken adapters that would have to be
rebuild for newer Squid/libecap anyway.


I used "Store-ID" for the name of the meta header (an ICAP response
header or an eCAP option name; not an HTTP header!). Other alternatives
considered but rejected:

Store-Id: Most registered MIME headers use -ID, not -Id.
http://www.iana.org/assignments/message-headers/message-headers.xhtml

X-Store-ID: Trying to avoid adding X-Store-ID now and then renaming it
to Store-ID ten years later, while adding more code for backward
compatibility reasons. The Store-ID header is not registered. I am not
going to write an Internet Draft to describe it, but somebody could try
to submit a _provisional_ Store-ID IANA registration that does not
require a Draft or an RFC AFAICT. However, the header would still need a
description on the web somewhere. Perhaps somebody can volunteer to take
a lead on that?

Archived-At: This is a registered header that is almost perfect for our
needs. The was worried that other developers would object to using a
completely different name compared to StoreID helpers (and the feature
name itself).

Content-ID: This is a registered header that may be reused for our
needs. However, StoreID is not restricted to (and typically does not
use) unique content identifiers, content checksums, etc. All StoreID
helpers I know map URLs and do not really know anything about the
corresponding response content. Archived-At reasoning above applies here
as well.


Any other reusable and registered (or at least "known") headers to consider?

I am happy to rename if a consensus builds around a different name,
including those above.


Thank you,

Alex.

Allow eCAP and ICAP transactions to set StoreID
by returning a Store-ID eCAP transaction option or ICAP header.

Also merged eCAP and ICAP code that updates adaptation history (an old TODO).

After these changes, broken eCAP adapters that implemented
adapter::Xaction::option() without also implementing
adapter::Xaction::visitEachOption() will stop working. Similarly, broken eCAP
adapters that implemented visitEachOption() without also implementing option()
may start working.

=== modified file 'src/adaptation/History.cc'
--- src/adaptation/History.cc	2013-11-12 14:48:50 +0000
+++ src/adaptation/History.cc	2014-06-27 22:49:42 +0000
@@ -1,57 +1,60 @@
 #include "squid.h"
 #include "adaptation/Config.h"
 #include "adaptation/History.h"
+#include "adaptation/ServiceConfig.h"
 #include "base/TextException.h"
 #include "Debug.h"
 #include "globals.h"
+#include "HttpRequest.h"
 #include "SquidTime.h"
 
 /// impossible services value to identify unset theNextServices
 const static char *TheNullServices = ",null,";
 
 Adaptation::History::Entry::Entry(const String &serviceId, const timeval &when):
         service(serviceId), start(when), theRptm(-1), retried(false)
 {
 }
 
 Adaptation::History::Entry::Entry():
         start(current_time), theRptm(-1), retried(false)
 {
 }
 
 void Adaptation::History::Entry::stop()
 {
     // theRptm may already be set if the access log entry has already been made
     (void)rptm(); // will cache result in theRptm if not set already
 }
 
 int Adaptation::History::Entry::rptm()
 {
     if (theRptm < 0)
         theRptm = tvSubMsec(start, current_time);
     return theRptm;
 }
 
 Adaptation::History::History():
         lastMeta(hoReply),
         allMeta(hoReply),
+        store_id(),
         theNextServices(TheNullServices)
 {
 }
 
 int Adaptation::History::recordXactStart(const String &serviceId, const timeval &when, bool retrying)
 {
     // the history will be empty on retries if it was enabled after the failure
     if (retrying && !theEntries.empty())
         theEntries.back().retried = true;
 
     theEntries.push_back(Adaptation::History::Entry(serviceId, when));
     return theEntries.size() - 1; // record position becomes history ID
 }
 
 void Adaptation::History::recordXactFinish(int hid)
 {
     Must(0 <= hid && hid < static_cast<int>(theEntries.size()));
     theEntries[hid].stop();
 }
 
@@ -132,40 +135,72 @@ void Adaptation::History::updateNextServ
 bool Adaptation::History::extractNextServices(String &value)
 {
     if (theNextServices == TheNullServices)
         return false;
 
     value = theNextServices;
     theNextServices = TheNullServices; // prevents resetting the plan twice
     return true;
 }
 
 void Adaptation::History::recordMeta(const HttpHeader *lm)
 {
     lastMeta.clean();
     lastMeta.update(lm, NULL);
 
     allMeta.update(lm, NULL);
     allMeta.compact();
 }
 
 void
+Adaptation::History::RecordMeta(const ServiceConfig &cfg, const HttpRequest &request, const HttpHeader &meta)
+{
+    // update the cross-transactional database if needed
+    if (const char *xxName = Adaptation::Config::masterx_shared_name) {
+        String value;
+        if (meta.getByNameIfPresent(xxName, value))
+            request.adaptHistory(true)->updateXxRecord(xxName, value);
+    }
+
+    // update the adaptation plan if needed
+    if (cfg.routing) {
+        String value;
+        if (meta.getList(HDR_X_NEXT_SERVICES, &value))
+            request.adaptHistory(true)->updateNextServices(value);
+    } // TODO: else warn (occasionally!) if we got HDR_X_NEXT_SERVICES
+
+    // update StoreID if needed
+    if (cfg.storeId) {
+        String value;
+        if (meta.getByNameIfPresent("Store-ID", value))
+            request.adaptHistory(true)->store_id = value;
+    } // TODO: else warn (occasionally!) if we got Store-ID
+
+    // Store the received meta headers for adapt::<last_h logformat code use.
+    // If we already have stored headers from a previous adaptation xaction
+    // related to the same master transction, they will be replaced.
+    const Adaptation::History::Pointer ah = request.adaptLogHistory();
+    if (ah != NULL)
+        ah->recordMeta(&meta);
+}
+
+void
 Adaptation::History::recordAdaptationService(SBuf &srvId)
 {
     theAdaptationServices.push_back(srvId);
 }
 
 void
 Adaptation::History::setFutureServices(const DynamicGroupCfg &services)
 {
     if (!theFutureServices.empty())
         debugs(93,3, HERE << "old future services: " << theFutureServices);
     debugs(93,3, HERE << "new future services: " << services);
     theFutureServices = services; // may be empty
 }
 
 bool Adaptation::History::extractFutureServices(DynamicGroupCfg &value)
 {
     if (theFutureServices.empty())
         return false;
 
     value = theFutureServices;

=== modified file 'src/adaptation/History.h'
--- src/adaptation/History.h	2014-06-24 22:21:48 +0000
+++ src/adaptation/History.h	2014-06-27 21:26:31 +0000
@@ -1,24 +1,25 @@
 #ifndef SQUID_ADAPT_HISTORY_H
 #define SQUID_ADAPT_HISTORY_H
 
 #include "adaptation/DynamicGroupCfg.h"
+#include "adaptation/forward.h"
 #include "base/RefCount.h"
 #include "HttpHeader.h"
 #include "Notes.h"
 #include "SBuf.h"
 #include "SquidString.h"
 
 namespace Adaptation
 {
 
 /// collects information about adaptations related to a master transaction
 class History: public RefCountable
 {
 public:
     typedef RefCount<Adaptation::History> Pointer;
 
     History();
 
     /// record the start of a xact, return xact history ID
     int recordXactStart(const String &serviceId, const timeval &when, bool retrying);
 
@@ -30,58 +31,64 @@ public:
 
     /// dump xaction times, merging retried and retry times together
     void sumLogString(const char *serviceId, String &buf);
 
     /// sets or resets a cross-transactional database record
     void updateXxRecord(const char *name, const String &value);
 
     /// returns true and fills the record fields iff there is a db record
     bool getXxRecord(String &name, String &value) const;
 
     /// sets or resets next services for the Adaptation::Iterator to notice
     void updateNextServices(const String &services);
 
     /// returns true, fills the value, and resets iff next services were set
     bool extractNextServices(String &value);
 
     /// store the last meta header fields received from the adaptation service
     void recordMeta(const HttpHeader *lm);
 
     void recordAdaptationService(SBuf &srvId);
+
+    /// creates (if needed) and updates history with adaptation xaction meta
+    static void RecordMeta(const ServiceConfig &cfg, const HttpRequest &request, const HttpHeader &meta);
+
 public:
     /// Last received meta header (REQMOD or RESPMOD, whichever comes last).
     HttpHeader lastMeta;
     /// All REQMOD and RESPMOD meta headers merged. Last field wins conflicts.
     HttpHeader allMeta;
     /// key:value pairs set by adaptation_meta, to be added to
     /// AccessLogEntry::notes when ALE becomes available
     NotePairs::Pointer metaHeaders;
 
     typedef std::vector<SBuf> AdaptationServices;
     AdaptationServices theAdaptationServices; ///< The service groups used
 
     /// sets future services for the Adaptation::AccessCheck to notice
     void setFutureServices(const DynamicGroupCfg &services);
 
     /// returns true, fills the value, and resets iff future services were set
     bool extractFutureServices(DynamicGroupCfg &services);
 
+    String store_id; ///< store ID (pre-cache REQMOD only for now)
+
 private:
     /// single Xaction stats (i.e., a historical record entry)
     class Entry
     {
     public:
         Entry(const String &serviceId, const timeval &when);
         Entry(); // required by Vector<>
 
         void stop(); ///< updates stats on transaction end
         int rptm(); ///< returns response time [msec], calculates it if needed
 
         String service; ///< adaptation service ID
         timeval start; ///< when the xaction was started
 
     private:
         int theRptm; ///< calculated and cached response time value in msec
 
     public:
         bool retried; ///< whether the xaction was replaced by another
     };

=== modified file 'src/adaptation/ServiceConfig.cc'
--- src/adaptation/ServiceConfig.cc	2013-07-21 19:24:35 +0000
+++ src/adaptation/ServiceConfig.cc	2014-06-27 21:07:43 +0000
@@ -1,36 +1,38 @@
 /*
  * DEBUG: section 93    Adaptation
  */
 
 #include "squid.h"
 #include "adaptation/ServiceConfig.h"
 #include "ConfigParser.h"
 #include "Debug.h"
 #include "globals.h"
 #include "ip/tools.h"
 #include <set>
 
 Adaptation::ServiceConfig::ServiceConfig():
         port(-1), method(methodNone), point(pointNone),
         bypass(false), maxConn(-1), onOverload(srvWait),
-        routing(false), ipv6(false)
+        routing(false),
+        storeId(false),
+        ipv6(false)
 {}
 
 const char *
 Adaptation::ServiceConfig::methodStr() const
 {
     return Adaptation::methodStr(method);
 }
 
 const char *
 Adaptation::ServiceConfig::vectPointStr() const
 {
     return Adaptation::vectPointStr(point);
 }
 
 Adaptation::Method
 Adaptation::ServiceConfig::parseMethod(const char *str) const
 {
     if (!strncasecmp(str, "REQMOD", 6))
         return Adaptation::methodReqmod;
 
@@ -50,41 +52,41 @@ Adaptation::ServiceConfig::parseVectPoin
         t = q + 1;
 
     if (!strcmp(t, "precache"))
         return Adaptation::pointPreCache;
 
     if (!strcmp(t, "postcache"))
         return Adaptation::pointPostCache;
 
     return Adaptation::pointNone;
 }
 
 bool
 Adaptation::ServiceConfig::parse()
 {
     key = ConfigParser::NextToken();
     String method_point = ConfigParser::NextToken();
     method = parseMethod(method_point.termedBuf());
     point = parseVectPoint(method_point.termedBuf());
 
     // reset optional parameters in case we are reconfiguring
-    bypass = routing = false;
+    bypass = routing = storeId = false;
 
     // handle optional service name=value parameters
     bool grokkedUri = false;
     bool onOverloadSet = false;
     std::set<std::string> options;
 
     while (char *option = ConfigParser::NextToken()) {
         const char *name = option;
         const char *value = "";
         if (strcmp(option, "0") == 0) { // backward compatibility
             name = "bypass";
             value = "off";
             debugs(3, DBG_PARSE_NOTE(DBG_IMPORTANT), "UPGRADE: Please use 'bypass=off' option to disable service bypass");
         }  else if (strcmp(option, "1") == 0) { // backward compatibility
             name = "bypass";
             value = "on";
             debugs(3, DBG_PARSE_NOTE(DBG_IMPORTANT), "UPGRADE: Please use 'bypass=on' option to enable service bypass");
         } else {
             char *eq = strstr(option, "=");
             const char *sffx = strstr(option, "://");
@@ -93,73 +95,82 @@ Adaptation::ServiceConfig::parse()
                 value = option;
             }  else { // a normal name=value option
                 *eq = '\0'; // terminate option name
                 value = eq + 1; // skip '='
             }
         }
 
         // Check if option is set twice
         if (options.find(name) != options.end()) {
             debugs(3, DBG_CRITICAL, cfg_filename << ':' << config_lineno << ": " <<
                    "Duplicate option \"" << name << "\" in adaptation service definition");
             return false;
         }
         options.insert(name);
 
         bool grokked = false;
         if (strcmp(name, "bypass") == 0) {
             grokked = grokBool(bypass, name, value);
         } else if (strcmp(name, "routing") == 0)
             grokked = grokBool(routing, name, value);
+        else if (strcmp(name, "store-id") == 0)
+            grokked = grokBool(storeId, name, value);
         else if (strcmp(name, "uri") == 0)
             grokked = grokkedUri = grokUri(value);
         else if (strcmp(name, "ipv6") == 0) {
             grokked = grokBool(ipv6, name, value);
             if (grokked && ipv6 && !Ip::EnableIpv6)
                 debugs(3, DBG_PARSE_NOTE(DBG_IMPORTANT), "WARNING: IPv6 is disabled. ICAP service option ignored.");
         } else if (strcmp(name, "max-conn") == 0)
             grokked = grokLong(maxConn, name, value);
         else if (strcmp(name, "on-overload") == 0) {
             grokked = grokOnOverload(onOverload, value);
             onOverloadSet = true;
         } else
             grokked = grokExtension(name, value);
 
         if (!grokked)
             return false;
     }
 
     // set default on-overload value if needed
     if (!onOverloadSet)
         onOverload = bypass ? srvBypass : srvWait;
 
     // is the service URI set?
     if (!grokkedUri) {
         debugs(3, DBG_CRITICAL, cfg_filename << ':' << config_lineno << ": " <<
                "No \"uri\" option in adaptation service definition");
         return false;
     }
 
+    if (storeId && !(method == Adaptation::methodReqmod && point == Adaptation::pointPreCache)) {
+        debugs(3, DBG_CRITICAL, cfg_filename << ':' << config_lineno << ": " <<
+               "store-id is not supported at the " << method_point <<
+               " vectoring point");
+        return false;
+    }
+
     debugs(3,5, cfg_filename << ':' << config_lineno << ": " <<
            "adaptation_service " << key << ' ' <<
            methodStr() << "_" << vectPointStr() << ' ' <<
-           bypass << routing << ' ' <<
+           bypass << routing << storeId << ' ' <<
            uri);
 
     return true;
 }
 
 bool
 Adaptation::ServiceConfig::grokUri(const char *value)
 {
     // TODO: find core code that parses URLs and extracts various parts
     // AYJ: most of this is duplicate of urlParse() in src/url.cc
 
     if (!value || !*value) {
         debugs(3, DBG_CRITICAL, HERE << cfg_filename << ':' << config_lineno << ": " <<
                "empty adaptation service URI");
         return false;
     }
 
     uri = value;
 
     // extract scheme and use it as the service_configConfig protocol

=== modified file 'src/adaptation/ServiceConfig.h'
--- src/adaptation/ServiceConfig.h	2013-10-25 00:13:46 +0000
+++ src/adaptation/ServiceConfig.h	2014-06-27 21:07:29 +0000
@@ -20,39 +20,40 @@ public:
     bool parse();
 
 public:
     String key;    // service_configConfig name in the configuration file
     String uri;    // service_configConfig URI
 
     // service_configConfig URI components
     String protocol;
     String host;
     String resource;
     int port;
 
     Method method;   // what is being adapted (REQMOD vs RESPMOD)
     VectPoint point; // where the adaptation happens (pre- or post-cache)
     bool bypass;
 
     // options
     long maxConn; ///< maximum number of concurrent service transactions
     SrvBehaviour onOverload; ///< how to handle Max-Connections feature
     bool routing; ///< whether this service may determine the next service(s)
+    bool storeId; ///< whether this service sets StoreID
     bool ipv6;    ///< whether this service uses IPv6 transport (default IPv4)
 
 protected:
     Method parseMethod(const char *buf) const;
     VectPoint parseVectPoint(const char *buf) const;
 
     /// interpret parsed values
     bool grokBool(bool &var, const char *name, const char *value);
     bool grokUri(const char *value);
     bool grokLong(long &var, const char *name, const char *value);
     /// handle on-overload configuration option
     bool grokOnOverload(SrvBehaviour &var, const char *value);
     /// handle name=value configuration option with name unknown to Squid
     virtual bool grokExtension(const char *name, const char *value);
 };
 
 } // namespace Adaptation
 
 #endif /* SQUID_ADAPTATION__SERVICE_CONFIG_H */

=== modified file 'src/adaptation/ecap/XactionRep.cc'
--- src/adaptation/ecap/XactionRep.cc	2013-12-05 11:04:45 +0000
+++ src/adaptation/ecap/XactionRep.cc	2014-06-27 22:15:49 +0000
@@ -1,53 +1,55 @@
 /*
  * DEBUG: section 93    eCAP Interface
  */
 #include "squid.h"
 #include <libecap/common/area.h>
 #include <libecap/common/delay.h>
 #include <libecap/common/named_values.h>
 #include <libecap/common/names.h>
 #include <libecap/adapter/xaction.h>
 #include "adaptation/Answer.h"
 #include "adaptation/ecap/Config.h"
 #include "adaptation/ecap/XactionRep.h"
 #include "adaptation/Initiator.h"
 #include "base/AsyncJobCalls.h"
 #include "base/TextException.h"
 #include "format/Format.h"
 #include "HttpReply.h"
 #include "HttpRequest.h"
+#include "log/Config.h"
 #include "SquidTime.h"
 
 CBDATA_NAMESPACED_CLASS_INIT(Adaptation::Ecap::XactionRep, XactionRep);
 
 /// a libecap Visitor for converting adapter transaction options to HttpHeader
 class OptionsExtractor: public libecap::NamedValueVisitor
 {
 public:
     typedef libecap::Name Name;
     typedef libecap::Area Area;
 
     OptionsExtractor(HttpHeader &aMeta): meta(aMeta) {}
 
     // libecap::NamedValueVisitor API
     virtual void visit(const Name &name, const Area &value) {
+        // TODO: optimize convertion of libecap::Name/Area to Squid String
         meta.putExt(name.image().c_str(), value.toString().c_str());
     }
 
     HttpHeader &meta; ///< where to put extracted options
 };
 
 Adaptation::Ecap::XactionRep::XactionRep(
     HttpMsg *virginHeader, HttpRequest *virginCause, AccessLogEntry::Pointer &alp,
     const Adaptation::ServicePointer &aService):
         AsyncJob("Adaptation::Ecap::XactionRep"),
         Adaptation::Initiate("Adaptation::Ecap::XactionRep"),
         theService(aService),
         theVirginRep(virginHeader), theCauseRep(NULL),
         makingVb(opUndecided), proxyingAb(opUndecided),
         adaptHistoryId(-1),
         vbProductionFinished(false),
         abProductionFinished(false), abProductionAtEnd(false),
         al(alp)
 {
     if (virginCause)
@@ -442,72 +444,50 @@ Adaptation::Ecap::XactionRep::blockVirgi
 
     sinkVb("blockVirgin");
 
     updateHistory(NULL);
     sendAnswer(Answer::Block(service().cfg().key));
     Must(done());
 }
 
 /// 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(HttpMsg *adapted)
 {
     if (!theMaster) // all updates rely on being able to query the adapter
         return;
 
     const HttpRequest *request = dynamic_cast<const HttpRequest*>(theCauseRep ?
                                  theCauseRep->raw().header : theVirginRep.raw().header);
     Must(request);
 
-    // TODO: move common ICAP/eCAP logic to Adaptation::Xaction or similar
-    // TODO: optimize Area-to-String conversion
-
-    // update the cross-transactional database if needed
-    if (const char *xxNameStr = Adaptation::Config::masterx_shared_name) {
-        Adaptation::History::Pointer ah = request->adaptHistory(true);
-        if (ah != NULL) {
-            libecap::Name xxName(xxNameStr); // TODO: optimize?
-            if (const libecap::Area val = theMaster->option(xxName))
-                ah->updateXxRecord(xxNameStr, val.toString().c_str());
-        }
-    }
-
-    // update the adaptation plan if needed
-    if (service().cfg().routing) {
-        String services;
-        if (const libecap::Area services = theMaster->option(libecap::metaNextServices)) {
-            Adaptation::History::Pointer ah = request->adaptHistory(true);
-            if (ah != NULL)
-                ah->updateNextServices(services.toString().c_str());
-        }
-    } // TODO: else warn (occasionally!) if we got libecap::metaNextServices
-
-    // Store received meta headers for adapt::<last_h logformat code use.
-    // If we already have stored headers from a previous adaptation transaction
-    // related to the same master transction, they will be replaced.
-    Adaptation::History::Pointer ah = request->adaptLogHistory();
-    if (ah != NULL) {
+    const bool needMeta =
+        Adaptation::Config::masterx_shared_name ||
+        service().cfg().routing ||
+        service().cfg().storeId ||
+        Log::TheConfig.hasAdaptToken; // we need to log transaction options
+    if (needMeta) {
         HttpHeader meta(hoReply);
         OptionsExtractor extractor(meta);
         theMaster->visitEachOption(extractor);
-        ah->recordMeta(&meta);
+        Adaptation::History::RecordMeta(service().cfg(), *request, meta);
     }
 
     // Add just-created history to the adapted/cloned request that lacks it.
     if (HttpRequest *adaptedReq = dynamic_cast<HttpRequest*>(adapted))
         adaptedReq->adaptHistoryImport(*request);
 }
 
 void
 Adaptation::Ecap::XactionRep::vbDiscard()
 {
     Must(makingVb == opUndecided);
     // if adapter does not need vb, we do not need to send it
     sinkVb("vbDiscard");
     Must(makingVb == opNever);
 }
 
 void
 Adaptation::Ecap::XactionRep::vbMake()
 {
     Must(makingVb == opUndecided);

=== modified file 'src/adaptation/icap/ModXact.cc'
--- src/adaptation/icap/ModXact.cc	2014-03-30 12:00:34 +0000
+++ src/adaptation/icap/ModXact.cc	2014-06-27 21:29:55 +0000
@@ -798,67 +798,42 @@ void Adaptation::Icap::ModXact::parseIca
         break;
 
     case Http::scNoContent:
         handle204NoContent();
         break;
 
     case Http::scPartialContent:
         handle206PartialContent();
         break;
 
     default:
         debugs(93, 5, "ICAP status " << icapReply->sline.status());
         handleUnknownScode();
         break;
     }
 
     const HttpRequest *request = dynamic_cast<HttpRequest*>(adapted.header);
     if (!request)
         request = &virginRequest();
 
-    // update the cross-transactional database if needed (all status codes!)
-    if (const char *xxName = Adaptation::Config::masterx_shared_name) {
-        Adaptation::History::Pointer ah = request->adaptHistory(true);
-        if (ah != NULL) { // TODO: reorder checks to avoid creating history
-            const String val = icapReply->header.getByName(xxName);
-            if (val.size() > 0) // XXX: HttpHeader lacks empty value detection
-                ah->updateXxRecord(xxName, val);
-        }
-    }
-
-    // update the adaptation plan if needed (all status codes!)
-    if (service().cfg().routing) {
-        String services;
-        if (icapReply->header.getList(HDR_X_NEXT_SERVICES, &services)) {
-            Adaptation::History::Pointer ah = request->adaptHistory(true);
-            if (ah != NULL)
-                ah->updateNextServices(services);
-        }
-    } // TODO: else warn (occasionally!) if we got HDR_X_NEXT_SERVICES
-
-    // We need to store received ICAP headers for <icapLastHeader logformat option.
-    // If we already have stored headers from previous ICAP transaction related to this
-    // request, old headers will be replaced with the new one.
-
-    Adaptation::History::Pointer ah = request->adaptLogHistory();
-    if (ah != NULL)
-        ah->recordMeta(&icapReply->header);
+    // update history using meta headers regardless of the ICAP status code
+    Adaptation::History::RecordMeta(service().cfg(), *request, icapReply->header);
 
     // handle100Continue() manages state.writing on its own.
     // Non-100 status means the server needs no postPreview data from us.
     if (state.writing == State::writingPaused)
         stopWriting(true);
 }
 
 bool Adaptation::Icap::ModXact::validate200Ok()
 {
     if (ICAP::methodRespmod == service().cfg().method) {
         if (!gotEncapsulated("res-hdr"))
             return false;
 
         return true;
     }
 
     if (ICAP::methodReqmod == service().cfg().method) {
         if (!gotEncapsulated("res-hdr") && !gotEncapsulated("req-hdr"))
             return false;
 

=== modified file 'src/cf.data.pre'
--- src/cf.data.pre	2014-06-24 22:52:53 +0000
+++ src/cf.data.pre	2014-06-27 22:42:11 +0000
@@ -7979,40 +7979,58 @@ DOC_START
 		returned to the HTTP client.
 
 		Bypass is off by default: services are treated as essential.
 
 	routing=on|off|1|0
 		If set to 'on' or '1', the ICAP service is allowed to
 		dynamically change the current message adaptation plan by
 		returning a chain of services to be used next. The services
 		are specified using the X-Next-Services ICAP response header
 		value, formatted as a comma-separated list of service names.
 		Each named service should be configured in squid.conf. Other
 		services are ignored. An empty X-Next-Services value results
 		in an empty plan which ends the current adaptation.
 
 		Dynamic adaptation plan may cross or cover multiple supported
 		vectoring points in their natural processing order.
 
 		Routing is not allowed by default: the ICAP X-Next-Services
 		response header is ignored.
 
+	store-id=on|off|1|0
+		If set to 'on' or '1', the service transactions set StoreID
+		via an ICAP (not HTTP!) response header named "Store-ID".
+		This is currently supported for pre-cache REQMOD services
+		only. Responses with any status code, including ICAP 200 (Use
+		Adapted) and 204 (Use Virgin) may return this option, but keep
+		in mind that Squid currently does not cache request
+		satisfaction responses.
+
+		If a transaction does not return an ICAP Store-ID header or is
+		not allowed to return one by store-id=off, then Squid does not
+		update the current StoreID value. If a transaction returns an
+		allowed Store-ID header with an empty value, then Squid clears
+		the previously set StoreID, if any.  Note that all adaptation
+		services are executed before any store_id_program helpers.
+
+		Setting StoreIDs is not allowed by default.
+
 	ipv6=on|off
 		Only has effect on split-stack systems. The default on those systems
 		is to use IPv4-only connections. When set to 'on' this option will
 		make Squid use IPv6-only connections to contact this ICAP service.
 
 	on-overload=block|bypass|wait|force
 		If the service Max-Connections limit has been reached, do
 		one of the following for each new ICAP transaction:
 		  * block:  send an HTTP error response to the client
 		  * bypass: ignore the "over-connected" ICAP service
 		  * wait:   wait (in a FIFO queue) for an ICAP connection slot
 		  * force:  proceed, ignoring the Max-Connections limit 
 
 		In SMP mode with N workers, each worker assumes the service
 		connection limit is Max-Connections/N, even though not all
 		workers may use a given service.
 
 		The default value is "bypass" if service is bypassable,
 		otherwise it is set to "wait".
 		
@@ -8106,40 +8124,57 @@ DOC_START
 		If set to 'on' or '1', the eCAP service is treated as optional.
 		If the service cannot be reached or malfunctions, Squid will try
 		to ignore any errors and process the message as if the service
 		was not enabled. No all eCAP errors can be bypassed.
 		If set to 'off' or '0', the eCAP service is treated as essential
 		and all eCAP errors will result in an error page returned to the
 		HTTP client.
 
                 Bypass is off by default: services are treated as essential.
 
 	routing=on|off|1|0
 		If set to 'on' or '1', the eCAP service is allowed to
 		dynamically change the current message adaptation plan by
 		returning a chain of services to be used next.
 
 		Dynamic adaptation plan may cross or cover multiple supported
 		vectoring points in their natural processing order.
 
 		Routing is not allowed by default.
 
+	store-id=on|off|1|0
+		If set to 'on' or '1', the service transactions set StoreID
+		via an eCAP transaction option called "Store-ID".  This is
+		currently supported for pre-cache REQMOD services only. All
+		eCAP actions (i.e., useVirgin, useAdapted, and blockVirgin)
+		may return this option, but keep in mind that Squid currently
+		does not cache request satisfaction responses.
+
+		If a transaction does not return a Store-ID option or is not
+		allowed to return one by store-id=off, then Squid does not
+		update the current StoreID value.  If a transaction returns an
+		allowed Store-ID option with an empty value, Squid clears the
+		previously set StoreID, if any. Note that all adaptation
+		services are executed before any store_id_program helpers.
+
+		Setting StoreIDs is not allowed by default.
+
 	Older ecap_service format without optional named parameters is
 	deprecated but supported for backward compatibility.
 
 
 Example:
 ecap_service s1 reqmod_precache ecap://filters.R.us/leakDetector?on_error=block bypass=off
 ecap_service s2 respmod_precache ecap://filters.R.us/virusFilter config=/etc/vf.cfg bypass=on
 DOC_END
 
 NAME: loadable_modules
 TYPE: wordlist
 IFDEF: USE_LOADABLE_MODULES
 LOC: Config.loadable_module_names
 DEFAULT: none
 DOC_START
 	Instructs Squid to load the specified dynamic module(s) or activate
 	preloaded module(s).
 Example:
 loadable_modules @DEFAULT_PREFIX@/lib/MinimalAdapter.so
 DOC_END

=== modified file 'src/client_side_request.cc'
--- src/client_side_request.cc	2014-06-24 22:52:53 +0000
+++ src/client_side_request.cc	2014-06-27 22:22:18 +0000
@@ -1369,52 +1369,44 @@ ClientRequestContext::clientStoreIdDone(
     case HelperReply::Unknown:
     case HelperReply::TT:
         // Handler in redirect.cc should have already mapped Unknown
         // IF it contained valid entry for the old helper protocol
         debugs(85, DBG_IMPORTANT, "ERROR: storeID helper returned invalid result code. Wrong helper? " << reply);
         break;
 
     case HelperReply::BrokenHelper:
         debugs(85, DBG_IMPORTANT, "ERROR: storeID helper: " << reply << ", attempt #" << (store_id_fail_count+1) << " of 2");
         if (store_id_fail_count < 2) { // XXX: make this configurable ?
             ++store_id_fail_count;
             // reset state flag to try StoreID again from scratch.
             store_id_done = false;
         }
         break;
 
     case HelperReply::Error:
         // no change to be done.
         break;
 
-    case HelperReply::Okay: {
-        const char *urlNote = reply.notes.findFirst("store-id");
-
-        // prevent broken helpers causing too much damage. If old URL == new URL skip the re-write.
-        if (urlNote != NULL && strcmp(urlNote, http->uri) ) {
-            // Debug section required for some very specific cases.
-            debugs(85, 9, "Setting storeID with: " << urlNote );
-            http->request->store_id = urlNote;
-            http->store_id = urlNote;
-        }
-    }
-    break;
+    case HelperReply::Okay:
+        if (const char *urlNote = reply.notes.findFirst("store-id"))
+            http->resetStoreId(urlNote);
+        break;
     }
 
     http->doCallouts();
 }
 
 /** Test cache allow/deny configuration
  *  Sets flags.cachable=1 if caching is not denied.
  */
 void
 ClientRequestContext::checkNoCache()
 {
     if (Config.accessList.noCache) {
         acl_checklist = clientAclChecklistCreate(Config.accessList.noCache, http);
         acl_checklist->nonBlockingCheck(checkNoCacheDoneWrapper, this);
     } else {
         /* unless otherwise specified, we try to cache. */
         checkNoCacheDone(ACCESS_ALLOWED);
     }
 }
 
@@ -1833,40 +1825,61 @@ ClientHttpRequest::doCallouts()
     calloutContext = NULL;
 #if HEADERS_LOG
 
     headersLog(0, 1, request->method, request);
 #endif
 
     debugs(83, 3, HERE << "calling processRequest()");
     processRequest();
 
 #if ICAP_CLIENT
     Adaptation::Icap::History::Pointer ih = request->icapHistory();
     if (ih != NULL)
         ih->logType = logType;
 #endif
 }
 
 #if !_USE_INLINE_
 #include "client_side_request.cci"
 #endif
 
+void
+ClientHttpRequest::resetStoreId(const String &newId)
+{
+    if (newId == store_id)
+        return; // no change: still the same StoreID
+
+    if (uri && newId == uri) { // a StoreID that is the same as URI
+        if (!store_id.size())
+            return; // no change: still no StoreID
+        debugs(85, 3, "forgetting storeID: " << store_id);
+        store_id.clean();
+    } else {
+        // either store_id or newId may be emtpy here and that is OK
+        debugs(85, 3, "resetting storeID from " << store_id << " to " << newId);
+        store_id = newId;
+    }
+
+    if (request)
+        request->store_id = store_id; // keep the two in-sync
+}
+
 #if USE_ADAPTATION
 /// Initiate an asynchronous adaptation transaction which will call us back.
 void
 ClientHttpRequest::startAdaptation(const Adaptation::ServiceGroupPointer &g)
 {
     debugs(85, 3, HERE << "adaptation needed for " << this);
     assert(!virginHeadSource);
     assert(!adaptedBodySource);
     virginHeadSource = initiateAdaptation(
                            new Adaptation::Iterator(request, NULL, al, g));
 
     // we could try to guess whether we can bypass this adaptation
     // initiation failure, but it should not really happen
     Must(initiated(virginHeadSource));
 }
 
 void
 ClientHttpRequest::noteAdaptationAnswer(const Adaptation::Answer &answer)
 {
     assert(cbdataReferenceValid(this));		// indicates bug
@@ -1884,72 +1897,78 @@ ClientHttpRequest::noteAdaptationAnswer(
 
     case Adaptation::Answer::akError:
         handleAdaptationFailure(ERR_DETAIL_CLT_REQMOD_ABORT, !answer.final);
         break;
     }
 }
 
 void
 ClientHttpRequest::handleAdaptedHeader(HttpMsg *msg)
 {
     assert(msg);
 
     if (HttpRequest *new_req = dynamic_cast<HttpRequest*>(msg)) {
         /*
          * Replace the old request with the new request.
          */
         HTTPMSGUNLOCK(request);
         request = new_req;
         HTTPMSGLOCK(request);
 
+        const Adaptation::History::Pointer ah = request->adaptHistory(false);
+        if (ah != NULL)
+            resetStoreId(ah->store_id);
+
         // update the new message to flag whether URL re-writing was done on it
         if (strcmp(urlCanonical(request),uri) != 0)
             request->flags.redirected = 1;
 
         /*
          * Store the new URI for logging
          */
         xfree(uri);
         uri = xstrdup(urlCanonical(request));
         setLogUri(this, urlCanonicalClean(request));
         assert(request->method.id());
     } else if (HttpReply *new_rep = dynamic_cast<HttpReply*>(msg)) {
         debugs(85,3,HERE << "REQMOD reply is HTTP reply");
 
         // subscribe to receive reply body
         if (new_rep->body_pipe != NULL) {
             adaptedBodySource = new_rep->body_pipe;
             int consumer_ok = adaptedBodySource->setConsumerIfNotLate(this);
             assert(consumer_ok);
         }
 
         clientStreamNode *node = (clientStreamNode *)client_stream.tail->prev->data;
         clientReplyContext *repContext = dynamic_cast<clientReplyContext *>(node->data.getRaw());
         assert(repContext);
         repContext->createStoreEntry(request->method, request->flags);
 
         EBIT_CLR(storeEntry()->flags, ENTRY_FWD_HDR_WAIT);
         request_satisfaction_mode = true;
         request_satisfaction_offset = 0;
         storeEntry()->replaceHttpReply(new_rep);
         storeEntry()->timestampsSet();
 
+        // If we start caching satisfaction responses, then resetStoreId().
+
         if (!adaptedBodySource) // no body
             storeEntry()->complete();
         clientGetMoreData(node, this);
     }
 
     // we are done with getting headers (but may be receiving body)
     clearAdaptation(virginHeadSource);
 
     if (!request_satisfaction_mode)
         doCallouts();
 }
 
 void
 ClientHttpRequest::handleAdaptationBlock(const Adaptation::Answer &answer)
 {
     request->detailError(ERR_ACCESS_DENIED, ERR_DETAIL_REQMOD_BLOCK);
     AclMatchedName = answer.ruleId.termedBuf();
     assert(calloutContext);
     calloutContext->clientAccessCheckDone(ACCESS_DENIED);
     AclMatchedName = NULL;

=== modified file 'src/client_side_request.h'
--- src/client_side_request.h	2014-06-05 14:57:58 +0000
+++ src/client_side_request.h	2014-06-27 18:30:18 +0000
@@ -65,40 +65,43 @@ public:
     ClientHttpRequest& operator=(ClientHttpRequest const &);
 
     String rangeBoundaryStr() const;
     void freeResources();
     void updateCounters();
     void logRequest();
     _SQUID_INLINE_ MemObject * memObject() const;
     bool multipartRangeRequest() const;
     void processRequest();
     void httpStart();
     bool onlyIfCached()const;
     bool gotEnough() const;
     _SQUID_INLINE_ StoreEntry *storeEntry() const;
     void storeEntry(StoreEntry *);
     _SQUID_INLINE_ StoreEntry *loggingEntry() const;
     void loggingEntry(StoreEntry *);
 
     _SQUID_INLINE_ ConnStateData * getConn() const;
     _SQUID_INLINE_ void setConn(ConnStateData *);
 
+    /// change store ID if needed after a helper or adaptation service response
+    void resetStoreId(const String &id);
+
     /** Details of the client socket which produced us.
      * Treat as read-only for the lifetime of this HTTP request.
      */
     Comm::ConnectionPointer clientConnection;
 
     HttpRequest *request;		/* Parsed URL ... */
     char *uri;
     char *log_uri;
     String store_id; /* StoreID for transactions where the request member is nil */
 
     struct {
         int64_t offset;
         int64_t size;
         size_t headers_sz;
     } out;
 
     HttpHdrRangeIter range_iter;	/* data for iterating thru range specs */
     size_t req_sz;		/* raw request size on input, not current request size */
 
     /// the processing tags associated with this request transaction.

Reply via email to