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 5729db8..273fb8b 100644
--- a/include/types/proxy.h
+++ b/include/types/proxy.h
@@ -340,8 +340,6 @@
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 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 49a91c0..cc515a2 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -1837,12 +1837,17 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
/* 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 15c14e3..d4c1189 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -932,12 +932,14 @@
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);
+ 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->uniqueid_format_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 7bfe954..f909d20 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -409,10 +409,12 @@
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);
}