Remove HttpRequest::peer_host (#2032)

HttpRequest::peer_host was added in 2009 commit 9ca29d23 so that
httpFixupAuthentication() could pass copied raw CachePeer::host pointer
value to peer_proxy_negotiate_auth(). Unfortunately, raw peer_host
pointer (to CachePeer::host memory) becomes dangling when CachePeer is
reconfigured away. Instead of maintaining this problematic field, we can
safely obtain the same CachePeer::host value from HttpStateData::_peer.

diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc
index 9700e7ec1baf8964c8622a3f416f7700221f1242..eb715f29cba72445e63ab98bbd0722e473070956 100644 (file)
--- a/src/HttpRequest.cc
+++ b/src/HttpRequest.cc
@@ -93,7 +93,6 @@ HttpRequest::init()
     error.clear();
     peer_login = nullptr;      // not allocated/deallocated by this class
     peer_domain = nullptr;     // not allocated/deallocated by this class
-    peer_host = nullptr;
     vary_headers = SBuf();
     myportname = null_string;
     tag = null_string;

diff --git a/src/HttpRequest.h b/src/HttpRequest.h
index df554d77b4115427916fc532d887a99cf36b1dbb..e026fcba24b26482b7278d99305424e49155da98 100644 (file)
--- a/src/HttpRequest.h
+++ b/src/HttpRequest.h
@@ -162,8 +162,6 @@ public:
 
     char *peer_login;       /* Configured peer login:password */
 
-    char *peer_host;           /* Selected peer host*/
-
     time_t lastmod;     /* Used on refreshes */
 
     /// The variant second-stage cache key. Generated from Vary header pattern for this request.

diff --git a/src/clients/HttpTunneler.cc b/src/clients/HttpTunneler.cc
index 48b3206863f9d8064ae239f312bbd06537940fac..8ecf537f79687b568e6d0e164eaeaffbc71ffe8e 100644 (file)
--- a/src/clients/HttpTunneler.cc
+++ b/src/clients/HttpTunneler.cc
@@ -150,6 +150,7 @@ Http::Tunneler::writeRequest()
                                               nullptr, // StoreEntry
                                               al,
                                               &hdr_out,
+                                              connection->getPeer(),
                                               flags);
         hdr_out.packInto(&mb);
         hdr_out.clean();

diff --git a/src/htcp.cc b/src/htcp.cc
index a87af95d3fdabd6f8e46743ddb408ae8efda1459..fe9ba50a0bdd003e670768436e4b36cb6c0e99c9 100644 (file)
--- a/src/htcp.cc
+++ b/src/htcp.cc
@@ -1533,7 +1533,7 @@ htcpQuery(StoreEntry * e, HttpRequest * req, CachePeer * p)
     stuff.S.method = sb.c_str();
     stuff.S.uri = (char *) e->url();
     stuff.S.version = vbuf;
-    HttpStateData::httpBuildRequestHeader(req, e, nullptr, &hdr, flags);
+    HttpStateData::httpBuildRequestHeader(req, e, nullptr, &hdr, p, flags);
     MemBuf mb;
     mb.init();
     hdr.packInto(&mb);
@@ -1588,7 +1588,7 @@ htcpClear(StoreEntry * e, HttpRequest * req, const HttpRequestMethod &, CachePee
     stuff.S.uri = uri.c_str();
     stuff.S.version = vbuf;
     if (reason != HTCP_CLR_INVALIDATION) {
-        HttpStateData::httpBuildRequestHeader(req, e, nullptr, &hdr, flags);
+        HttpStateData::httpBuildRequestHeader(req, e, nullptr, &hdr, p, flags);
         mb.init();
         hdr.packInto(&mb);
         hdr.clean();

diff --git a/src/http.cc b/src/http.cc
index 55b56e3474e4e501fb316a8667810c0b3ae7a404..7fa9c6773e41d74904e49dbed1823d3c6b1a14c2 100644 (file)
--- a/src/http.cc
+++ b/src/http.cc
@@ -1790,12 +1790,16 @@ HttpStateData::doneWithServer() const
  * Fixup authentication request headers for special cases
  */
 static void
-httpFixupAuthentication(HttpRequest * request, const HttpHeader * hdr_in, HttpHeader * hdr_out, const Http::StateFlags &flags)
+httpFixupAuthentication(HttpRequest * request, const HttpHeader * hdr_in, HttpHeader * hdr_out, const CachePeer * const peer, const Http::StateFlags &flags)
 {
     /* Nothing to do unless we are forwarding to a peer */
     if (!flags.peering)
         return;
 
+    // do nothing if our cache_peer was reconfigured away
+    if (!peer)
+        return;
+
     // This request is going "through" rather than "to" our _peer.
     if (flags.tunneling)
         return;
@@ -1881,7 +1885,7 @@ httpFixupAuthentication(HttpRequest * request, const HttpHeader * hdr_in, HttpHe
         if (request->flags.auth_no_keytab) {
             negotiate_flags |= PEER_PROXY_NEGOTIATE_NOKEYTAB;
         }
-        Token = peer_proxy_negotiate_auth(PrincipalName, request->peer_host, negotiate_flags);
+        Token = peer_proxy_negotiate_auth(PrincipalName, peer->host, negotiate_flags);
         if (Token) {
             httpHeaderPutStrf(hdr_out, header, "Negotiate %s",Token);
         }
@@ -1905,6 +1909,7 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request,
                                       StoreEntry * entry,
                                       const AccessLogEntryPointer &al,
                                       HttpHeader * hdr_out,
+                                      const CachePeer * const peer,
                                       const Http::StateFlags &flags)
 {
     /* building buffer for complex strings */
@@ -2023,7 +2028,7 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request,
     }
 
     /* Fixup (Proxy-)Authorization special cases. Plain relaying dealt with above */
-    httpFixupAuthentication(request, hdr_in, hdr_out, flags);
+    httpFixupAuthentication(request, hdr_in, hdr_out, peer, flags);
 
     /* append Cache-Control, add max-age if not there already */
     {
@@ -2377,7 +2382,8 @@ HttpStateData::buildRequestPrefix(MemBuf * mb)
     {
         HttpHeader hdr(hoRequest);
         forwardUpgrade(hdr); // before httpBuildRequestHeader() for CONNECTION
-        httpBuildRequestHeader(request.getRaw(), entry, fwd->al, &hdr, flags);
+        const auto peer = cbdataReferenceValid(_peer) ? _peer : nullptr;
+        httpBuildRequestHeader(request.getRaw(), entry, fwd->al, &hdr, peer, flags);
 
         if (request->flags.pinned && request->flags.connectionAuth)
             request->flags.authSent = true;
@@ -2476,7 +2482,6 @@ HttpStateData::sendRequest()
     }
 
     mb.init();
-    request->peer_host=_peer?_peer->host:nullptr;
     buildRequestPrefix(&mb);
 
     debugs(11, 2, "HTTP Server " << serverConnection);

diff --git a/src/http.h b/src/http.h
index 71b1c15977877e207a48198decdbd5c22b5ccef2..b14f66805aec774179fa03f460b7371e47400900 100644 (file)
--- a/src/http.h
+++ b/src/http.h
@@ -50,6 +50,7 @@ public:
                                        StoreEntry * entry,
                                        const AccessLogEntryPointer &al,
                                        HttpHeader * hdr_out,
+                                       const CachePeer *peer,
                                        const Http::StateFlags &flags);
 
     const Comm::ConnectionPointer & dataConnection() const override;

diff --git a/src/peer_proxy_negotiate_auth.cc b/src/peer_proxy_negotiate_auth.cc
index 11b5d2595daf341ba827e2bd14d07fe1fb565565..b5dc6713ecf881d3e0e5e127f2694342eadc32cc 100644 (file)
--- a/src/peer_proxy_negotiate_auth.cc
+++ b/src/peer_proxy_negotiate_auth.cc
@@ -483,7 +483,7 @@ restart:
  * peer_proxy_negotiate_auth gets a GSSAPI token for principal_name
  * and base64 encodes it.
  */
-char *peer_proxy_negotiate_auth(char *principal_name, char *proxy, int flags) {
+char *peer_proxy_negotiate_auth(char *principal_name, const char * const proxy, int flags) {
     int rc = 0;
     OM_uint32 major_status, minor_status;
     gss_ctx_id_t gss_context = GSS_C_NO_CONTEXT;

diff --git a/src/peer_proxy_negotiate_auth.h b/src/peer_proxy_negotiate_auth.h
index f5c592cba2e8986149dfed9472e1378401d81b2e..aa0ac939e246b6c4e91764f97a6b19fb7ad8c78c 100644 (file)
--- a/src/peer_proxy_negotiate_auth.h
+++ b/src/peer_proxy_negotiate_auth.h
@@ -14,7 +14,7 @@
 #if HAVE_AUTH_MODULE_NEGOTIATE && HAVE_KRB5 && HAVE_GSSAPI
 
 /* upstream proxy authentication */
-char *peer_proxy_negotiate_auth(char *principal_name, char *proxy, int flags);
+char *peer_proxy_negotiate_auth(char *principal_name, const char *proxy, int flags);
 #endif
 
 #endif /* SQUID_SRC_PEER_PROXY_NEGOTIATE_AUTH_H */

diff --git a/src/tunnel.cc b/src/tunnel.cc
index ff85638f0592de3e4b68392b99bd35b15bf99584..60833d92394d7fb1e937a958f486b48576510bd1 100644 (file)
--- a/src/tunnel.cc
+++ b/src/tunnel.cc
@@ -1155,8 +1155,6 @@ TunnelStateData::connectDone(const Comm::ConnectionPointer &conn, const char *or
 
     netdbPingSite(request->url.host());
 
-    request->peer_host = conn->getPeer() ? conn->getPeer()->host : nullptr;
-
     bool toOrigin = false; // same semantics as StateFlags::toOrigin
     if (const auto * const peer = conn->getPeer()) {
         request->prepForPeering(*peer);
@@ -1566,8 +1564,6 @@ switchToTunnel(HttpRequest *request, const Comm::ConnectionPointer &clientConn,
         tunnelState->server.setDelayId(DelayId::DelayClient(context->http));
 #endif
 
-    request->peer_host = srvConn->getPeer() ? srvConn->getPeer()->host : nullptr;
-
     debugs(26, 4, "determine post-connect handling pathway.");
     if (const auto peer = srvConn->getPeer())
         request->prepForPeering(*peer);