io/fs, path, path/filepath, testing/fstest: validate patterns in Match, 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>

diff --git a/src/io/fs/glob.go b/src/io/fs/glob.go
index 77f6ebbaaf18bef1075e68643af5ab70dbce7f65..cde6c49f3db0c6012b513101649fe580e803294c 100644 (file)
--- a/src/io/fs/glob.go
+++ b/src/io/fs/glob.go
@@ -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

diff --git a/src/io/fs/glob_test.go b/src/io/fs/glob_test.go
index 0183a49b6cf75b3bba539514ab0066c787299631..5c8ac3fbf32bd968f240fcbe5fd0b150b9039d35 100644 (file)
--- a/src/io/fs/glob_test.go
+++ b/src/io/fs/glob_test.go
@@ -7,6 +7,7 @@ package fs_test
 import (
        . "io/fs"
        "os"
+       "path"
        "testing"
 )
 
@@ -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)
+               }
        }
 }
 

diff --git a/src/path/filepath/match.go b/src/path/filepath/match.go
index 20a334805ba8a1c0e43233be578b2179a0cc2af8..c77a26952a657f2c53bd9dafbf98deb4a7071f56 100644 (file)
--- a/src/path/filepath/match.go
+++ b/src/path/filepath/match.go
@@ -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
@@ -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 {
@@ -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
 }
 
@@ -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

diff --git a/src/path/filepath/match_test.go b/src/path/filepath/match_test.go
index b8657626bc7ea02c01c0b7967001b259427f55ab..1c3b567fa38238bdfad4314315678d682d678fe8 100644 (file)
--- a/src/path/filepath/match_test.go
+++ b/src/path/filepath/match_test.go
@@ -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},
 }
 
@@ -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)
+               }
        }
 }
 

diff --git a/src/path/match.go b/src/path/match.go
index 837eb8bb8b724777695caf0e067cf9f9518c0d57..918624c60e80a31881a318ca8ce2928a366d6dfd 100644 (file)
--- a/src/path/match.go
+++ b/src/path/match.go
@@ -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
@@ -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
@@ -140,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 {
@@ -153,34 +169,40 @@ func matchChunk(chunk, s string) (rest string, ok bool, err error) {
                                }
                                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
 }
 

diff --git a/src/path/match_test.go b/src/path/match_test.go
index 3e027e1f682a481eaef2a0d5c537768f690e4a72..996bd691eb82b60c8ae875dddcbffedb98da9c91 100644 (file)
--- a/src/path/match_test.go
+++ b/src/path/match_test.go
@@ -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},
 }
 

diff --git a/src/testing/fstest/testfs.go b/src/testing/fstest/testfs.go
index 21cd00e5b608f2c079a3a9ff63840425b9dfc420..4912a271b26adc6986512930a5c920ba5a4da79c 100644 (file)
--- a/src/testing/fstest/testfs.go
+++ b/src/testing/fstest/testfs.go
@@ -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++ {