[git commit] ash: [PARSER] Report substition errors at expansion time

[Denys Vlasenko]

commit: https://git.busybox.net/busybox/commit/?id=88e15703acdbfb182440cf35fdb8972fc4931dd2
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master

Upstreams commit:

    Date: Mon, 8 Oct 2007 21:32:25 +0800
    [PARSER] Report substition errors at expansion time

    On Wed, Apr 11, 2007 at 01:24:21PM -0700, Micah Cowan wrote:
    > This operation fails on Ubuntu:
    >
    >     $ /bin/sh -c 'if false; then d="${foo/bar}"; fi'
    >     /bin/sh: Syntax error: Bad substitution
    >
    > When used with other POSIX shells it succeeds. While semantically the
    > variable reference ${foo/bar} is not valid, this is not a syntax error
    > according to POSIX, and since the variable assignment expression is
    > never invoked (because it's within an "if false") it should not be seen
    > as an error.
    >
    > I ran into this because after restarting my system I could no longer log
    > in. It turns out that the problem was (a) I had edited .gnomerc to
    > source my .bashrc file so that my environment would be set properly, and
    > (b) I had added some new code to my .bashrc WITHIN A CHECK FOR BASH!
    > that used bash's ${var/match/sub} feature. Even though this code was
    > within a "case $BASH_VERSION; in *[0-9]*) ... esac (so dash would never
    > execute it since that variable is not set), it still caused dash to
    > throw up.
    >
    > FYI, some relevant details from POSIX:
    >
    > Section 2.3, Token Recognition:
    >
    > 5. If the current character is an unquoted '$' or '`', the shell shall
    > identify the start of any candidates for parameter expansion ( Parameter
    > Expansion), command substitution ( Command Substitution), or arithmetic
    > expansion ( Arithmetic Expansion) from their introductory unquoted
    > character sequences: '$' or "${", "$(" or '`', and "$((", respectively.
    > The shell shall read sufficient input to determine the end of the unit
    > to be expanded (as explained in the cited sections).
    >
    > Section 2.6.2, Parameter Expansion:
    >
    > The format for parameter expansion is as follows:
    >
    >     ${expression}
    >
    > where expression consists of all characters until the matching '}'. Any
    > '}' escaped by a backslash or within a quoted string, and characters in
    > embedded arithmetic expansions, command substitutions, and variable
    > expansions, shall not be examined in determining the matching '}'.
    > [...]
    >
    > The parameter name or symbol can be enclosed in braces, which are
    > optional except for positional parameters with more than one digit or
    > when parameter is followed by a character that could be interpreted as
    > part of the name. The matching closing brace shall be determined by
    > counting brace levels, skipping over enclosed quoted strings, and
    > command substitutions.
    > ---
    > In addition to bash I've checked Solaris /bin/sh and ksh and they don't
    > report an error.
    >
    > -----
    > Micah Cowan:
    >
    > The applicable portion of POSIX is in XCU 2.10.1:
    >
    > "The WORD tokens shall have the word expansion rules applied to them
    > immediately before the associated command is executed, not at the time
    > the command is parsed."
    >
    > This seems fairly clear to me.

    This patch moves the error detection to expansion time.

    Test case:

    	if false; then
    		echo ${a!7}
    	fi
    	echo OK

    Old result:

	dash: Syntax error: Bad substitution

    New result:

	OK

function                                             old     new   delta
evalvar                                              574     585     +11
readtoken1                                          2763    2750     -13

Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
 shell/ash.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/shell/ash.c b/shell/ash.c
index dc26bc1..e30d7fe 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -6752,6 +6752,10 @@ evalvar(char *p, int flag, struct strlist *var_str_list)
 
 	varflags = (unsigned char) *p++;
 	subtype = varflags & VSTYPE;
+
+	if (!subtype)
+		raise_error_syntax("bad substitution");
+
 	quoted = flag & EXP_QUOTED;
 	var = p;
 	easy = (!quoted || (*var == '@' && shellparam.nparam));
@@ -11699,7 +11703,7 @@ parseredir: {
 parsesub: {
 	unsigned char subtype;
 	int typeloc;
-	int flags;
+	int flags = 0;
 
 	c = pgetc_eatbnl();
 	if (c > 255 /* PEOA or PEOF */
@@ -11759,15 +11763,13 @@ parsesub: {
 			USTPUTC(c, out);
 			c = pgetc_eatbnl();
 		} else {
- badsub:
-			raise_error_syntax("bad substitution");
+			goto badsub;
 		}
 		if (c != '}' && subtype == VSLENGTH) {
 			/* ${#VAR didn't end with } */
 			goto badsub;
 		}
 
-		STPUTC('=', out);
 		flags = 0;
 		if (subtype == 0) {
 			static const char types[] ALIGN1 = "}-+?=";
@@ -11784,7 +11786,7 @@ parsesub: {
 				if (!strchr(types, c)) {
 					subtype = VSSUBSTR;
 					pungetc();
-					break; /* "goto do_pungetc" is bigger (!) */
+					break; /* "goto badsub" is bigger (!) */
 				}
 #endif
 				flags = VSNUL;
@@ -11792,7 +11794,7 @@ parsesub: {
 			default: {
 				const char *p = strchr(types, c);
 				if (p == NULL)
-					goto badsub;
+					break;
 				subtype = p - types + VSNORMAL;
 				break;
 			}
@@ -11802,7 +11804,7 @@ parsesub: {
 				subtype = (c == '#' ? VSTRIMLEFT : VSTRIMRIGHT);
 				c = pgetc_eatbnl();
 				if (c != cc)
-					goto do_pungetc;
+					goto badsub;
 				subtype++;
 				break;
 			}
@@ -11814,13 +11816,13 @@ parsesub: {
 				subtype = VSREPLACE;
 				c = pgetc_eatbnl();
 				if (c != '/')
-					goto do_pungetc;
+					goto badsub;
 				subtype++; /* VSREPLACEALL */
 				break;
 #endif
 			}
 		} else {
- do_pungetc:
+ badsub:
 			pungetc();
 		}
 		((unsigned char *)stackblock())[typeloc] = subtype | flags;
@@ -11830,6 +11832,7 @@ parsesub: {
 				dqvarnest++;
 			}
 		}
+		STPUTC('=', out);
 	}
 	goto parsesub_return;
 }