[PATCH v2] expand: Fix multiple issues with EXP_DISCARD in evalvar

[Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>]

On Fri, Sep 07, 2018 at 06:55:29AM +0200, Martijn Dekker wrote:
>
> But now I'm encountering another, similar bug, that was a bit harder to
> track down:
> 
> $ src/dash -c 'foo=bar; echo ${foo=BUG}; echo $foo'
> barBUG
> bar
> $ src/dash -c 'foo=bar; echo ${foo:=BUG}; echo $foo'
> barBUG
> bar
> 
> Expected output: 'bar' twice in both cases. The ${foo=BUG} and ${foo:=BUG}
> expansions fail to discard the word 'BUG' if foo is set.

Thanks for the update.  In this case we didn't call the parsing
function subevalvar at all when we should have called it with
EXP_DISCARD.

As patchwork was having issues I've rolled all three patches into
one.

Cheers,

---8<---
The commit 3cd538634f71538370f5af239f342aec48b7470b broke parameter
expansion in multiple ways because the EXP_DISCARD flag wasn't set
or tested for various cases:

	$ src/dash -c 'var=; echo ${var:+nonempty}'
	nonempty
        $ src/dash -u -c 'unset foo bar; echo ${foo+${bar}}'
        dash: 1: bar: parameter not set
        $ src/dash -c 'foo=bar; echo ${foo=BUG}; echo $foo'
        barBUG
        bar
        $

This patch fixes them by introducing a new discard variable that
tracks whether the extra word should be discarded or not when it
is parsed.

Reported-by: Martijn Dekker <martijn@xxxxxxxx>
Fixes: 3cd538634f71 ("expand: Do not reprocess data when...")
Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>

diff --git a/src/expand.c b/src/expand.c
index 14daa63..856b7a9 100644
--- a/src/expand.c
+++ b/src/expand.c
@@ -698,6 +698,7 @@ evalvar(char *p, int flag)
 	int patloc;
 	int startloc;
 	ssize_t varlen;
+	int discard;
 	int quoted;
 
 	varflags = *p++;
@@ -713,41 +714,41 @@ again:
 	if (varflags & VSNUL)
 		varlen--;
 
+	discard = varlen < 0 ? EXP_DISCARD : 0;
+
 	switch (subtype) {
 	case VSPLUS:
-		varlen = -1 - varlen;
+		discard ^= EXP_DISCARD;
 		/* fall through */
 
 	case 0:
 	case VSMINUS:
-		p = argstr(p, flag | EXP_TILDE | EXP_WORD);
-		if (varlen < 0)
-			return p;
+		p = argstr(p, flag | EXP_TILDE | EXP_WORD |
+			      (discard ^ EXP_DISCARD));
 		goto record;
 
 	case VSASSIGN:
 	case VSQUESTION:
-		if (varlen >= 0)
-			goto record;
-
 		p = subevalvar(p, var, 0, startloc, varflags,
-			       flag & ~QUOTES_ESC);
+			       (flag & ~QUOTES_ESC) |
+			       (discard ^ EXP_DISCARD));
 
-		if (flag & EXP_DISCARD)
-			return p;
+		if ((flag | ~discard) & EXP_DISCARD)
+			goto record;
 
 		varflags &= ~VSNUL;
+		subtype = VSNORMAL;
 		goto again;
 	}
 
-	if (varlen < 0 && uflag)
+	if ((discard & ~flag) && uflag)
 		varunset(p, var, 0, 0);
 
 	if (subtype == VSLENGTH) {
 		if (flag & EXP_DISCARD)
 			return p;
 		cvtnum(varlen > 0 ? varlen : 0, flag);
-		goto record;
+		goto really_record;
 	}
 
 	if (subtype == VSNORMAL)
@@ -765,7 +766,7 @@ again:
 	}
 #endif
 
-	flag |= varlen < 0 ? EXP_DISCARD : 0;
+	flag |= discard;
 	if (!(flag & EXP_DISCARD)) {
 		/*
 		 * Terminate the string and start recording the pattern
@@ -778,9 +779,10 @@ again:
 	p = subevalvar(p, NULL, patloc, startloc, varflags, flag);
 
 record:
-	if (flag & EXP_DISCARD)
+	if ((flag | discard) & EXP_DISCARD)
 		return p;
 
+really_record:
 	if (quoted) {
 		quoted = *var == '@' && shellparam.nparam;
 		if (!quoted)
-- 
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt