]> git.ipfire.org Git - thirdparty/squid.git/commit - src/adaptation/icap/ModXact.cc
- Many ICAP fixes from Alex Rousskov accumulated on the
authorwessels <>
Wed, 1 Nov 2006 06:30:55 +0000 (06:30 +0000)
committerwessels <>
Wed, 1 Nov 2006 06:30:55 +0000 (06:30 +0000)
commitc99de60701b56be31c01be2045d204ed411e33ca
treeb738eb2dcb1806770873b2091ceefb33de5954dc
parentcda5938d385bb733a82e59ec280b7e0e46b9484a
- Many ICAP fixes from Alex Rousskov accumulated on the
  sourceforge squid3-icap branch since 2006/10, including:

        - Polished ICAP service selection code and implemented bypass of
          optional services. The code implements icap_class
          configuration directive which is currently used as a "set of
          interchangeable ICAP services". Squid2 and current squid.conf
          may imply otherwise.

        - Support Transfer-* ICAP OPTIONS response header. If Squid
          knows that a service does not want the URL, Squid will not use
          the service, even if it is an essential service with
          bypass=0. Note that we may make this decision before we know
          what the service wants. Eventually, ACLs should initiate and
          wait for the OPTIONS transaction for yet-unprobed services.

        - When ICAP transactions fail to connect to the service many
          times, the service is suspended until the next OPTIONS
          update. The limit is currently hard-coded to 10. Suspended
          service is a down service and will be skipped by the ACL
          service selection algorithm.

        - Rewrote the code updating ICAP service options. We no longer
          mark the service being updated as "down". Only presence of
          valid and fresh options is important. We also try to update
          the options before they expire to avoid any service downtime
          or use of stale options.

        - Report interesting changes in the ICAP service state, some
          with debugging level one to alert the cache administrator.

        - When cloning a request during an ICAP 204 "No Content" REQMOD
          response, preserve the client address so that the rest of the
          code has access to it. This change appears to fix Squid Bug
          #1712.

        - After ICAP 100 Continue, expect new ICAP headers instead of
          HTTP headers. Reset ICAP message object to be ready to parse
          ICAP headers again. (Tsantilas Christos
          <chtsanti@users.sourceforge.net>)

        - The ieof HTTP chunk-extension was written after chunk-data
          instead of being written after the chunk-size. (Tsantilas
          Christos <chtsanti@users.sourceforge.net>)

        - Merged common code from the ICAPClientReqmodPrecache and
          ICAPClientReqmodPrecache classes into the newly added
          ICAPClientVector class.  The specific vectors do not have a
          common owner (yet?) because ServerStateData and
          ClientHttpRequest do not have a common base class. Thus,
          ICAPClientVector has to rely on its kids to communicate with
          their owners. However, at least 50% of the logic was common
          and has been moved. Eventually, we may want to create a
          simple ICAPOwner API that ServerStateData and
          ClientHttpRequest can implement and ICAPClientVector can rely
          on. This will make the code simpler and more efficient.  The
          big merge was motivated by a couple of bugs that were found
          in one vector class but that did not exist or behaved
          differently in the other vector, mostly likely due to natural
          diversion of used-to-be identical code.

        - Rewrote communication between a server-side ICAPClient*mod*
          vector and its owner.  When a server-side ICAPClient*mod*
          vector was notifying its owner of more adapted data, the
          owner could delete the vector (by calling icap->ownerAbort)
          if the store entry was not willing to accept the data.  The
          same deletion could happen when a vector was notifying the
          owner of a successful termination. In all those cases, the
          vector did not expect to be deleted and could continue to do
          something, causing segmentation faults.  Now, when more data
          is available, the vector calls its owner and checks the
          return value of the call. If it is false, the vector knows it
          has been deleted and quits. When vector terminates, it calls
          its owner and trusts the owner to always delete the vector.
          The "check return value and quit" design is not perfect, but
          we are paying the price for isolating the vectors from their
          owners while using direct calls between them (instead of
          MsgPipe or a similar less efficient indirect approach we use
          elsewhere).

        - Renamed doIcap to startIcap and moved more common code there.
          Changed its return type to bool. We now handle three cases
          when ICAP ACLs call back:  1) No service was selected
          (because there was no applicable service or because all
          applicable services were broken and optional). We proceed as
          if ICAP was not configured.  2) The selected essential
          service is broken. This is a fatal transaction error and we
          return an "ICAP protocol error" HTTP error response. We could
          proceed with the ICAP stuff, but it saves a lot of cycles to
          abort early.  3) The selected service is not broken. We
          proceed with the ICAP stuff.  The old code did not detect
          case #2, even though there was code to handle that case (with
          dangerous XXX suggestions that are now gone).  The code
          should probably be polished further to move common ftp/http
          logic from icapAclCheckDone()s into ServerStateData.

        - Make sure there is an accept callback when we are accepting.
          If there is no callback and we accept, we will silently leak
          the accepted FD.  When we are running out of FDs, there is
          often no accept callback.  The old code, when running out of
          FDs, would create lots of "orphaned" or "forgotten" FDs that
          will eventually get into a CLOSED_WAIT state and remain there
          until Squid quits.  The new code does not call accept() if
          there is no accept callback and does not register the accept
          FD for reading if the AcceptLimiter is deferring, because
          when the AcceptLimiter kicks in, it will register the accept
          FD for reading. There are most likely other places/cases
          where accept FD should not be registered for reading.

        - When an exception is caught, mark the ICAP connection as
          non-reusable so that it is not recycled while a write is
          pending but simply closed instead. Our write callback will
          still be called, unfortunately, because there is no way to
          clear the callback without invalidating its data (i.e., the
          transaction pointer).  This change prevents pconn.cc:253:
          "!comm_has_incomplete_write(fd)" assertion from firing when
          things go wrong (e.g., the ICAP server cannot be contacted to
          retrieve OPTIONS).  Not all exceptions caught by the ICAP
          xaction should lead to the ICAP connection termination, but
          it is very difficult if not impossible to reliably detect
          exceptional conditions when it is safe to reuse the ICAP
          connection, and it is probably not worth it anyway.

        - Added Tsantilas Christos <chtsanti@users.sourceforge.net>
          to CONTRIBUTORS for fixing ICAP bugs.

        - Polished debugging.
35 files changed:
CONTRIBUTORS
src/HttpReply.cc
src/HttpRequest.cc
src/ICAP/ChunkedCodingParser.cc
src/ICAP/ICAPClientReqmodPrecache.cc
src/ICAP/ICAPClientReqmodPrecache.h
src/ICAP/ICAPClientRespmodPrecache.cc
src/ICAP/ICAPClientRespmodPrecache.h
src/ICAP/ICAPClientVector.cc [new file with mode: 0644]
src/ICAP/ICAPClientVector.h [new file with mode: 0644]
src/ICAP/ICAPConfig.cc
src/ICAP/ICAPConfig.h
src/ICAP/ICAPModXact.cc
src/ICAP/ICAPModXact.h
src/ICAP/ICAPOptXact.cc
src/ICAP/ICAPOptXact.h
src/ICAP/ICAPOptions.cc
src/ICAP/ICAPOptions.h
src/ICAP/ICAPServiceRep.cc
src/ICAP/ICAPServiceRep.h
src/ICAP/ICAPXaction.cc
src/ICAP/ICAPXaction.h
src/ICAP/MsgPipe.cc
src/ICAP/MsgPipeData.h
src/ICAP/TextException.h
src/Makefile.am
src/Server.cc
src/Server.h
src/client_side.cc
src/client_side_request.cc
src/comm.cc
src/comm.h
src/ftp.cc
src/http.cc
src/http.h