Skip to content

Commit

Permalink
io/fs, path, path/filepath, testing/fstest: validate patterns in Matc…
Browse files Browse the repository at this point in the history
…h, Glob

According to #28614, proposal review agreed in December 2018 that
Match should return an error for failed matches where the unmatched
part of the pattern has a syntax error. (The failed match has to date
caused the scan of the pattern to stop early.)

This change implements that behavior: the match loop continues
scanning to the end of the pattern, even after a confirmed mismatch,
to check whether the pattern is even well-formed.

The change applies to both path.Match and filepath.Match.
Then filepath.Glob and fs.Glob make a single validity-checking
call to Match before beginning their usual processing.

Also update fstest.TestFS to check for correct validation in custom
Glob implementations.

Fixes #28614.

Change-Id: Ic1d35a4bb9c3565184ae83dbefc425c5c96318e7
Reviewed-on: https://go-review.googlesource.com/c/go/+/264397
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
  • Loading branch information
rsc committed Oct 23, 2020
1 parent 4a2cc73 commit b5ddc42
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 51 deletions.
4 changes: 4 additions & 0 deletions src/io/fs/glob.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ func Glob(fsys FS, pattern string) (matches []string, err error) {
return fsys.Glob(pattern)
}

// Check pattern is well-formed.
if _, err := path.Match(pattern, ""); err != nil {
return nil, err
}
if !hasMeta(pattern) {
if _, err = Stat(fsys, pattern); err != nil {
return nil, nil
Expand Down
10 changes: 7 additions & 3 deletions src/io/fs/glob_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package fs_test
import (
. "io/fs"
"os"
"path"
"testing"
)

Expand Down Expand Up @@ -44,9 +45,12 @@ func TestGlob(t *testing.T) {
}

func TestGlobError(t *testing.T) {
_, err := Glob(os.DirFS("."), "[]")
if err == nil {
t.Error("expected error for bad pattern; got none")
bad := []string{`[]`, `nonexist/[]`}
for _, pattern := range bad {
_, err := Glob(os.DirFS("."), pattern)
if err != path.ErrBadPattern {
t.Errorf("Glob(fs, %#q) returned err=%v, want path.ErrBadPattern", pattern, err)
}
}
}

Expand Down
61 changes: 37 additions & 24 deletions src/path/filepath/match.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,25 +122,28 @@ Scan:
// If so, it returns the remainder of s (after the match).
// Chunk is all single-character operators: literals, char classes, and ?.
func matchChunk(chunk, s string) (rest string, ok bool, err error) {
// failed records whether the match has failed.
// After the match fails, the loop continues on processing chunk,
// checking that the pattern is well-formed but no longer reading s.
failed := false
for len(chunk) > 0 {
if len(s) == 0 {
return
if !failed && len(s) == 0 {
failed = true
}
switch chunk[0] {
case '[':
// character class
r, n := utf8.DecodeRuneInString(s)
s = s[n:]
chunk = chunk[1:]
// We can't end right after '[', we're expecting at least
// a closing bracket and possibly a caret.
if len(chunk) == 0 {
err = ErrBadPattern
return
var r rune
if !failed {
var n int
r, n = utf8.DecodeRuneInString(s)
s = s[n:]
}
chunk = chunk[1:]
// possibly negated
negated := chunk[0] == '^'
if negated {
negated := false
if len(chunk) > 0 && chunk[0] == '^' {
negated = true
chunk = chunk[1:]
}
// parse all ranges
Expand All @@ -153,12 +156,12 @@ func matchChunk(chunk, s string) (rest string, ok bool, err error) {
}
var lo, hi rune
if lo, chunk, err = getEsc(chunk); err != nil {
return
return "", false, err
}
hi = lo
if chunk[0] == '-' {
if hi, chunk, err = getEsc(chunk[1:]); err != nil {
return
return "", false, err
}
}
if lo <= r && r <= hi {
Expand All @@ -167,35 +170,41 @@ func matchChunk(chunk, s string) (rest string, ok bool, err error) {
nrange++
}
if match == negated {
return
failed = true
}

case '?':
if s[0] == Separator {
return
if !failed {
if s[0] == Separator {
failed = true
}
_, n := utf8.DecodeRuneInString(s)
s = s[n:]
}
_, n := utf8.DecodeRuneInString(s)
s = s[n:]
chunk = chunk[1:]

case '\\':
if runtime.GOOS != "windows" {
chunk = chunk[1:]
if len(chunk) == 0 {
err = ErrBadPattern
return
return "", false, ErrBadPattern
}
}
fallthrough

default:
if chunk[0] != s[0] {
return
if !failed {
if chunk[0] != s[0] {
failed = true
}
s = s[1:]
}
s = s[1:]
chunk = chunk[1:]
}
}
if failed {
return "", false, nil
}
return s, true, nil
}

Expand Down Expand Up @@ -232,6 +241,10 @@ func getEsc(chunk string) (r rune, nchunk string, err error) {
// The only possible returned error is ErrBadPattern, when pattern
// is malformed.
func Glob(pattern string) (matches []string, err error) {
// Check pattern is well-formed.
if _, err := Match(pattern, ""); err != nil {
return nil, err
}
if !hasMeta(pattern) {
if _, err = os.Lstat(pattern); err != nil {
return nil, nil
Expand Down
12 changes: 8 additions & 4 deletions src/path/filepath/match_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,10 @@ var matchTests = []MatchTest{
{"[", "a", false, ErrBadPattern},
{"[^", "a", false, ErrBadPattern},
{"[^bc", "a", false, ErrBadPattern},
{"a[", "a", false, nil},
{"a[", "a", false, ErrBadPattern},
{"a[", "ab", false, ErrBadPattern},
{"a[", "x", false, ErrBadPattern},
{"a/b[", "x", false, ErrBadPattern},
{"*x", "xxx", true, nil},
}

Expand Down Expand Up @@ -155,9 +157,11 @@ func TestGlob(t *testing.T) {
}

func TestGlobError(t *testing.T) {
_, err := Glob("[]")
if err == nil {
t.Error("expected error for bad pattern; got none")
bad := []string{`[]`, `nonexist/[]`}
for _, pattern := range bad {
if _, err := Glob(pattern); err != ErrBadPattern {
t.Errorf("Glob(%#q) returned err=%v, want ErrBadPattern", pattern, err)
}
}
}

Expand Down
60 changes: 41 additions & 19 deletions src/path/match.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,14 @@ Pattern:
}
}
}
// Before returning false with no error,
// check that the remainder of the pattern is syntactically valid.
for len(pattern) > 0 {
_, chunk, pattern = scanChunk(pattern)
if _, _, err := matchChunk(chunk, ""); err != nil {
return false, err
}
}
return false, nil
}
return len(name) == 0, nil
Expand Down Expand Up @@ -114,20 +122,28 @@ Scan:
// If so, it returns the remainder of s (after the match).
// Chunk is all single-character operators: literals, char classes, and ?.
func matchChunk(chunk, s string) (rest string, ok bool, err error) {
// failed records whether the match has failed.
// After the match fails, the loop continues on processing chunk,
// checking that the pattern is well-formed but no longer reading s.
failed := false
for len(chunk) > 0 {
if len(s) == 0 {
return
if !failed && len(s) == 0 {
failed = true
}
switch chunk[0] {
case '[':
// character class
r, n := utf8.DecodeRuneInString(s)
s = s[n:]
var r rune
if !failed {
var n int
r, n = utf8.DecodeRuneInString(s)
s = s[n:]
}
chunk = chunk[1:]
// possibly negated
notNegated := true
negated := false
if len(chunk) > 0 && chunk[0] == '^' {
notNegated = false
negated = true
chunk = chunk[1:]
}
// parse all ranges
Expand All @@ -140,47 +156,53 @@ func matchChunk(chunk, s string) (rest string, ok bool, err error) {
}
var lo, hi rune
if lo, chunk, err = getEsc(chunk); err != nil {
return
return "", false, err
}
hi = lo
if chunk[0] == '-' {
if hi, chunk, err = getEsc(chunk[1:]); err != nil {
return
return "", false, err
}
}
if lo <= r && r <= hi {
match = true
}
nrange++
}
if match != notNegated {
return
if match == negated {
failed = true
}

case '?':
if s[0] == '/' {
return
if !failed {
if s[0] == '/' {
failed = true
}
_, n := utf8.DecodeRuneInString(s)
s = s[n:]
}
_, n := utf8.DecodeRuneInString(s)
s = s[n:]
chunk = chunk[1:]

case '\\':
chunk = chunk[1:]
if len(chunk) == 0 {
err = ErrBadPattern
return
return "", false, ErrBadPattern
}
fallthrough

default:
if chunk[0] != s[0] {
return
if !failed {
if chunk[0] != s[0] {
failed = true
}
s = s[1:]
}
s = s[1:]
chunk = chunk[1:]
}
}
if failed {
return "", false, nil
}
return s, true, nil
}

Expand Down
4 changes: 3 additions & 1 deletion src/path/match_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,10 @@ var matchTests = []MatchTest{
{"[", "a", false, ErrBadPattern},
{"[^", "a", false, ErrBadPattern},
{"[^bc", "a", false, ErrBadPattern},
{"a[", "a", false, nil},
{"a[", "a", false, ErrBadPattern},
{"a[", "ab", false, ErrBadPattern},
{"a[", "x", false, ErrBadPattern},
{"a/b[", "x", false, ErrBadPattern},
{"*x", "xxx", true, nil},
}

Expand Down
6 changes: 6 additions & 0 deletions src/testing/fstest/testfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,12 @@ func (t *fsTester) checkGlob(dir string, list []fs.DirEntry) {
glob = strings.Join(elem, "/") + "/"
}

// Test that malformed patterns are detected.
// The error is likely path.ErrBadPattern but need not be.
if _, err := t.fsys.(fs.GlobFS).Glob(glob + "nonexist/[]"); err == nil {
t.errorf("%s: Glob(%#q): bad pattern not detected", dir, glob+"nonexist/[]")
}

// Try to find a letter that appears in only some of the final names.
c := rune('a')
for ; c <= 'z'; c++ {
Expand Down

0 comments on commit b5ddc42

Please sign in to comment.