From: Alex Rousskov Date: Wed, 30 Mar 2022 15:51:20 +0000 (+0000) Subject: Logformat %lp expands to "-" in wildcard listening port configs (#997) X-Git-Tag: SQUID_6_0_1~217 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b6c0f0dc13cbf0e89adce74d40d94e1d30758761;p=thirdparty%2Fsquid.git Logformat %lp expands to "-" in wildcard listening port configs (#997) FindListeningPortAddress() and its helpers look for "local" address of an accepted connection. When FindListeningPortAddress() is called in %la context, we must skip Ip::Address objects with INADDR_ANY IP addresses because we are looking for a specific IP address, not a wildcard. However, when called in %lp context, skipping those Ip::Address objects may, in some cases, result in skipping the only object that actually contains the port information, resulting in %lp expanding to "-". Similarly, zero-port Ip::Address objects (with non-any IPs) could be, in theory, returned instead of continuing the search for an object with a non-zero port number, although this case was not observed in tests. Now we configure the address searching helpers with a "good Ip::Address object" filter so that each %code code path can customize its search. The problem was introduced in commit ea35939 that expanded INADDR_ANY check scope from %la to %lp. --- diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc index 5d43aee163..08d43019ff 100644 --- a/src/HttpRequest.cc +++ b/src/HttpRequest.cc @@ -824,30 +824,25 @@ HttpRequest::canonicalCleanUrl() const return urlCanonicalCleanWithoutRequest(effectiveRequestUri(), method, url.getScheme()); } -/// a helper for validating FindListeningPortAddress()-found address candidates -static const Ip::Address * -FindListeningPortAddressInAddress(const Ip::Address *ip) -{ - // FindListeningPortAddress() callers do not want INADDR_ANY addresses - return (ip && !ip->isAnyAddr()) ? ip : nullptr; -} - /// a helper for handling PortCfg cases of FindListeningPortAddress() +template static const Ip::Address * -FindListeningPortAddressInPort(const AnyP::PortCfgPointer &port) +FindGoodListeningPortAddressInPort(const AnyP::PortCfgPointer &port, const Filter isGood) { - return port ? FindListeningPortAddressInAddress(&port->s) : nullptr; + return (port && isGood(port->s)) ? &port->s : nullptr; } /// a helper for handling Connection cases of FindListeningPortAddress() +template static const Ip::Address * -FindListeningPortAddressInConn(const Comm::ConnectionPointer &conn) +FindGoodListeningPortAddressInConn(const Comm::ConnectionPointer &conn, const Filter isGood) { - return conn ? FindListeningPortAddressInAddress(&conn->local) : nullptr; + return (conn && isGood(conn->local)) ? &conn->local : nullptr; } +template const Ip::Address * -FindListeningPortAddress(const HttpRequest *callerRequest, const AccessLogEntry *ale) +FindGoodListeningPortAddress(const HttpRequest *callerRequest, const AccessLogEntry *ale, const Filter filter) { // Check all sources of usable listening port information, giving // HttpRequest and masterXaction a preference over ALE. @@ -858,18 +853,35 @@ FindListeningPortAddress(const HttpRequest *callerRequest, const AccessLogEntry if (!request) return nullptr; // not enough information - const Ip::Address *ip = FindListeningPortAddressInPort(request->masterXaction->squidPort); + auto ip = FindGoodListeningPortAddressInPort(request->masterXaction->squidPort, filter); if (!ip && ale) - ip = FindListeningPortAddressInPort(ale->cache.port); + ip = FindGoodListeningPortAddressInPort(ale->cache.port, filter); // XXX: also handle PROXY protocol here when we have a flag to identify such request if (ip || request->flags.interceptTproxy || request->flags.intercepted) return ip; /* handle non-intercepted cases that were not handled above */ - ip = FindListeningPortAddressInConn(request->masterXaction->tcpClient); + ip = FindGoodListeningPortAddressInConn(request->masterXaction->tcpClient, filter); if (!ip && ale) - ip = FindListeningPortAddressInConn(ale->tcpClient); + ip = FindGoodListeningPortAddressInConn(ale->tcpClient, filter); return ip; // may still be nil } +const Ip::Address * +FindListeningPortAddress(const HttpRequest *callerRequest, const AccessLogEntry *ale) +{ + return FindGoodListeningPortAddress(callerRequest, ale, [](const Ip::Address &address) { + // FindListeningPortAddress() callers do not want INADDR_ANY addresses + return !address.isAnyAddr(); + }); +} + +unsigned short +FindListeningPortNumber(const HttpRequest *callerRequest, const AccessLogEntry *ale) +{ + const auto ip = FindGoodListeningPortAddress(callerRequest, ale, [](const Ip::Address &address) { + return address.port() > 0; + }); + return ip ? ip->port() : 0; +} diff --git a/src/HttpRequest.h b/src/HttpRequest.h index 3eaf2ac5e7..98aeea9b56 100644 --- a/src/HttpRequest.h +++ b/src/HttpRequest.h @@ -285,5 +285,9 @@ void UpdateRequestNotes(ConnStateData *csd, HttpRequest &request, NotePairs cons /// nil parameter(s) indicate missing caller information and are handled safely const Ip::Address *FindListeningPortAddress(const HttpRequest *, const AccessLogEntry *); +/// \returns listening/*_port port number used by the client connection (or zero) +/// nil parameter(s) indicate missing caller information and are handled safely +unsigned short FindListeningPortNumber(const HttpRequest *, const AccessLogEntry *); + #endif /* SQUID_HTTPREQUEST_H */ diff --git a/src/format/Format.cc b/src/format/Format.cc index 968fb235a4..4d23236e7b 100644 --- a/src/format/Format.cc +++ b/src/format/Format.cc @@ -507,8 +507,8 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS break; case LFT_LOCAL_LISTENING_PORT: - if (const auto addr = FindListeningPortAddress(nullptr, al.getRaw())) { - outint = addr->port(); + if (const auto port = FindListeningPortNumber(nullptr, al.getRaw())) { + outint = port; doint = 1; } break;