From 42d598017b1944ab041862e57292cec3bf29d001 Mon Sep 17 00:00:00 2001 From: Willie Slepecki Date: Mon, 18 Oct 2021 16:49:28 -0400 Subject: [PATCH 1/4] Added Infinite loop protection --- .../IBranchConfigurationCalculator.cs | 2 +- .../BranchConfigurationCalculator.cs | 18 +++++++++++++----- .../Core/GitVersionContextFactory.cs | 2 +- .../InfiniteLoopProtectionException.cs | 12 ++++++++++++ 4 files changed, 27 insertions(+), 7 deletions(-) create mode 100644 src/GitVersion.Core/Model/Exceptions/InfiniteLoopProtectionException.cs diff --git a/src/GitVersion.Core/Configuration/Abstractions/IBranchConfigurationCalculator.cs b/src/GitVersion.Core/Configuration/Abstractions/IBranchConfigurationCalculator.cs index ed49a4e085..92aeb7b74c 100644 --- a/src/GitVersion.Core/Configuration/Abstractions/IBranchConfigurationCalculator.cs +++ b/src/GitVersion.Core/Configuration/Abstractions/IBranchConfigurationCalculator.cs @@ -7,5 +7,5 @@ public interface IBranchConfigurationCalculator /// /// Gets the for the current commit. /// - BranchConfig GetBranchConfiguration(IBranch targetBranch, ICommit? currentCommit, Config configuration, IList? excludedInheritBranches = null); + BranchConfig GetBranchConfiguration(int recursiveProtection, IBranch targetBranch, ICommit? currentCommit, Config configuration, IList? excludedInheritBranches = null); } diff --git a/src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs b/src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs index 3a45736b5b..795d846fd6 100644 --- a/src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs +++ b/src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs @@ -12,6 +12,7 @@ public class BranchConfigurationCalculator : IBranchConfigurationCalculator private readonly ILog log; private readonly IRepositoryStore repositoryStore; + private readonly int _infiniteLoopProtectionLevel = 50; public BranchConfigurationCalculator(ILog log, IRepositoryStore repositoryStore) { @@ -22,8 +23,13 @@ public BranchConfigurationCalculator(ILog log, IRepositoryStore repositoryStore) /// /// Gets the for the current commit. /// - public BranchConfig GetBranchConfiguration(IBranch targetBranch, ICommit? currentCommit, Config configuration, IList? excludedInheritBranches = null) + public BranchConfig GetBranchConfiguration(int recursiveLevel, IBranch targetBranch, ICommit? currentCommit, Config configuration, IList? excludedInheritBranches = null) { + if (recursiveLevel >= _infiniteLoopProtectionLevel) + { + throw new InfiniteLoopProtectionException("Inherited branch configuration caused an infinite loop...breaking."); + } + var matchingBranches = configuration.GetConfigForBranch(targetBranch.Name.WithoutRemote); if (matchingBranches == null) @@ -41,7 +47,7 @@ public BranchConfig GetBranchConfiguration(IBranch targetBranch, ICommit? curren if (matchingBranches.Increment == IncrementStrategy.Inherit) { - matchingBranches = InheritBranchConfiguration(targetBranch, matchingBranches, currentCommit, configuration, excludedInheritBranches); + matchingBranches = InheritBranchConfiguration(recursiveLevel, targetBranch, matchingBranches, currentCommit, configuration, excludedInheritBranches); if (matchingBranches.Name!.IsEquivalentTo(FallbackConfigName) && matchingBranches.Increment == IncrementStrategy.Inherit) { // We tried, and failed to inherit, just fall back to patch @@ -53,10 +59,12 @@ public BranchConfig GetBranchConfiguration(IBranch targetBranch, ICommit? curren } // TODO I think we need to take a fresh approach to this.. it's getting really complex with heaps of edge cases - private BranchConfig InheritBranchConfiguration(IBranch targetBranch, BranchConfig branchConfiguration, ICommit? currentCommit, Config configuration, IList? excludedInheritBranches) + private BranchConfig InheritBranchConfiguration(int recursiveLevel, IBranch targetBranch, BranchConfig branchConfiguration, ICommit? currentCommit, Config configuration, IList? excludedInheritBranches) { using (this.log.IndentLog("Attempting to inherit branch configuration from parent branch")) { + recursiveLevel += 1; + var excludedBranches = new[] { targetBranch }; // Check if we are a merge commit. If so likely we are a pull request var parentCount = currentCommit?.Parents.Count(); @@ -106,7 +114,7 @@ private BranchConfig InheritBranchConfiguration(IBranch targetBranch, BranchConf if (possibleParents.Count == 1) { - var branchConfig = GetBranchConfiguration(possibleParents[0], currentCommit, configuration, excludedInheritBranches); + var branchConfig = GetBranchConfiguration(recursiveLevel, possibleParents[0], currentCommit, configuration, excludedInheritBranches); // If we have resolved a fallback config we should not return that we have got config if (branchConfig.Name != FallbackConfigName) { @@ -154,7 +162,7 @@ private BranchConfig InheritBranchConfiguration(IBranch targetBranch, BranchConf }; } - var inheritingBranchConfig = GetBranchConfiguration(chosenBranch, currentCommit, configuration, excludedInheritBranches)!; + var inheritingBranchConfig = GetBranchConfiguration(recursiveLevel, chosenBranch, currentCommit, configuration, excludedInheritBranches)!; var configIncrement = inheritingBranchConfig.Increment; if (inheritingBranchConfig.Name!.IsEquivalentTo(FallbackConfigName) && configIncrement == IncrementStrategy.Inherit) { diff --git a/src/GitVersion.Core/Core/GitVersionContextFactory.cs b/src/GitVersion.Core/Core/GitVersionContextFactory.cs index c249ce2c38..e90514980e 100644 --- a/src/GitVersion.Core/Core/GitVersionContextFactory.cs +++ b/src/GitVersion.Core/Core/GitVersionContextFactory.cs @@ -36,7 +36,7 @@ public GitVersionContext Create(GitVersionOptions? gitVersionOptions) currentBranch = branchForCommit ?? currentBranch; } - var currentBranchConfig = this.branchConfigurationCalculator.GetBranchConfiguration(currentBranch, currentCommit, configuration); + var currentBranchConfig = this.branchConfigurationCalculator.GetBranchConfiguration(0, currentBranch, currentCommit, configuration); var effectiveConfiguration = configuration.CalculateEffectiveConfiguration(currentBranchConfig); var currentCommitTaggedVersion = this.repositoryStore.GetCurrentCommitTaggedVersion(currentCommit, effectiveConfiguration); var numberOfUncommittedChanges = this.repositoryStore.GetNumberOfUncommittedChanges(); diff --git a/src/GitVersion.Core/Model/Exceptions/InfiniteLoopProtectionException.cs b/src/GitVersion.Core/Model/Exceptions/InfiniteLoopProtectionException.cs new file mode 100644 index 0000000000..e7cb11d303 --- /dev/null +++ b/src/GitVersion.Core/Model/Exceptions/InfiniteLoopProtectionException.cs @@ -0,0 +1,12 @@ +using System; + +namespace GitVersion.Model.Exceptions +{ + public class InfiniteLoopProtectionException : Exception + { + public InfiniteLoopProtectionException(string messageFormat) + : base(messageFormat) + { + } + } +} From 5f59feddc34ec980d2862b59913052518dae6366 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Asbj=C3=B8rn=20Ulsberg?= Date: Wed, 2 Mar 2022 13:02:48 +0100 Subject: [PATCH 2/4] Refactor recursion protection to internal method Refactor recursion protection from the public `GetBranchConfiguration` method to an internal `GetBranchConfigurationInternal` method. --- .../IBranchConfigurationCalculator.cs | 2 +- .../BranchConfigurationCalculator.cs | 18 ++++++++++++------ .../Core/GitVersionContextFactory.cs | 2 +- .../InfiniteLoopProtectionException.cs | 2 -- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/GitVersion.Core/Configuration/Abstractions/IBranchConfigurationCalculator.cs b/src/GitVersion.Core/Configuration/Abstractions/IBranchConfigurationCalculator.cs index 92aeb7b74c..ed49a4e085 100644 --- a/src/GitVersion.Core/Configuration/Abstractions/IBranchConfigurationCalculator.cs +++ b/src/GitVersion.Core/Configuration/Abstractions/IBranchConfigurationCalculator.cs @@ -7,5 +7,5 @@ public interface IBranchConfigurationCalculator /// /// Gets the for the current commit. /// - BranchConfig GetBranchConfiguration(int recursiveProtection, IBranch targetBranch, ICommit? currentCommit, Config configuration, IList? excludedInheritBranches = null); + BranchConfig GetBranchConfiguration(IBranch targetBranch, ICommit? currentCommit, Config configuration, IList? excludedInheritBranches = null); } diff --git a/src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs b/src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs index 795d846fd6..ea9b321bbf 100644 --- a/src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs +++ b/src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs @@ -3,16 +3,17 @@ using GitVersion.Extensions; using GitVersion.Logging; using GitVersion.Model.Configuration; +using GitVersion.Model.Exceptions; namespace GitVersion.Configuration; public class BranchConfigurationCalculator : IBranchConfigurationCalculator { private const string FallbackConfigName = "Fallback"; + private const int MaxRecursions = 50; private readonly ILog log; private readonly IRepositoryStore repositoryStore; - private readonly int _infiniteLoopProtectionLevel = 50; public BranchConfigurationCalculator(ILog log, IRepositoryStore repositoryStore) { @@ -23,11 +24,14 @@ public BranchConfigurationCalculator(ILog log, IRepositoryStore repositoryStore) /// /// Gets the for the current commit. /// - public BranchConfig GetBranchConfiguration(int recursiveLevel, IBranch targetBranch, ICommit? currentCommit, Config configuration, IList? excludedInheritBranches = null) + public BranchConfig GetBranchConfiguration(IBranch targetBranch, ICommit? currentCommit, Config configuration, IList? excludedInheritBranches = null) => + GetBranchConfigurationInternal(0, targetBranch, currentCommit, configuration, excludedInheritBranches); + + private BranchConfig GetBranchConfigurationInternal(int recursiveLevel, IBranch targetBranch, ICommit? currentCommit, Config configuration, IList? excludedInheritBranches = null) { - if (recursiveLevel >= _infiniteLoopProtectionLevel) + if (recursiveLevel >= MaxRecursions) { - throw new InfiniteLoopProtectionException("Inherited branch configuration caused an infinite loop...breaking."); + throw new InfiniteLoopProtectionException($"Inherited branch configuration caused {recursiveLevel} recursions. Aborting!"); } var matchingBranches = configuration.GetConfigForBranch(targetBranch.Name.WithoutRemote); @@ -56,6 +60,7 @@ public BranchConfig GetBranchConfiguration(int recursiveLevel, IBranch targetBra } return matchingBranches; + } // TODO I think we need to take a fresh approach to this.. it's getting really complex with heaps of edge cases @@ -114,7 +119,7 @@ private BranchConfig InheritBranchConfiguration(int recursiveLevel, IBranch targ if (possibleParents.Count == 1) { - var branchConfig = GetBranchConfiguration(recursiveLevel, possibleParents[0], currentCommit, configuration, excludedInheritBranches); + var branchConfig = GetBranchConfigurationInternal(recursiveLevel, possibleParents[0], currentCommit, configuration, excludedInheritBranches); // If we have resolved a fallback config we should not return that we have got config if (branchConfig.Name != FallbackConfigName) { @@ -162,13 +167,14 @@ private BranchConfig InheritBranchConfiguration(int recursiveLevel, IBranch targ }; } - var inheritingBranchConfig = GetBranchConfiguration(recursiveLevel, chosenBranch, currentCommit, configuration, excludedInheritBranches)!; + var inheritingBranchConfig = GetBranchConfigurationInternal(recursiveLevel, chosenBranch, currentCommit, configuration, excludedInheritBranches)!; var configIncrement = inheritingBranchConfig.Increment; if (inheritingBranchConfig.Name!.IsEquivalentTo(FallbackConfigName) && configIncrement == IncrementStrategy.Inherit) { this.log.Warning("Fallback config inherits by default, dropping to patch increment"); configIncrement = IncrementStrategy.Patch; } + return new BranchConfig(branchConfiguration) { Increment = configIncrement, diff --git a/src/GitVersion.Core/Core/GitVersionContextFactory.cs b/src/GitVersion.Core/Core/GitVersionContextFactory.cs index e90514980e..c249ce2c38 100644 --- a/src/GitVersion.Core/Core/GitVersionContextFactory.cs +++ b/src/GitVersion.Core/Core/GitVersionContextFactory.cs @@ -36,7 +36,7 @@ public GitVersionContext Create(GitVersionOptions? gitVersionOptions) currentBranch = branchForCommit ?? currentBranch; } - var currentBranchConfig = this.branchConfigurationCalculator.GetBranchConfiguration(0, currentBranch, currentCommit, configuration); + var currentBranchConfig = this.branchConfigurationCalculator.GetBranchConfiguration(currentBranch, currentCommit, configuration); var effectiveConfiguration = configuration.CalculateEffectiveConfiguration(currentBranchConfig); var currentCommitTaggedVersion = this.repositoryStore.GetCurrentCommitTaggedVersion(currentCommit, effectiveConfiguration); var numberOfUncommittedChanges = this.repositoryStore.GetNumberOfUncommittedChanges(); diff --git a/src/GitVersion.Core/Model/Exceptions/InfiniteLoopProtectionException.cs b/src/GitVersion.Core/Model/Exceptions/InfiniteLoopProtectionException.cs index e7cb11d303..0828ae18c1 100644 --- a/src/GitVersion.Core/Model/Exceptions/InfiniteLoopProtectionException.cs +++ b/src/GitVersion.Core/Model/Exceptions/InfiniteLoopProtectionException.cs @@ -1,5 +1,3 @@ -using System; - namespace GitVersion.Model.Exceptions { public class InfiniteLoopProtectionException : Exception From fe04f26e6153e486e8df227833cc6bdf1f07c584 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Asbj=C3=B8rn=20Ulsberg?= Date: Wed, 2 Mar 2022 13:03:59 +0100 Subject: [PATCH 3/4] Add fallback for main and develop branch regex Add fallback for `null` in the configuration for the for `main` and `develop` branch regular expressions. --- .../Configuration/BranchConfigurationCalculator.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs b/src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs index ea9b321bbf..1e786aad7f 100644 --- a/src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs +++ b/src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs @@ -226,8 +226,8 @@ private IBranch[] CalculateWhenMultipleParents(ICommit currentCommit, ref IBranc private static BranchConfig? ChooseMainOrDevelopIncrementStrategyIfTheChosenBranchIsOneOfThem(IBranch chosenBranch, BranchConfig branchConfiguration, Config config) { BranchConfig? mainOrDevelopConfig = null; - var developBranchRegex = config.Branches[Config.DevelopBranchKey]?.Regex; - var mainBranchRegex = config.Branches[Config.MainBranchKey]?.Regex; + var developBranchRegex = config.Branches[Config.DevelopBranchKey]?.Regex ?? Config.DevelopBranchRegex; + var mainBranchRegex = config.Branches[Config.MainBranchKey]?.Regex ?? Config.MainBranchRegex; if (Regex.IsMatch(chosenBranch.Name.Friendly, developBranchRegex, RegexOptions.IgnoreCase)) { // Normally we would not expect this to happen but for safety we add a check From 3be6c8f3eabd5744d1ba4094752fa607af61eae3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Asbj=C3=B8rn=20Ulsberg?= Date: Wed, 2 Mar 2022 13:06:18 +0100 Subject: [PATCH 4/4] Rename `recursiveLevel` to `recurions` --- .../BranchConfigurationCalculator.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs b/src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs index 1e786aad7f..2a2f966257 100644 --- a/src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs +++ b/src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs @@ -27,11 +27,11 @@ public BranchConfigurationCalculator(ILog log, IRepositoryStore repositoryStore) public BranchConfig GetBranchConfiguration(IBranch targetBranch, ICommit? currentCommit, Config configuration, IList? excludedInheritBranches = null) => GetBranchConfigurationInternal(0, targetBranch, currentCommit, configuration, excludedInheritBranches); - private BranchConfig GetBranchConfigurationInternal(int recursiveLevel, IBranch targetBranch, ICommit? currentCommit, Config configuration, IList? excludedInheritBranches = null) + private BranchConfig GetBranchConfigurationInternal(int recursions, IBranch targetBranch, ICommit? currentCommit, Config configuration, IList? excludedInheritBranches = null) { - if (recursiveLevel >= MaxRecursions) + if (recursions >= MaxRecursions) { - throw new InfiniteLoopProtectionException($"Inherited branch configuration caused {recursiveLevel} recursions. Aborting!"); + throw new InfiniteLoopProtectionException($"Inherited branch configuration caused {recursions} recursions. Aborting!"); } var matchingBranches = configuration.GetConfigForBranch(targetBranch.Name.WithoutRemote); @@ -51,7 +51,7 @@ private BranchConfig GetBranchConfigurationInternal(int recursiveLevel, IBranch if (matchingBranches.Increment == IncrementStrategy.Inherit) { - matchingBranches = InheritBranchConfiguration(recursiveLevel, targetBranch, matchingBranches, currentCommit, configuration, excludedInheritBranches); + matchingBranches = InheritBranchConfiguration(recursions, targetBranch, matchingBranches, currentCommit, configuration, excludedInheritBranches); if (matchingBranches.Name!.IsEquivalentTo(FallbackConfigName) && matchingBranches.Increment == IncrementStrategy.Inherit) { // We tried, and failed to inherit, just fall back to patch @@ -64,11 +64,11 @@ private BranchConfig GetBranchConfigurationInternal(int recursiveLevel, IBranch } // TODO I think we need to take a fresh approach to this.. it's getting really complex with heaps of edge cases - private BranchConfig InheritBranchConfiguration(int recursiveLevel, IBranch targetBranch, BranchConfig branchConfiguration, ICommit? currentCommit, Config configuration, IList? excludedInheritBranches) + private BranchConfig InheritBranchConfiguration(int recursions, IBranch targetBranch, BranchConfig branchConfiguration, ICommit? currentCommit, Config configuration, IList? excludedInheritBranches) { using (this.log.IndentLog("Attempting to inherit branch configuration from parent branch")) { - recursiveLevel += 1; + recursions += 1; var excludedBranches = new[] { targetBranch }; // Check if we are a merge commit. If so likely we are a pull request @@ -119,7 +119,7 @@ private BranchConfig InheritBranchConfiguration(int recursiveLevel, IBranch targ if (possibleParents.Count == 1) { - var branchConfig = GetBranchConfigurationInternal(recursiveLevel, possibleParents[0], currentCommit, configuration, excludedInheritBranches); + var branchConfig = GetBranchConfigurationInternal(recursions, possibleParents[0], currentCommit, configuration, excludedInheritBranches); // If we have resolved a fallback config we should not return that we have got config if (branchConfig.Name != FallbackConfigName) { @@ -167,7 +167,7 @@ private BranchConfig InheritBranchConfiguration(int recursiveLevel, IBranch targ }; } - var inheritingBranchConfig = GetBranchConfigurationInternal(recursiveLevel, chosenBranch, currentCommit, configuration, excludedInheritBranches)!; + var inheritingBranchConfig = GetBranchConfigurationInternal(recursions, chosenBranch, currentCommit, configuration, excludedInheritBranches)!; var configIncrement = inheritingBranchConfig.Increment; if (inheritingBranchConfig.Name!.IsEquivalentTo(FallbackConfigName) && configIncrement == IncrementStrategy.Inherit) {