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
