diff --git a/pkg/path/match.go b/pkg/path/match.go index 7f371f755..4e191cb72 100644 --- a/pkg/path/match.go +++ b/pkg/path/match.go @@ -27,6 +27,8 @@ import ( // ErrBadPattern indicates a pattern was malformed. var ErrBadPattern = errors.New("syntax error in pattern") +var errStarStarDisallowed = errors.New("'**' is not supported in patterns as of yet") + // Match reports whether name matches the shell file name pattern. // The pattern syntax is: // @@ -51,13 +53,19 @@ var ErrBadPattern = errors.New("syntax error in pattern") // // On Windows, escaping is disabled. Instead, '\\' is treated as // path separator. +// +// A pattern may not contain '**', as a wildcard matching separator characters +// is not supported at this time. func Match(pattern, name string, o OS) (matched bool, err error) { os := getOS(o) Pattern: for len(pattern) > 0 { var star bool var chunk string - star, chunk, pattern = scanChunk(pattern, os) + star, chunk, pattern, err = scanChunk(pattern, os) + if err != nil { + return false, err + } if star && chunk == "" { // Trailing * matches rest of string unless it has a /. return !strings.Contains(name, string(os.Separator)), nil @@ -92,6 +100,14 @@ Pattern: } } } + // Before returning false with no error, + // check that the remainder of the pattern is syntactically valid. + for len(pattern) > 0 { + _, chunk, pattern, err = scanChunk(pattern, os) + if err != nil { + return false, err + } + } return false, nil } return len(name) == 0, nil @@ -99,10 +115,14 @@ Pattern: // scanChunk gets the next segment of pattern, which is a non-star string // possibly preceded by a star. -func scanChunk(pattern string, os os) (star bool, chunk, rest string) { - for len(pattern) > 0 && pattern[0] == '*' { +func scanChunk(pattern string, os os) (star bool, chunk, rest string, _ error) { + if len(pattern) > 0 && pattern[0] == '*' { pattern = pattern[1:] star = true + if len(pattern) > 0 && pattern[0] == '*' { + // ** is disallowed to allow for future functionality. + return false, "", "", errStarStarDisallowed + } } inrange := false var i int @@ -126,7 +146,7 @@ Scan: } } } - return star, pattern[0:i], pattern[i:] + return star, pattern[0:i], pattern[i:], nil } // matchChunk checks whether chunk matches the beginning of s. diff --git a/pkg/path/match_test.go b/pkg/path/match_test.go index a08551f7d..45a347e1a 100644 --- a/pkg/path/match_test.go +++ b/pkg/path/match_test.go @@ -86,10 +86,9 @@ var matchTests = []MatchTest{ {"a[", "x", false, ErrBadPattern}, {"a/b[", "x", false, ErrBadPattern}, {"*x", "xxx", true, nil}, - // TODO(mvdan): this should fail; right now "**" happens to behave like "*". - {"**", "ab/c", false, nil}, - {"**/c", "ab/c", true, nil}, - {"a/b/**", "", false, nil}, + {"**", "ab/c", false, errStarStarDisallowed}, + {"**/c", "ab/c", false, errStarStarDisallowed}, + {"a/b/**", "", false, errStarStarDisallowed}, {"\\**", "*ab", true, nil}, {"[x**y]", "*", true, nil}, } diff --git a/pkg/tool/file/file.go b/pkg/tool/file/file.go index 3dbed397a..a6b62689b 100644 --- a/pkg/tool/file/file.go +++ b/pkg/tool/file/file.go @@ -17,10 +17,12 @@ package file import ( "os" "path/filepath" + "runtime" "cuelang.org/go/cue" "cuelang.org/go/cue/errors" "cuelang.org/go/internal/task" + pkgpath "cuelang.org/go/pkg/path" ) func init() { @@ -110,6 +112,16 @@ func (c *cmdGlob) Run(ctx *task.Context) (res interface{}, err error) { if ctx.Err != nil { return nil, ctx.Err } + // Validate that the glob pattern is valid per [pkgpath.Match]. + // Note that we use the current OS to match the semantics of [filepath.Glob], + // and since the APIs in this package are meant to support native paths. + os := pkgpath.Unix + if runtime.GOOS == "windows" { + os = pkgpath.Windows + } + if _, err := pkgpath.Match(glob, "", os); err != nil { + return nil, err + } m, err := filepath.Glob(glob) for i, s := range m { m[i] = filepath.ToSlash(s) diff --git a/pkg/tool/file/file_test.go b/pkg/tool/file/file_test.go index 206811826..8c7cc5797 100644 --- a/pkg/tool/file/file_test.go +++ b/pkg/tool/file/file_test.go @@ -125,13 +125,12 @@ func TestGlob(t *testing.T) { qt.Assert(t, qt.DeepEquals(got, any(map[string]any{"files": []string{"testdata/input.foo"}}))) // globstar or recursive globbing is not supported. - // TODO(mvdan): this should fail; right now "**" happens to behave like "*". v = parse(t, "tool/file.Glob", `{ glob: "testdata/**/glob.leaf" }`) got, err = (*cmdGlob).Run(nil, &task.Context{Obj: v}) - qt.Assert(t, qt.IsNil(err)) - qt.Assert(t, qt.DeepEquals(got, any(map[string]any{"files": []string{"testdata/glob1/glob.leaf"}}))) + qt.Assert(t, qt.IsNotNil(err)) + qt.Assert(t, qt.IsNil(got)) } func TestGlobEscapeStar(t *testing.T) { @@ -151,9 +150,8 @@ func TestGlobEscapeStar(t *testing.T) { }`) got, err := (*cmdGlob).Run(nil, &task.Context{Obj: v}) if runtime.GOOS == "windows" { - // TODO(mvdan): this should fail; right now "**" happens to behave like "*". - qt.Assert(t, qt.IsNil(err)) - qt.Assert(t, qt.DeepEquals(got, any(map[string]any{"files": []string(nil)}))) + qt.Assert(t, qt.IsNotNil(err)) + qt.Assert(t, qt.Equals(got, nil)) } else { qt.Assert(t, qt.IsNil(err)) qt.Assert(t, qt.DeepEquals(got, any(map[string]any{"files": []string{leafFile}})))