]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MINOR: log: fix global lf_expr node options behavior
authorAurelien DARRAGON <adarragon@haproxy.com>
Mon, 29 Apr 2024 09:55:27 +0000 (11:55 +0200)
committerAurelien DARRAGON <adarragon@haproxy.com>
Mon, 29 Apr 2024 12:47:37 +0000 (14:47 +0200)
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

index e3ab29a5034e99e89a1bd1c21b32ba2f96a7b087..a7cc713fcae3f526768c0825395b38266756b044 100644 (file)
--- 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))) {