From 88e15703acdbfb182440cf35fdb8972fc4931dd2 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Wed, 26 Oct 2016 01:55:56 +0200 Subject: [PATCH] ash: [PARSER] Report substition errors at expansion time 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 --- shell/ash.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/shell/ash.c b/shell/ash.c index dc26bc142..e30d7fe67 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; } -- 2.25.1