Bug 4855: re-enable querying private entries for HTCP/ICP (#214)

This was broken since 4310f8b: HTCP/ICP misused Store as a storage of
private entries during queries (e.g., see
neighborsUdpPing()/neighborsUdpAck()). A smarter HTCP/ICP
implementation would maintain its own StoreEntry cache for this purpose
(just like the existing queried_keys array for cache keys). However,
fixing this is beyond this issue scope.

diff --git a/src/neighbors.cc b/src/neighbors.cc
index 07299129b44a41a067812add2ab5ceda6ea7af1b..f76bf617b6cfb523f0a07bd2594a72fa6bdc78b3 100644 (file)
--- a/src/neighbors.cc
+++ b/src/neighbors.cc
@@ -1005,7 +1005,7 @@ neighborsUdpAck(const cache_key * key, icp_common_t * header, const Ip::Address
 
     debugs(15, 6, "neighborsUdpAck: opcode " << opcode << " '" << storeKeyText(key) << "'");
 
-    if ((entry = Store::Root().findCallback(key)))
+    if ((entry = Store::Root().findCallbackXXX(key)))
         mem = entry->mem_obj;
 
     if ((p = whichPeer(from)))
@@ -1721,7 +1721,7 @@ dump_peers(StoreEntry * sentry, CachePeer * peers)
 void
 neighborsHtcpReply(const cache_key * key, HtcpReplyData * htcp, const Ip::Address &from)
 {
-    StoreEntry *e = Store::Root().findCallback(key);
+    StoreEntry *e = Store::Root().findCallbackXXX(key);
     MemObject *mem = NULL;
     CachePeer *p;
     peer_t ntype = PEER_NONE;

diff --git a/src/store/Controller.cc b/src/store/Controller.cc
index bebc65e05cec987dcf70a29b370bfbf5044fef56..be514e0caca248363c27a24a79fe4e3bb36db24a 100644 (file)
--- a/src/store/Controller.cc
+++ b/src/store/Controller.cc
@@ -349,14 +349,16 @@ Store::Controller::allowSharing(StoreEntry &entry, const cache_key *key)
 }
 
 StoreEntry *
-Store::Controller::findCallback(const cache_key *key)
+Store::Controller::findCallbackXXX(const cache_key *key)
 {
     // We could check for mem_obj presence (and more), moving and merging some
     // of the duplicated neighborsUdpAck() and neighborsHtcpReply() code here,
     // but that would mean polluting Store with HTCP/ICP code. Instead, we
     // should encapsulate callback-related data in a protocol-neutral MemObject
     // member or use an HTCP/ICP-specific index rather than store_table.
-    return peekAtLocal(key);
+
+    // cannot reuse peekAtLocal() because HTCP/ICP callbacks may use private keys
+    return static_cast<StoreEntry*>(hash_lookup(store_table, key));
 }
 
 /// \returns either an existing local reusable StoreEntry object or nil

diff --git a/src/store/Controller.h b/src/store/Controller.h
index 3a5290059c4daf3b28de00b06fa84330c0843231..c715d91783b3f51a4c9b47f2f3be696b4d48c540 100644 (file)
--- a/src/store/Controller.h
+++ b/src/store/Controller.h
@@ -56,9 +56,9 @@ public:
 
     /// \returns matching StoreEntry associated with local ICP/HTCP transaction
     /// Warning: The returned StoreEntry is not synced and may be marked for
-    /// deletion. Use it only for extracting transaction callback details.
-    /// TODO: Group and return just that callback-related data instead?
-    StoreEntry *findCallback(const cache_key *);
+    /// deletion. It can only be used for extracting transaction callback details.
+    /// New code should be designed to avoid this deprecated API.
+    StoreEntry *findCallbackXXX(const cache_key *);
 
     /// Whether a transient entry with the given public key exists and (but) was
     /// marked for removal some time ago; get(key) returns nil in such cases.