From c0ab3caeb3fb8cb7782f7c70d2ac9fae46efc619 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Sat, 25 Jun 2022 20:14:49 -0700 Subject: [PATCH] Fix glob expansion that includes broken symlinks 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 --- 0.19.0-release-notes.md | 3 +++ pkg/glob/glob.go | 8 ++++---- pkg/glob/glob_test.go | 29 +++++++++++++++++++++++------ 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/0.19.0-release-notes.md b/0.19.0-release-notes.md index 19a7a9cfd..6314f65dc 100644 --- a/0.19.0-release-notes.md +++ b/0.19.0-release-notes.md @@ -21,6 +21,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. diff --git a/pkg/glob/glob.go b/pkg/glob/glob.go index 32d0c8369..277a6b4dd 100644 --- a/pkg/glob/glob.go +++ b/pkg/glob/glob.go @@ -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 @@ -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 } diff --git a/pkg/glob/glob_test.go b/pkg/glob/glob_test.go index 93c05f185..91d21268b 100644 --- a/pkg/glob/glob_test.go +++ b/pkg/glob/glob_test.go @@ -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 { @@ -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"}}, @@ -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 {