On 21/02/11 18:38, Alex Rousskov wrote:
Hello,
The eCAP project recently released two new versions of the eCAP
library, with several important features added. The attached patch adds
Squid support for the latest libecap v0.2.0 and fixes a few bugs we
found along the way. I will summarize the changes below. My
lp:~rousskov/squid/3p2-ecap branch contains detailed messages for these
changes.
The new eCAP features revolve around adapter configuration (similar to
ICAP OPTIONS exchange) and transaction meta-information (similar to ICAP
message headers, not to be confused with ICAP-encapsulated HTTP
headers). There is also request satisfaction and message blocking
support. These features are needed for many adaptation projects. For
example, with these additions, we were able to write a ClamAV virus
scanning adapter (to be published after this work is completed).
Summary of changes:
libecap v0.2.0 support: accept/update/log eCAP transaction meta-info.
libecap v0.2.0 support: supply client IP and username to eCAP adapter.
libecap v0.1.0 support: Support blockVirgin() API with ERR_ACCESS_DENIED
Use pkg-config's PKG_CHECK_MODULES to check for and link with libecap.
Support adapter-specific parameters as a part of ecap_service
configuration. Allow uri=value parameter when specifying adaptation
service URIs.
Fixed virgin body handling in our eCAP transaction wrapper
(Ecap::XactionRep). Fixed BodyPipe.cc:144 "!theConsumer" assertion.
Log "important" messages from eCAP adapters with DBG_IMPORTANT not DBG_DATA!
Added XXXs to identify old unrelated problems to be fixed separately.
Thank you,
Alex.
Okay the polish audit results...
configure.ac:
* can you explain a bit more about what the pkg-check problem is with
library names and libtool?
src/AccessLogEntry.h:
* please note the TODO on the "headers" member. New uses of that area
is not desirable. The ideal structure for AccessLogEntry has separate
sub-classes to match http::, icap::, adapt::, ecap:: namespaces in the
logformat tokens.
Please add an "adapt" sub-class to match the namespace for
adapt::<last_h.
src/Server.cc:
* HERE << "handleAdaptationBlocked: " is redundant. HERE contains the
function name.
src/adaptation/Config.h:
* on createService() you are not using const reference for the
parameter. Same for ParseServiceGroup(). Any particular reason?
src/adaptation/Initiator.cc:
* please enact that TODO: Move to src/adaptation/Answer.*
src/adaptation/Makefile.am:
* Why is an automake macro called *_LIBS being added to LDFLAGS?
I can't see where it is being set to know if its wrongly named or
wrongly copied.
- the comment in src/adaptation/ecap/Makefile.am indicates its
wrongly used. Since adding *_LIBS and *_LDADD auto-magically updates
dependencies. Adding *FLAGS does not.
src/adaptation/ecap/Makefile.am:
* please use CXXFLAGS instead of CPPFLAGS like we do everywhere else
src/adaptation/Service.h,cc:
* please use const ServiceConfigPointer& for passing to constructor.
(less refcounting overheads as you keep pointing out to me).
- same in other places too.
src/adaptation/ServiceConfig.cc:
* in Adaptation::ServiceConfig::grokExtension() please use
DBG_CRITICAL for 0-level debugs instead of "0".
- it also needs a suitable WARNING/ERROR/FATAL etc. to highlight the
effect of leaving it.
src/adaptation/ecap/MessageRep.cc:
* the includes seem to be violating our policy on "" before <>. Not a
major for this patch, but it may be hiding bugs and needs fixing. Same
for a lot of these adaptation files.
src/adaptation/ecap/MessageRep.h:
* zero documentation on visitEach() - what does it actually do?
pass the headers freshly received from ecap back to ecap? part of
the chaining? something else?
src/adaptation/ecap/ServiceRep.cc:
* I think the starting eCAP service: message should be at level 2. Out
of regular sight but easy to get to. If its just a startup/restart then
it might even go at level-1 with the other module starting/stopping
messages.
src/cf.data.pre:
* while you are updating the documentation for
icap_client_username_header please correct the typo which mentions a
non-existent "send_username" to be "adaptation_send_username"
* in the documentation for adaptation_masterx_shared_names please
split the ICAP and eCAP methods of setting into two paragraphs.
src/cf_gen_defines:
* missing entry for FOLLOW_X_FORWARDED_FOR&&USE_ADAPTATION to generate
readable documentation on adaptation_uses_indirect_client
src/log/FormatSquidCustom.cc:
* in LFT_ADAPTATION_LAST_HEADER you haev an XXX. Please resolve it for
the new cases. No need to perpetuate the error.
Amos
--
Please be using
Current Stable Squid 2.7.STABLE9 or 3.1.11
Beta testers wanted for 3.2.0.5
