[PARSER] Report substition errors at expansion time

On Wed, Apr 11, 2007 at 01:24:21PM -0700, Micah Cowan wrote:
> Package: dash
> Version: 0.5.3-3
> 
> Bug first reported against Ubuntu at 
> https://bugs.launchpad.net/ubuntu/+source/dash/+bug/105634
> by Paul Smith
> 
> The description and some comments from that bug report follow.
> 
> -----
> 
> 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

2 files changed, 8 insertions, 3 deletions

diff --git a/src/expand.c b/src/expand.c
index e3e5d8f..54fe908 100644
--- a/src/expand.c
+++ b/src/expand.c
@@ -749,6 +749,10 @@ evalvar(char *p, int flag)
 
 	varflags = *p++;
 	subtype = varflags & VSTYPE;
+
+	if (!subtype)
+		sh_error("Bad substitution");
+
 	quoted = flag & EXP_QUOTED;
 	var = p;
 	easy = (!quoted || (*var == '@' && shellparam.nparam));
diff --git a/src/parser.c b/src/parser.c
index d0e0553..4b8a5fe 100644
--- a/src/parser.c
+++ b/src/parser.c
@@ -1204,9 +1204,8 @@ varname:
 			USTPUTC(cc, out);
 		}
 		else
-badsub:			synerror("Bad substitution");
+			goto badsub;
 
-		STPUTC('=', out);
 		if (subtype == 0) {
 			switch (c) {
 			case ':':
@@ -1216,7 +1215,7 @@ badsub:			synerror("Bad substitution");
 			default:
 				p = strchr(types, c);
 				if (p == NULL)
-					goto badsub;
+					break;
 				subtype |= p - types + VSNORMAL;
 				break;
 			case '%':
@@ -1234,6 +1233,7 @@ badsub:			synerror("Bad substitution");
 				}
 			}
 		} else {
+badsub:
 			pungetc();
 		}
 		*((char *)stackblock() + typeloc) = subtype;
@@ -1242,6 +1242,7 @@ badsub:			synerror("Bad substitution");
 			if (dblquote)
 				dqvarnest++;
 		}
+		STPUTC('=', out);
 	}
 	goto parsesub_return;
 }