From 4f7e41410a4e3df8a812118ecc6c3e7ba9e244dc Mon Sep 17 00:00:00 2001 From: Jojo-1000 <33495614+Jojo-1000@users.noreply.github.com> Date: Thu, 12 Oct 2023 13:20:44 +0200 Subject: [PATCH] Fix incorrect filter behavior when combining multiple regexp. Closes #4590 With multiple regexp filters, if an earlier filter matches only part of a path, any other filters were ignored. --- Duplicati/Library/Utility/FilterExpression.cs | 77 ++++++++++--------- Duplicati/Library/Utility/Utility.cs | 2 +- Duplicati/UnitTest/FilterTest.cs | 44 +++++++++-- 3 files changed, 82 insertions(+), 41 deletions(-) diff --git a/Duplicati/Library/Utility/FilterExpression.cs b/Duplicati/Library/Utility/FilterExpression.cs index 60ca22588..80202075a 100644 --- a/Duplicati/Library/Utility/FilterExpression.cs +++ b/Duplicati/Library/Utility/FilterExpression.cs @@ -72,7 +72,7 @@ namespace Duplicati.Library.Utility /// The regular expression version of the filter /// public readonly Regex Regexp; - + /// /// The single wildcard character (DOS style) /// @@ -112,7 +112,14 @@ namespace Duplicati.Library.Utility { this.Type = FilterType.Regexp; this.Filter = filter.Substring(1, filter.Length - 2); - this.Regexp = new Regex(this.Filter, REGEXP_OPTIONS); + if (Filter.StartsWith("^") && Filter.EndsWith("$")) + { + this.Regexp = new Regex(this.Filter, REGEXP_OPTIONS); + } + else + { + this.Regexp = new Regex("^(" + this.Filter + ")$", REGEXP_OPTIONS); + } } else if (filter.StartsWith("{", StringComparison.Ordinal) && filter.EndsWith("}", StringComparison.Ordinal)) { @@ -181,14 +188,14 @@ namespace Duplicati.Library.Utility regexString = "(" + string.Join(")|(", regexStrings) + ")"; } - result = new Regex(regexString, REGEXP_OPTIONS); + result = new Regex("^(" + regexString + ")$", REGEXP_OPTIONS); FilterEntry.filterGroupRegexCache[filterGroup] = result; return result; } } - + /// /// Tests whether specified string can be matched against provided pattern string. Pattern may contain single- and multiple-replacing /// wildcard characters. @@ -211,7 +218,7 @@ namespace Duplicati.Library.Utility inputPos++; patternPos++; } - + // Push this position to stack if it points to end of pattern or to a general wildcard if (patternPos == pattern.Length || pattern[patternPos] == MULTIPLE_WILDCARD) { @@ -232,10 +239,10 @@ namespace Duplicati.Library.Utility if (inputPos == input.Length && (patternPos == pattern.Length || (patternPos == pattern.Length - 1 && pattern[patternPos] == MULTIPLE_WILDCARD))) matched = true; // Reached end of both pattern and input string, hence matching is successful else - { + { // First character in next pattern block is guaranteed to be multiple wildcard // So skip it and search for all matches in value string until next multiple wildcard character is reached in pattern - for(int curInputStart = inputPos; curInputStart < input.Length; curInputStart++) + for (int curInputStart = inputPos; curInputStart < input.Length; curInputStart++) { int curInputPos = curInputStart; int curPatternPos = patternPos + 1; @@ -255,7 +262,7 @@ namespace Duplicati.Library.Utility // If we have reached next multiple wildcard character in pattern without breaking the matching sequence, then we have another candidate for full match // This candidate should be pushed to stack for further processing // At the same time, pair (input position, pattern position) will be marked as tested, so that it will not be pushed to stack later again - if (((curPatternPos == pattern.Length && curInputPos == input.Length) || (curPatternPos < pattern.Length && pattern[curPatternPos] == MULTIPLE_WILDCARD)) + if (((curPatternPos == pattern.Length && curInputPos == input.Length) || (curPatternPos < pattern.Length && pattern[curPatternPos] == MULTIPLE_WILDCARD)) && !pointTested[curInputPos, curPatternPos]) { pointTested[curInputPos, curPatternPos] = true; @@ -267,7 +274,7 @@ namespace Duplicati.Library.Utility } return matched; } - + /// /// Gets a value indicating if the filter matches the path /// @@ -285,7 +292,7 @@ namespace Duplicati.Library.Utility var m = this.Regexp.Match(path); return m.Success && m.Length == path.Length; default: - return false; + return false; } } @@ -304,12 +311,12 @@ namespace Duplicati.Library.Utility } } } - + /// /// The internal list of expressions /// private readonly List m_filters; - + /// /// Gets the type of the filter /// @@ -337,7 +344,7 @@ namespace Duplicati.Library.Utility else throw new InvalidOperationException($"Cannot extract simple list when the type is: {this.Type}"); } - + /// /// Gets a value indicating if the filter matches the path /// @@ -351,14 +358,14 @@ namespace Duplicati.Library.Utility match = null; return false; } - + if (m_filters.Any(x => x.Matches(path))) { match = this; result = this.Result; return true; } - + match = null; return false; } @@ -367,7 +374,7 @@ namespace Duplicati.Library.Utility public string GetFilterHash() { var hash = MD5HashHelper.GetHash(m_filters?.Select(x => x.Filter)); - return Utility.ByteArrayAsHexString(hash); + return Utility.ByteArrayAsHexString(hash); } /// @@ -395,26 +402,26 @@ namespace Duplicati.Library.Utility public FilterExpression(IEnumerable filter, bool result = true) { this.Result = result; - + if (filter == null) { this.Type = FilterType.Empty; return; } - + m_filters = Compact( (from n in filter - let nx = new FilterEntry(n) - where nx.Type != FilterType.Empty - select nx) + let nx = new FilterEntry(n) + where nx.Type != FilterType.Empty + select nx) ); - + if (m_filters.Count == 0) this.Type = FilterType.Empty; else this.Type = m_filters.Max((a) => a.Type); } - + private static IEnumerable Expand(string filter) { if (string.IsNullOrWhiteSpace(filter)) @@ -434,7 +441,7 @@ namespace Duplicati.Library.Utility return filter.Split(new char[] { System.IO.Path.PathSeparator }, StringSplitOptions.RemoveEmptyEntries); } - + private static List Compact(IEnumerable items) { var r = new List(); @@ -478,7 +485,7 @@ namespace Duplicati.Library.Utility if (combined.Length > 0) r.Add(new FilterEntry("[" + combined.Append("]"))); - return r; + return r; } /// @@ -516,7 +523,7 @@ namespace Duplicati.Library.Utility // Check for cached results if (filter != null) - lock(_matchLock) + lock (_matchLock) if (_matchFallbackLookup.TryGetValue(filter, out cacheLookup)) { includes = cacheLookup.Item1; @@ -549,7 +556,7 @@ namespace Duplicati.Library.Utility } // Populate the cache - lock(_matchLock) + lock (_matchLock) { if (_matchFallbackLookup.Count > 10) _matchFallbackLookup.Remove(_matchFallbackLookup.Keys.Skip(new Random().Next(0, _matchFallbackLookup.Count)).First()); @@ -571,7 +578,7 @@ namespace Duplicati.Library.Utility match = null; return true; } - + bool result; if (filter.Matches(path, out result, out match)) return result; @@ -594,7 +601,7 @@ namespace Duplicati.Library.Utility } } - + /// /// Combine the specified filter expressions. /// @@ -638,12 +645,12 @@ namespace Duplicati.Library.Utility { if (this.Empty) return ""; - - return + + return "(" + string.Join(") || (", (from n in m_filters - select n.ToString()) + select n.ToString()) ) + ")"; } @@ -659,7 +666,7 @@ namespace Duplicati.Library.Utility return (from n in m_filters - select $"{(this.Result ? "+" : "-")}{n.ToString()}" + select $"{(this.Result ? "+" : "-")}{n.ToString()}" ).ToArray(); } @@ -671,7 +678,7 @@ namespace Duplicati.Library.Utility { if (filter == null || filter.Empty) return new string[0]; - + IEnumerable res = new string[0]; var work = new Stack(); work.Push(filter); @@ -705,7 +712,7 @@ namespace Duplicati.Library.Utility return null; IFilter res = null; - foreach(var n in filters) + foreach (var n in filters) { bool include; if (n.StartsWith("+", StringComparison.Ordinal)) diff --git a/Duplicati/Library/Utility/Utility.cs b/Duplicati/Library/Utility/Utility.cs index 1c7bb8e0e..1f89048b0 100644 --- a/Duplicati/Library/Utility/Utility.cs +++ b/Duplicati/Library/Utility/Utility.cs @@ -201,7 +201,7 @@ namespace Duplicati.Library.Utility //Replace the globbing expressions with the corresponding regular expressions globexp = globexp.Replace('?', '.').Replace("*", ".*"); - return globexp; + return "^" + globexp + "$"; } /// diff --git a/Duplicati/UnitTest/FilterTest.cs b/Duplicati/UnitTest/FilterTest.cs index a40da0a17..381838a8c 100644 --- a/Duplicati/UnitTest/FilterTest.cs +++ b/Duplicati/UnitTest/FilterTest.cs @@ -62,7 +62,7 @@ namespace Duplicati.UnitTest // Create a fileset with all data present using (var c = new Library.Main.Controller("file://" + TARGETFOLDER, testopts, null)) { - IBackupResults backupResults = c.Backup(new string[] {DATAFOLDER}); + IBackupResults backupResults = c.Backup(new string[] { DATAFOLDER }); Assert.AreEqual(0, backupResults.Errors.Count()); Assert.AreEqual(0, backupResults.Warnings.Count()); } @@ -87,7 +87,7 @@ namespace Duplicati.UnitTest testopts["ignore-filenames"] = "exclude.me"; using (var c = new Library.Main.Controller("file://" + TARGETFOLDER, testopts, null)) { - IBackupResults backupResults = c.Backup(new string[] {DATAFOLDER}); + IBackupResults backupResults = c.Backup(new string[] { DATAFOLDER }); Assert.AreEqual(0, backupResults.Errors.Count()); Assert.AreEqual(0, backupResults.Warnings.Count()); } @@ -112,7 +112,7 @@ namespace Duplicati.UnitTest testopts["exclude-empty-folders"] = "true"; using (var c = new Library.Main.Controller("file://" + TARGETFOLDER, testopts, null)) { - IBackupResults backupResults = c.Backup(new string[] {DATAFOLDER}); + IBackupResults backupResults = c.Backup(new string[] { DATAFOLDER }); Assert.AreEqual(0, backupResults.Errors.Count()); Assert.AreEqual(0, backupResults.Warnings.Count()); } @@ -137,7 +137,7 @@ namespace Duplicati.UnitTest var excludefilter = new Library.Utility.FilterExpression($"*{System.IO.Path.DirectorySeparatorChar}myfile.txt", false); using (var c = new Library.Main.Controller("file://" + TARGETFOLDER, testopts, null)) { - IBackupResults backupResults = c.Backup(new string[] {DATAFOLDER}, excludefilter); + IBackupResults backupResults = c.Backup(new string[] { DATAFOLDER }, excludefilter); Assert.AreEqual(0, backupResults.Errors.Count()); Assert.AreEqual(0, backupResults.Warnings.Count()); } @@ -162,7 +162,7 @@ namespace Duplicati.UnitTest File.Delete(Path.Combine(source, "toplevel", "normal", "standard.txt")); using (var c = new Library.Main.Controller("file://" + TARGETFOLDER, testopts, null)) { - IBackupResults backupResults = c.Backup(new string[] {DATAFOLDER}, excludefilter); + IBackupResults backupResults = c.Backup(new string[] { DATAFOLDER }, excludefilter); Assert.AreEqual(0, backupResults.Errors.Count()); Assert.AreEqual(0, backupResults.Warnings.Count()); } @@ -214,5 +214,39 @@ namespace Duplicati.UnitTest Assert.IsFalse(filter.Matches(entry.Value, out _, out _)); } } + + [Test] + [Category("Filter")] + public static void CombineRegexp() + { + FilterExpression f1 = new FilterExpression(@"[/(a|b)/]"); + FilterExpression f2 = new FilterExpression(@"[/a/c/]"); + FilterExpression f3 = new FilterExpression(@"/b/c/"); + FilterExpression f4 = new FilterExpression(@"[/b/c/d/]"); + FilterExpression combined = FilterExpression.Combine(f1, FilterExpression.Combine(f2, FilterExpression.Combine(f3, f4))); + + List shouldMatch = new List() + { + "/a/", + "/b/", + "/a/c/", + "/b/c/", + "/b/c/d/" + }; + List shouldNotMatch = new List() + { + "/b/d/", + "/b/d/e", + "/a/d/", + }; + foreach (string s in shouldMatch) + { + Assert.IsTrue(combined.Matches(s, out _, out _)); + } + foreach (string s in shouldNotMatch) + { + Assert.IsFalse(combined.Matches(s, out _, out _)); + } + } } }