From: Amos Jeffries <squid3@treenet.co.nz>
Date: Mon, 19 Jun 2017 10:31:04 +0000 (+1200)
Subject: Improve config parsing of logformat definitions
X-Git-Tag: SQUID_4_0_21~26
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9d63df880fb1da85f8b17cefde5da4ef2a794555;p=thirdparty%2Fsquid.git

Improve config parsing of logformat definitions

Squid has for some time ignored custom definitions using the same name
as internally defined formats. And overwritten custom formats when there
was a repeated definition.

* Detect logformat duplicates and produce ERROR message indicating the
  format name, config line and action taken.

* Add some missing FATAL labels on parse abort when access_log has
  multiple logformat= options configured.

* Add missing FATAL error message when logformat lines has no name
  parameter (and thus no tokens either).

* Omit the default "logformat=squid" option from cachemgr config dumps.
---

diff --git a/src/cache_cf.cc b/src/cache_cf.cc
index 4194906bbe..7b65098a6e 100644
--- a/src/cache_cf.cc
+++ b/src/cache_cf.cc
@@ -4038,7 +4038,7 @@ setLogformat(CustomLog *cl, const char *logdef_name, const bool dieWhenMissing)
     debugs(3, 9, "possible " << cl->filename << " logformat: " << logdef_name);
 
     if (cl->type != Log::Format::CLF_UNKNOWN) {
-        debugs(3, DBG_CRITICAL, "Second logformat name in one access_log: " <<
+        debugs(3, DBG_CRITICAL, "FATAL: Second logformat name in one access_log: " <<
                logdef_name << " " << cl->type << " ? " << Log::Format::CLF_NONE);
         self_destruct();
         return false;
@@ -4077,7 +4077,7 @@ setLogformat(CustomLog *cl, const char *logdef_name, const bool dieWhenMissing)
     } else if (strcmp(logdef_name, "referrer") == 0) {
         cl->type = Log::Format::CLF_REFERER;
     } else if (dieWhenMissing) {
-        debugs(3, DBG_CRITICAL, "Log format '" << logdef_name << "' is not defined");
+        debugs(3, DBG_CRITICAL, "FATAL: Log format '" << logdef_name << "' is not defined");
         self_destruct();
         return false;
     } else {
@@ -4112,7 +4112,8 @@ dump_access_log(StoreEntry * entry, const char *name, CustomLog * logs)
             break;
 
         case Log::Format::CLF_SQUID:
-            storeAppendPrintf(entry, "%s logformat=squid", log->filename);
+            // this is the default, no need to add to the dump
+            //storeAppendPrintf(entry, "%s logformat=squid", log->filename);
             break;
 
         case Log::Format::CLF_COMBINED:
diff --git a/src/log/Config.cc b/src/log/Config.cc
index a0e80d1cf6..21111e309a 100644
--- a/src/log/Config.cc
+++ b/src/log/Config.cc
@@ -20,7 +20,27 @@ Log::LogConfig::parseFormats()
     char *name, *def;
 
     if (!(name = ConfigParser::NextToken())) {
+        debugs(3, DBG_CRITICAL, "FATAL: missing logformat details in " << cfg_filename << " line " << config_lineno);
         self_destruct();
+        return;
+    }
+
+    // check for re-definition of built-in formats
+    if (strcmp(name, "squid") == 0 ||
+            strcmp(name, "common") == 0 ||
+            strcmp(name, "combined") == 0 ||
+            strcmp(name, "useragent") == 0 ||
+            strcmp(name, "referrer") == 0) {
+        debugs(3, DBG_PARSE_NOTE(DBG_IMPORTANT), "ERROR: logformat " << name << " is already defined. Ignoring.");
+        return;
+    }
+
+    // check for re-definition of custom formats
+    for (auto i = logformats; i ; i = i->next) {
+        if (strcmp(i->name, name) == 0) {
+            debugs(3, DBG_PARSE_NOTE(DBG_IMPORTANT), "ERROR: logformat " << name << " is already defined. Ignoring.");
+            return;
+        }
     }
 
     ::Format::Format *nlf = new ::Format::Format(name);