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.
This commit is contained in:
Aurelien DARRAGON 2024-04-29 11:55:27 +02:00
parent 9bdea51d7e
commit 98b44e8edb

View File

@ -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))) {