From: Willy Tarreau <w@1wt.eu>
Date: Tue, 24 Jun 2025 16:18:18 +0000 (+0200)
Subject: BUG/MEDIUM: config: solve the empty argument problem again
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=1968731765adfdd311c56f8bcbc718680ac3f23a;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: config: solve the empty argument problem again

This mostly reverts commit ff8db5a85 ("BUG/MINOR: config: Stopped parsing
upon unmatched environment variables").

As explained in commit #2367, finally the fix above was incorrect because
it causes other trouble such as this:

     log "192.168.100.${NODE}" "local0"

being resolved to this:

     log 192.168.100.local0

when NODE does not exist due to the loss of the spaces. In fact, while none
of us was well aware of this, when the user had:

     server app 127.0.0.1:80 "${NO_CHECK}" weight 123

in fact they should have written it this way:

     server app 127.0.0.1:80 "${NO_CHECK[*]}" weight 123

so that the variable is expanded to zero, one or multiple words, leaving
no empty arg (like in shell). This is supported since 2.3 with commit
fa41cb6 so the right fix is in the config, let's revert the fix and
properly address the issue.

Some changes are necessary however, since after that patch, the in_arg
checks were added and are now inserting an empty argument even for
proper error reporting. For example, the following statement:

    acl foo path "/a" "${FOO[*]}" "/b"

would complain about an empty arg at FOO due to in_arg=1, while dropping
this in_arg=1 with the following config:

    acl foo path "/a" "${FOO}" "/b"

would silently stop after "/a" instead of complaining about an empty
field. So the approach here consists in noting whether or not something
was written since the quotes were emitted, in order to decide whether
or not to produce an argument. This way, "" continues to be an explicitly
empty arg, just like the same with an unknown variable, while "${FOO[*]}"
is allowed to prevent the creation of an argument if empty.

This should be backported to *some* versions, but the risk that some
configs were altered to rely on the broken fix is not null. At least
recent LTS should be reverted. Note that this requires previous commit:

    BUG/MINOR: config: emit warning for empty args when *not* in discovery mode

otherwise this will break again configs relying on HAPROXY_LOCALPEER and
maybe a few other variables set at the end of discovery.
---

diff --git a/src/tools.c b/src/tools.c
index 3acbc56c7..7f29e807b 100644
--- a/src/tools.c
+++ b/src/tools.c
@@ -6147,6 +6147,7 @@ int is_dir_present(const char *path_fmt, ...)
 		if (!err)					       \
 			out[outpos] = __c;			       \
 		outpos++;					       \
+		empty_quote = 0;				       \
 	} while (0)
 
 /* Parse <in>, copy it into <out> split into isolated words whose pointers
@@ -6194,6 +6195,7 @@ uint32_t parse_line(char *in, char *out, size_t *outlen, char **args, int *nbarg
 	uint32_t err = 0;
 	int in_arg = 0;
 	int prev_in_arg = 0;
+	int empty_quote = 0;
 
 	*nbargs = 0;
 	*outlen = 0;
@@ -6223,26 +6225,30 @@ uint32_t parse_line(char *in, char *out, size_t *outlen, char **args, int *nbarg
 			break;
 		}
 		else if (*in == '"' && !squote && (opts & PARSE_OPT_DQUOTE)) {  /* double quote outside single quotes */
-			in_arg = 1;
 			if (dquote) {
+				if (empty_quote) // force arg creation if empty
+					in_arg = 1;
 				dquote = 0;
 				quote = NULL;
 			}
 			else {
 				dquote = 1;
 				quote = in;
+				empty_quote = 1;
 			}
 			in++;
 		}
 		else if (*in == '\'' && !dquote && (opts & PARSE_OPT_SQUOTE)) { /* single quote outside double quotes */
-			in_arg = 1;
 			if (squote) {
+				if (empty_quote) // force arg creation if empty
+					in_arg = 1;
 				squote = 0;
 				quote = NULL;
 			}
 			else {
 				squote = 1;
 				quote = in;
+				empty_quote = 1;
 			}
 			in++;
 		}
@@ -6452,21 +6458,20 @@ uint32_t parse_line(char *in, char *out, size_t *outlen, char **args, int *nbarg
 					arg_start = outpos;
 				}
 			}
+			else if (word_expand) {
+				/* An empty or unknown env variable was passed as an array.
+				 * This is permitted and results in not creating any arg and
+				 * dropping the surrounding quotes. Here we pretend the quotes
+				 * were not empty, which will be sufficient to avoid creating
+				 * a new arg.
+				 */
+				empty_quote = 0;
+			}
 			else {
-				/* An unmatched environment variable was parsed.
-				 * Let's skip the trailing double-quote character
-				 * and spaces.
+				/* An unmatched environment variable was parsed, it must
+				 * count as an argument.
 				 */
 				in_arg = 1;
-				if (likely(*var_name != '.') && *in == '"') {
-					in++;
-					while (isspace((unsigned char)*in))
-						in++;
-					if (dquote) {
-						dquote = 0;
-						quote = NULL;
-					}
-				}
 			}
 			word_expand = NULL;
 		}