Author: Christos Tsantilas <chtsanti@users.sourceforge.net>

Bug #1971 fix, part 1: Check whether the store entry got aborted while
we were waiting for an asynchronous ICAP ACL check.

Also moved common icapAclCheckDone() code from FtpStateData and
HttpStateData into Server. Removed "if (eof) serverComplete()" checks from
HttpStateData::processReplyHeader and HttpStateData::failReply methods as
eof can no longer be true there.

diff --git a/src/Server.cc b/src/Server.cc
index cbd3183732c41d1c07b51de272d1a14084adcf6e..d8c59130e5c370827818b7b1d1acd31b1161936e 100644 (file)
--- a/src/Server.cc
+++ b/src/Server.cc
@@ -1,5 +1,5 @@
 /*
- * $Id: Server.cc,v 1.12 2007/06/19 20:58:26 rousskov Exp $
+ * $Id: Server.cc,v 1.13 2007/06/25 22:34:24 rousskov Exp $
  *
  * DEBUG:
  * AUTHOR: Duane Wessels
@@ -43,7 +43,8 @@
 #include "ICAP/ICAPModXact.h"
 #endif
 
-ServerStateData::ServerStateData(FwdState *theFwdState): requestSender(NULL)
+ServerStateData::ServerStateData(FwdState *theFwdState): requestSender(NULL),
+    icapAccessCheckPending(false)
 {
     fwd = theFwdState;
     entry = fwd->entry;
@@ -481,4 +482,44 @@ ServerStateData::handleIcapAborted(bool bypassable)
     abortTransaction("ICAP failure");
 }
 
+HttpRequest *
+ServerStateData::originalRequest()
+{
+    return request;
+}
+
+void
+ServerStateData::icapAclCheckDone(ICAPServiceRep::Pointer service)
+{
+    icapAccessCheckPending = false;
+
+    if (abortOnBadEntry("entry went bad while waiting for ICAP ACL check"))
+        return;
+
+    const bool startedIcap = startIcap(service, originalRequest());
+
+    if (!startedIcap && (!service || service->bypass)) {
+        // handle ICAP start failure when no service was selected
+        // or where the selected service was optional
+        entry->replaceHttpReply(reply);
+
+        haveParsedReplyHeaders();
+        processReplyBody();
+
+        return;
+    }
+
+    if (!startedIcap) {
+        // handle start failure for an essential ICAP service
+        ErrorState *err = errorCon(ERR_ICAP_FAILURE,
+            HTTP_INTERNAL_SERVER_ERROR, originalRequest());
+        err->xerrno = errno;
+        errorAppendEntry(entry, err);
+        abortTransaction("ICAP start failure");
+        return;
+    }
+
+    processReplyBody();
+}
+
 #endif

diff --git a/src/Server.h b/src/Server.h
index 34ce848521765ea0d2b9fa263b9ad4b744ebae71..1b3f26bf72c0060a141a2cfe065970c831704416 100644 (file)
--- a/src/Server.h
+++ b/src/Server.h
@@ -1,6 +1,6 @@
 
 /*
- * $Id: Server.h,v 1.4 2007/05/08 16:46:37 rousskov Exp $
+ * $Id: Server.h,v 1.5 2007/06/25 22:34:24 rousskov Exp $
  *
  * AUTHOR: Duane Wessels
  *
@@ -86,7 +86,9 @@ public:
     virtual void abortTransaction(const char *reason) = 0;
 
 #if ICAP_CLIENT
-    virtual void icapAclCheckDone(ICAPServiceRep::Pointer) = 0;
+    void icapAclCheckDone(ICAPServiceRep::Pointer);
+    // a hack to reach HttpStateData::orignal_request
+    virtual  HttpRequest *originalRequest();
 
     // ICAPInitiator: start an ICAP transaction and receive adapted headers.
     virtual void noteIcapAnswer(HttpMsg *message);
@@ -96,6 +98,7 @@ public:
     virtual void noteMoreBodySpaceAvailable(BodyPipe &);
     virtual void noteBodyConsumerAborted(BodyPipe &);
 #endif
+    virtual void processReplyBody() = 0;
 
 public: // should be protected
     void serverComplete(); // call when no server communication is expected

diff --git a/src/ftp.cc b/src/ftp.cc
index 922a49d11630e93f3ca5f5706032d0edd07082dd..47fc452e20dce9fdc5ef71213aba6b89a64d613a 100644 (file)
--- a/src/ftp.cc
+++ b/src/ftp.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: ftp.cc,v 1.425 2007/06/19 20:27:00 rousskov Exp $
+ * $Id: ftp.cc,v 1.426 2007/06/25 22:34:24 rousskov Exp $
  *
  * DEBUG: section 9     File Transfer Protocol (FTP)
  * AUTHOR: Harvest Derived
@@ -228,14 +228,6 @@ public:
 private:
     // BodyConsumer for HTTP: consume request body.
     virtual void handleRequestBodyProducerAborted();
-
-#if ICAP_CLIENT
-public:
-    void icapAclCheckDone(ICAPServiceRep::Pointer);
-
-    bool icapAccessCheckPending;
-#endif
-
 };
 
 CBDATA_CLASS_INIT(FtpStateData);
@@ -3396,32 +3388,4 @@ icapAclCheckDoneWrapper(ICAPServiceRep::Pointer service, void *data)
     ftpState->icapAclCheckDone(service);
 }
 
-// TODO: merge with http.cc and move to Server.cc?
-void
-FtpStateData::icapAclCheckDone(ICAPServiceRep::Pointer service)
-{
-    icapAccessCheckPending = false;
-
-    const bool startedIcap = startIcap(service, request);
-
-    if (!startedIcap && (!service || service->bypass)) {
-        // handle ICAP start failure when no service was selected
-        // or where the selected service was optional
-        entry->replaceHttpReply(reply);
-        processReplyBody();
-        return;
-    }
-
-    if (!startedIcap) {
-        // handle start failure for an essential ICAP service
-        ErrorState *err = errorCon(ERR_ICAP_FAILURE, HTTP_INTERNAL_SERVER_ERROR, request);
-        err->xerrno = errno;
-        errorAppendEntry(entry, err);
-        comm_close(ctrl.fd);
-        return;
-    }
-
-    processReplyBody();
-}
-
 #endif

diff --git a/src/http.cc b/src/http.cc
index 899234a11ea7824fe24c315de11d8bae6e7a2a22..b6d6737cbf69ee5d340ce39dc58b60a3f95e4b85 100644 (file)
--- a/src/http.cc
+++ b/src/http.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: http.cc,v 1.526 2007/06/19 21:19:04 rousskov Exp $
+ * $Id: http.cc,v 1.527 2007/06/25 22:34:24 rousskov Exp $
  *
  * DEBUG: section 11    Hypertext Transfer Protocol (HTTP)
  * AUTHOR: Harvest Derived
@@ -659,10 +659,6 @@ HttpStateData::failReply(HttpReply *reply, http_status const & status)
     reply->sline.version = HttpVersion(1, 0);
     reply->sline.status = status;
     entry->replaceHttpReply(reply);
-
-    if (eof == 1) {
-        serverComplete();
-    }
 }
 
 void
@@ -783,10 +779,6 @@ HttpStateData::processReplyHeader()
 
     haveParsedReplyHeaders();
 
-    if (eof == 1) {
-        serverComplete();
-    }
-
     ctx_exit(ctx);
 }
 
@@ -1960,37 +1952,11 @@ icapAclCheckDoneWrapper(ICAPServiceRep::Pointer service, void *data)
     http->icapAclCheckDone(service);
 }
 
-void
-HttpStateData::icapAclCheckDone(ICAPServiceRep::Pointer service)
+// TODO: why does FtpStateData not need orig_request?
+HttpRequest *
+HttpStateData::originalRequest()
 {
-    icapAccessCheckPending = false;
-
-    const bool startedIcap = startIcap(service, orig_request);
-
-    if (!startedIcap && (!service || service->bypass)) {
-        // handle ICAP start failure when no service was selected
-        // or where the selected service was optional
-        entry->replaceHttpReply(reply);
-
-        haveParsedReplyHeaders();
-        processReplyBody();
-
-        if (eof == 1)
-            serverComplete();
-
-        return;
-    }
-
-    if (!startedIcap) {
-        // handle start failure for an essential ICAP service
-        ErrorState *err = errorCon(ERR_ICAP_FAILURE, HTTP_INTERNAL_SERVER_ERROR, orig_request);
-        err->xerrno = errno;
-        errorAppendEntry(entry, err);
-        comm_close(fd);
-        return;
-    }
-
-    processReplyBody();
+    return orig_request;
 }
 
 #endif

diff --git a/src/http.h b/src/http.h
index 2c3bb478d76397deb61c2b55a7ffaeae4dd2022d..258dcfc3770161d7df8b352e7d56ffcf01dfe494 100644 (file)
--- a/src/http.h
+++ b/src/http.h
@@ -1,6 +1,6 @@
 
 /*
- * $Id: http.h,v 1.28 2007/04/20 07:29:47 wessels Exp $
+ * $Id: http.h,v 1.29 2007/06/25 22:34:24 rousskov Exp $
  *
  *
  * SQUID Web Proxy Cache          http://www.squid-cache.org/
@@ -84,17 +84,17 @@ public:
 
     void processSurrogateControl(HttpReply *);
 
-#if ICAP_CLIENT
-    void icapAclCheckDone(ICAPServiceRep::Pointer);
-    bool icapAccessCheckPending;
-#endif
-
     /*
      * getReply() public only because it is called from a static function
      * as httpState->getReply()
      */
     const HttpReply * getReply() const { assert(reply); return reply; }
 
+protected:
+#if ICAP_CLIENT
+    virtual HttpRequest *originalRequest();
+#endif
+
 private:
     enum ConnectionStatus {
         INCOMPLETE_MSG,