From f950ad113cb28924f66b88ba87728883306f963a Mon Sep 17 00:00:00 2001 From: Eduard Bagdasaryan Date: Wed, 30 May 2018 11:58:49 +0000 Subject: [PATCH] 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. --- src/neighbors.cc | 4 ++-- src/store/Controller.cc | 6 ++++-- src/store/Controller.h | 6 +++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/neighbors.cc b/src/neighbors.cc index 3d48543849..b943a24d20 100644 --- a/src/neighbors.cc +++ b/src/neighbors.cc @@ -984,7 +984,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))) @@ -1692,7 +1692,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 bebc65e05c..be514e0cac 100644 --- 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(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 3a5290059c..c715d91783 100644 --- 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. -- 2.47.2