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 4194906bbe6a382618fda544fdfdbdde29c90721..7b65098a6e69b40214d5c241f06c174346ca8f81 100644 (file)
--- 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 a0e80d1cf66de65b3a1d250063aee320efcd0efb..21111e309ab7046f66cd5e6470198a2f16c76316 100644 (file)
--- 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);