ICP: Fix HttpRequest lifetime for ICP v3 queries

src/ICP.h

@@ -94,10 +94,7 @@ extern Ip::Address theIcpPublicHostID;
 const char *icpGetUrl(const Ip::Address &from, const char *, const icp_common_t &);
 
 /// \ingroup ServerProtocolICPAPI
-HttpRequest *icpGetRequest(const char *url, int reqnum, int fd, const Ip::Address &from);
-
-/// \ingroup ServerProtocolICPAPI
-bool icpAccessAllowed(Ip::Address &from, HttpRequest * icp_request);
+HttpRequestPointer icpGetRequest(const char *url, int reqnum, int fd, const Ip::Address &from);
 
 /// \ingroup ServerProtocolICPAPI
 void icpCreateAndSend(icp_opcode, int flags, char const *url, int reqnum, int pad, int fd, const Ip::Address &from, AccessLogEntryPointer);

src/icp_v2.cc

@@ -438,8 +438,9 @@ icpDenyAccess(const Ip::Address &from, const char * const url, const int reqnum,
     }
 }
 
-bool
-icpAccessAllowed(Ip::Address &from, HttpRequest * icp_request)
+/// icpGetRequest() helper that determines whether squid.conf allows the given ICP query
+static bool
+icpAccessAllowed(const Ip::Address &from, HttpRequest * icp_request)
 {
     if (!Config.accessList.icp) {
         debugs(12, 2, "Access Denied due to lack of ICP access rules.");
@@ -490,7 +491,7 @@ icpGetUrl(const Ip::Address &from, const char * const buf, const icp_common_t &h
     return url;
 }
 
-HttpRequest *
+HttpRequest::Pointer
 icpGetRequest(const char * const url, const int reqnum, const int fd, const Ip::Address &from)
 {
     if (strpbrk(url, w_space)) {
@@ -499,12 +500,17 @@ icpGetRequest(const char * const url, const int reqnum, const int fd, const Ip::
     }
 
     const auto mx = MasterXaction::MakePortless<XactionInitiator::initIcp>();
-    auto *result = HttpRequest::FromUrlXXX(url, mx);
-    if (!result)
-        icpCreateAndSend(ICP_ERR, 0, url, reqnum, 0, fd, from, nullptr);
+    if (const HttpRequest::Pointer request = HttpRequest::FromUrlXXX(url, mx)) {
+        if (!icpAccessAllowed(from, request.getRaw())) {
+            icpDenyAccess(from, url, reqnum, fd);
+            return nullptr;
+        }
 
-    return result;
+        return request;
+    }
 
+    icpCreateAndSend(ICP_ERR, 0, url, reqnum, 0, fd, from, nullptr);
+    return nullptr;
 }
 
 static void
@@ -520,18 +526,11 @@ doV2Query(const int fd, Ip::Address &from, const char * const buf, icp_common_t
         return;
     }
 
-    HttpRequest *icp_request = icpGetRequest(url, header.reqnum, fd, from);
+    const auto icp_request = icpGetRequest(url, header.reqnum, fd, from);
 
     if (!icp_request)
         return;
 
-    HTTPMSGLOCK(icp_request);
-
-    if (!icpAccessAllowed(from, icp_request)) {
-        icpDenyAccess(from, url, header.reqnum, fd);
-        HTTPMSGUNLOCK(icp_request);
-        return;
-    }
 #if USE_ICMP
     if (header.flags & ICP_FLAG_SRC_RTT) {
         rtt = netdbHostRtt(icp_request->url.host());
@@ -544,7 +543,7 @@ doV2Query(const int fd, Ip::Address &from, const char * const buf, icp_common_t
 #endif /* USE_ICMP */
 
     /* The peer is allowed to use this cache */
-    ICP2State state(header, icp_request);
+    ICP2State state(header, icp_request.getRaw());
     state.fd = fd;
     state.from = from;
     state.url = xstrdup(url);
@@ -573,8 +572,6 @@ doV2Query(const int fd, Ip::Address &from, const char * const buf, icp_common_t
     }
 
     icpCreateAndSend(codeToSend, flags, url, header.reqnum, src_rtt, fd, from, state.al);
-
-    HTTPMSGUNLOCK(icp_request);
 }
 
 void

src/icp_v3.cc

@@ -40,19 +40,13 @@ doV3Query(int fd, Ip::Address &from, const char * const buf, icp_common_t header
         return;
     }
 
-    HttpRequest *icp_request = icpGetRequest(url, header.reqnum, fd, from);
+    const auto icp_request = icpGetRequest(url, header.reqnum, fd, from);
 
     if (!icp_request)
         return;
 
-    if (!icpAccessAllowed(from, icp_request)) {
-        icpDenyAccess (from, url, header.reqnum, fd);
-        delete icp_request;
-        return;
-    }
-
     /* The peer is allowed to use this cache */
-    ICP3State state(header, icp_request);
+    ICP3State state(header, icp_request.getRaw());
     state.fd = fd;
     state.from = from;
     state.url = xstrdup(url);

src/tests/stub_icp.cc

@@ -9,6 +9,7 @@
 #include "squid.h"
 #include "AccessLogEntry.h"
 #include "comm/Connection.h"
+#include "HttpRequest.h"
 #include "ICP.h"
 
 #define STUB_API "icp_*.cc"
@@ -30,8 +31,7 @@ Comm::ConnectionPointer icpOutgoingConn;
 Ip::Address theIcpPublicHostID;
 
 const char *icpGetUrl(const Ip::Address &, const char *, const icp_common_t &) STUB_RETVAL(nullptr)
-HttpRequest* icpGetRequest(const char *, int, int, const Ip::Address &) STUB_RETVAL(nullptr)
-bool icpAccessAllowed(Ip::Address &, HttpRequest *) STUB_RETVAL(false)
+HttpRequest::Pointer icpGetRequest(const char *, int, int, const Ip::Address &) STUB_RETVAL(nullptr)
 void icpCreateAndSend(icp_opcode, int, char const *, int, int, int, const Ip::Address &, AccessLogEntryPointer) STUB
 icp_opcode icpGetCommonOpcode() STUB_RETVAL(ICP_INVALID)
 void icpDenyAccess(const Ip::Address &, const char *, int, int) STUB