bug in parameter expansion

bug in parameter expansion

[Scott Moser]

This issue was initially reported against shell code delivered in
cloud-init at [1], but it seems to me to be a bug in zsh's posix shell
behavior.

$ for shell in /bin/dash /bin/bash /bin/ksh /bin/zsh; do \
  echo "=== $shell ==="; $shell -c 'echo "${1%%=*}"' \
  -- A=B; done
=== /bin/dash ===
A
=== /bin/bash ===
A
=== /bin/ksh ===
A
=== /bin/zsh ===
zsh:1: * not found

It also has the issue when matching prefix (#):
$ zsh -c 'echo ${1#=?}' -- =AB
zsh:1: ? not found
$ bash -c 'echo ${1#=?}' -- =AB
B

It is seemingly easily worked around by escaping the '=' like:
  $ zsh -c 'echo "${1%%\=*}"' -- A=B
  A
or:
  zsh -c 'equal="="; echo "${1%%${equal}*}"' -- A=B
  A

Scott
--
[1] https://bugs.launchpad.net/ubuntu/+source/cloud-init/+bug/1073077

Re: bug in parameter expansion

[Peter Stephenson]

On Wed, 7 Nov 2012 14:13:36 -0500 (EST)
Scott Moser <smoser@ubuntu.com> wrote:
> This issue was initially reported against shell code delivered in
> cloud-init at [1], but it seems to me to be a bug in zsh's posix shell
> behavior.

As it happens, that's not quite true, because zsh doesn't have POSIX
behaviour by default, and it will work as you expect if you invoke it as
sh or "setopt noequals".

It's still pretty funny, even in the native mode, though.  That's a very
strange place for =-expansion to happen.

It's happening because we look at the string "=*" and we tokenise that to
make patterns (such as the "*") active if it was in quotes.  We tokenise
the "=" the same as if it were a command line argument.  I can't think
of any occasion we'd want to do that if we know it's *not* a command
line argument: there's already a flag for this, so the fix is simple.
Maybe someone can think of a pathological case where we should do the
=-expansion (or =(...) expansion) even in a substituted string, but
I can't think of one.  This doesn't trigger any test failures.

Index: Src/lex.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/lex.c,v
retrieving revision 1.73
diff -p -u -r1.73 lex.c
--- Src/lex.c	5 Oct 2012 21:35:06 -0000	1.73
+++ Src/lex.c	7 Nov 2012 20:37:05 -0000
@@ -1265,51 +1265,53 @@ gettokstr(int c, int sub)
 		break;
 	    goto brk;
 	case LX2_EQUALS:
-	    if (intpos) {
-		e = hgetc();
-		if (e != '(') {
-		    hungetc(e);
-		    lexstop = 0;
-		    c = Equals;
-		} else {
-		    add(Equals);
-		    if (skipcomm()) {
-			peek = LEXERR;
-			goto brk;
-		    }
-		    c = Outpar;
-		}
-	    } else if (!sub && peek != ENVSTRING &&
-		       incmdpos && !bct && !brct) {
-		char *t = tokstr;
-		if (idigit(*t))
-		    while (++t < bptr && idigit(*t));
-		else {
-		    int sav = *bptr;
-		    *bptr = '\0';
-		    t = itype_end(t, IIDENT, 0);
-		    if (t < bptr) {
-			skipparens(Inbrack, Outbrack, &t);
+	    if (!sub) {
+		if (intpos) {
+		    e = hgetc();
+		    if (e != '(') {
+			hungetc(e);
+			lexstop = 0;
+			c = Equals;
 		    } else {
-			*bptr = sav;
+			add(Equals);
+			if (skipcomm()) {
+			    peek = LEXERR;
+			    goto brk;
+			}
+			c = Outpar;
 		    }
-		}
-		if (*t == '+')
-                    t++;
-		if (t == bptr) {
-		    e = hgetc();
-		    if (e == '(' && incmdpos) {
+		} else if (peek != ENVSTRING &&
+			   incmdpos && !bct && !brct) {
+		    char *t = tokstr;
+		    if (idigit(*t))
+			while (++t < bptr && idigit(*t));
+		    else {
+			int sav = *bptr;
 			*bptr = '\0';
-			return ENVARRAY;
+			t = itype_end(t, IIDENT, 0);
+			if (t < bptr) {
+			    skipparens(Inbrack, Outbrack, &t);
+			} else {
+			    *bptr = sav;
+			}
 		    }
-		    hungetc(e);
-		    lexstop = 0;
-		    peek = ENVSTRING;
-		    intpos = 2;
+		    if (*t == '+')
+			t++;
+		    if (t == bptr) {
+			e = hgetc();
+			if (e == '(' && incmdpos) {
+			    *bptr = '\0';
+			    return ENVARRAY;
+			}
+			hungetc(e);
+			lexstop = 0;
+			peek = ENVSTRING;
+			intpos = 2;
+		    } else
+			c = Equals;
 		} else
 		    c = Equals;
-	    } else
-		c = Equals;
+	    }
 	    break;
 	case LX2_BKSLASH:
 	    c = hgetc();

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: bug in parameter expansion

[Bart Schaefer]

On Nov 7,  8:45pm, Peter Stephenson wrote:
}
} It's happening because we look at the string "=*" and we tokenise that to
} make patterns (such as the "*") active if it was in quotes.  We tokenise
} the "=" the same as if it were a command line argument.  I can't think
} of any occasion we'd want to do that if we know it's *not* a command
} line argument: there's already a flag for this, so the fix is simple.

This looks OK to me, but just to be sure I understand:  The flag you're
talking about in this context is "sub" rather than "incmdpos"?  For those
who may be attempting to learn the code, "incmdpos" really does mean "in
the command position", e.g., the first word in the buffer or the first
word after a separator such as ";" or "|", among other cases.

The "sub" flag doesn't directly mean "not a command line argument" here,
unless I'm mistaken?

Re: bug in parameter expansion

[Peter Stephenson]

On Thu, 08 Nov 2012 06:53:31 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Nov 7,  8:45pm, Peter Stephenson wrote:
> } It's happening because we look at the string "=*" and we tokenise
> } that to make patterns (such as the "*") active if it was in
> } quotes.  We tokenise the "=" the same as if it were a command line
> } argument.  I can't think of any occasion we'd want to do that if
> } we know it's *not* a command line argument: there's already a flag
> } for this, so the fix is simple.
> 
> This looks OK to me, but just to be sure I understand:  The flag
> you're talking about in this context is "sub" rather than
> "incmdpos"?  For those who may be attempting to learn the code,
> "incmdpos" really does mean "in the command position", e.g., the
> first word in the buffer or the first word after a separator such as
> ";" or "|", among other cases.

Yes, the flag I'm talking about is "sub".

I think you probably mean intpos rather than incmdpos; it says we're at
the start of the word, which is the only place you want to do =cmd
expansion.  However, I don't think the code even relies on that --- it
tokenises the "=" and relies on the fact that expansion later only looks
for an active =cmd expansion by checking the first character in the
argument.  Later still tokens are turned back to normal characters.

> The "sub" flag doesn't directly mean "not a command line argument"
> here, unless I'm mistaken?

Umm, yes and no.  No, it's not explicitly saying that.  However, it is
only used in a context where we're doing some substitution to a string
that isn't by itself a command line argument, though it may form part of
a command line argument later.

I think that's OK because at all such points the command line
tokenisation has taken place and it's the normal zsh behaviour that
substitution and globbing doesn't take place on substituted strings.  If
GLOB_SUBST is set, the (re)tokenisation takes place later, because we
don't expect to get tokens out of the early stages of expansion.

pws