From: Eduard Bagdasaryan Date: Thu, 20 Mar 2025 19:44:48 +0000 (+0000) Subject: Remove HttpRequest::peer_host (#2032) X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=16b17ba916edef35196879481f0ebabc4b988815;p=thirdparty%2Fsquid.git 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 9700e7ec1b..eb715f29cb 100644 --- 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 df554d77b4..e026fcba24 100644 --- 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 48b3206863..8ecf537f79 100644 --- 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 a87af95d3f..fe9ba50a0b 100644 --- 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 55b56e3474..7fa9c6773e 100644 --- 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 71b1c15977..b14f66805a 100644 --- 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 11b5d2595d..b5dc6713ec 100644 --- 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 f5c592cba2..aa0ac939e2 100644 --- 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 ff85638f05..60833d9239 100644 --- 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);