lib/ignore: Ignore duplicate lines in .stignore

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4350
LGTM: AudriusButkevicius, calmh

lib/ignore/ignore.go

@@ -132,19 +132,13 @@ func (m *Matcher) Load(file string) error {
 		return nil
 	}
 
-	fd, err := m.fs.Open(file)
+	fd, info, err := loadIgnoreFile(m.fs, file, m.changeDetector)
 	if err != nil {
 		m.parseLocked(&bytes.Buffer{}, file)
 		return err
 	}
 	defer fd.Close()
 
-	info, err := fd.Stat()
-	if err != nil {
-		m.parseLocked(&bytes.Buffer{}, file)
-		return err
-	}
-
 	m.changeDetector.Reset()
 	m.changeDetector.Remember(m.fs, file, info.ModTime())
 
@@ -158,7 +152,7 @@ func (m *Matcher) Parse(r io.Reader, file string) error {
 }
 
 func (m *Matcher) parseLocked(r io.Reader, file string) error {
-	lines, patterns, err := parseIgnoreFile(m.fs, r, file, m.changeDetector)
+	lines, patterns, err := parseIgnoreFile(m.fs, r, file, m.changeDetector, make(map[string]struct{}))
 	// Error is saved and returned at the end. We process the patterns
 	// (possibly blank) anyway.
 
@@ -300,11 +294,21 @@ func hashPatterns(patterns []Pattern) string {
 	return fmt.Sprintf("%x", h.Sum(nil))
 }
 
-func loadIgnoreFile(filesystem fs.Filesystem, file string, cd ChangeDetector) ([]string, []Pattern, error) {
+func loadIgnoreFile(fs fs.Filesystem, file string, cd ChangeDetector) (fs.File, fs.FileInfo, error) {
-	if cd.Seen(filesystem, file) {
+	fd, err := fs.Open(file)
-		return nil, nil, fmt.Errorf("multiple include of ignore file %q", file)
+	if err != nil {
+		return fd, nil, err
 	}
 
+	info, err := fd.Stat()
+	if err != nil {
+		fd.Close()
+	}
+
+	return fd, info, err
+}
+
+func loadParseIncludeFile(filesystem fs.Filesystem, file string, cd ChangeDetector, linesSeen map[string]struct{}) ([]string, []Pattern, error) {
 	// Allow escaping the folders filesystem.
 	// TODO: Deprecate, somehow?
 	if filesystem.Type() == fs.FilesystemTypeBasic {
@@ -316,23 +320,22 @@ func loadIgnoreFile(filesystem fs.Filesystem, file string, cd ChangeDetector) ([
 		}
 	}
 
-	fd, err := filesystem.Open(file)
+	if cd.Seen(filesystem, file) {
+		return nil, nil, fmt.Errorf("multiple include of ignore file %q", file)
+	}
+
+	fd, info, err := loadIgnoreFile(filesystem, file, cd)
 	if err != nil {
 		return nil, nil, err
 	}
 	defer fd.Close()
 
-	info, err := fd.Stat()
-	if err != nil {
-		return nil, nil, err
-	}
-
 	cd.Remember(filesystem, file, info.ModTime())
 
-	return parseIgnoreFile(filesystem, fd, file, cd)
+	return parseIgnoreFile(filesystem, fd, file, cd, linesSeen)
 }
 
-func parseIgnoreFile(fs fs.Filesystem, fd io.Reader, currentFile string, cd ChangeDetector) ([]string, []Pattern, error) {
+func parseIgnoreFile(fs fs.Filesystem, fd io.Reader, currentFile string, cd ChangeDetector, linesSeen map[string]struct{}) ([]string, []Pattern, error) {
 	var lines []string
 	var patterns []Pattern
 
@@ -399,7 +402,7 @@ func parseIgnoreFile(fs fs.Filesystem, fd io.Reader, currentFile string, cd Chan
 		} else if strings.HasPrefix(line, "#include ") {
 			includeRel := strings.TrimSpace(line[len("#include "):])
 			includeFile := filepath.Join(filepath.Dir(currentFile), includeRel)
-			_, includePatterns, err := loadIgnoreFile(fs, includeFile, cd)
+			_, includePatterns, err := loadParseIncludeFile(fs, includeFile, cd, linesSeen)
 			if err != nil {
 				return fmt.Errorf("include of %q: %v", includeRel, err)
 			}
@@ -429,6 +432,10 @@ func parseIgnoreFile(fs fs.Filesystem, fd io.Reader, currentFile string, cd Chan
 	for scanner.Scan() {
 		line := strings.TrimSpace(scanner.Text())
 		lines = append(lines, line)
+		if _, ok := linesSeen[line]; ok {
+			continue
+		}
+		linesSeen[line] = struct{}{}
 		switch {
 		case line == "":
 			continue

lib/ignore/ignore_test.go

@@ -872,6 +872,7 @@ func TestLines(t *testing.T) {
 
 	!/a
 	/*
+	!/a
 	`
 
 	pats := New(fs.NewFilesystem(fs.FilesystemTypeBasic, "."), WithCache(true))
@@ -886,6 +887,7 @@ func TestLines(t *testing.T) {
 		"",
 		"!/a",
 		"/*",
+		"!/a",
 		"",
 	}
 
@@ -898,5 +900,33 @@ func TestLines(t *testing.T) {
 			t.Fatalf("Lines()[%d] == %s, expected %s", i, lines[i], expectedLines[i])
 		}
 	}
-
+}
+
+func TestDuplicateLines(t *testing.T) {
+	stignore := `
+	!/a
+	/*
+	!/a
+	`
+	stignoreFiltered := `
+	!/a
+	/*
+	`
+
+	pats := New(fs.NewFilesystem(fs.FilesystemTypeBasic, "testdata"), WithCache(true))
+
+	err := pats.Parse(bytes.NewBufferString(stignore), ".stignore")
+	if err != nil {
+		t.Fatal(err)
+	}
+	patsLen := len(pats.patterns)
+
+	err = pats.Parse(bytes.NewBufferString(stignoreFiltered), ".stignore")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if patsLen != len(pats.patterns) {
+		t.Fatalf("Parsed patterns differ when manually removing duplicate lines")
+	}
 }