mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-08-06 23:27:04 +02:00
Code Issues Projects Releases Wiki Activity

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

Browse Source 
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.
This commit is contained in:
Willy Tarreau 2013-04-12 18:13:46 +02:00
parent d16a1b2a81
commit 62a6123fed
4 changed files with 101 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
include/types/proxy.h
View File

@ -340,8 +340,6 @@ struct proxy {
int no_options; /* PR_O_REDISP, PR_O_TRANSP, ... */ int no_options; /* PR_O_REDISP, PR_O_TRANSP, ... */
int no_options2; /* PR_O2_* */ int no_options2; /* PR_O2_* */
char *logformat_string; /* log format string */
char *uniqueid_format_string; /* unique-id format string */
struct { struct {
char *file; /* file where the section appears */ char *file; /* file where the section appears */
int line; /* line 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 bind; /* list of bind settings */
struct list listeners; /* list of listeners belonging to this frontend */ struct list listeners; /* list of listeners belonging to this frontend */
struct arg_list args; /* sample arg list that need to be resolved */ 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 */ } conf; /* config information */
void *parent; /* parent of the proxy when applicable */ void *parent; /* parent of the proxy when applicable */
struct comp *comp; /* http compression */ struct comp *comp; /* http compression */

143
src/cfgparse.c
View File

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

12
src/haproxy.c
View File

@ -932,12 +932,14 @@ void deinit(void)
free(p->capture_name); free(p->capture_name);
free(p->monitor_uri); free(p->monitor_uri);
free(p->rdp_cookie_name); free(p->rdp_cookie_name);
if (p->logformat_string != default_http_log_format && if (p->conf.logformat_string != default_http_log_format &&
p->logformat_string != default_tcp_log_format && p->conf.logformat_string != default_tcp_log_format &&
p->logformat_string != clf_http_log_format) p->conf.logformat_string != clf_http_log_format)
free(p->logformat_string); 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++) for (i = 0; i < HTTP_ERR_SIZE; i++)
chunk_destroy(&p->errmsg[i]); chunk_destroy(&p->errmsg[i]);

10
src/proxy.c
View File

@ -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", Warning("config : 'option httplog' not usable with %s '%s' (needs 'mode http'). Falling back to 'option tcplog'.\n",
proxy_type_str(curproxy), curproxy->id); proxy_type_str(curproxy), curproxy->id);
} }
if (curproxy->logformat_string == default_http_log_format || if (curproxy->conf.logformat_string == default_http_log_format ||
curproxy->logformat_string == clf_http_log_format) { curproxy->conf.logformat_string == clf_http_log_format) {
curproxy->logformat_string = default_tcp_log_format; /* Note: we don't change the directive's file:line number */
Warning("config : 'option httplog' not usable with %s '%s' (needs 'mode http'). Falling back to 'option tcplog'.\n", 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); proxy_type_str(curproxy), curproxy->id);
} }