Skip to content

Commit 785ad11

Browse files
Honny1Honny1
committed
fileutils: fix wildcard negation directory descent in archive.TarWithOptions
The `strings.HasPrefix` check in `tarWithOptionsTo` only worked for literal negation patterns (e.g. `!cmd/main.go`) but failed for wildcard negations like `!**/*.go`, `!*/*.go`, or `!cmd/image*`. This caused excluded directories to be skipped entirely even when negation patterns should have re-included files within them. Add `ShouldDescendExcludedDir` to `pkg/fileutils` which extracts the literal prefix before the first wildcard character and checks whether the directory is at or under that prefix. When no literal prefix exists (e.g. `!**/*.go`), always descend. Replace the inline loop in tarWithOptionsTo with a call to the new function. Signed-off-by: Jan Rodák <hony.com@seznam.cz>
1 parent b17b0a4 commit 785ad11

3 files changed

Lines changed: 213 additions & 17 deletions

File tree

‎storage/pkg/archive/archive.go‎

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1012,24 +1012,9 @@ func tarWithOptionsTo(dest io.WriteCloser, srcPath string, options *TarOptions)
10121012
return nil
10131013
}
10141014

1015-
// No exceptions (!...) in patterns so just skip dir
1016-
if !pm.Exclusions() {
1017-
return filepath.SkipDir
1018-
}
1019-
1020-
dirSlash := relFilePath + string(filepath.Separator)
1021-
1022-
for _, pat := range pm.Patterns() {
1023-
if !pat.Exclusion() {
1024-
continue
1025-
}
1026-
if strings.HasPrefix(pat.String()+string(filepath.Separator), dirSlash) {
1027-
// found a match - so can't skip this dir
1028-
return nil
1029-
}
1015+
if fileutils.ShouldDescendExcludedDir(relFilePath, pm) {
1016+
return nil
10301017
}
1031-
1032-
// No matching exclusion dir so just skip dir
10331018
return filepath.SkipDir
10341019
}
10351020

‎storage/pkg/fileutils/fileutils.go‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,50 @@ func Matches(file string, patterns []string) (bool, error) {
299299
return pm.IsMatch(file)
300300
}
301301

302+
// ShouldDescendExcludedDir checks whether an excluded directory should still be
303+
// descended into because a negation pattern in pm might match files under it.
304+
// It handles literal prefix matches (e.g. !cmd/main.go for dir "cmd") and
305+
// wildcard negations (e.g. !**/*.go, !*/*.go). The wildcard check extracts
306+
// the literal prefix before the first wildcard and may intentionally
307+
// overmatch (descend into directories that won't ultimately contain matches),
308+
// which is safe because actual file-level matching happens later.
309+
func ShouldDescendExcludedDir(dirPath string, pm *PatternMatcher) bool {
310+
if pm == nil || !pm.Exclusions() {
311+
return false
312+
}
313+
dir := filepath.ToSlash(strings.Trim(dirPath, string(os.PathSeparator)))
314+
for _, pattern := range pm.Patterns() {
315+
if !pattern.Exclusion() {
316+
continue
317+
}
318+
slashPattern := filepath.ToSlash(strings.Trim(pattern.String(), string(os.PathSeparator)))
319+
320+
// Literal-prefix check: the negation spec starts with this
321+
// directory path, for example: !cmd/main.go matches dir "cmd"
322+
if strings.HasPrefix(slashPattern, dir+"/") {
323+
return true
324+
}
325+
326+
// Wildcard-aware check: extract the literal prefix before
327+
// the first wildcard character (*, ?, [), for example: !cmd/**/*.go matches dir "cmd"
328+
// if the directory is at or under that literal prefix, a file beneath this
329+
// directory could match the negation, so keep descending.
330+
if firstWild := strings.IndexAny(slashPattern, "*?["); firstWild >= 0 {
331+
var literalPrefix string
332+
if idx := strings.LastIndex(slashPattern[:firstWild], "/"); idx >= 0 {
333+
literalPrefix = slashPattern[:idx]
334+
}
335+
if literalPrefix == "" {
336+
return true
337+
}
338+
if dir == literalPrefix || strings.HasPrefix(dir, literalPrefix+"/") {
339+
return true
340+
}
341+
}
342+
}
343+
return false
344+
}
345+
302346
// CopyFile copies from src to dst until either EOF is reached
303347
// on src or an error occurs. It verifies src exists and removes
304348
// the dst if it exists.

‎storage/pkg/fileutils/fileutils_test.go‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"github.com/stretchr/testify/assert"
1313
"github.com/stretchr/testify/require"
14+
"go.podman.io/storage/pkg/fileutils"
1415
)
1516

1617
const windows = "windows"
@@ -617,3 +618,169 @@ func TestMatchesAmount(t *testing.T) {
617618
assert.Equal(t, testCase.isMatch, isMatch, desc)
618619
}
619620
}
621+
622+
func TestShouldDescendExcludedDir(t *testing.T) {
623+
tests := []struct {
624+
name string
625+
path string
626+
patterns []string
627+
want bool
628+
}{
629+
{
630+
name: "nil matcher",
631+
path: "cmd",
632+
patterns: nil,
633+
want: false,
634+
},
635+
{
636+
name: "no exclusions",
637+
path: "cmd",
638+
patterns: []string{"*"},
639+
want: false,
640+
},
641+
{
642+
name: "literal prefix match",
643+
path: "cmd",
644+
patterns: []string{"*", "!cmd/main.go"},
645+
want: true,
646+
},
647+
{
648+
name: "literal prefix no match",
649+
path: "other",
650+
patterns: []string{"*", "!cmd/main.go"},
651+
want: false,
652+
},
653+
{
654+
name: "double star at start matches any dir",
655+
path: "cmd",
656+
patterns: []string{"**", "!**/*.go"},
657+
want: true,
658+
},
659+
{
660+
name: "double star at start matches nested dir",
661+
path: "cmd/sub",
662+
patterns: []string{"**", "!**/*.go"},
663+
want: true,
664+
},
665+
{
666+
name: "double star with prefix matches dir under prefix",
667+
path: "cmd/sub",
668+
patterns: []string{"**", "!cmd/**/*.go"},
669+
want: true,
670+
},
671+
{
672+
name: "double star with prefix no match for other dir",
673+
path: "other",
674+
patterns: []string{"**", "!cmd/**/*.go"},
675+
want: false,
676+
},
677+
{
678+
name: "single star at start matches any dir",
679+
path: "cmd",
680+
patterns: []string{"*", "!*/*.go"},
681+
want: true,
682+
},
683+
{
684+
name: "single star at start matches nested dir",
685+
path: "cmd/sub",
686+
patterns: []string{"*", "!*/*.go"},
687+
want: true,
688+
},
689+
{
690+
name: "single star with prefix matches dir under prefix",
691+
path: "src/pkg",
692+
patterns: []string{"**", "!src/*/*.go"},
693+
want: true,
694+
},
695+
{
696+
name: "single star with prefix no match for other dir",
697+
path: "other",
698+
patterns: []string{"**", "!src/*/*.go"},
699+
want: false,
700+
},
701+
{
702+
name: "leading slash is stripped",
703+
path: "/cmd",
704+
patterns: []string{"*", "!cmd/main.go"},
705+
want: true,
706+
},
707+
{
708+
name: "deep nested with double star prefix",
709+
path: "src/internal/pkg",
710+
patterns: []string{"**", "!src/**/*.go"},
711+
want: true,
712+
},
713+
{
714+
name: "dir prefix match is not a partial match",
715+
path: "cmds",
716+
patterns: []string{"*", "!cmd/main.go"},
717+
want: false,
718+
},
719+
{
720+
name: "wildcard mid-segment descends parent dir",
721+
path: "cmd/images",
722+
patterns: []string{"**", "!cmd/image*/main.go"},
723+
want: true,
724+
},
725+
{
726+
name: "wildcard mid-segment matches parent",
727+
path: "cmd",
728+
patterns: []string{"**", "!cmd/image*"},
729+
want: true,
730+
},
731+
{
732+
name: "question mark wildcard matches any dir",
733+
path: "cmd",
734+
patterns: []string{"**", "!cm?/*.go"},
735+
want: true,
736+
},
737+
{
738+
name: "question mark wildcard no literal prefix matches any dir",
739+
path: "other",
740+
patterns: []string{"**", "!?md/*.go"},
741+
want: true,
742+
},
743+
{
744+
name: "bracket wildcard matches dir",
745+
path: "cmd",
746+
patterns: []string{"**", "!cm[d]/*.go"},
747+
want: true,
748+
},
749+
{
750+
name: "bracket wildcard no literal prefix matches any dir",
751+
path: "other",
752+
patterns: []string{"**", "![c]md/*.go"},
753+
want: true,
754+
},
755+
{
756+
name: "bracket wildcard with prefix matches dir under prefix",
757+
path: "src/cmd",
758+
patterns: []string{"**", "!src/cm[d]/*.go"},
759+
want: true,
760+
},
761+
{
762+
name: "bracket wildcard with prefix no match for other dir",
763+
path: "other",
764+
patterns: []string{"**", "!src/cm[d]/*.go"},
765+
want: false,
766+
},
767+
{
768+
name: "non-exclusion patterns are ignored",
769+
path: "cmd",
770+
patterns: []string{"cmd/**/*.go"},
771+
want: false,
772+
},
773+
}
774+
for _, tt := range tests {
775+
t.Run(tt.name, func(t *testing.T) {
776+
var pm *fileutils.PatternMatcher
777+
if tt.patterns != nil {
778+
var err error
779+
pm, err = fileutils.NewPatternMatcher(tt.patterns)
780+
require.NoError(t, err)
781+
}
782+
got := ShouldDescendExcludedDir(tt.path, pm)
783+
assert.Equal(t, tt.want, got, "ShouldDescendExcludedDir(%q)", tt.path)
784+
})
785+
}
786+
}

0 commit comments

Comments
 (0)