From: Aurelien DARRAGON <adarragon@haproxy.com>
Date: Mon, 29 Apr 2024 13:58:36 +0000 (+0200)
Subject: BUG/MINOR: log: fix global lf_expr node options behavior (2nd try)
X-Git-Tag: v3.0-dev10~36
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9931a62c3fd5a2252fe603833b717652302af1d5;p=thirdparty%2Fhaproxy.git

BUG/MINOR: log: fix global lf_expr node options behavior (2nd try)

In 98b44e8 ("BUG/MINOR: log: fix global lf_expr node options behavior"),
I properly restored global node options behavior for when encoding is
not used, however the fix is not optimal when encoding is involved:

Indeed, encoding logic in sess_build_logline() relies on global node
options to know if encoding must be handled expression-wide or
individually. However, because of the above fix, if an expression is
made of 1 or multiple nodes that all set an encoding option manually
(without '%o'), we consider that the option was set globally, but
that's probably not what the user intended. Instead we should only
evaluate global options from '%o', so that it remains possible to
skip global encoding when needed.

No backport needed.
---

diff --git a/include/haproxy/log-t.h b/include/haproxy/log-t.h
index f563968749..e3a3e8f41b 100644
--- a/include/haproxy/log-t.h
+++ b/include/haproxy/log-t.h
@@ -189,7 +189,7 @@ struct lf_expr {
 	union {
 		struct {
 			struct list list; /* logformat_node list */
-			int options;      /* global options (common to all nodes) */
+			int options;      /* global '%o' options (common to all nodes) */
 		} nodes;
 		char *str;                /* original string prior to parsing (NULL once compiled) */
 	};
diff --git a/src/log.c b/src/log.c
index 3b78bc09e0..ee22e94ac1 100644
--- a/src/log.c
+++ b/src/log.c
@@ -442,6 +442,21 @@ static int parse_logformat_tag(char *arg, int arg_len, char *name, int name_len,
 			}
 			if (node->tag->type == LOG_FMT_GLOBAL) {
 				*defoptions = node->options;
+				if (lf_expr->nodes.options == LOG_OPT_NONE)
+					lf_expr->nodes.options = node->options;
+				else {
+					/* global options were previously set and were
+					 * overwritten for nodes that appear after the
+					 * current one.
+					 *
+					 * However, for lf_expr->nodes.options we must
+					 * keep a track of options common to ALL nodes,
+					 * thus we take previous global options into
+					 * account to compute the new logformat
+					 * expression wide (global) node options.
+					 */
+					lf_expr->nodes.options &= node->options;
+				}
 				free_logformat_node(node);
 			} else {
 				LIST_APPEND(list_format, &node->list);
@@ -620,6 +635,7 @@ int lf_expr_compile(struct lf_expr *lf_expr,
 	 * returning.
 	 */
 	LIST_INIT(&lf_expr->nodes.list);
+	lf_expr->nodes.options = LOG_OPT_NONE;
 	/* we must set the compiled flag now for proper deinit in case of failure */
 	lf_expr->flags |= LF_FL_COMPILED;
 
@@ -885,11 +901,6 @@ 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;
@@ -902,7 +913,7 @@ int lf_expr_postcheck(struct lf_expr *lf_expr, struct proxy *px, char **err)
 					          expr->fetch->kw);
 					goto fail;
 				}
-				goto next_node;
+				continue;
 			}
 			/* 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 */
@@ -931,14 +942,6 @@ 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_NODE_WITH_OPT(lf)) {
-			/* 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))) {