[PATCH] StoreID via eCAP and ICAP
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 +
+++ src/adaptation/History.cc 2014-06-27 22:49:42 +
@@ -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(theEntries.size()));
theEntries[hid].stop();
}
@@ -132,40 +135,72 @@ void Adaptation::History::upda
Re: [PATCH] StoreID via eCAP and ICAP
Hey Alex, I have seen the patch but it will take me time to read here and there. I think I can describe the header with the whole feature description. Where is this description should be filed at? I have written in the past a StoreID helper which first fetch the url and looks at the properties of it to verify that it's not a 302 and if so then use a StoreID that will not cause a 302 redirect endless looping. The helper could exploit any details about the request HEAD response. Thanks, Eliezer P.S. I know that my StoreID is maybe not coded to match very high standards but any bug that will come from it will reveal usage of a specific variable which was used couple times in a weird way and places. On 06/28/2014 02:54 AM, Alex Rousskov wrote: 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.
Re: [PATCH] StoreID via eCAP and ICAP
On 07/06/2014 03:19 PM, Eliezer Croitoru wrote: > I think I can describe the header with the whole feature description. > Where is this description should be filed at? As you know, the StoreID feature is described at http://wiki.squid-cache.org/Features/StoreID I do not know whether IANA is going to accept a reference to a subsection of that wiki page as the Store-ID header description, but one can try and find out. The registration procedure for provisional headers is described by RFC 3864. I do not know how well that documentation matches the reality, > P.S. I know that my StoreID is maybe not coded to match very high > standards but any bug that will come from it will reveal usage of a > specific variable which was used couple times in a weird way and places. Sorry, I do not understand why you are saying the above. AFAICT, the proposed patch does not really change how StoreID helper responses are processed. Thank you, Alex. > On 06/28/2014 02:54 AM, Alex Rousskov wrote: >> 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. >>
Re: [PATCH] StoreID via eCAP and ICAP
On 28/06/2014 11:54 a.m., Alex Rousskov wrote: > 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. > Audit: in src/adaptation/History.h: * store_id member is not following the camelCase member naming for this class. in src/cf.data.pre: * please omit the 0|1 values from the documentation. on|off is sufficient for new option parameters and allows non-numeric implementation in future. in src/client_side_request.cc: * s/emtpy/empty/ +1. The above can be done and the patch applied without another audit review IMO. > > 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. Use of X- prefix for experimental headers is officially frowned upon by IETF and IANA for the same reasons. I agree with not going there. > 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). I'm not worried about the naming. The semantics fit. If you want to use this it makes a lot of sense and saves all the work of documenting an identical header. Amos
Re: [PATCH] StoreID via eCAP and ICAP
Any examples of a header description would help to understand what is needed for IANA registration. Eliezer On 07/29/2014 11:59 AM, Amos Jeffries wrote: 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). I'm not worried about the naming. The semantics fit. If you want to use this it makes a lot of sense and saves all the work of documenting an identical header. Amos
Re: [PATCH] StoreID via eCAP and ICAP
On 30/07/2014 1:09 a.m., Eliezer Croitoru wrote: > Any examples of a header description would help to understand what is > needed for IANA registration. Any of the documents linked from the registry can be used as an example. It would be worth checking ICAP protocol spec for anything about new header requirements or recommendations. I know HTTP, mail and netnews have some messaging details that need to be condsidered and mentioned for new headers. But this is ICAP header so things are different. PS. though I am happy with Archived-At if you want to avoid work. Amos
