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.

diff --git a/src/urn.cc b/src/urn.cc
index 8f73ae1f870f08a163b589f9e116938743ca330f..efadc88151239793212aa5dc121135083ba2d83a 100644 (file)
--- a/src/urn.cc
+++ b/src/urn.cc
@@ -78,6 +78,15 @@ CBDATA_CLASS_INIT(UrnState);
 
 UrnState::~UrnState()
 {
+    if (urlres_e) {
+        if (sc)
+            storeUnregister(sc, urlres_e, this);
+        urlres_e->unlock("~UrnState+res");
+    }
+
+    if (entry)
+        entry->unlock("~UrnState+prime");
+
     safe_free(urlres);
 }
 
@@ -223,14 +232,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)
@@ -252,8 +253,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;
     }
 
@@ -262,15 +263,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,
@@ -284,7 +285,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;
     }
 
@@ -300,7 +301,7 @@ urnHandleReply(void *data, StoreIOBuffer result)
         err->url = xstrdup(e->url());
         errorAppendEntry(e, err);
         delete rep;
-        urnHandleReplyError(urnState, urlres_e);
+        delete urnState;
         return;
     }
 
@@ -316,7 +317,7 @@ urnHandleReply(void *data, StoreIOBuffer result)
         err = new ErrorState(ERR_URN_RESOLVE, Http::scNotFound, urnState->request.getRaw(), urnState->ale);
         err->url = xstrdup(e->url());
         errorAppendEntry(e, err);
-        urnHandleReplyError(urnState, urlres_e);
+        delete urnState;
         return;
     }
 
@@ -374,9 +375,8 @@ urnHandleReply(void *data, StoreIOBuffer result)
     }
 
     safe_free(urls);
-    storeUnregister(urnState->sc, urlres_e, urnState);
 
-    urnHandleReplyError(urnState, urlres_e);
+    delete urnState;
 }
 
 static url_entry *