From 02c777ffbf66f3775130353e3e9d690aa2ce0a3f Mon Sep 17 00:00:00 2001
From: Michele Caci <michele.caci@gmail.com>
Date: Wed, 9 Oct 2019 14:06:46 +0200
Subject: [PATCH] filter: prevent mix opts when filesfrom is present - fixes
 #3599

---
 fs/filter/filter.go      |  5 ++++
 fs/filter/filter_test.go | 62 +++++++++++++++++++++++++++++++++-------
 2 files changed, 57 insertions(+), 10 deletions(-)

diff --git a/fs/filter/filter.go b/fs/filter/filter.go
index b693c2129..460fad6a6 100644
--- a/fs/filter/filter.go
+++ b/fs/filter/filter.go
@@ -191,7 +191,12 @@ func NewFilter(opt *Opt) (f *Filter, err error) {
 			return nil, err
 		}
 	}
+
+	inActive := f.InActive()
 	for _, rule := range f.Opt.FilesFrom {
+		if !inActive {
+			return nil, fmt.Errorf("The usage of --files-from overrides all other filters, it should be used alone")
+		}
 		f.initAddFile() // init to show --files-from set even if no files within
 		err := forEachLine(rule, func(line string) error {
 			return f.AddFile(line)
diff --git a/fs/filter/filter_test.go b/fs/filter/filter_test.go
index 36c425cba..773b3eb04 100644
--- a/fs/filter/filter_test.go
+++ b/fs/filter/filter_test.go
@@ -42,7 +42,58 @@ func testFile(t *testing.T, contents string) string {
 	return s
 }
 
-func TestNewFilterFull(t *testing.T) {
+func TestNewFilterForbiddenMixOfFilesFromAndFilterRule(t *testing.T) {
+	Opt := DefaultOpt
+
+	// Set up the input
+	Opt.FilterRule = []string{"- filter1", "- filter1b"}
+	Opt.FilesFrom = []string{testFile(t, "#comment\nfiles1\nfiles2\n")}
+
+	rm := func(p string) {
+		err := os.Remove(p)
+		if err != nil {
+			t.Logf("error removing %q: %v", p, err)
+		}
+	}
+	// Reset the input
+	defer func() {
+		rm(Opt.FilesFrom[0])
+	}()
+
+	_, err := NewFilter(&Opt)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "The usage of --files-from overrides all other filters")
+}
+
+func TestNewFilterWithFilesFromAlone(t *testing.T) {
+	Opt := DefaultOpt
+
+	// Set up the input
+	Opt.FilesFrom = []string{testFile(t, "#comment\nfiles1\nfiles2\n")}
+
+	rm := func(p string) {
+		err := os.Remove(p)
+		if err != nil {
+			t.Logf("error removing %q: %v", p, err)
+		}
+	}
+	// Reset the input
+	defer func() {
+		rm(Opt.FilesFrom[0])
+	}()
+
+	f, err := NewFilter(&Opt)
+	require.NoError(t, err)
+	assert.Len(t, f.files, 2)
+	for _, name := range []string{"files1", "files2"} {
+		_, ok := f.files[name]
+		if !ok {
+			t.Errorf("Didn't find file %q in f.files", name)
+		}
+	}
+}
+
+func TestNewFilterFullExceptFilesFromOpt(t *testing.T) {
 	Opt := DefaultOpt
 
 	mins := fs.SizeSuffix(100 * 1024)
@@ -56,7 +107,6 @@ func TestNewFilterFull(t *testing.T) {
 	Opt.ExcludeFrom = []string{testFile(t, "#comment\nexclude2\nexclude3\n")}
 	Opt.IncludeRule = []string{"include1"}
 	Opt.IncludeFrom = []string{testFile(t, "#comment\ninclude2\ninclude3\n")}
-	Opt.FilesFrom = []string{testFile(t, "#comment\nfiles1\nfiles2\n")}
 	Opt.MinSize = mins
 	Opt.MaxSize = maxs
 
@@ -71,7 +121,6 @@ func TestNewFilterFull(t *testing.T) {
 		rm(Opt.FilterFrom[0])
 		rm(Opt.ExcludeFrom[0])
 		rm(Opt.IncludeFrom[0])
-		rm(Opt.FilesFrom[0])
 	}()
 
 	f, err := NewFilter(&Opt)
@@ -96,13 +145,6 @@ func TestNewFilterFull(t *testing.T) {
 + ^.*$
 - ^.*$`
 	assert.Equal(t, want, got)
-	assert.Len(t, f.files, 2)
-	for _, name := range []string{"files1", "files2"} {
-		_, ok := f.files[name]
-		if !ok {
-			t.Errorf("Didn't find file %q in f.files", name)
-		}
-	}
 	assert.False(t, f.InActive())
 }