Skip to content

Commit

Permalink
Fix serious regression in pathname expansion (re: 4361b6a)
Browse files Browse the repository at this point in the history
@dicktyr (Richard Taityr) reports:
> globs are not expanded with the following:
>
>   % echo "/"{bin,sbin}"/*"
>   /bin/* /sbin/*
>
>   % v=/; echo "$v"bin"/*"
>   /bin/*
>
> but globbing is unexpectedly performed if both parameter
> substitution and brace expansion are present:
>
>   % v=/; echo "$v"{bin,sbin}"/*"
>   [output omitted for the sake of brevity]

So, quoted pattern characters are expanded. No es bueno.

The closest I've been able to come to fixing this so far is:

src/cmd/ksh93/sh/macro.c: endfield():
- Do not set musttrim if mp->quoted is set, i.e., if the argument
  node contains any quoted parts.

This fixes the major bug. But it also partially reintroduces the
previously fixed bug <#549> for
cases where any part of the word is quoted -- even if the quoted
part is empty. For example:

    $ mkdir testdir
    $ cd testdir
    $ touch a b c ./-
    $ p='[a\-c]'
    $ echo $p    # OK
    - a c
    $ echo ""$p  # BUG
    a b c
    $ echo $p""  # BUG
    a b c

The fundamental problem is that, by the time endfield() is reached,
we have a complete word and we can no longer distinguish between
the quoted and unquoted parts of it. The Mac_t flags, such as
mp->quoted, apply to the entire word.

This is a fundamental flaw in the current design, where quotes are
internally translated to backslash escapes for certain characters.
To the best of my knowledge (and I've tried hard), it is not
possible to fix this without introducing other regressions.

A radical redesign of this entire mechanism is needed where this
internal backslash escaping is replaced by a way for every 'struct
argnod' argument node to track for each individual character
whether it is quoted or not, without modifying the argument string
itself. Because it is incorrect to modify the argument string.

But, for the foreseeable future, that is a pipe dream, because
there is no one who fully understands all this code with all its
myriad kludges and workarounds going decades back.

Resolves: #660
  • Loading branch information
McDutchie committed Jun 13, 2023
1 parent e288d1f commit 4c7727c
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 1 deletion.
6 changes: 6 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ This documents significant changes in the 1.0 branch of ksh 93u+m.
For full details, see the git log at: https://github.com/ksh93/ksh/tree/1.0
Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.

2023-06-13:

- Fixed a serious regression in pathname expansion where quoted wildcard
characters were incorrectly expanded if a pattern contains both a brace
expansion and a variable expansion.

2023-06-12:

- Fixed a bug where ^X^E in emacs or 'v' in vi, which invoke a full-screen
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/sh/macro.c
Original file line number Diff line number Diff line change
Expand Up @@ -2582,7 +2582,7 @@ static void endfield(Mac_t *mp,int split)
mp->atmode = 0;
if(mp->patfound)
{
int musttrim = mp->wasexpan && !mp->noextpat && strchr(argp->argval,'\\');
int musttrim = mp->wasexpan && !mp->quoted && !mp->noextpat && strchr(argp->argval,'\\');
sh.argaddr = 0;
#if SHOPT_BRACEPAT
/* in POSIX mode, disallow brace expansion for unquoted expansions */
Expand Down
14 changes: 14 additions & 0 deletions src/cmd/ksh93/tests/glob.sh
Original file line number Diff line number Diff line change
Expand Up @@ -532,5 +532,19 @@ unquoted_patvar='a\\\\b.*'
test_glob '<a\\b.txt>' a\\\\b.*
test_glob '<a\\b.txt>' $unquoted_patvar

# ======
# 93u+m/1.0.5 regression - glob expansion with brace expansion and parameter exapansion
v='./'
mkdir -p bin/BAD sbin/WRONG
test_glob '<./bin/*> <./sbin/*>' "./"{bin,sbin}"/*"
test_glob '<./bin/*>' "$v"bin"/*"
test_glob '<./bin/*> <./sbin/*>' "$v"{bin,sbin}"/*"
unset null
test_glob '<b*> <s*>' {b,s}"*"
test_glob '<b*> <s*>' $null{b,s}"*"
test_glob '<*b> <*c>' $null"*"{b,c}
test_glob '<*b> <*c>' "*"$null{b,c}
test_glob '<*>' $null"*"

# ======
exit $((Errors<125?Errors:125))

0 comments on commit 4c7727c

Please sign in to comment.