From: Nathan Hoad Date: Fri, 25 Mar 2016 13:03:30 +0000 (+1300) Subject: Fix memory leak of AccessLogentry::url X-Git-Tag: SQUID_4_0_8~19 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=f57ae909fef52f5088b8304a4eb21d0c3978f533;p=thirdparty%2Fsquid.git Fix memory leak of AccessLogentry::url ... created by ACLFilledChecklist::syncAle(). ::syncAle() is the only place in the codebase that assigns a URL that AccessLogEntry is expected to free(), which AccessLogEntry doesn't do. This results in a memory leak. This is submitted on behalf of Bloomberg L.P. --- diff --git a/src/AccessLogEntry.h b/src/AccessLogEntry.h index 5968ab2f0f..6665bd887e 100644 --- a/src/AccessLogEntry.h +++ b/src/AccessLogEntry.h @@ -21,6 +21,7 @@ #include "LogTags.h" #include "MessageSizes.h" #include "Notes.h" +#include "sbuf/SBuf.h" #if ICAP_CLIENT #include "adaptation/icap/Elements.h" #endif @@ -56,7 +57,7 @@ public: /// Fetch the transaction method string (ICP opcode, HTCP opcode or HTTP method) SBuf getLogMethod() const; - const char *url; + SBuf url; /// TCP/IP level details about the client connection Comm::ConnectionPointer tcpClient; diff --git a/src/acl/FilledChecklist.cc b/src/acl/FilledChecklist.cc index dd5973407a..271c6bc2f6 100644 --- a/src/acl/FilledChecklist.cc +++ b/src/acl/FilledChecklist.cc @@ -103,9 +103,9 @@ ACLFilledChecklist::syncAle() const HTTPMSGLOCK(al->adapted_request); } - if (!al->url) { + if (al->url.isEmpty()) { showDebugWarning("URL"); - al->url = xstrdup(request->url.absolute().c_str()); + al->url = request->url.absolute(); } } diff --git a/src/format/Format.cc b/src/format/Format.cc index c096aa4ee3..7ca4f53036 100644 --- a/src/format/Format.cc +++ b/src/format/Format.cc @@ -34,6 +34,8 @@ /// Convert a string to NULL pointer if it is "" #define strOrNull(s) ((s)==NULL||(s)[0]=='\0'?NULL:(s)) +const SBuf Format::Dash("-"); + Format::Format::Format(const char *n) : format(NULL), next(NULL) @@ -1039,7 +1041,11 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS break; case LFT_REQUEST_URI: - out = al->url; + if (!al->url.isEmpty()) { + const SBuf &s = al->url; + sb.append(s.rawContent(), s.length()); + out = sb.termedBuf(); + } break; case LFT_REQUEST_VERSION_OLD_2X: diff --git a/src/format/Format.h b/src/format/Format.h index 4e764dace8..08249ecc9d 100644 --- a/src/format/Format.h +++ b/src/format/Format.h @@ -11,6 +11,7 @@ #include "base/RefCount.h" #include "ConfigParser.h" +#include "sbuf/SBuf.h" /* * Squid configuration allows users to define custom formats in @@ -32,6 +33,8 @@ class StoreEntry; namespace Format { +extern const SBuf Dash; + class Token; // XXX: inherit from linked list diff --git a/src/log/FormatHttpdCombined.cc b/src/log/FormatHttpdCombined.cc index 6af7e74005..a063f8928b 100644 --- a/src/log/FormatHttpdCombined.cc +++ b/src/log/FormatHttpdCombined.cc @@ -47,13 +47,13 @@ Log::Format::HttpdCombined(const AccessLogEntry::Pointer &al, Logfile * logfile) const SBuf method(al->getLogMethod()); - logfilePrintf(logfile, "%s %s %s [%s] \"" SQUIDSBUFPH " %s %s/%d.%d\" %d %" PRId64 " \"%s\" \"%s\" %s:%s%s", + logfilePrintf(logfile, "%s %s %s [%s] \"" SQUIDSBUFPH " " SQUIDSBUFPH " %s/%d.%d\" %d %" PRId64 " \"%s\" \"%s\" %s:%s%s", clientip, user_ident ? user_ident : dash_str, user_auth ? user_auth : dash_str, Time::FormatHttpd(squid_curtime), SQUIDSBUFPRINT(method), - al->url, + SQUIDSBUFPRINT(al->url), AnyP::ProtocolType_str[al->http.version.protocol], al->http.version.major, al->http.version.minor, al->http.code, diff --git a/src/log/FormatHttpdCommon.cc b/src/log/FormatHttpdCommon.cc index 0d191c1740..b14036ec03 100644 --- a/src/log/FormatHttpdCommon.cc +++ b/src/log/FormatHttpdCommon.cc @@ -34,13 +34,13 @@ Log::Format::HttpdCommon(const AccessLogEntry::Pointer &al, Logfile * logfile) const SBuf method(al->getLogMethod()); - logfilePrintf(logfile, "%s %s %s [%s] \"" SQUIDSBUFPH " %s %s/%d.%d\" %d %" PRId64 " %s:%s%s", + logfilePrintf(logfile, "%s %s %s [%s] \"" SQUIDSBUFPH " " SQUIDSBUFPH " %s/%d.%d\" %d %" PRId64 " %s:%s%s", clientip, user_ident ? user_ident : dash_str, user_auth ? user_auth : dash_str, Time::FormatHttpd(squid_curtime), SQUIDSBUFPRINT(method), - al->url, + SQUIDSBUFPRINT(al->url), AnyP::ProtocolType_str[al->http.version.protocol], al->http.version.major, al->http.version.minor, al->http.code, diff --git a/src/log/FormatSquidNative.cc b/src/log/FormatSquidNative.cc index 562631ed19..594e11b75d 100644 --- a/src/log/FormatSquidNative.cc +++ b/src/log/FormatSquidNative.cc @@ -50,7 +50,7 @@ Log::Format::SquidNative(const AccessLogEntry::Pointer &al, Logfile * logfile) const SBuf method(al->getLogMethod()); - logfilePrintf(logfile, "%9ld.%03d %6ld %s %s/%03d %" PRId64 " " SQUIDSBUFPH " %s %s %s%s/%s %s%s", + logfilePrintf(logfile, "%9ld.%03d %6ld %s %s/%03d %" PRId64 " " SQUIDSBUFPH " " SQUIDSBUFPH " %s %s%s/%s %s%s", (long int) current_time.tv_sec, (int) current_time.tv_usec / 1000, tvToMsec(al->cache.trTime), @@ -59,7 +59,7 @@ Log::Format::SquidNative(const AccessLogEntry::Pointer &al, Logfile * logfile) al->http.code, al->http.clientReplySz.messageTotal(), SQUIDSBUFPRINT(method), - al->url, + SQUIDSBUFPRINT(al->url), user ? user : dash_str, al->hier.ping.timedout ? "TIMEOUT_" : "", hier_code_str[al->hier.code], diff --git a/src/log/FormatSquidReferer.cc b/src/log/FormatSquidReferer.cc index 0ccadcfb1b..1efc2b0f18 100644 --- a/src/log/FormatSquidReferer.cc +++ b/src/log/FormatSquidReferer.cc @@ -28,11 +28,13 @@ Log::Format::SquidReferer(const AccessLogEntry::Pointer &al, Logfile *logfile) char clientip[MAX_IPSTRLEN]; al->getLogClientIp(clientip, MAX_IPSTRLEN); - logfilePrintf(logfile, "%9ld.%03d %s %s %s\n", + const SBuf url = !al->url.isEmpty() ? al->url : ::Format::Dash; + + logfilePrintf(logfile, "%9ld.%03d %s %s " SQUIDSBUFPH "\n", (long int) current_time.tv_sec, (int) current_time.tv_usec / 1000, clientip, referer, - al->url ? al->url : "-"); + SQUIDSBUFPRINT(url)); } diff --git a/src/log/access_log.cc b/src/log/access_log.cc index 8c8cb8f649..a3fc35a4b0 100644 --- a/src/log/access_log.cc +++ b/src/log/access_log.cc @@ -76,8 +76,8 @@ void accessLogLogTo(CustomLog* log, AccessLogEntry::Pointer &al, ACLChecklist * checklist) { - if (al->url == NULL) - al->url = dash_str; + if (al->url.isEmpty()) + al->url = Format::Dash; if (!al->http.content_type || *al->http.content_type == '\0') al->http.content_type = dash_str; @@ -160,8 +160,8 @@ accessLogLog(AccessLogEntry::Pointer &al, ACLChecklist * checklist) else { unsigned int ibuf[365]; size_t isize; - xstrncpy((char *) ibuf, al->url, 364 * sizeof(int)); - isize = ((strlen(al->url) + 8) / 8) * 2; + xstrncpy((char *) ibuf, al->url.c_str(), 364 * sizeof(int)); + isize = ((al->url.length() + 8) / 8) * 2; if (isize > 364) isize = 364;