From 9dd02e0c1bb6463de5d79f14ce06d078ef876a32 Mon Sep 17 00:00:00 2001 From: Ronald Martin Date: Fri, 12 Feb 2021 22:37:15 -0500 Subject: Checkks all custom actions for cycles. Version 3. Fixes wixtoolset/issues#6201 --- .../Bind/SequenceActionsCommand.cs | 78 +++++++++++++++++----- .../CustomActionFixture.cs | 37 ++++++++-- .../CustomAction/CustomActionCycleWithTail.wxs | 20 ++++++ 3 files changed, 114 insertions(+), 21 deletions(-) create mode 100644 src/test/WixToolsetTest.CoreIntegration/TestData/CustomAction/CustomActionCycleWithTail.wxs diff --git a/src/WixToolset.Core.WindowsInstaller/Bind/SequenceActionsCommand.cs b/src/WixToolset.Core.WindowsInstaller/Bind/SequenceActionsCommand.cs index d33836c3..056b129b 100644 --- a/src/WixToolset.Core.WindowsInstaller/Bind/SequenceActionsCommand.cs +++ b/src/WixToolset.Core.WindowsInstaller/Bind/SequenceActionsCommand.cs @@ -67,7 +67,7 @@ namespace WixToolset.Core.WindowsInstaller.Bind } else // not a supported unscheduled action. { - throw new InvalidOperationException("Found an action with no Sequence, Before, or After column set."); + throw new InvalidOperationException($"Found an action [{actionSymbol.Id.Id}] at [{actionSymbol.SourceLineNumbers}] with no Sequence, Before, or After column set."); } } @@ -129,6 +129,9 @@ namespace WixToolset.Core.WindowsInstaller.Bind } } + // A dictionary used for detecting cyclic references among action symbols. + var firstReference = new Dictionary(); + // Build up dependency trees of the relatively scheduled actions. // Use ToList() to create a copy of the required action symbols so that new symbols can // be added while enumerating. @@ -142,7 +145,7 @@ namespace WixToolset.Core.WindowsInstaller.Bind this.Messaging.Write(ErrorMessages.StandardActionRelativelyScheduledInModule(actionSymbol.SourceLineNumbers, actionSymbol.SequenceTable.ToString(), actionSymbol.Action)); } - this.SequenceActionSymbol(actionSymbol, requiredActionSymbols); + this.SequenceActionSymbol(actionSymbol, requiredActionSymbols, firstReference); } else if (SectionType.Module == this.Section.Type && 0 < actionSymbol.Sequence && !WindowsInstallerStandard.IsStandardAction(actionSymbol.Action)) // check for custom actions and dialogs that have a sequence number { @@ -566,7 +569,7 @@ namespace WixToolset.Core.WindowsInstaller.Bind /// /// The action symbol to be sequenced. /// Collection of actions which must be included. - private void SequenceActionSymbol(WixActionSymbol actionSymbol, Dictionary requiredActionSymbols) + private void SequenceActionSymbol(WixActionSymbol actionSymbol, Dictionary requiredActionSymbols, Dictionary firstReference) { var after = false; @@ -576,7 +579,7 @@ namespace WixToolset.Core.WindowsInstaller.Bind } else if (actionSymbol.Before == null) { - throw new InvalidOperationException("Found an action with no Sequence, Before, or After column set."); + throw new InvalidOperationException($"Found an action [{actionSymbol.Id.Id}] at [{actionSymbol.SourceLineNumbers}] with no Sequence, Before, or After column set."); } var parentActionName = (after ? actionSymbol.After : actionSymbol.Before); @@ -597,10 +600,8 @@ namespace WixToolset.Core.WindowsInstaller.Bind throw new InvalidOperationException(String.Format(CultureInfo.CurrentUICulture, "Found an action with a non-existent {0} action: {1}.", (after ? "After" : "Before"), parentActionName)); } } - else if (actionSymbol == parentActionSymbol || this.ContainsChildActionSymbol(actionSymbol, parentActionSymbol)) // cycle detected - { - throw new WixException(ErrorMessages.ActionCircularDependency(actionSymbol.SourceLineNumbers, actionSymbol.SequenceTable.ToString(), actionSymbol.Action, parentActionSymbol.Action)); - } + + this.CheckForCircularActionReference(actionSymbol, requiredActionSymbols, firstReference); // Add this action to the appropriate list of dependent action symbols. var relativeActions = this.GetRelativeActions(parentActionSymbol); @@ -608,19 +609,66 @@ namespace WixToolset.Core.WindowsInstaller.Bind relatedSymbols.Add(actionSymbol); } - private bool ContainsChildActionSymbol(WixActionSymbol childSymbol, WixActionSymbol parentSymbol) + /// + /// Check the specified action symbol to see if it leads to a cycle. + /// + /// Use the provided dictionary to note the initial action symbol that first led to each action + /// symbol. Any action symbol encountered that has already been encountered starting from a different + /// initial action symbol inherits the loop characteristics of that initial action symbol, and thus is + /// also not part of a cycle. However, any action symbol encountered that has already been encountered + /// starting from the same initial action symbol is an indication that the current action symbol is + /// part of a cycle. + /// + /// The action symbol to be checked. + /// Collection of actions which must be included. + /// The first encountered action symbol that led to each action symbol. + private void CheckForCircularActionReference(WixActionSymbol actionSymbol, Dictionary requiredActionSymbols, Dictionary firstReference) { - var result = false; + WixActionSymbol currentActionSymbol = null; + var parentActionSymbol = actionSymbol; - if (this.RelativeActionsForActions.TryGetValue(childSymbol.Id.Id, out var relativeActions)) + do { - result = relativeActions.NextActions.Any(a => a.SequenceTable == parentSymbol.SequenceTable && a.Id.Id == parentSymbol.Id.Id) || - relativeActions.PreviousActions.Any(a => a.SequenceTable == parentSymbol.SequenceTable && a.Id.Id == parentSymbol.Id.Id); - } + var previousActionSymbol = currentActionSymbol ?? parentActionSymbol; + currentActionSymbol = parentActionSymbol; - return result; + if (!firstReference.TryGetValue(currentActionSymbol, out var existingInitialActionSymbol)) + { + firstReference[currentActionSymbol] = actionSymbol; + } + else if (existingInitialActionSymbol == actionSymbol) + { + throw new WixException(ErrorMessages.ActionCircularDependency(currentActionSymbol.SourceLineNumbers, currentActionSymbol.SequenceTable.ToString(), currentActionSymbol.Action, previousActionSymbol.Action)); + } + + parentActionSymbol = this.GetParentActionSymbol(currentActionSymbol, requiredActionSymbols); + } while (null != parentActionSymbol); } + /// + /// Get the action symbol that is the parent of the given action symbol. + /// + /// The given action symbol. + /// Collection of actions which must be included. + /// Null if there is no parent. Used for loop termination. + private WixActionSymbol GetParentActionSymbol(WixActionSymbol actionSymbol, Dictionary requiredActionSymbols) + { + if (null == actionSymbol.Before && null == actionSymbol.After) + { + return null; + } + + var parentActionKey = actionSymbol.SequenceTable.ToString() + "/" + (actionSymbol.After ?? actionSymbol.Before); + + if (!requiredActionSymbols.TryGetValue(parentActionKey, out var parentActionSymbol)) + { + WindowsInstallerStandard.TryGetStandardAction(parentActionKey, out parentActionSymbol); + } + + return parentActionSymbol; + } + + private RelativeActions GetRelativeActions(WixActionSymbol action) { if (!this.RelativeActionsForActions.TryGetValue(action.Id.Id, out var relativeActions)) diff --git a/src/test/WixToolsetTest.CoreIntegration/CustomActionFixture.cs b/src/test/WixToolsetTest.CoreIntegration/CustomActionFixture.cs index 65f4be31..7980ea61 100644 --- a/src/test/WixToolsetTest.CoreIntegration/CustomActionFixture.cs +++ b/src/test/WixToolsetTest.CoreIntegration/CustomActionFixture.cs @@ -11,7 +11,7 @@ namespace WixToolsetTest.CoreIntegration public class CustomActionFixture { - [Fact(Skip = "https://github.com/wixtoolset/issues/issues/6201")] + [Fact] public void CanDetectCustomActionCycle() { var folder = TestData.Get(@"TestData"); @@ -22,8 +22,8 @@ namespace WixToolsetTest.CoreIntegration var intermediateFolder = Path.Combine(baseFolder, "obj"); var msiPath = Path.Combine(baseFolder, @"bin\test.msi"); - var result = WixRunner.Execute(new[] - { + var exception = Assert.Throws(() => WixRunner.Execute(new[] + { "build", Path.Combine(folder, "CustomAction", "CustomActionCycle.wxs"), Path.Combine(folder, "ProductWithComponentGroupRef", "MinimalComponentGroup.wxs"), @@ -31,10 +31,35 @@ namespace WixToolsetTest.CoreIntegration "-bindpath", Path.Combine(folder, "SingleFile", "data"), "-intermediateFolder", intermediateFolder, "-o", msiPath - }); + })); + + Assert.Equal("The InstallExecuteSequence table contains an action 'Action1' that is scheduled to come before or after action 'Action3', which is also scheduled to come before or after action 'Action1'. Please remove this circular dependency by changing the Before or After attribute for one of the actions.", exception.Message); + } + } + + [Fact] + public void CanDetectCustomActionCycleWithTail() + { + var folder = TestData.Get(@"TestData"); + + using (var fs = new DisposableFileSystem()) + { + var baseFolder = fs.GetFolder(); + var intermediateFolder = Path.Combine(baseFolder, "obj"); + var msiPath = Path.Combine(baseFolder, @"bin\test.msi"); + + var exception = Assert.Throws(() => WixRunner.Execute(new[] + { + "build", + Path.Combine(folder, "CustomAction", "CustomActionCycleWithTail.wxs"), + Path.Combine(folder, "ProductWithComponentGroupRef", "MinimalComponentGroup.wxs"), + Path.Combine(folder, "ProductWithComponentGroupRef", "Product.wxs"), + "-bindpath", Path.Combine(folder, "SingleFile", "data"), + "-intermediateFolder", intermediateFolder, + "-o", msiPath + })); - var warnings = result.Messages.Where(m => m.Level == MessageLevel.Warning).ToArray(); - Assert.False(result.ExitCode == 0 && warnings.Length == 0); + Assert.Equal("The InstallExecuteSequence table contains an action 'Action2' that is scheduled to come before or after action 'Action4', which is also scheduled to come before or after action 'Action2'. Please remove this circular dependency by changing the Before or After attribute for one of the actions.", exception.Message); } } diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/CustomAction/CustomActionCycleWithTail.wxs b/src/test/WixToolsetTest.CoreIntegration/TestData/CustomAction/CustomActionCycleWithTail.wxs new file mode 100644 index 00000000..c64ef143 --- /dev/null +++ b/src/test/WixToolsetTest.CoreIntegration/TestData/CustomAction/CustomActionCycleWithTail.wxs @@ -0,0 +1,20 @@ + + + + + + + + + + + + + + + + + + + + \ No newline at end of file -- cgit v1.2.3-55-g6feb