mvdan.cc / cue
this repo has no description
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

pkg: forbid `**` in path.Match and tool/file.Glob

Currently the double-star pattern term `**` behaves like a star `*`,
as one star matches any sequence of non-separator characters
and the second star matches nothing.

This is confusing, as one may write the pattern `**/*.json` and it can
match strings like `foo/bar.json`, making it seem like `**` works as
a double star that would also match `foo.json` or `foo/bar/baz.json`
when in fact it does not.

If we choose to support `**` patterns in the future to match separators,
this would mean changing the behavior for existing users in a subtle way
which could lead to unintended behavior or latent bugs.
It is clearer for existing users to treat `**` as an invalid pattern,
which would allow us to add the feature in the future without concern
about existing users already using such patterns without error.

We also mirror Go's path.Match behavior to consume the entire pattern
to validate that it is always syntactically valid, so that we may
use it to check for `**` even when the matching has otherwise ended.
See https://go.dev/issue/28614 for details; we largely copied these
semantics from Go 1.15 already, but we omitted a bit of code that was
necessary for trailing double-stars to be caught as an error.

This change is driven by the embed proposal, where recursively embedding
entire directory trees is a relatively common use case that we want
to support somehow, and users might be misled by `**` silently working
in a way that doesn't match the user's expectation. We want path.Match,
tool/file.Glob, and embed patterns to behave consistently, so for that
reason, start consistently treating `**` as an error.

Note that this is a breaking change for any users whose pattern
includes `**` as two consecutive wildcards, and this is on purpose
for the reasons outlined above. Users who run into this problem
may not have been aware that it behaved like a single wildcard,
and in the majority of cases they should be able to use `*` instead.

For #1919.
Fixes #3315.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: Ifeb81f4818a863cd0e4c5c13bc0fc2ac54a0f22f
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1198636
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Roger Peppe <rogpeppe@gmail.com>

Daniel Martí 987a85e6 25bb3d4f

+43 -14
+24 -4
pkg/path/match.go
··· 27 27 // ErrBadPattern indicates a pattern was malformed. 28 28 var ErrBadPattern = errors.New("syntax error in pattern") 29 29 30 + var errStarStarDisallowed = errors.New("'**' is not supported in patterns as of yet") 31 + 30 32 // Match reports whether name matches the shell file name pattern. 31 33 // The pattern syntax is: 32 34 // ··· 51 53 // 52 54 // On Windows, escaping is disabled. Instead, '\\' is treated as 53 55 // path separator. 56 + // 57 + // A pattern may not contain '**', as a wildcard matching separator characters 58 + // is not supported at this time. 54 59 func Match(pattern, name string, o OS) (matched bool, err error) { 55 60 os := getOS(o) 56 61 Pattern: 57 62 for len(pattern) > 0 { 58 63 var star bool 59 64 var chunk string 60 - star, chunk, pattern = scanChunk(pattern, os) 65 + star, chunk, pattern, err = scanChunk(pattern, os) 66 + if err != nil { 67 + return false, err 68 + } 61 69 if star && chunk == "" { 62 70 // Trailing * matches rest of string unless it has a /. 63 71 return !strings.Contains(name, string(os.Separator)), nil ··· 92 100 } 93 101 } 94 102 } 103 + // Before returning false with no error, 104 + // check that the remainder of the pattern is syntactically valid. 105 + for len(pattern) > 0 { 106 + _, chunk, pattern, err = scanChunk(pattern, os) 107 + if err != nil { 108 + return false, err 109 + } 110 + } 95 111 return false, nil 96 112 } 97 113 return len(name) == 0, nil ··· 99 115 100 116 // scanChunk gets the next segment of pattern, which is a non-star string 101 117 // possibly preceded by a star. 102 - func scanChunk(pattern string, os os) (star bool, chunk, rest string) { 103 - for len(pattern) > 0 && pattern[0] == '*' { 118 + func scanChunk(pattern string, os os) (star bool, chunk, rest string, _ error) { 119 + if len(pattern) > 0 && pattern[0] == '*' { 104 120 pattern = pattern[1:] 105 121 star = true 122 + if len(pattern) > 0 && pattern[0] == '*' { 123 + // ** is disallowed to allow for future functionality. 124 + return false, "", "", errStarStarDisallowed 125 + } 106 126 } 107 127 inrange := false 108 128 var i int ··· 126 146 } 127 147 } 128 148 } 129 - return star, pattern[0:i], pattern[i:] 149 + return star, pattern[0:i], pattern[i:], nil 130 150 } 131 151 132 152 // matchChunk checks whether chunk matches the beginning of s.
+3 -4
pkg/path/match_test.go
··· 86 86 {"a[", "x", false, ErrBadPattern}, 87 87 {"a/b[", "x", false, ErrBadPattern}, 88 88 {"*x", "xxx", true, nil}, 89 - // TODO(mvdan): this should fail; right now "**" happens to behave like "*". 90 - {"**", "ab/c", false, nil}, 91 - {"**/c", "ab/c", true, nil}, 92 - {"a/b/**", "", false, nil}, 89 + {"**", "ab/c", false, errStarStarDisallowed}, 90 + {"**/c", "ab/c", false, errStarStarDisallowed}, 91 + {"a/b/**", "", false, errStarStarDisallowed}, 93 92 {"\\**", "*ab", true, nil}, 94 93 {"[x**y]", "*", true, nil}, 95 94 }
+12
pkg/tool/file/file.go
··· 17 17 import ( 18 18 "os" 19 19 "path/filepath" 20 + "runtime" 20 21 21 22 "cuelang.org/go/cue" 22 23 "cuelang.org/go/cue/errors" 23 24 "cuelang.org/go/internal/task" 25 + pkgpath "cuelang.org/go/pkg/path" 24 26 ) 25 27 26 28 func init() { ··· 109 111 glob := ctx.String("glob") 110 112 if ctx.Err != nil { 111 113 return nil, ctx.Err 114 + } 115 + // Validate that the glob pattern is valid per [pkgpath.Match]. 116 + // Note that we use the current OS to match the semantics of [filepath.Glob], 117 + // and since the APIs in this package are meant to support native paths. 118 + os := pkgpath.Unix 119 + if runtime.GOOS == "windows" { 120 + os = pkgpath.Windows 121 + } 122 + if _, err := pkgpath.Match(glob, "", os); err != nil { 123 + return nil, err 112 124 } 113 125 m, err := filepath.Glob(glob) 114 126 for i, s := range m {
+4 -6
pkg/tool/file/file_test.go
··· 125 125 qt.Assert(t, qt.DeepEquals(got, any(map[string]any{"files": []string{"testdata/input.foo"}}))) 126 126 127 127 // globstar or recursive globbing is not supported. 128 - // TODO(mvdan): this should fail; right now "**" happens to behave like "*". 129 128 v = parse(t, "tool/file.Glob", `{ 130 129 glob: "testdata/**/glob.leaf" 131 130 }`) 132 131 got, err = (*cmdGlob).Run(nil, &task.Context{Obj: v}) 133 - qt.Assert(t, qt.IsNil(err)) 134 - qt.Assert(t, qt.DeepEquals(got, any(map[string]any{"files": []string{"testdata/glob1/glob.leaf"}}))) 132 + qt.Assert(t, qt.IsNotNil(err)) 133 + qt.Assert(t, qt.IsNil(got)) 135 134 } 136 135 137 136 func TestGlobEscapeStar(t *testing.T) { ··· 151 150 }`) 152 151 got, err := (*cmdGlob).Run(nil, &task.Context{Obj: v}) 153 152 if runtime.GOOS == "windows" { 154 - // TODO(mvdan): this should fail; right now "**" happens to behave like "*". 155 - qt.Assert(t, qt.IsNil(err)) 156 - qt.Assert(t, qt.DeepEquals(got, any(map[string]any{"files": []string(nil)}))) 153 + qt.Assert(t, qt.IsNotNil(err)) 154 + qt.Assert(t, qt.Equals(got, nil)) 157 155 } else { 158 156 qt.Assert(t, qt.IsNil(err)) 159 157 qt.Assert(t, qt.DeepEquals(got, any(map[string]any{"files": []string{leafFile}})))