Skip to content

Commit

Permalink
Fix glob expansion that includes broken symlinks
Browse files Browse the repository at this point in the history
Glob expansion should use os.Lstat rather than os.Stat so that broken
symlinks don't stop glob expansion. It's not the place of glob expansion
to decide if a particular path is good.

The symlink test cases and logic were borrowed from
pkg/mods/path/path_test.go.

Fixes #1240
  • Loading branch information
Kurtis Rader authored and xiaq committed Aug 28, 2022
1 parent 7c75510 commit a72224d
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 10 deletions.
3 changes: 3 additions & 0 deletions 0.19.0-release-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ and compiled, even if it is not executed:
- Temporary assignment on an unset environment variables no longer leave it
set to an empty string ([#1448](https://b.elv.sh/1448)).

- Glob expansion that includes broken symlinks no longer short-circuits the
expansion ([#1240](https://b.elv.sh/1240)).

# Notable new features

- A new `inexact-num` converts its argument to an inexact number.
Expand Down
8 changes: 4 additions & 4 deletions pkg/glob/glob.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,19 +72,19 @@ func glob(segs []Segment, dir string, cb func(PathInfo) bool) bool {
elem := segs[0].(Literal).Data
segs = segs[2:]
dir += elem + "/"
if info, err := os.Stat(dir); err != nil || !info.IsDir() {
if info, err := os.Lstat(dir); err != nil || !info.IsDir() {
return true
}
}

if len(segs) == 0 {
if info, err := os.Stat(dir); err == nil {
if info, err := os.Lstat(dir); err == nil {
return cb(PathInfo{dir, info})
}
return true
} else if len(segs) == 1 && IsLiteral(segs[0]) {
path := dir + segs[0].(Literal).Data
if info, err := os.Stat(path); err == nil {
if info, err := os.Lstat(path); err == nil {
return cb(PathInfo{path, info})
}
return true
Expand Down Expand Up @@ -161,7 +161,7 @@ func glob(segs []Segment, dir string, cb func(PathInfo) bool) bool {
name := info.Name()
if matchElement(segs, name) {
dirname := dir + name
info, err := os.Stat(dirname)
info, err := os.Lstat(dirname)
if err != nil {
return true
}
Expand Down
29 changes: 23 additions & 6 deletions pkg/glob/glob_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@ var (
"lorem", "ipsum",
"d1/e/f/g/X", "d2/e/f/g/X"}
createDots = []string{".x", ".el/x"}
symlinks = []struct {
path string
target string
}{
{"d1/s-f", "f"},
{"s-d", "d2"},
{"s-d-f", "d1/f"},
{"s-bad", "bad"},
}
)

type globCase struct {
Expand All @@ -28,22 +37,21 @@ type globCase struct {
}

var globCases = []globCase{
{"*", []string{"a", "b", "c", "d1", "d2", "dX", "dXY", "lorem", "ipsum"}},
{"*", []string{"a", "b", "c", "d1", "d2", "dX", "dXY", "lorem", "ipsum", "s-bad", "s-d", "s-d-f"}},
{".", []string{"."}},
{"./*", []string{"./a", "./b", "./c", "./d1", "./d2", "./dX", "./dXY", "./lorem", "./ipsum"}},
{"./*", []string{"./a", "./b", "./c", "./d1", "./d2", "./dX", "./dXY", "./lorem", "./ipsum", "./s-bad", "./s-d", "./s-d-f"}},
{"..", []string{".."}},
{"a/..", []string{"a/.."}},
{"a/../*", []string{"a/../a", "a/../b", "a/../c", "a/../d1", "a/../d2", "a/../dX", "a/../dXY", "a/../lorem", "a/../ipsum"}},
{"a/../*", []string{"a/../a", "a/../b", "a/../c", "a/../d1", "a/../d2", "a/../dX", "a/../dXY", "a/../ipsum", "a/../lorem", "a/../s-bad", "a/../s-d", "a/../s-d-f"}},
{"*/", []string{"a/", "b/", "c/", "d1/", "d2/"}},
{"**", append(mkdirs, creates...)},
{"**", []string{"a", "a/X", "a/Y", "b", "b/X", "c", "c/Y", "d1", "d1/e", "d1/e/f", "d1/e/f/g", "d1/e/f/g/X", "d1/s-f", "d2", "d2/e", "d2/e/f", "d2/e/f/g", "d2/e/f/g/X", "dX", "dXY", "ipsum", "lorem", "s-bad", "s-d", "s-d-f"}},
{"*/X", []string{"a/X", "b/X"}},
{"**X", []string{"a/X", "b/X", "dX", "d1/e/f/g/X", "d2/e/f/g/X"}},
{"*/*/*", []string{"d1/e/f", "d2/e/f"}},
{"l*m", []string{"lorem"}},
{"d*", []string{"d1", "d2", "dX", "dXY"}},
{"d*/", []string{"d1/", "d2/"}},
{"d**", []string{"d1", "d1/e", "d1/e/f", "d1/e/f/g", "d1/e/f/g/X",
"d2", "d2/e", "d2/e/f", "d2/e/f/g", "d2/e/f/g/X", "dX", "dXY"}},
{"d**", []string{"d1", "d1/e", "d1/e/f", "d1/e/f/g", "d1/e/f/g/X", "d1/s-f", "d2", "d2/e", "d2/e/f", "d2/e/f/g", "d2/e/f/g/X", "dX", "dXY"}},
{"?", []string{"a", "b", "c"}},
{"??", []string{"d1", "d2", "dX"}},

Expand Down Expand Up @@ -80,6 +88,15 @@ func testGlob(t *testing.T, abs bool) {
}
f.Close()
}
for _, link := range symlinks {
err := os.Symlink(link.target, link.path)
if err != nil {
// Creating symlinks requires a special permission on Windows. If
// the user doesn't have that permission, just skip the whole test.
t.Skip(err)
}
}

for _, tc := range globCases {
pattern := tc.pattern
if abs {
Expand Down

0 comments on commit a72224d

Please sign in to comment.