parser confusions with braces inside ${ cmd; } form of command substitution

[stephane-chazelas]

Using braces inside ${ ...; } command substitution sometimes causes some `{' unmatched errors:

$ ksh -c 'echo ${ echo {a.b}; }'
ksh: syntax error at line 1: `{' unmatched
$ ksh -c 'echo ${ echo "{a,b}c; }'
ksh: syntax error at line 1: `"' unmatched
$ ksh -c 'echo ${ echo {fd[0]}< /dev/null; }'
ksh: syntax error at line 1: `{' unmatched

These are OK for some reason:

$ ksh -c 'echo ${ echo {acb}; }'
{acb}
$ ksh -c 'echo ${ echo {a b}; }'
{a b}
$ ksh -c 'echo ${ echo x{a,b} ;}'
xa xb
$ ksh -c 'echo ${ echo ${0}; }'
ksh
$ ksh -c 'echo ${ a={a,b}c; echo $a }'
ac bc

Quoting the cmdsubst doesn't help:

$ ksh -c 'echo "${ echo {a,b}; }"'
ksh: syntax error at line 1: `"' unmatched

Escaping/quoting the { avoids the error:

$ ksh -c 'echo ${ echo \{a,b}c; }'
ac bc
$ ksh -c 'echo "{"a,b}c'
{a,b}c
$ ksh -c 'echo ${ echo "{"a,b}c; }'
ac bc

I guess the brace expansion is done on the output of unquoted command substitution:

$ ksh -o posix -c 'echo ${ echo \{a,b} ;}'
{a,b}

$ ksh --version
  version         sh (AT&T Research) 93u+m/1.0.4 2022-10-22

[McDutchie]

This is the reason for bugs like these:

ksh/src/cmd/ksh93/sh/lex.c

Lines 1497 to 1523 in 4dacec2

	/*
	* read to end of command substitution
	* of the form $(...) or ${ ...;}
	* or arithmetic expansion $((...))
	*
	* Ugly hack alert: At parse time, command substitutions and arithmetic expansions are read
	* without parsing, using lexical analysis only. This is only to determine their length, so
	* that their literal source text can be stored in the parse tree. They are then actually
	* parsed at runtime (!) each time they are executed (!) via comsubst() in macro.c.
	*
	* This approach is okay for arithmetic expansions, but for command substitutions it is an
	* unreliable hack. The lexer does not have real shell grammar knowledge; that's what the
	* parser is for. However, a clean separation between lexical analysis and parsing is not
	* possible, because the design of the shell language is fundamentally messy. So we need the
	* parser to set the some flags in the lexer at the appropriate times to avoid spurious
	* syntax errors (these are the non-private Lex_t struct members). But the parser obviously
	* cannot do this if we're not using it.
	*
	* The comsub() hack below, along with all the dolparen checks in the lexer, tries to work
	* around this fundamental problem as best we can to make it work in all but corner cases.
	* It sets the lexd.dolparen, lexd.dolparen_eqparen and lexd.dolparen_arithexp flags for the
	* rest of the lexer code to execute lots of workarounds.
	*
	* TODO: to achieve correctness, actually parse command substitutions at parse time.
	*/
	static int comsub(Lex_t *lp, int endtok)
	{

So far, my experiments in that direction have not been successful.

[phidebian]

@McDutchie before adressing @stephane-chazelas Issue#691 there is another profound bug that need to be arbitrated.

Pardon the abuse of href in the following docco excerp, that's the only way I found to highlight the text, and note as well the cited text is not formatted as CODE i.e <pre> because it defeat the href.

The docco sez

Command Substitution.
The standard output from a command list enclosed in parentheses pre‐
ceded by a dollar sign ( $(list) ), or in a brace group preceded by a
dollar sign ( ${ list;} ), or in a pair of grave accents (``) may be
used as part or all of a word; trailing new-lines are removed. In the
second case, the { and } are treated as a reserved words so that { must
be followed by a blank and } must appear at the beginning of the line
or follow a ;. In the third (obsolete) form, the string between the
quotes is processed for special quoting characters before the command
is executed (see Quoting below). The command substitution $(cat file)
can be replaced by the equivalent but faster $(<file). The command
substitution $(n<#) will expand to the current byte offset for file de‐
scriptor n. Except for the second form, the command list is run in a
subshell so that no side effects are possible. For the second form,
the final } will be recognized as a reserved word after any token.

I found it confusing, because I can't say in the highlighted text above if In the second case... is the same as For the second form.

At the moment I assume they both apply to ${ list; } and if so they seems to contradict themselves, the first one sez that ${ list... must be closed by '\n}' or ';}' and the second one seem to say ${ list... is closed by a '}' following any token (like a word).

The implementation seem to favor the second form and reject the first form as seen here

function ec1
{ echo "[EC1 $@ EC1]"
}

function ec2
{ echo "[EC2 $@ EC2]"
}

ec1 ${ ec2 word } } }    
ec1 ${ ec2 word } } ; }   

$ ./arch/linux.i386-64/bin/ksh ~/bug691.ksh
[EC1 [EC2 word EC2] } } EC1]
/home/phi/bug691.ksh: line 9: syntax error at line 10: `}' unexpected

As we can see here '${...' is closed on first '}' as specified by For the second form, the final } will be recognized as a reserved word after any token.

Yet the 'ec1 ${ ec2 word } } ; }' is unrecognised despite what the docco sez.

Bash (in its latest version) works with the ';}' close (that make sense to me) and barf if '[;\n]}' is not found

$ cat  ~/bug691.sh
function ec1
{ echo "[EC1 $@ EC1]"
}

function ec2
{ echo "[EC2 $@ EC2]"
}

ec1 ${ ec2 word } } ; }    
ec1 ${ ec2 word } }   
}
ec1 ${ ec2 word } } }    

┌─bug-691 /d1/ksh-latest
└─PW$ /d1/bash-oot/B-dbg/bash ~/bug691.sh
[EC1 [EC2 word } } EC2] EC1]
[EC1 [EC2 word } } EC2] EC1]
/home/phi/bug691.sh: line 14: unexpected EOF while looking for matching `}'

The big problem here is that if we fix it so that '[;\n]}' become the closing for '${...}' then it break an handfull QA tests, making me thinking that may be out there there are some now relying on 'ec1 ${ ec2 word } } }' working like it works today, that is really odd IMHO.
Let me know if you think we should implement the '[;\n]}' close, if so it imply

• Break existing script that rely on current implemention.
• This include a significant amount of QA test.

May be the fix should be implemented through an option (May be bash compat kind TBD) and only when this option is turned on the closing would be '[;\n]}' .

Lemme know your thinking.

Only when '[;\n]}' would work then 'echo ${ echo {a.b}; }' would work too.

Cheers

[McDutchie]

Yes, that's yet another profoundly misguided AT&T hack. I noticed it some time ago but it looks like I hadn't yet noticed it when I made my previous comment in this thread.

They made v=${ echo foo } work without the terminating ; that is required for correct processing of { and } as shell keywords/reserved words, and of course this hack introduced bugs (like v=${ echo foo } ; } not working without quoting the } argument to echo.

Unfortunately we can't fix it now without breaking backward compatibility, because this is the sort of thing that multiple existing scripts are likely to rely on.

On the other hand, perhaps it's worth doing for the 1.1 release and up, because this is some egregious breakage. It might well be due to this hack that all of Stéphane's reproducers above break.

[phidebian]

Ok, I've been warned, thanx, I will look at ideas as time permit, I'm about to go offline for 2 weeks... I let you know my findings (if any...)

Cheers

[phidebian]

I just pushed a PR candidate #868 for this issue #691.

The proposed fix aim at fixing another issue that is the implementation of ${ list ;} actually implement something else that is ${ list } with the closing of ${ done with the first token (word) }.

The documented ${ list ;} is never recognised when non reserved word } are mixed inside the ${ list ;} like ${ list...}... ;} the inside ...}... do close the ${ list} before reaching the real ;} or \n}

Because the original implementation (AT&T?) first token } predate the implemention of a closing ;} or \n} I suggest the iplementation of a new ksh93 option, in the patch it is called set -o comsub_brace_greedy it is off by default so all the existing scripts are not broken, i.e the actual QA test suite run with no modifications.

When ON ${ list is closed with a reserved keyword ;} or \n}

A new test greedy_cumsub.sh is added, it turns ON set -o comsub_brace_greedy and then torture the scanner/parser.

I implemented in there @stephane-chazelas test along with a series of trivial cases.

Please do torture it to the extreme so I could fix it if need, the way is is now is good enough for my own needs, the motivation for this fix.

There is only one case from @stephane-chazelas that I didn;t implemented in greedy_comsub.sh that is

$ ksh -c 'echo ${ echo "{a,b}c; }'
```
The ["]() need to be closed, so instead I implemented this

${ echo '{a,b}c' ;}

Cheers,

[phidebian]

I forgot to say that if this patch is accepted, the docco need to be fixed accordingly, it require technical writing skill I don't have unfortunatly...

[phidebian]

Ouch didn't noticed the QA test fail on shcomp.

Actually implementing the feature via set -o comsub_brace_greedy make no sense with shcomp as it is a runtime feature, shcomp is then in the default mode and will never compile in the new mode.

So the question is does shcomp really used ? if so I should see to find a way to catch set -o comsub_brace_greedy during shcomp time and set shcomp scan/parse state accordingly. Else I could simply no run shcomp for greedy_comsub.sh

lemme know

[phidebian]

I submitted a new PR with a fix for shcomp so now all QA runs ok on x86_64 Ubuntu 24.04.2 LTS

[phidebian]

QA run ok on s390x too on Ubuntu 24.04.2 LTS