Only allow MsiPackage and MspPackage inside MSI transactions.

Improve 64-bit package detection.

8 files changed, 124 insertions, 20 deletions

diff --git a/src/api/wix/WixToolset.Data/ErrorMessages.cs b/src/api/wix/WixToolset.Data/ErrorMessages.cs
index a833452d..186acd29 100644
--- a/src/api/wix/WixToolset.Data/ErrorMessages.cs
+++ b/src/api/wix/WixToolset.Data/ErrorMessages.cs
@@ -1494,9 +1494,24 @@ namespace WixToolset.Data
             return Message(null, Ids.MissingValidatorExtension, "The validator requires at least one extension. Add \"ValidatorExtension, Wix\" for the default implementation.");
         }
 
-        public static Message MsiTransactionX86BeforeX64(SourceLineNumber sourceLineNumbers)
+        public static Message MsiTransactionInvalidPackage(SourceLineNumber sourceLineNumbers, string packageId, string packageType)
         {
-            return Message(sourceLineNumbers, Ids.MsiTransactionX86BeforeX64, "MSI transactions must install all x64 packages before any x86 package.");
+            return Message(sourceLineNumbers, Ids.MsiTransactionInvalidPackage, "Invalid package '{0}' in MSI transaction. It is type '{1}' but must be Msi or Msp.", packageId, packageType);
+        }
+
+        public static Message MsiTransactionInvalidPackage2(SourceLineNumber sourceLineNumbers)
+        {
+            return Message(sourceLineNumbers, Ids.MsiTransactionInvalidPackage2, "Location of rollback boundary related to previous error.");
+        }
+
+        public static Message MsiTransactionX86BeforeX64Package(SourceLineNumber sourceLineNumbers, string x64PackageId, string x86PackageId)
+        {
+            return Message(sourceLineNumbers, Ids.MsiTransactionX86BeforeX64Package, "Package '{0}' is x64 but Package '{1}' is x86. MSI transactions must install all x64 packages before any x86 package.", x64PackageId, x86PackageId);
+        }
+
+        public static Message MsiTransactionX86BeforeX64Package2(SourceLineNumber sourceLineNumbers)
+        {
+            return Message(sourceLineNumbers, Ids.MsiTransactionX86BeforeX64Package2, "Location of x86 package related to previous error.");
         }
 
         public static Message MultipleEntrySections(SourceLineNumber sourceLineNumbers, string sectionName1, string sectionName2)
@@ -2672,7 +2687,7 @@ namespace WixToolset.Data
             InlineDirectorySyntaxRequiresPath = 387,
             InsecureBundleFilename = 388,
             PayloadMustBeRelativeToCache = 389,
-            MsiTransactionX86BeforeX64 = 390,
+            MsiTransactionX86BeforeX64Package = 390,
             NoSourceFiles = 391,
             WixiplSourceFileIsExclusive = 392,
             UnableToConvertFieldToNumber = 393,
@@ -2692,6 +2707,9 @@ namespace WixToolset.Data
             MissingPackagePayload = 407,
             ExpectedAttributeWithoutOtherAttributes = 408,
             InvalidBundleCondition = 409,
+            MsiTransactionX86BeforeX64Package2 = 410,
+            MsiTransactionInvalidPackage = 411,
+            MsiTransactionInvalidPackage2 = 412,
         }
     }
 }
diff --git a/src/api/wix/WixToolset.Data/WarningMessages.cs b/src/api/wix/WixToolset.Data/WarningMessages.cs
index 0c026b68..ecd76392 100644
--- a/src/api/wix/WixToolset.Data/WarningMessages.cs
+++ b/src/api/wix/WixToolset.Data/WarningMessages.cs
@@ -222,6 +222,11 @@ namespace WixToolset.Data
             return Message(sourceLineNumbers, Ids.DiscardedRollbackBoundary, "The RollbackBoundary '{0}' was discarded because it was not followed by a package. Without a package the rollback boundary doesn't do anything. Verify that the RollbackBoundary element is not followed by another RollbackBoundary and that the element is not at the end of the chain.", rollbackBoundaryId);
         }
 
+        public static Message DiscardedRollbackBoundary2(SourceLineNumber sourceLineNumbers)
+        {
+            return Message(sourceLineNumbers, Ids.DiscardedRollbackBoundary2, "Location of rollback boundary related to previous warning.");
+        }
+
         public static Message DiscouragedAllUsersValue(SourceLineNumber sourceLineNumbers, string path, string machineOrUser)
         {
             return Message(sourceLineNumbers, Ids.DiscouragedAllUsersValue, "Bundles require a package to be either per-machine or per-user. The MSI '{0}' ALLUSERS Property is set to '2' which may change from per-user to per-machine at install time. The Bundle will assume the package is per-{1} and will not work correctly if that changes. If possible, remove the Property with Id='ALLUSERS' and use Package/@InstallScope attribute instead.", path, machineOrUser);
@@ -821,6 +826,7 @@ namespace WixToolset.Data
             InvalidEnvironmentVariable = 1157,
             WindowsInstallerFileTooLarge = 1158,
             UnavailableBundleConditionVariable = 1159,
+            DiscardedRollbackBoundary2 = 1160,
         }
     }
 }
diff --git a/src/wix/WixToolset.Core.Burn/Bundles/OrderPackagesAndRollbackBoundariesCommand.cs b/src/wix/WixToolset.Core.Burn/Bundles/OrderPackagesAndRollbackBoundariesCommand.cs
index df798583..87c10190 100644
--- a/src/wix/WixToolset.Core.Burn/Bundles/OrderPackagesAndRollbackBoundariesCommand.cs
+++ b/src/wix/WixToolset.Core.Burn/Bundles/OrderPackagesAndRollbackBoundariesCommand.cs
@@ -48,7 +48,7 @@ namespace WixToolset.Core.Burn.Bundles
             // defined.
             var pendingRollbackBoundary = new WixBundleRollbackBoundarySymbol(null, new Identifier(AccessModifier.Section, BurnConstants.BundleDefaultBoundaryId)) { Vital = true };
             var lastRollbackBoundary = pendingRollbackBoundary;
-            var boundaryHadX86Package = false;
+            PackageFacade msiTransactionX86Package = null;
             var warnedMsiTransaction = false;
 
             foreach (var groupSymbol in groupSymbols)
@@ -57,7 +57,7 @@ namespace WixToolset.Core.Burn.Bundles
                 {
                     if (this.PackageFacades.TryGetValue(groupSymbol.ChildId, out var facade))
                     {
-                        var insideMsiTransaction = lastRollbackBoundary?.Transaction ?? false;
+                        var insideMsiTransaction = lastRollbackBoundary.Transaction ?? false;
 
                         if (null != pendingRollbackBoundary)
                         {
@@ -76,18 +76,32 @@ namespace WixToolset.Core.Burn.Bundles
                             usedBoundaries.Add(pendingRollbackBoundary);
                             facade.PackageSymbol.RollbackBoundaryRef = pendingRollbackBoundary.Id.Id;
                             pendingRollbackBoundary = null;
-
-                            boundaryHadX86Package = !facade.PackageSymbol.Win64;
+                            msiTransactionX86Package = null;
                         }
 
-                        // Error if MSI transaction has x86 package preceding x64 packages
-                        if (insideMsiTransaction && boundaryHadX86Package && facade.PackageSymbol.Win64)
+                        if (insideMsiTransaction)
                         {
-                            this.Messaging.Write(ErrorMessages.MsiTransactionX86BeforeX64(facade.PackageSymbol.SourceLineNumbers));
+                            if (facade.PackageSymbol.Type != WixBundlePackageType.Msi && facade.PackageSymbol.Type != WixBundlePackageType.Msp)
+                            {
+                                this.Messaging.Write(ErrorMessages.MsiTransactionInvalidPackage(facade.PackageSymbol.SourceLineNumbers, facade.PackageId, facade.PackageSymbol.Type.ToString()));
+                                this.Messaging.Write(ErrorMessages.MsiTransactionInvalidPackage2(lastRollbackBoundary.SourceLineNumbers));
+                            }
+                            // Not possible to tell the bitness of Msp.
+                            else if (facade.PackageSymbol.Type == WixBundlePackageType.Msi)
+                            {
+                                if (msiTransactionX86Package == null && !facade.PackageSymbol.Win64)
+                                {
+                                    msiTransactionX86Package = facade;
+                                }
+                                // Error if MSI transaction has x86 package preceding x64 packages
+                                else if (msiTransactionX86Package != null && facade.PackageSymbol.Win64)
+                                {
+                                    this.Messaging.Write(ErrorMessages.MsiTransactionX86BeforeX64Package(facade.PackageSymbol.SourceLineNumbers, facade.PackageId, msiTransactionX86Package.PackageId));
+                                    this.Messaging.Write(ErrorMessages.MsiTransactionX86BeforeX64Package2(msiTransactionX86Package.PackageSymbol.SourceLineNumbers));
+                                }
+                            }
                         }
 
-                        boundaryHadX86Package |= !facade.PackageSymbol.Win64;
-
                         orderedFacades.Add(facade);
                     }
                     else // must be a rollback boundary.
@@ -97,6 +111,7 @@ namespace WixToolset.Core.Burn.Bundles
                         if (null != pendingRollbackBoundary && pendingRollbackBoundary.Id.Id != BurnConstants.BundleDefaultBoundaryId)
                         {
                             this.Messaging.Write(WarningMessages.DiscardedRollbackBoundary(nextRollbackBoundary.SourceLineNumbers, nextRollbackBoundary.Id.Id));
+                            this.Messaging.Write(WarningMessages.DiscardedRollbackBoundary2(lastRollbackBoundary.SourceLineNumbers));
                         }
                         else
                         {
diff --git a/src/wix/WixToolset.Core.Burn/Bundles/ProcessMsiPackageCommand.cs b/src/wix/WixToolset.Core.Burn/Bundles/ProcessMsiPackageCommand.cs
index d9560cb0..c2bceb32 100644
--- a/src/wix/WixToolset.Core.Burn/Bundles/ProcessMsiPackageCommand.cs
+++ b/src/wix/WixToolset.Core.Burn/Bundles/ProcessMsiPackageCommand.cs
@@ -87,10 +87,9 @@ namespace WixToolset.Core.Burn.Bundles
                         // "Elevated privileges are not required to install this package."
                         // in MSI 4.5 and below, if this bit is 0, elevation is required.
                         var perMachine = (0 == (fileAndElevateFlags & 8));
-                        var x64 = platformsAndLanguages.Contains("x64");
 
                         this.Facade.PackageSymbol.PerMachine = perMachine ? YesNoDefaultType.Yes : YesNoDefaultType.No;
-                        this.Facade.PackageSymbol.Win64 = x64;
+                        this.Facade.PackageSymbol.Win64 = this.IsWin64(packagePayload.SourceLineNumbers, sourcePath, platformsAndLanguages);
                     }
 
                     string packageName = null;
@@ -216,6 +215,29 @@ namespace WixToolset.Core.Burn.Bundles
             return new HashSet<string>(properties, StringComparer.Ordinal);
         }
 
+        // https://docs.microsoft.com/en-us/windows/win32/msi/template-summary
+        private bool IsWin64(SourceLineNumber sourceLineNumbers, string sourcePath, string platformsAndLanguages)
+        {
+            var separatorIndex = platformsAndLanguages.IndexOf(';');
+            var platformValue = separatorIndex > 0 ? platformsAndLanguages.Substring(0, separatorIndex) : platformsAndLanguages;
+
+            switch (platformValue)
+            {
+                case "Arm64":
+                case "Intel64":
+                case "x64":
+                    return true;
+
+                case "Arm":
+                case "Intel":
+                    return false;
+
+                default:
+                    this.Messaging.Write(BurnBackendWarnings.UnknownMsiPackagePlatform(sourceLineNumbers, sourcePath, platformValue));
+                    return true;
+            }
+        }
+
         private void SetPerMachineAppropriately(string allusers, WixBundleMsiPackageSymbol msiPackage, string sourcePath)
         {
             // Can ignore ALLUSERS from MsiProperties because it is not allowed there.
diff --git a/src/wix/WixToolset.Core.Burn/BurnBackendWarnings.cs b/src/wix/WixToolset.Core.Burn/BurnBackendWarnings.cs
index 1f9a4102..7235c792 100644
--- a/src/wix/WixToolset.Core.Burn/BurnBackendWarnings.cs
+++ b/src/wix/WixToolset.Core.Burn/BurnBackendWarnings.cs
@@ -26,9 +26,9 @@ namespace WixToolset.Core.Burn
             return Message(sourceLineNumbers, Ids.FailedToExtractAttachedContainers, "Failed to extract attached container. This most often happens when extracting a stripped bundle from the package cache, which is not supported.");
         }
 
-        public static Message UnknownCoffMachineType(SourceLineNumber sourceLineNumbers, string bundleExecutable, ushort machineType)
+        public static Message HiddenBundleNotSupported(SourceLineNumber sourceLineNumbers, string bundleExecutable)
         {
-            return Message(sourceLineNumbers, Ids.UnknownCoffMachineType, "The bundle '{0}' has an unknown COFF machine type: {1}. It is assumed to be 32-bit.", bundleExecutable, machineType);
+            return Message(sourceLineNumbers, Ids.HiddenBundleNotSupported, "The bundle '{0}' does not support hiding its ARP registration.", bundleExecutable);
         }
 
         public static Message UnknownBundleRelationAction(SourceLineNumber sourceLineNumbers, string bundleExecutable, string action)
@@ -36,9 +36,14 @@ namespace WixToolset.Core.Burn
             return Message(sourceLineNumbers, Ids.UnknownBundleRelationAction, "The manifest for the bundle '{0}' contains an unknown related bundle action '{1}'. It will be ignored.", bundleExecutable, action);
         }
 
-        public static Message HiddenBundleNotSupported(SourceLineNumber sourceLineNumbers, string bundleExecutable)
+        public static Message UnknownCoffMachineType(SourceLineNumber sourceLineNumbers, string bundleExecutable, ushort machineType)
         {
-            return Message(sourceLineNumbers, Ids.HiddenBundleNotSupported, "The bundle '{0}' does not support hiding its ARP registration.", bundleExecutable);
+            return Message(sourceLineNumbers, Ids.UnknownCoffMachineType, "The bundle '{0}' has an unknown COFF machine type: {1}. It is assumed to be 32-bit.", bundleExecutable, machineType);
+        }
+
+        public static Message UnknownMsiPackagePlatform(SourceLineNumber sourceLineNumbers, string msiPath, string platform)
+        {
+            return Message(sourceLineNumbers, Ids.UnknownMsiPackagePlatform, "The MsiPackage '{0}' has an unknown platform: '{1}'. It is assumed to be 64-bit.", msiPath, platform);
         }
 
         private static Message Message(SourceLineNumber sourceLineNumber, Ids id, string format, params object[] args)
@@ -55,6 +60,7 @@ namespace WixToolset.Core.Burn
             UnknownCoffMachineType = 8504,
             UnknownBundleRelationAction = 8505,
             HiddenBundleNotSupported = 8506,
+            UnknownMsiPackagePlatform = 8507,
         } // last available is 8999. 9000 is VerboseMessages.
     }
 }
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/MsiTransactionFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/MsiTransactionFixture.cs
index 573ae4b4..431c131b 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/MsiTransactionFixture.cs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/MsiTransactionFixture.cs
@@ -11,7 +11,34 @@ namespace WixToolsetTest.CoreIntegration
     public class MsiTransactionFixture
     {
         [Fact]
-        public void CantBuildX64AfterX86Bundle()
+        public void CannotBuildExePackageInMsiTransaction()
+        {
+            var folder = TestData.Get(@"TestData");
+
+            using (var fs = new DisposableFileSystem())
+            {
+                var baseFolder = fs.GetFolder();
+                var intermediateFolder = Path.Combine(baseFolder, "obj");
+                var exePath = Path.Combine(baseFolder, @"bin\test.exe");
+
+                var result = WixRunner.Execute(new[]
+                {
+                    "build",
+                    "-sw1151", // this is expected for this test
+                    Path.Combine(folder, "MsiTransaction", "ExeInMsiTransactionBundle.wxs"),
+                    Path.Combine(folder, "BundleWithPackageGroupRef", "Bundle.wxs"),
+                    "-bindpath", Path.Combine(folder, "SimpleBundle", "data"),
+                    "-bindpath", Path.Combine(folder, ".Data"),
+                    "-intermediateFolder", intermediateFolder,
+                    "-o", exePath,
+                });
+
+                Assert.Equal(412, result.ExitCode);
+            }
+        }
+
+        [Fact]
+        public void CannotBuildX64AfterX86Bundle()
         {
             var folder = TestData.Get(@"TestData");
 
@@ -36,7 +63,7 @@ namespace WixToolsetTest.CoreIntegration
                     "-o", exePath,
                 });
 
-                Assert.Equal(390, result.ExitCode);
+                Assert.Equal(410, result.ExitCode);
             }
         }
 
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/RollbackBoundaryFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/RollbackBoundaryFixture.cs
index c69ff894..3a79878d 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/RollbackBoundaryFixture.cs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/RollbackBoundaryFixture.cs
@@ -103,6 +103,7 @@ namespace WixToolsetTest.CoreIntegration
                 WixAssert.CompareLineByLine(new[]
                 {
                     "The RollbackBoundary 'Second' was discarded because it was not followed by a package. Without a package the rollback boundary doesn't do anything. Verify that the RollbackBoundary element is not followed by another RollbackBoundary and that the element is not at the end of the chain.",
+                    "Location of rollback boundary related to previous warning.",
                 }, result.Messages.Select(m => m.ToString()).ToArray());
 
                 Assert.True(File.Exists(exePath));
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/ExeInMsiTransactionBundle.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/ExeInMsiTransactionBundle.wxs
new file mode 100644
index 00000000..23bd47a1
--- /dev/null
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/ExeInMsiTransactionBundle.wxs
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+    <Fragment>
+        <PackageGroup Id="BundlePackages">
+            <RollbackBoundary Id="HasExe" Transaction="yes" />
+            <ExePackage SourceFile="burn.exe" DetectCondition="none" Permanent="yes" />
+        </PackageGroup>
+    </Fragment>
+</Wix>