[U-Boot] [PATCH V2] hush: fix some quoted variable expansion issues

[Stephen Warren]

The following shell command fails:

if test -z "$x"; then echo "zero"; else echo "non-zero"; fi

(assuming $x does not exist, it prints "non-zero" rather than "zero").

... since "$x" expands to nothing, and the argument is completely
dropped, causing too few to be passed to -z, causing cmd_test() to
error out early.

This is because when variable expansions are processed by make_string(),
the expanded results are concatenated back into a new string. However,
no quoting is applied when doing so, so any empty variables simply don't
generate any parameter when the combined string is parsed again.

Fix this by explicitly replacing quoting any argument that was originally
quoted when re-generating a string from the already-parsed argument list.

This also fixes loss of whitespace in commands such as:

setenv space " "
setenv var " 1${space}${space} 2 "
echo ">>${var}<<"

Reported-by: Russell King <linux at arm.linux.org.uk>
Signed-off-by: Stephen Warren <swarren at wwwdotorg.org>
---
v2:
* Save the "nonnull" value from the argument parser, and pass this to
  make_string(), so that it only quotes the values in the resultant
  string if the original value was quoted.
* Add some more unit tests to ensure that whitespace in quoted text is
  expanded and re-concatenated without loss.
* Revert accidental s/SUBSTED_VAR_SYMBOL/q/
---
 common/hush.c     | 24 +++++++++++++++++++-----
 test/command_ut.c | 17 +++++++++++++++++
 2 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/common/hush.c b/common/hush.c
index 3f3a79c..66ece41 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -221,6 +221,7 @@ struct child_prog {
 	pid_t pid;					/* 0 if exited */
 #endif
 	char **argv;				/* program name and arguments */
+	int *argv_nonnull;
 #ifdef __U_BOOT__
 	int    argc;                            /* number of program arguments */
 #endif
@@ -467,7 +468,7 @@ static int process_command_subs(o_string *dest, struct p_context *ctx, struct in
 static int parse_group(o_string *dest, struct p_context *ctx, struct in_str *input, int ch);
 #endif
 static char *lookup_param(char *src);
-static char *make_string(char **inp);
+static char *make_string(char **inp, int *nonnull);
 static int handle_dollar(o_string *dest, struct p_context *ctx, struct in_str *input);
 #ifndef __U_BOOT__
 static int parse_string(o_string *dest, struct p_context *ctx, const char *src);
@@ -1613,7 +1614,8 @@ static int run_pipe_real(struct pipe *pi)
 		if (child->sp) {
 			char * str = NULL;
 
-			str = make_string((child->argv + i));
+			str = make_string(child->argv + i,
+					  child->argv_nonnull + i);
 			parse_string_outer(str, FLAG_EXIT_FROM_LOOP | FLAG_REPARSING);
 			free(str);
 			return last_return_code;
@@ -1940,7 +1942,8 @@ static int free_pipe(struct pipe *pi, int indent)
 			for (a = 0; a < child->argc; a++) {
 				free(child->argv[a]);
 			}
-					free(child->argv);
+			free(child->argv);
+			free(child->argv_nonnull);
 			child->argc = 0;
 #endif
 			child->argv=NULL;
@@ -2470,8 +2473,14 @@ static int done_word(o_string *dest, struct p_context *ctx)
 		argc = ++child->argc;
 		child->argv = realloc(child->argv, (argc+1)*sizeof(*child->argv));
 		if (child->argv == NULL) return 1;
+		child->argv_nonnull = realloc(child->argv_nonnull,
+					(argc+1)*sizeof(*child->argv_nonnull));
+		if (child->argv_nonnull == NULL)
+			return 1;
 		child->argv[argc-1]=str;
+		child->argv_nonnull[argc-1] = dest->nonnull;
 		child->argv[argc]=NULL;
+		child->argv_nonnull[argc] = 0;
 		for (s = dest->data; s && *s; s++,str++) {
 			if (*s == '\\') s++;
 			*str = *s;
@@ -2537,6 +2546,7 @@ static int done_command(struct p_context *ctx)
 	prog->redirects = NULL;
 #endif
 	prog->argv = NULL;
+	prog->argv_nonnull = NULL;
 #ifndef __U_BOOT__
 	prog->is_stopped = 0;
 #endif
@@ -3586,7 +3596,7 @@ static char **make_list_in(char **inp, char *name)
 }
 
 /* Make new string for parser */
-static char * make_string(char ** inp)
+static char *make_string(char **inp, int *nonnull)
 {
 	char *p;
 	char *str = NULL;
@@ -3600,13 +3610,17 @@ static char * make_string(char ** inp)
 		noeval = 1;
 	for (n = 0; inp[n]; n++) {
 		p = insert_var_value_sub(inp[n], noeval);
-		str = xrealloc(str, (len + strlen(p)));
+		str = xrealloc(str, (len + strlen(p) + (2 * nonnull[n])));
 		if (n) {
 			strcat(str, " ");
 		} else {
 			*str = '\0';
 		}
+		if (nonnull[n])
+			strcat(str, "'");
 		strcat(str, p);
+		if (nonnull[n])
+			strcat(str, "'");
 		len = strlen(str) + 3;
 		if (p != inp[n]) free(p);
 	}
diff --git a/test/command_ut.c b/test/command_ut.c
index 620a297..56041e9 100644
--- a/test/command_ut.c
+++ b/test/command_ut.c
@@ -137,6 +137,23 @@ static int do_ut_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	HUSH_TEST(or_1_0_inv_inv, "! ! aaa = aaa -o ! ! bbb != bbb", y);
 	HUSH_TEST(or_1_1_inv_inv, "! ! aaa = aaa -o ! ! bbb = bbb", y);
 
+	setenv("ut_var_nonexistent", NULL);
+	setenv("ut_var_exists", "1");
+	HUSH_TEST(z_varexp_quoted, "-z \"$ut_var_nonexistent\"", y);
+	HUSH_TEST(z_varexp_quoted, "-z \"$ut_var_exists\"", n);
+	setenv("ut_var_exists", NULL);
+
+	run_command("setenv ut_var_space \" \"", 0);
+	assert(!strcmp(getenv("ut_var_space"), " "));
+	run_command("setenv ut_var_test $ut_var_space", 0);
+	assert(!getenv("ut_var_test"));
+	run_command("setenv ut_var_test \"$ut_var_space\"", 0);
+	assert(!strcmp(getenv("ut_var_test"), " "));
+	run_command("setenv ut_var_test \" 1${ut_var_space}${ut_var_space} 2 \"", 0);
+	assert(!strcmp(getenv("ut_var_test"), " 1   2 "));
+	setenv("ut_var_space", NULL);
+	setenv("ut_var_test", NULL);
+
 #ifdef CONFIG_SANDBOX
 	/*
 	 * File existence
-- 
1.8.3.2