Przeglądaj źródła

feat(ignore): add .stignore escaping on Windows (#10205)

Based on the discussion in
https://forum.syncthing.net/t/towards-syncthing-2-0/24072/35 This PR
adds the ability for Windows users to use the pipe character (|) to
escape the metacharacters *, ?, [, and { in .stignore files.

Additionally, this PR adds the ability for the user to set the escape
character to backslash, or any character they want, by adding a line in
the form:

  #escape=X

(where X is any single rune), to the top of an .stignore file.

This would allow users to use the same .stignore file across platforms,
by simply adding

  #escape=\

to the top of the file.

### Testing

All tests pass in CI.

### Documentation

See https://github.com/syncthing/docs/pull/919

Fixes #10057: Support escaping in .stignore files on Windows
Fixes #7547: Ignore pattern with \[ and \] does not work
Ross Smith II 2 miesięcy temu
rodzic
commit
49462448d0
2 zmienionych plików z 441 dodań i 1 usunięć
  1. 55 1
      lib/ignore/ignore.go
  2. 386 0
      lib/ignore/ignore_test.go

+ 55 - 1
lib/ignore/ignore.go

@@ -13,9 +13,11 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"os"
 	"path/filepath"
 	"strings"
 	"time"
+	"unicode/utf8"
 
 	"github.com/gobwas/glob"
 	"golang.org/x/text/unicode/norm"
@@ -27,6 +29,17 @@ import (
 	"github.com/syncthing/syncthing/lib/sync"
 )
 
+const escapePrefix = "#escape"
+
+var defaultEscapeChar = '\\'
+
+func init() {
+	if os.PathSeparator == defaultEscapeChar {
+		// The pipe character (|) is not allowed in filenames on Windows
+		defaultEscapeChar = '|'
+	}
+}
+
 // A ParseError signifies an error with contents of an ignore file,
 // including I/O errors on included files. An I/O error on the root level
 // ignore file is not a ParseError.
@@ -238,6 +251,7 @@ func (m *Matcher) Match(file string) (result ignoreresult.R) {
 		return ignoreresult.NotIgnored
 	}
 
+	// Change backslashes to slashes (on Windows only)
 	file = filepath.ToSlash(file)
 
 	if m.matches != nil {
@@ -504,8 +518,36 @@ func parseIgnoreFile(fs fs.Filesystem, fd io.Reader, currentFile string, cd Chan
 		return nil, nil, err
 	}
 
+	escapeChar := defaultEscapeChar
+
 	var err error
+	escapePrefixSeen := false
+	includedPatterns := 0
 	for _, line := range lines {
+		if strings.HasPrefix(line, escapePrefix) {
+			if escapePrefixSeen {
+				return nil, nil, errors.New("mutiple #escape= lines found in ignore file")
+			}
+
+			if len(patterns)-includedPatterns > 0 {
+				return nil, nil, errors.New("#escape= line found after patterns in ignore file")
+			}
+
+			escapePrefixSeen = true
+			trimmed := strings.TrimSpace(strings.TrimPrefix(line, escapePrefix))
+			before, esc, ok := strings.Cut(trimmed, "=")
+			if ok && before == "" {
+				esc = strings.TrimSpace(esc)
+				// avoids allocation of a new slice.
+				if utf8.RuneCountInString(esc) == 1 {
+					escapeChar, _ = utf8.DecodeRuneInString(esc)
+					continue
+				}
+			}
+
+			return nil, nil, fmt.Errorf("failed to parse #escape= line in ignore file: %q", line)
+		}
+
 		if _, ok := linesSeen[line]; ok {
 			continue
 		}
@@ -517,7 +559,18 @@ func parseIgnoreFile(fs fs.Filesystem, fd io.Reader, currentFile string, cd Chan
 			continue
 		}
 
-		line = filepath.ToSlash(line)
+		if escapeChar != '\\' {
+			// ToSlash changes backslashes to forward slashes on Windows only,
+			// so we only need to do this, if escapeChar is not a backslash.
+			// If escapeChar is a backslash, then the user is using forward
+			// slashes for path separators, and we leave backslashes alone.
+			line = filepath.ToSlash(line)
+			// Replace all escapeChars with backslashes
+			line = strings.ReplaceAll(line, string(escapeChar), `\`)
+			// Now restore double escapeChars to actually escape the escapeChar.
+			line = strings.ReplaceAll(line, `\\`, `\`+string(escapeChar))
+		}
+
 		switch {
 		case strings.HasPrefix(line, "#include"):
 			fields := strings.SplitN(line, " ", 2)
@@ -536,6 +589,7 @@ func parseIgnoreFile(fs fs.Filesystem, fd io.Reader, currentFile string, cd Chan
 			var includePatterns []Pattern
 			if includePatterns, err = loadParseIncludeFile(fs, includeFile, cd, linesSeen); err == nil {
 				patterns = append(patterns, includePatterns...)
+				includedPatterns += len(includePatterns)
 			} else {
 				// Wrap the error, as if the include does not exist, we get a
 				// IsNotExists(err) == true error, which we use to check

+ 386 - 0
lib/ignore/ignore_test.go

@@ -10,6 +10,7 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -22,6 +23,8 @@ import (
 	"github.com/syncthing/syncthing/lib/rand"
 )
 
+const escapePrefixEqual = escapePrefix + "="
+
 var testFiles = map[string]string{
 	".stignore": `#include excludes
 bfile
@@ -1345,3 +1348,386 @@ func TestWindowsLineEndings(t *testing.T) {
 		t.Error("expected there to be a non-zero number of Windows line endings")
 	}
 }
+
+type escapeTest struct {
+	pattern string
+	match   string
+	want    bool
+}
+
+// pathSepIsBackslash could also be set to build.IsWindows, but this will work
+// on any platform where the os.PathSeparator is a backslash (which is
+// currently only Windows).
+const pathSepIsBackslash = os.PathSeparator == '\\'
+
+var backslashTests = []escapeTest{
+	{`a`, `a`, true},
+
+	{`a*`, `a`, true},
+	{`a*b`, `ab`, true},
+	{`*a`, `a`, true},
+	{`*a*`, `a`, true},
+
+	{`a?`, `ab`, true},
+	{`a?b`, `acb`, true},
+	{`?a`, `ba`, true},
+	{`?a?`, `bac`, true},
+
+	{`a[bc]`, `ab`, true},
+	{`a[bc]d`, `abd`, true},
+	{`[ab]c`, `ac`, true},
+	{`[ab]c[de]`, `acd`, true},
+
+	{`a{b,c}`, `ab`, true},
+	{`a{b,c}d`, `abd`, true},
+	{`{a,b}c`, `ac`, true},
+	{`{a,b}c{d,e}`, `acd`, true},
+
+	{`a/**`, `a/b/c`, true},
+	{`a**c`, `a/b/c`, true},
+	{`**/c`, `a/b/c`, true},
+	{`a**b**c`, `a/b/c`, true},
+	{`**/c/**`, `a/b/c/d/e`, true},
+
+	{`a]b`, `a]b`, true},
+	{`a}b`, `a}b`, true},
+
+	{`a\*`, `a*`, !pathSepIsBackslash},
+	{`a\*b`, `a*b`, !pathSepIsBackslash},
+	{`\*a`, `*a`, true}, // backslash is first character
+	{`\*a\*`, `*a*`, !pathSepIsBackslash},
+
+	{`a\?`, `a?`, !pathSepIsBackslash},
+	{`a\?b`, `a?b`, !pathSepIsBackslash},
+	{`\?a`, `?a`, true}, // backslash is first character
+	{`\?a\?`, `?a?`, !pathSepIsBackslash},
+
+	{`a\[bc\]`, `a[bc]`, !pathSepIsBackslash},
+	{`a\[bc\]d`, `a[bc]d`, !pathSepIsBackslash},
+	{`\[ab\]c`, `[ab]c`, !pathSepIsBackslash},
+	{`\[ab\]c\[de\]`, `[ab]c[de]`, !pathSepIsBackslash},
+
+	{`a\{b,c\}`, `a{b,c}`, !pathSepIsBackslash},
+	{`a\{b,c\}d`, `a{b,c}d`, !pathSepIsBackslash},
+	{`\{a,b\}c`, `{a,b}c`, !pathSepIsBackslash},
+	{`\{a,b\}c\{d,e\}`, `{a,b}c{d,e}`, !pathSepIsBackslash},
+
+	{`a/\*\*`, `a/**`, !pathSepIsBackslash},
+	{`a\*\*c`, `a**c`, !pathSepIsBackslash},
+	{`\*\*/c`, `**/c`, !pathSepIsBackslash},
+	{`a\*\*b\*\*c`, `a**b**c`, !pathSepIsBackslash},
+	{`\*\*/c/\*\*`, `**/c/**`, !pathSepIsBackslash},
+
+	{`\a`, `a`, true}, // backslash is first character
+
+	{`\a\b`, `ab`, !pathSepIsBackslash},
+	{`\a\b`, `a/b`, pathSepIsBackslash},
+
+	{`a\r\n`, `arn`, !pathSepIsBackslash},
+	{`a\r\n`, `a/r/n`, pathSepIsBackslash},
+
+	{`\a\r\n`, `arn`, !pathSepIsBackslash},
+	{`\a\r\n`, `a/r/n`, pathSepIsBackslash},
+	{`\a\r\n`, `/a/r/n`, false}, // leading backslash is stripped off
+}
+
+// TestEscapeBackslash tests backslash (\) as the escape character.
+func TestEscapeBackslash(t *testing.T) {
+	testEscape(t, backslashTests, true)
+}
+
+// pipeTests contains the same wants as backslashTests, but
+// !pathSepIsBackslash is changed to true and
+// pathSepIsBackslash is changed to false.
+var pipeTests = []escapeTest{
+	{`a|*`, `a*`, true},
+	{`a|*b`, `a*b`, true},
+	{`|*a`, `*a`, true}, // backslash is first character
+	{`|*a|*`, `*a*`, true},
+
+	{`a|?`, `a?`, true},
+	{`a|?b`, `a?b`, true},
+	{`|?a`, `?a`, true}, // backslash is first character
+	{`|?a|?`, `?a?`, true},
+
+	{`a|[bc|]`, `a[bc]`, true},
+	{`a|[bc|]d`, `a[bc]d`, true},
+	{`|[ab|]c`, `[ab]c`, true},
+	{`|[ab|]c|[de|]`, `[ab]c[de]`, true},
+
+	{`a|{b,c|}`, `a{b,c}`, true},
+	{`a|{b,c|}d`, `a{b,c}d`, true},
+	{`|{a,b|}c`, `{a,b}c`, true},
+	{`|{a,b|}c|{d,e|}`, `{a,b}c{d,e}`, true},
+
+	{`a/|*|*`, `a/**`, true},
+	{`a|*|*c`, `a**c`, true},
+	{`|*|*/c`, `**/c`, true},
+	{`a|*|*b|*|*c`, `a**b**c`, true},
+	{`|*|*/c/|*|*`, `**/c/**`, true},
+
+	{`a]b`, `a]b`, true},
+	{`a}b`, `a}b`, true},
+
+	{`|a`, `a`, true}, // backslash is first character
+
+	{`|a|b`, `ab`, true},
+	{`|a|b`, `a/b`, false},
+
+	{`a|r|n`, `arn`, true},
+	{`a|r|n`, `a/r/n`, false},
+
+	{`|a|r|n`, `arn`, true},
+	{`|a|r|n`, `a/r/n`, false},
+	{`|a|r|n`, `/a/r/n`, false}, // leading backslash is stripped off
+}
+
+// TestEscapePipe tests when pipe (|) is the defaultEscapeChar character
+// (as it is on Windows).
+func TestEscapePipe(t *testing.T) {
+	if defaultEscapeChar != '|' {
+		t.Skip("Skipping: defaultEscapeChar is not a '|'")
+	}
+
+	testEscape(t, pipeTests, true)
+}
+
+// overrideBackslashTests has the same wants as the pipeTests tests.
+// The only difference in the tests is the pipe symbol in the pattern has been
+// changed to a backslash. This could be done programatically, if desired.
+var overrideBackslashTests = []escapeTest{
+	{`a\*`, `a*`, true},
+	{`a\*b`, `a*b`, true},
+	{`\*a`, `*a`, true}, // backslash is first character
+	{`\*a\*`, `*a*`, true},
+
+	{`a\?`, `a?`, true},
+	{`a\?b`, `a?b`, true},
+	{`\?a`, `?a`, true}, // backslash is first character
+	{`\?a\?`, `?a?`, true},
+
+	{`a\[bc\]`, `a[bc]`, true},
+	{`a\[bc\]d`, `a[bc]d`, true},
+	{`\[ab\]c`, `[ab]c`, true},
+	{`\[ab\]c\[de\]`, `[ab]c[de]`, true},
+
+	{`a\{b,c\}`, `a{b,c}`, true},
+	{`a\{b,c\}d`, `a{b,c}d`, true},
+	{`\{a,b\}c`, `{a,b}c`, true},
+	{`\{a,b\}c\{d,e\}`, `{a,b}c{d,e}`, true},
+
+	{`a/\*\*`, `a/**`, true},
+	{`a\*\*c`, `a**c`, true},
+	{`\*\*/c`, `**/c`, true},
+	{`a\*\*b\*\*c`, `a**b**c`, true},
+	{`\*\*/c/\*\*`, `**/c/**`, true},
+
+	{`a]b`, `a]b`, true},
+	{`a}b`, `a}b`, true},
+
+	{`\a`, `a`, true}, // backslash is first character
+
+	{`\a\b`, `ab`, true},
+	{`\a\b`, `a/b`, false},
+
+	{`a\r\n`, `arn`, true},
+	{`a\r\n`, `a/r/n`, false},
+
+	{`\a\r\n`, `arn`, true},
+	{`\a\r\n`, `a/r/n`, false},
+	{`\a\r\n`, `/a/r/n`, false}, // leading backslash is stripped off
+}
+
+// TestEscapeOverrideBackslash tests when #escape=\ is in the .stignore file.
+func TestEscapeOverrideBackslash(t *testing.T) {
+	tests := make([]escapeTest, 0, len(overrideBackslashTests))
+
+	for _, test := range overrideBackslashTests {
+		tests = append(tests, escapeTest{
+			escapePrefixEqual + "\\\n" + test.pattern,
+			test.match,
+			test.want,
+		})
+	}
+
+	testEscape(t, tests, true)
+}
+
+// TestEscapeOverridePipe tests when #escape=| (or another character) is in the
+// .stignore file.
+func TestEscapeOverridePipe(t *testing.T) {
+	escapeChars := []string{
+		"|",
+		">",
+		"\u241B", // ␛
+	}
+
+	tests := make([]escapeTest, 0, len(pipeTests))
+
+	for _, test := range pipeTests {
+		for _, escapeChar := range escapeChars {
+			tests = append(tests, escapeTest{
+				escapePrefixEqual + escapeChar + "\n" + strings.ReplaceAll(test.pattern, "|", escapeChar),
+				test.match,
+				test.want,
+			})
+		}
+	}
+
+	testEscape(t, tests, true)
+}
+
+var escapePrefixes = []string{
+	"",
+	"\n",
+	"// comment\n",
+	"\n// comment\n",
+	"#include escape-excludes\n",
+	"// comment\n#include escape-excludes\n",
+	"#include escape-excludes\n//comment\n",
+	"// comment\n#include escape-excludes\n//comment\n",
+}
+
+// TestEscapeBeforePattern tests when #escape= is found before a pattern in the
+// .stignore file.
+func TestEscapeBeforePattern(t *testing.T) {
+	tests := make([]escapeTest, 0, len(overrideBackslashTests)*len(escapePrefixes))
+
+	for _, test := range overrideBackslashTests {
+		for _, prefix := range escapePrefixes {
+			tests = append(tests, escapeTest{
+				// Use backslash, as it should not be ignored,
+				// so test against the overrideBackslashTests.
+				prefix + escapePrefixEqual + "\\\n" + test.pattern,
+				test.match,
+				test.want,
+			})
+		}
+	}
+
+	testEscape(t, tests, true)
+}
+
+// TestEscapeEmpty tests when #escape= (no char) is in the .stignore file.
+func TestEscapeEmpty(t *testing.T) {
+	suffixes := []string{"", " ", "\t", "=", "= ", "=\t", "x"}
+
+	tests := make([]escapeTest, 0, len(backslashTests)*len(suffixes))
+
+	for _, test := range backslashTests {
+		for _, suffix := range suffixes {
+			tests = append(tests, escapeTest{
+				escapePrefix + suffix + "\n" + test.pattern,
+				test.match,
+				false,
+			})
+		}
+	}
+
+	testEscape(t, tests, false)
+}
+
+// TestEscapeInvalid tests when #escape=x has extra characters after it
+func TestEscapeInvalid(t *testing.T) {
+	suffixes := []string{"\\\\", "||", "\u241B\u241B", "xx"} // ␛
+
+	tests := make([]escapeTest, 0, len(backslashTests)*len(suffixes))
+
+	for _, test := range backslashTests {
+		for _, suffix := range suffixes {
+			tests = append(tests, escapeTest{
+				escapePrefixEqual + suffix + "\n" + test.pattern,
+				test.match,
+				false,
+			})
+		}
+	}
+
+	testEscape(t, tests, false)
+}
+
+// TestEscapeAfterPattern tests when #escape= is found after a pattern in the
+// .stignore file.
+func TestEscapeAfterPattern(t *testing.T) {
+	suffixes := []string{
+		"pattern\n",
+		"pattern/\n",
+		"pattern/**\n",
+	}
+
+	tests := make([]escapeTest, 0, len(backslashTests)*len(escapePrefixes)*len(suffixes))
+
+	for _, test := range backslashTests {
+		for _, prefix := range escapePrefixes {
+			for _, suffix := range suffixes {
+				tests = append(tests, escapeTest{
+					// Use a different character, as it should be ignored,
+					// so test against the backslashTests.
+					prefix + suffix + escapePrefixEqual + "\u241B\n" + test.pattern,
+					test.match,
+					false,
+				})
+			}
+		}
+	}
+
+	testEscape(t, tests, false)
+}
+
+// TestEscapeDoubled tests when #escape= is found more than once.
+func TestEscapeDoubled(t *testing.T) {
+	suffixes := []string{
+		"#escape\n",
+		"#escape=\n",
+		"#escape=\\\n",
+		"#escape=|\n",
+	}
+
+	tests := make([]escapeTest, 0, len(backslashTests)*len(suffixes))
+
+	for _, test := range backslashTests {
+		for _, suffix := range suffixes {
+			tests = append(tests, escapeTest{
+				escapePrefixEqual + "\\\n" + suffix + test.pattern,
+				test.match,
+				false,
+			})
+		}
+	}
+
+	testEscape(t, tests, false)
+}
+
+var testEscapeFiles = map[string]string{
+	"escape-excludes": "dir4\n",
+}
+
+func testEscape(t *testing.T, tests []escapeTest, noErrors bool) {
+	t.Helper()
+
+	for i, test := range tests {
+		testFS := fs.NewFilesystem(fs.FilesystemTypeFake, rand.String(32)+"?content=true&nostfolder=true")
+
+		for name, content := range testEscapeFiles {
+			fs.WriteFile(testFS, name, []byte(content), 0o666)
+		}
+		pats := New(testFS, WithCache(true))
+
+		err := pats.Parse(bytes.NewBufferString(test.pattern), ".stignore")
+		if noErrors {
+			if err != nil {
+				t.Fatalf("%q: err=%v (test %d)", test.pattern, err, i+1)
+			}
+		} else {
+			if err == nil {
+				t.Fatalf("%q: got nil, want error (test %d)", test.pattern, i+1)
+			}
+			continue
+		}
+
+		got := pats.Match(test.match).IsIgnored()
+		if got != test.want {
+			t.Errorf("%-20q: %-20q: got %v, want %v (test %d)", test.pattern, test.match, got, test.want, i+1)
+		}
+	}
+}