From 98b44e8edb439ef54d021de03efec1e8fd019fd0 Mon Sep 17 00:00:00 2001 From: Aurelien DARRAGON Date: Mon, 29 Apr 2024 11:55:27 +0200 Subject: [PATCH] BUG/MINOR: log: fix global lf_expr node options behavior In 507223d5 ("MINOR: log: global lf_expr node options"), a mistake was made because it was assumed that only the last occurence of %o (LOG_FMT_GLOBAL) should be kept as global node options. However, although not documented, it is possible to have multiple %o within a single logformat expression to change the global settings on the fly. For instance, consider this example: log-format "%{+X}o test1=%ms %{-X}o test2=%ms %{+X}o test3=%ms" Prior to 3f2e8d0ed ("MEDIUM: log: lf_* build helpers now take a ctx argument"), this would output something like this: test1=18B test2=395 test3=18B This is because global options is properly updated as the lf_expr string is parsed. But now due to 507223d5 and 3f2e8d0ed, only the last %o occurence is considered. With the above example, this gives: test1=18B test2=18B test3=18B To restore historical behavior, let's partially revert 507223d5: to compute global node options, we now start with all options enabled and then for each configurable node in lf_expr_postcheck(), we keep options common to the current node and previous nodes using AND masking, this way we really end up with options common to all nodes. No backport needed. --- src/log.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/src/log.c b/src/log.c index e3ab29a503..a7cc713fca 100644 --- a/src/log.c +++ b/src/log.c @@ -415,7 +415,7 @@ int parse_logformat_tag_args(char *args, struct logformat_node *node, char **err */ static int parse_logformat_tag(char *arg, int arg_len, char *name, int name_len, int typecast, char *tag, int tag_len, struct lf_expr *lf_expr, - char **err) + int *defoptions, char **err) { int j; struct list *list_format= &lf_expr->nodes.list; @@ -434,14 +434,14 @@ static int parse_logformat_tag(char *arg, int arg_len, char *name, int name_len, node->typecast = typecast; if (name && name_len) node->name = my_strndup(name, name_len); - node->options = lf_expr->nodes.options; + node->options = *defoptions; if (arg_len) { node->arg = my_strndup(arg, arg_len); if (!parse_logformat_tag_args(node->arg, node, err)) goto error_free; } if (node->tag->type == LOG_FMT_GLOBAL) { - lf_expr->nodes.options = node->options; + *defoptions = node->options; free_logformat_node(node); } else { LIST_APPEND(list_format, &node->list); @@ -510,7 +510,7 @@ int add_to_logformat_list(char *start, char *end, int type, struct lf_expr *lf_e */ static int add_sample_to_logformat_list(char *text, char *name, int name_len, int typecast, char *arg, int arg_len, struct lf_expr *lf_expr, - struct arg_list *al, int cap, char **err, char **endptr) + struct arg_list *al, int options, int cap, char **err, char **endptr) { char *cmd[2]; struct list *list_format = &lf_expr->nodes.list; @@ -539,7 +539,7 @@ static int add_sample_to_logformat_list(char *text, char *name, int name_len, in node->type = LOG_FMT_EXPR; node->typecast = typecast; node->expr = expr; - node->options = lf_expr->nodes.options; + node->options = options; if (arg_len) { node->arg = my_strndup(arg, arg_len); @@ -558,7 +558,7 @@ static int add_sample_to_logformat_list(char *text, char *name, int name_len, in goto error_free; } - if ((lf_expr->nodes.options & LOG_OPT_HTTP) && (expr->fetch->use & (SMP_USE_L6REQ|SMP_USE_L6RES))) { + if ((options & LOG_OPT_HTTP) && (expr->fetch->use & (SMP_USE_L6REQ|SMP_USE_L6RES))) { ha_warning("parsing [%s:%d] : L6 sample fetch <%s> ignored in HTTP log-format string.\n", lf_expr->conf.file, lf_expr->conf.line, text); } @@ -620,7 +620,6 @@ int lf_expr_compile(struct lf_expr *lf_expr, * returning. */ LIST_INIT(&lf_expr->nodes.list); - lf_expr->nodes.options = options; /* we must set the compiled flag now for proper deinit in case of failure */ lf_expr->flags |= LF_FL_COMPILED; @@ -737,7 +736,7 @@ int lf_expr_compile(struct lf_expr *lf_expr, * part of the expression, which MUST be the trailing * angle bracket. */ - if (!add_sample_to_logformat_list(tag, name, name_len, typecast, arg, arg_len, lf_expr, al, cap, err, &str)) + if (!add_sample_to_logformat_list(tag, name, name_len, typecast, arg, arg_len, lf_expr, al, options, cap, err, &str)) goto fail; if (*str == ']') { @@ -783,7 +782,7 @@ int lf_expr_compile(struct lf_expr *lf_expr, if (cformat != pformat || pformat == LF_SEPARATOR) { switch (pformat) { case LF_TAG: - if (!parse_logformat_tag(arg, arg_len, name, name_len, typecast, tag, tag_len, lf_expr, err)) + if (!parse_logformat_tag(arg, arg_len, name, name_len, typecast, tag, tag_len, lf_expr, &options, err)) goto fail; break; case LF_TEXT: @@ -886,6 +885,11 @@ int lf_expr_postcheck(struct lf_expr *lf_expr, struct proxy *px, char **err) if (!(px->flags & PR_FL_CHECKED)) px->to_log |= LW_INIT; + /* start with all options, then perform ANDmask with each node's + * options to end with options common to ALL nodes + */ + lf_expr->nodes.options = ~LOG_OPT_NONE; + list_for_each_entry(lf, &lf_expr->nodes.list, list) { if (lf->type == LOG_FMT_EXPR) { struct sample_expr *expr = lf->expr; @@ -898,7 +902,7 @@ int lf_expr_postcheck(struct lf_expr *lf_expr, struct proxy *px, char **err) expr->fetch->kw); goto fail; } - continue; + goto next_node; } /* check if we need to allocate an http_txn struct for HTTP parsing */ /* Note, we may also need to set curpx->to_log with certain fetches */ @@ -927,6 +931,14 @@ int lf_expr_postcheck(struct lf_expr *lf_expr, struct proxy *px, char **err) if (!(px->flags & PR_FL_CHECKED)) px->to_log |= lf->tag->lw; } + next_node: + if (lf->type == LOG_FMT_EXPR || lf->type == LOG_FMT_TAG) { + /* For configurable nodes, apply current node's option + * mask to global node options to keep options common + * to all nodes + */ + lf_expr->nodes.options &= lf->options; + } } if ((px->to_log & (LW_REQ | LW_RESP)) && (px->mode != PR_MODE_HTTP && !(px->options & PR_O_HTTP_UPG))) { -- 2.39.5