BUG/MEDIUM: log: fix regression on log-format handling

Commit a4312fa2 merged into dev18 improved log-format management by
processing "log-format" and "unique-id-format" where they were declared,
so that the faulty args could be reported with their correct line numbers.

Unfortunately, the log-format parser considers the proxy mode (TCP/HTTP)
and now if the directive is set before the "mode" statement, it can be
rejected and report warnings.

So we really need to parse these directives at the end of a section at
least. Right now we do not have an "end of section" event, so we need
to store the file name and line number for each of these directives,
and take care of them at the end.

One of the benefits is that now the line numbers can be inherited from
the line passing "option httplog" even if it's in a defaults section.

Future improvements should be performed to report line numbers in every
log-format processed by the parser.

diff --git a/include/types/proxy.h b/include/types/proxy.h
index 5729db80dcf609d52ccc26c768a4f40b8ad83f71..273fb8bc535379af372141531ce9248493494e3f 100644 (file)
--- a/include/types/proxy.h
+++ b/include/types/proxy.h
@@ -340,8 +340,6 @@ struct proxy {
        int no_options;                         /* PR_O_REDISP, PR_O_TRANSP, ... */
        int no_options2;                        /* PR_O2_* */
 
-       char *logformat_string;                 /* log format string */
-       char *uniqueid_format_string;           /* unique-id format string */
        struct {
                char *file;                     /* file where the section appears */
                int line;                       /* line where the section appears */
@@ -351,6 +349,12 @@ struct proxy {
                struct list bind;               /* list of bind settings */
                struct list listeners;          /* list of listeners belonging to this frontend */
                struct arg_list args;           /* sample arg list that need to be resolved */
+               char *logformat_string;         /* log format string */
+               char *lfs_file;                 /* file name where the logformat string appears (strdup) */
+               int   lfs_line;                 /* file name where the logformat string appears */
+               char *uniqueid_format_string;   /* unique-id format string */
+               char *uif_file;                 /* file name where the unique-id-format string appears (strdup) */
+               int   uif_line;                 /* file name where the unique-id-format string appears */
        } conf;                                 /* config information */
        void *parent;                           /* parent of the proxy when applicable */
        struct comp *comp;                      /* http compression */

diff --git a/src/cfgparse.c b/src/cfgparse.c
index 49a91c046ee70b97394b2fbe81516019b5c629a1..cc515a2058577044d09bdd92bf5c19271a1960f2 100644 (file)
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -1837,12 +1837,17 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
                                curproxy->defbe.name = strdup(defproxy.defbe.name);
 
                        /* get either a pointer to the logformat string or a copy of it */
-                       curproxy->logformat_string = defproxy.logformat_string;
-                       if (curproxy->logformat_string &&
-                           curproxy->logformat_string != default_http_log_format &&
-                           curproxy->logformat_string != default_tcp_log_format &&
-                           curproxy->logformat_string != clf_http_log_format)
-                               curproxy->logformat_string = strdup(curproxy->logformat_string);
+                       curproxy->conf.logformat_string = defproxy.conf.logformat_string;
+                       if (curproxy->conf.logformat_string &&
+                           curproxy->conf.logformat_string != default_http_log_format &&
+                           curproxy->conf.logformat_string != default_tcp_log_format &&
+                           curproxy->conf.logformat_string != clf_http_log_format)
+                               curproxy->conf.logformat_string = strdup(curproxy->conf.logformat_string);
+
+                       if (defproxy.conf.lfs_file) {
+                               curproxy->conf.lfs_file = strdup(defproxy.conf.lfs_file);
+                               curproxy->conf.lfs_line = defproxy.conf.lfs_line;
+                       }
                }
 
                if (curproxy->cap & PR_CAP_BE) {
@@ -1867,9 +1872,14 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
                        LIST_ADDQ(&curproxy->logsrvs, &node->list);
                }
 
-               curproxy->uniqueid_format_string = defproxy.uniqueid_format_string;
-               if (curproxy->uniqueid_format_string)
-                       curproxy->uniqueid_format_string = strdup(curproxy->uniqueid_format_string);
+               curproxy->conf.uniqueid_format_string = defproxy.conf.uniqueid_format_string;
+               if (curproxy->conf.uniqueid_format_string)
+                       curproxy->conf.uniqueid_format_string = strdup(curproxy->conf.uniqueid_format_string);
+
+               if (defproxy.conf.uif_file) {
+                       curproxy->conf.uif_file = strdup(defproxy.conf.uif_file);
+                       curproxy->conf.uif_line = defproxy.conf.uif_line;
+               }
 
                /* copy default header unique id */
                if (defproxy.header_unique_id)
@@ -1912,12 +1922,14 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
                free(defproxy.expect_str);
                if (defproxy.expect_regex) regfree(defproxy.expect_regex);
 
-               if (defproxy.logformat_string != default_http_log_format &&
-                   defproxy.logformat_string != default_tcp_log_format &&
-                   defproxy.logformat_string != clf_http_log_format)
-                       free(defproxy.logformat_string);
+               if (defproxy.conf.logformat_string != default_http_log_format &&
+                   defproxy.conf.logformat_string != default_tcp_log_format &&
+                   defproxy.conf.logformat_string != clf_http_log_format)
+                       free(defproxy.conf.logformat_string);
 
-               free(defproxy.uniqueid_format_string);
+               free(defproxy.conf.uniqueid_format_string);
+               free(defproxy.conf.lfs_file);
+               free(defproxy.conf.uif_file);
 
                for (rc = 0; rc < HTTP_ERR_SIZE; rc++)
                        chunk_destroy(&defproxy.errmsg[rc]);
@@ -3401,19 +3413,27 @@ stats_error_parsing:
                                        goto out;
                                }
                        }
-                       if (curproxy->logformat_string != default_http_log_format &&
-                           curproxy->logformat_string != default_tcp_log_format &&
-                           curproxy->logformat_string != clf_http_log_format)
-                               free(curproxy->logformat_string);
-                       curproxy->logformat_string = logformat;
+                       if (curproxy->conf.logformat_string != default_http_log_format &&
+                           curproxy->conf.logformat_string != default_tcp_log_format &&
+                           curproxy->conf.logformat_string != clf_http_log_format)
+                               free(curproxy->conf.logformat_string);
+                       curproxy->conf.logformat_string = logformat;
+
+                       free(curproxy->conf.lfs_file);
+                       curproxy->conf.lfs_file = strdup(curproxy->conf.args.file);
+                       curproxy->conf.lfs_line = curproxy->conf.args.line;
                }
                else if (!strcmp(args[1], "tcplog")) {
                        /* generate a detailed TCP log */
-                       if (curproxy->logformat_string != default_http_log_format &&
-                           curproxy->logformat_string != default_tcp_log_format &&
-                           curproxy->logformat_string != clf_http_log_format)
-                               free(curproxy->logformat_string);
-                       curproxy->logformat_string = default_tcp_log_format;
+                       if (curproxy->conf.logformat_string != default_http_log_format &&
+                           curproxy->conf.logformat_string != default_tcp_log_format &&
+                           curproxy->conf.logformat_string != clf_http_log_format)
+                               free(curproxy->conf.logformat_string);
+                       curproxy->conf.logformat_string = default_tcp_log_format;
+
+                       free(curproxy->conf.lfs_file);
+                       curproxy->conf.lfs_file = strdup(curproxy->conf.args.file);
+                       curproxy->conf.lfs_line = curproxy->conf.args.line;
                }
                else if (!strcmp(args[1], "tcpka")) {
                        /* enable TCP keep-alives on client and server sessions */
@@ -4829,20 +4849,12 @@ stats_error_parsing:
                        err_code |= ERR_ALERT | ERR_FATAL;
                        goto out;
                }
-               free(curproxy->uniqueid_format_string);
-               curproxy->uniqueid_format_string = strdup(args[1]);
+               free(curproxy->conf.uniqueid_format_string);
+               curproxy->conf.uniqueid_format_string = strdup(args[1]);
 
-               /* get a chance to improve log-format error reporting by
-                * reporting the correct line-number when possible.
-                */
-               if (curproxy != &defproxy) {
-                       curproxy->conf.args.ctx = ARGC_UIF;
-                       if (curproxy->uniqueid_format_string)
-                               parse_logformat_string(curproxy->uniqueid_format_string, curproxy, &curproxy->format_unique_id, 0,
-                                                      (proxy->cap & PR_CAP_FE) ? SMP_VAL_FE_HRQ_HDR : SMP_VAL_BE_HRQ_HDR);
-                       free(curproxy->uniqueid_format_string);
-                       curproxy->uniqueid_format_string = NULL;
-               }
+               free(curproxy->conf.uif_file);
+               curproxy->conf.uif_file = strdup(curproxy->conf.args.file);
+               curproxy->conf.uif_line = curproxy->conf.args.line;
        }
 
        else if (strcmp(args[0], "unique-id-header") == 0) {
@@ -4867,11 +4879,15 @@ stats_error_parsing:
                        goto out;
                }
 
-               if (curproxy->logformat_string != default_http_log_format &&
-                   curproxy->logformat_string != default_tcp_log_format &&
-                   curproxy->logformat_string != clf_http_log_format)
-                       free(curproxy->logformat_string);
-               curproxy->logformat_string = strdup(args[1]);
+               if (curproxy->conf.logformat_string != default_http_log_format &&
+                   curproxy->conf.logformat_string != default_tcp_log_format &&
+                   curproxy->conf.logformat_string != clf_http_log_format)
+                       free(curproxy->conf.logformat_string);
+               curproxy->conf.logformat_string = strdup(args[1]);
+
+               free(curproxy->conf.lfs_file);
+               curproxy->conf.lfs_file = strdup(curproxy->conf.args.file);
+               curproxy->conf.lfs_line = curproxy->conf.args.line;
 
                /* get a chance to improve log-format error reporting by
                 * reporting the correct line-number when possible.
@@ -4881,14 +4897,6 @@ stats_error_parsing:
                                file, linenum, curproxy->id);
                        err_code |= ERR_WARN;
                }
-               else if (curproxy->cap & PR_CAP_FE) {
-                       curproxy->conf.args.ctx = ARGC_LOG;
-                       if (curproxy->logformat_string)
-                               parse_logformat_string(curproxy->logformat_string, curproxy, &curproxy->logformat, LOG_OPT_MANDATORY,
-                                                      SMP_VAL_FE_LOG_END);
-                       free(curproxy->logformat_string);
-                       curproxy->logformat_string = NULL;
-               }
        }
 
        else if (!strcmp(args[0], "log") && kwm == KWM_NO) {
@@ -6471,22 +6479,35 @@ out_uri_auth_compat:
 
                /* compile the log format */
                if (!(curproxy->cap & PR_CAP_FE)) {
-                       if (curproxy->logformat_string != default_http_log_format &&
-                           curproxy->logformat_string != default_tcp_log_format &&
-                           curproxy->logformat_string != clf_http_log_format)
-                               free(curproxy->logformat_string);
-                       curproxy->logformat_string = NULL;
+                       if (curproxy->conf.logformat_string != default_http_log_format &&
+                           curproxy->conf.logformat_string != default_tcp_log_format &&
+                           curproxy->conf.logformat_string != clf_http_log_format)
+                               free(curproxy->conf.logformat_string);
+                       curproxy->conf.logformat_string = NULL;
+                       free(curproxy->conf.lfs_file);
+                       curproxy->conf.lfs_file = NULL;
+                       curproxy->conf.lfs_line = 0;
                }
 
-               curproxy->conf.args.ctx = ARGC_LOG;
-               if (curproxy->logformat_string)
-                       parse_logformat_string(curproxy->logformat_string, curproxy, &curproxy->logformat, LOG_OPT_MANDATORY,
+               if (curproxy->conf.logformat_string) {
+                       curproxy->conf.args.ctx = ARGC_LOG;
+                       curproxy->conf.args.file = curproxy->conf.lfs_file;
+                       curproxy->conf.args.line = curproxy->conf.lfs_line;
+                       parse_logformat_string(curproxy->conf.logformat_string, curproxy, &curproxy->logformat, LOG_OPT_MANDATORY,
                                               SMP_VAL_FE_LOG_END);
+                       curproxy->conf.args.file = NULL;
+                       curproxy->conf.args.line = 0;
+               }
 
-               curproxy->conf.args.ctx = ARGC_UIF;
-               if (curproxy->uniqueid_format_string)
-                       parse_logformat_string(curproxy->uniqueid_format_string, curproxy, &curproxy->format_unique_id, 0,
+               if (curproxy->conf.uniqueid_format_string) {
+                       curproxy->conf.args.ctx = ARGC_UIF;
+                       curproxy->conf.args.file = curproxy->conf.uif_file;
+                       curproxy->conf.args.line = curproxy->conf.uif_line;
+                       parse_logformat_string(curproxy->conf.uniqueid_format_string, curproxy, &curproxy->format_unique_id, 0,
                                               (proxy->cap & PR_CAP_FE) ? SMP_VAL_FE_HRQ_HDR : SMP_VAL_BE_HRQ_HDR);
+                       curproxy->conf.args.file = NULL;
+                       curproxy->conf.args.line = 0;
+               }
 
                /* only now we can check if some args remain unresolved */
                cfgerr += smp_resolve_args(curproxy);

diff --git a/src/haproxy.c b/src/haproxy.c
index 15c14e3029fb589477a1bd0bc78b8ae48cd99b31..d4c11897440e37a922844532cff10a63f303331f 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -932,12 +932,14 @@ void deinit(void)
                free(p->capture_name);
                free(p->monitor_uri);
                free(p->rdp_cookie_name);
-               if (p->logformat_string != default_http_log_format &&
-                   p->logformat_string != default_tcp_log_format &&
-                   p->logformat_string != clf_http_log_format)
-                       free(p->logformat_string);
-
-               free(p->uniqueid_format_string);
+               if (p->conf.logformat_string != default_http_log_format &&
+                   p->conf.logformat_string != default_tcp_log_format &&
+                   p->conf.logformat_string != clf_http_log_format)
+                       free(p->conf.logformat_string);
+
+               free(p->conf.lfs_file);
+               free(p->conf.uniqueid_format_string);
+               free(p->conf.uif_file);
 
                for (i = 0; i < HTTP_ERR_SIZE; i++)
                        chunk_destroy(&p->errmsg[i]);

diff --git a/src/proxy.c b/src/proxy.c
index 7bfe954a492775b4b5ed20d1fa07544cb341f7e7..f909d201d6c76a76f9d39bdc1abdca9d4143d119 100644 (file)
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -409,10 +409,12 @@ int proxy_cfg_ensure_no_http(struct proxy *curproxy)
                Warning("config : 'option httplog' not usable with %s '%s' (needs 'mode http'). Falling back to 'option tcplog'.\n",
                        proxy_type_str(curproxy), curproxy->id);
        }
-       if (curproxy->logformat_string == default_http_log_format ||
-           curproxy->logformat_string == clf_http_log_format) {
-               curproxy->logformat_string = default_tcp_log_format;
-               Warning("config : 'option httplog' not usable with %s '%s' (needs 'mode http'). Falling back to 'option tcplog'.\n",
+       if (curproxy->conf.logformat_string == default_http_log_format ||
+           curproxy->conf.logformat_string == clf_http_log_format) {
+               /* Note: we don't change the directive's file:line number */
+               curproxy->conf.logformat_string = default_tcp_log_format;
+               Warning("parsing [%s:%d] : 'option httplog' not usable with %s '%s' (needs 'mode http'). Falling back to 'option tcplog'.\n",
+                       curproxy->conf.lfs_file, curproxy->conf.lfs_line,
                        proxy_type_str(curproxy), curproxy->id);
        }