Fix URN response handling (#472)

urnHandleReply() may be called several times while copying the entry
from the store. Each time it must use the buffer length that is left
(from the previous call).

Also do not abandon a urn entry, still having clients attached.

Also allow urnHandleReply() to produce a reply if it receives a
zero-sized buffer. This may happen after the entry has been fully
stored.

CID 1453857: Error handling issues (UNCAUGHT_EXCEPT)

Due to various Store deficiencies, storeUnregister() might call swapout
code which Broadcast()s and throws Ipc::OneToOneUniQueue::ItemTooLarge.

diff --git a/src/urn.cc b/src/urn.cc
index e64da1ffba03a0cac7f33b534dba2498cf4488e3..a65ab94baee249d7ce7299e5d8df3cbdfa9a3dcd 100644 (file)
--- a/src/urn.cc
+++ b/src/urn.cc
@@ -9,6 +9,7 @@
 /* DEBUG: section 52    URN Parsing */
 
 #include "squid.h"
+#include "base/TextException.h"
 #include "cbdata.h"
 #include "errorpage.h"
 #include "FwdState.h"
@@ -69,7 +70,18 @@ CBDATA_CLASS_INIT(UrnState);
 
 UrnState::~UrnState()
 {
-    safe_free(urlres);
+    SWALLOW_EXCEPTIONS({
+        if (urlres_e) {
+            if (sc)
+                storeUnregister(sc, urlres_e, this);
+            urlres_e->unlock("~UrnState+res");
+        }
+
+        if (entry)
+            entry->unlock("~UrnState+prime");
+
+        safe_free(urlres);
+    });
 }
 
 static url_entry *
@@ -205,14 +217,6 @@ url_entry_sort(const void *A, const void *B)
         return u1->rtt - u2->rtt;
 }
 
-static void
-urnHandleReplyError(UrnState *urnState, StoreEntry *urlres_e)
-{
-    urlres_e->unlock("urnHandleReplyError+res");
-    urnState->entry->unlock("urnHandleReplyError+prime");
-    delete urnState;
-}
-
 /* TODO: use the clientStream support for this */
 static void
 urnHandleReply(void *data, StoreIOBuffer result)
@@ -235,8 +239,8 @@ urnHandleReply(void *data, StoreIOBuffer result)
 
     debugs(52, 3, "urnHandleReply: Called with size=" << result.length << ".");
 
-    if (EBIT_TEST(urlres_e->flags, ENTRY_ABORTED) || result.length == 0 || result.flags.error) {
-        urnHandleReplyError(urnState, urlres_e);
+    if (EBIT_TEST(urlres_e->flags, ENTRY_ABORTED) || result.flags.error) {
+        delete urnState;
         return;
     }
 
@@ -245,15 +249,15 @@ urnHandleReply(void *data, StoreIOBuffer result)
 
     /* Handle reqofs being bigger than normal */
     if (urnState->reqofs >= URN_REQBUF_SZ) {
-        urnHandleReplyError(urnState, urlres_e);
+        delete urnState;
         return;
     }
 
     /* If we haven't received the entire object (urn), copy more */
-    if (urlres_e->store_status == STORE_PENDING &&
-            urnState->reqofs < URN_REQBUF_SZ) {
+    if (urlres_e->store_status == STORE_PENDING) {
+        Must(result.length > 0); // zero length ought to imply STORE_OK
         tempBuffer.offset = urnState->reqofs;
-        tempBuffer.length = URN_REQBUF_SZ;
+        tempBuffer.length = URN_REQBUF_SZ - urnState->reqofs;
         tempBuffer.data = urnState->reqbuf + urnState->reqofs;
         storeClientCopy(urnState->sc, urlres_e,
                         tempBuffer,
@@ -267,7 +271,7 @@ urnHandleReply(void *data, StoreIOBuffer result)
 
     if (0 == k) {
         debugs(52, DBG_IMPORTANT, "urnHandleReply: didn't find end-of-headers for " << e->url()  );
-        urnHandleReplyError(urnState, urlres_e);
+        delete urnState;
         return;
     }
 
@@ -283,7 +287,7 @@ urnHandleReply(void *data, StoreIOBuffer result)
         err->url = xstrdup(e->url());
         errorAppendEntry(e, err);
         delete rep;
-        urnHandleReplyError(urnState, urlres_e);
+        delete urnState;
         return;
     }
 
@@ -299,7 +303,7 @@ urnHandleReply(void *data, StoreIOBuffer result)
         err = new ErrorState(ERR_URN_RESOLVE, Http::scNotFound, urnState->request.getRaw());
         err->url = xstrdup(e->url());
         errorAppendEntry(e, err);
-        urnHandleReplyError(urnState, urlres_e);
+        delete urnState;
         return;
     }
 
@@ -358,10 +362,7 @@ urnHandleReply(void *data, StoreIOBuffer result)
     }
 
     safe_free(urls);
-    /* mb was absorbed in httpBodySet call, so we must not clean it */
-    storeUnregister(urnState->sc, urlres_e, urnState);
-
-    urnHandleReplyError(urnState, urlres_e);
+    delete urnState;
 }
 
 static url_entry *