ArpEntry reads QuietUninstallString or UninstallString, and uses UninstallArguments for the uninstall command line

8 files changed, 135 insertions, 29 deletions

diff --git a/src/api/wix/WixToolset.Data/Symbols/WixBundleExePackageSymbol.cs b/src/api/wix/WixToolset.Data/Symbols/WixBundleExePackageSymbol.cs
index 74146716..0fcc6dd1 100644
--- a/src/api/wix/WixToolset.Data/Symbols/WixBundleExePackageSymbol.cs
+++ b/src/api/wix/WixToolset.Data/Symbols/WixBundleExePackageSymbol.cs
@@ -57,6 +57,7 @@ namespace WixToolset.Data.Symbols
         None = 0,
         Bundle = 1,
         ArpWin64 = 2,
+        ArpUseUninstallString = 4,
     }
 
     public class WixBundleExePackageSymbol : IntermediateSymbol
@@ -157,6 +158,22 @@ namespace WixToolset.Data.Symbols
             }
         }
 
+        public bool ArpUseUninstallString
+        {
+            get { return this.Attributes.HasFlag(WixBundleExePackageAttributes.ArpUseUninstallString); }
+            set
+            {
+                if (value)
+                {
+                    this.Attributes |= WixBundleExePackageAttributes.ArpUseUninstallString;
+                }
+                else
+                {
+                    this.Attributes &= ~WixBundleExePackageAttributes.ArpUseUninstallString;
+                }
+            }
+        }
+
         public bool Repairable => this.RepairCommand != null;
 
         public bool Uninstallable => this.UninstallCommand != null;
diff --git a/src/burn/engine/exeengine.cpp b/src/burn/engine/exeengine.cpp
index 6d326a5a..c0bab254 100644
--- a/src/burn/engine/exeengine.cpp
+++ b/src/burn/engine/exeengine.cpp
@@ -81,6 +81,14 @@ extern "C" HRESULT ExeEngineParsePackageFromXml(
         hr = XmlGetYesNoAttribute(pixnExePackage, L"ArpWin64", &pPackage->Exe.fArpWin64);
         ExitOnOptionalXmlQueryFailure(hr, fFoundXml, "Failed to get @ArpWin64.");
 
+        // @ArpUseUninstallString
+        hr = XmlGetYesNoAttribute(pixnExePackage, L"ArpUseUninstallString", &pPackage->Exe.fArpUseUninstallString);
+        ExitOnOptionalXmlQueryFailure(hr, fFoundXml, "Failed to get @ArpWin64.");
+
+        // @UninstallArguments
+        hr = XmlGetAttributeEx(pixnExePackage, L"UninstallArguments", &pPackage->Exe.sczUninstallArguments);
+        ExitOnOptionalXmlQueryFailure(hr, fFoundXml, "Failed to get @UninstallArguments.");
+
         pPackage->Exe.fUninstallable = TRUE;
     }
 
@@ -481,7 +489,7 @@ extern "C" HRESULT ExeEngineExecutePackage(
     }
     else if (BURN_EXE_DETECTION_TYPE_ARP == pPackage->Exe.detectionType && BOOTSTRAPPER_ACTION_STATE_UNINSTALL == pExecuteAction->exePackage.action)
     {
-        ExitOnNull(sczArpUninstallString, hr, E_INVALIDARG, "QuietUninstallString is null.");
+        ExitOnNull(sczArpUninstallString, hr, E_INVALIDARG, "%hs is null.", pPackage->Exe.fArpUseUninstallString ? "UninstallString" : "QuietUninstallString");
 
         hr = AppParseCommandLine(sczArpUninstallString, &argcArp, &argvArp);
         ExitOnFailure(hr, "Failed to parse QuietUninstallString: %ls.", sczArpUninstallString);
@@ -1122,8 +1130,20 @@ static HRESULT DetectArpEntry(
 
     if (psczQuietUninstallString)
     {
-        hr = RegReadString(hKey, L"QuietUninstallString", psczQuietUninstallString);
-        ExitOnPathFailure(hr, fExists, "Failed to read QuietUninstallString.");
+        LPCWSTR sczUninstallStringName = pPackage->Exe.fArpUseUninstallString ? L"UninstallString" : L"QuietUninstallString";
+
+        hr = RegReadString(hKey, sczUninstallStringName, psczQuietUninstallString);
+        ExitOnPathFailure(hr, fExists, "Failed to read %ls.", sczUninstallStringName);
+
+        // If the uninstall string is an executable path then ensure it is enclosed in quotes
+        if (fExists && *psczQuietUninstallString && (L'\"' != **psczQuietUninstallString) && FileExistsEx(*psczQuietUninstallString, nullptr))
+        {
+            hr = StrAllocPrefix(psczQuietUninstallString, L"\"", 0);
+            ExitOnFailure(hr, "Failed to prepend UninstallString with quote.");
+
+            hr = StrAllocConcat(psczQuietUninstallString, L"\"", 0);
+            ExitOnFailure(hr, "Failed to append quote to UninstallString.");
+        }
     }
 
 LExit:
diff --git a/src/burn/engine/package.h b/src/burn/engine/package.h
index bdebd5b6..1ea169e6 100644
--- a/src/burn/engine/package.h
+++ b/src/burn/engine/package.h
@@ -362,6 +362,7 @@ typedef struct _BURN_PACKAGE
             BURN_EXE_DETECTION_TYPE detectionType;
 
             BOOL fArpWin64;
+            BOOL fArpUseUninstallString;
             LPWSTR sczArpKeyPath;
             VERUTIL_VERSION* pArpDisplayVersion;
 
diff --git a/src/test/burn/TestData/ExePackageTests/PerMachineArpEntryWithUninstallStringExePackage/PerMachineArpEntryWithUninstallStringExePackage.wixproj b/src/test/burn/TestData/ExePackageTests/PerMachineArpEntryWithUninstallStringExePackage/PerMachineArpEntryWithUninstallStringExePackage.wixproj
new file mode 100644
index 00000000..afdd9e5b
--- /dev/null
+++ b/src/test/burn/TestData/ExePackageTests/PerMachineArpEntryWithUninstallStringExePackage/PerMachineArpEntryWithUninstallStringExePackage.wixproj
@@ -0,0 +1,19 @@
+<!-- Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information. -->
+<Project Sdk="WixToolset.Sdk">
+  <PropertyGroup>
+    <OutputType>Bundle</OutputType>
+    <InstallerPlatform>x64</InstallerPlatform>
+    <BA>TestBA_x64</BA>
+    <UpgradeCode>{0A3D66F0-21A8-41B5-BE9A-7A9ABDEC3AB5}</UpgradeCode>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="..\..\Templates\Bundle.wxs" Link="Bundle.wxs" />
+  </ItemGroup>
+  <ItemGroup>
+    <ProjectReference Include="..\..\TestBA\TestBAWixlib_x64\testbawixlib_x64.wixproj" />
+  </ItemGroup>
+  <ItemGroup>
+    <PackageReference Include="WixToolset.Bal.wixext" />
+    <PackageReference Include="WixToolset.NetFx.wixext" />
+  </ItemGroup>
+</Project>
diff --git a/src/test/burn/TestData/ExePackageTests/PerMachineArpEntryWithUninstallStringExePackage/PerMachineArpEntryWithUninstallStringExePackage.wxs b/src/test/burn/TestData/ExePackageTests/PerMachineArpEntryWithUninstallStringExePackage/PerMachineArpEntryWithUninstallStringExePackage.wxs
new file mode 100644
index 00000000..58198257
--- /dev/null
+++ b/src/test/burn/TestData/ExePackageTests/PerMachineArpEntryWithUninstallStringExePackage/PerMachineArpEntryWithUninstallStringExePackage.wxs
@@ -0,0 +1,18 @@
+<!-- Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information. -->
+
+<?define ArpId = {21E99C0F-E604-439C-97E0-33B9771394BC}?>
+<?define ArpKeyPath = HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\$(var.ArpId)?>
+<?define ArpVersion = 1.0.0.0?>
+
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+  <Fragment>
+    <PackageGroup Id="BundlePackages">
+      <ExePackage Id="TestExe" PerMachine="yes"
+                  InstallArguments="/regw &quot;$(var.ArpKeyPath),DisplayVersion,String,$(var.ArpVersion)&quot; /regw &quot;$(var.ArpKeyPath),UninstallString,String,\&quot;[WixBundleExecutePackageCacheFolder]testexe.exe\&quot; /regd \&quot;$(var.ArpKeyPath)\&quot;&quot;">
+        <ArpEntry Id="$(var.ArpId)" Version="$(var.ArpVersion)" Win64="yes" UseUninstallString="yes" />
+
+        <PayloadGroupRef Id="TestExePayloads_x64" />
+      </ExePackage>
+    </PackageGroup>
+  </Fragment>
+</Wix>
diff --git a/src/test/burn/WixToolsetTest.BurnE2E/ExePackageTests.cs b/src/test/burn/WixToolsetTest.BurnE2E/ExePackageTests.cs
index 42301f30..dc0b6b5a 100644
--- a/src/test/burn/WixToolsetTest.BurnE2E/ExePackageTests.cs
+++ b/src/test/burn/WixToolsetTest.BurnE2E/ExePackageTests.cs
@@ -34,6 +34,28 @@ namespace WixToolsetTest.BurnE2E
         }
 
         [RuntimeFact]
+        public void CanInstallAndUninstallPerMachineArpEntryWithUninstallStringExePackage()
+        {
+            var perMachineArpEntryExePackageBundle = this.CreateBundleInstaller(@"PerMachineArpEntryWithUninstallStringExePackage");
+            var arpEntryExePackage = this.CreateArpEntryInstaller(perMachineArpEntryExePackageBundle, "TestExe");
+            var arpId = arpEntryExePackage.ArpId;
+
+            arpEntryExePackage.VerifyRegistered(false);
+
+            var installLogPath = perMachineArpEntryExePackageBundle.Install();
+            perMachineArpEntryExePackageBundle.VerifyRegisteredAndInPackageCache();
+            arpEntryExePackage.VerifyRegistered(true);
+
+            Assert.True(LogVerifier.MessageInLogFile(installLogPath, $"TestExe.exe\" /regw \"HKLM\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\{arpId},DisplayVersion,String,1.0.0.0\" /regw \"HKLM\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\{arpId},UninstallString,String,\\\""));
+
+            var uninstallLogPath = perMachineArpEntryExePackageBundle.Uninstall();
+            perMachineArpEntryExePackageBundle.VerifyUnregisteredAndRemovedFromPackageCache();
+            arpEntryExePackage.VerifyRegistered(false);
+
+            Assert.True(LogVerifier.MessageInLogFile(uninstallLogPath, $"testexe.exe\" /regd HKLM\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\{arpId}"));
+        }
+
+        [RuntimeFact]
         public void CanRecacheAndReinstallPerMachineArpEntryExePackageOnUninstallRollback()
         {
             var packageTestExe = this.CreatePackageInstaller("PackageTestExe");
diff --git a/src/wix/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs b/src/wix/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs
index ec418bc1..0a11ea3a 100644
--- a/src/wix/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs
+++ b/src/wix/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs
@@ -436,6 +436,16 @@ namespace WixToolset.Core.Burn.Bundles
                                 {
                                     writer.WriteAttributeString("ArpWin64", "yes");
                                 }
+
+                                if (exePackage.ArpUseUninstallString)
+                                {
+                                    writer.WriteAttributeString("ArpUseUninstallString", "yes");
+                                }
+
+                                if (!String.IsNullOrEmpty(exePackage.UninstallCommand))
+                                {
+                                    writer.WriteAttributeString("UninstallArguments", exePackage.UninstallCommand);
+                                }
                                 break;
                             case WixBundleExePackageDetectionType.None:
                                 writer.WriteAttributeString("DetectionType", "none");
diff --git a/src/wix/WixToolset.Core/Compiler_Bundle.cs b/src/wix/WixToolset.Core/Compiler_Bundle.cs
index 89826cf6..7790d50d 100644
--- a/src/wix/WixToolset.Core/Compiler_Bundle.cs
+++ b/src/wix/WixToolset.Core/Compiler_Bundle.cs
@@ -1997,10 +1997,11 @@ namespace WixToolset.Core
             var bundle = YesNoType.NotSet;
             var slipstream = YesNoType.NotSet;
             var hasPayloadInfo = false;
-            WixBundleExePackageDetectionType? exeDetectionType = WixBundleExePackageDetectionType.None;
+            WixBundleExePackageDetectionType exeDetectionType = WixBundleExePackageDetectionType.None;
             string arpId = null;
             string arpDisplayVersion = null;
             var arpWin64 = YesNoType.NotSet;
+            var arpUseUninstallString = YesNoType.NotSet;
 
             var expectedNetFx4Args = new string[] { "/q", "/norestart" };
 
@@ -2117,6 +2118,7 @@ namespace WixToolset.Core
                         case "DetectCondition":
                             detectCondition = this.Core.GetAttributeValue(sourceLineNumbers, attrib, EmptyRule.CanBeEmpty);
                             allowed = (packageType == WixBundlePackageType.Exe || packageType == WixBundlePackageType.Msu);
+                            exeDetectionType = WixBundleExePackageDetectionType.Condition;
                             break;
                         case "Protocol":
                             protocol = this.Core.GetAttributeValue(sourceLineNumbers, attrib);
@@ -2183,11 +2185,6 @@ namespace WixToolset.Core
                 this.Core.ParseExtensionAttribute(node, attribute, contextValues);
             }
 
-            if (packageType == WixBundlePackageType.Exe && (detectCondition != null || uninstallArguments != null))
-            {
-                exeDetectionType = WixBundleExePackageDetectionType.Condition;
-            }
-
             foreach (var child in node.Elements())
             {
                 if (CompilerCore.WixNamespace == child.Name.Namespace)
@@ -2199,25 +2196,17 @@ namespace WixToolset.Core
                             allowed = packageType == WixBundlePackageType.Exe;
                             if (allowed)
                             {
-                                if (exeDetectionType == WixBundleExePackageDetectionType.Arp)
+                                if (exeDetectionType != WixBundleExePackageDetectionType.None)
                                 {
-                                    this.Core.Write(ErrorMessages.TooManyChildren(Preprocessor.GetSourceLineNumbers(child), node.Name.LocalName, child.Name.LocalName));
+                                    this.Core.Write(ErrorMessages.UnexpectedElementWithAttribute(sourceLineNumbers, node.Name.LocalName, child.Name.LocalName, "DetectCondition"));
                                 }
-                                else if (!exeDetectionType.HasValue || exeDetectionType.Value == WixBundleExePackageDetectionType.Condition)
+                                if (null != uninstallArguments)
                                 {
-                                    exeDetectionType = null;
-                                }
-                                else
-                                {
-                                    if (exeDetectionType.Value != WixBundleExePackageDetectionType.None)
-                                    {
-                                        throw new WixException($"Unexpected WixBundleExePackageDetectionType: {exeDetectionType}");
-                                    }
-
-                                    exeDetectionType = WixBundleExePackageDetectionType.Arp;
+                                    this.Core.Write(ErrorMessages.UnexpectedElementWithAttribute(sourceLineNumbers, node.Name.LocalName, child.Name.LocalName, "UninstallArguments"));
                                 }
 
-                                this.ParseExePackageArpEntryElement(child, out arpId, out arpDisplayVersion, out arpWin64);
+                                exeDetectionType = WixBundleExePackageDetectionType.Arp;
+                                this.ParseExePackageArpEntryElement(child, out arpId, out arpDisplayVersion, out arpWin64, out uninstallArguments, out arpUseUninstallString);
                             }
                             break;
                         case "SlipstreamMsp":
@@ -2282,6 +2271,11 @@ namespace WixToolset.Core
                 }
             }
 
+            if (packageType == WixBundlePackageType.Exe && exeDetectionType == WixBundleExePackageDetectionType.None && uninstallArguments != null)
+            {
+                exeDetectionType = WixBundleExePackageDetectionType.Condition;
+            }
+
             if (id.Id == BurnConstants.BundleDefaultBoundaryId)
             {
                 this.Messaging.Write(CompilerErrors.ReservedValue(sourceLineNumbers, node.Name.LocalName, "Id", id.Id));
@@ -2374,10 +2368,6 @@ namespace WixToolset.Core
                         this.Core.Write(WarningMessages.ExePackageDetectInformationRecommended(sourceLineNumbers));
                     }
                 }
-                else
-                {
-                    this.Core.Write(ErrorMessages.UnexpectedElementWithAttribute(sourceLineNumbers, node.Name.LocalName, "ArpEntry", String.IsNullOrEmpty(detectCondition) ? "UninstallArguments" : "DetectCondition"));
-                }
 
                 if (repairArguments == null && repairCondition != null)
                 {
@@ -2497,6 +2487,7 @@ namespace WixToolset.Core
                         WixBundleExePackageAttributes exeAttributes = 0;
                         exeAttributes |= (YesNoType.Yes == bundle) ? WixBundleExePackageAttributes.Bundle : 0;
                         exeAttributes |= (YesNoType.Yes == arpWin64) ? WixBundleExePackageAttributes.ArpWin64 : 0;
+                        exeAttributes |= (YesNoType.Yes == arpUseUninstallString) ? WixBundleExePackageAttributes.ArpUseUninstallString : 0;
 
                         this.Core.AddSymbol(new WixBundleExePackageSymbol(sourceLineNumbers, id)
                         {
@@ -2506,7 +2497,7 @@ namespace WixToolset.Core
                             RepairCommand = repairArguments,
                             UninstallCommand = uninstallArguments,
                             ExeProtocol = protocol,
-                            DetectionType = exeDetectionType.Value,
+                            DetectionType = exeDetectionType,
                             ArpId = arpId,
                             ArpDisplayVersion = arpDisplayVersion,
                         });
@@ -2971,12 +2962,14 @@ namespace WixToolset.Core
             }
         }
 
-        private void ParseExePackageArpEntryElement(XElement node, out string id, out string version, out YesNoType win64)
+        private void ParseExePackageArpEntryElement(XElement node, out string id, out string version, out YesNoType win64, out string uninstallArguments, out YesNoType arpUseUninstallString)
         {
             var sourceLineNumbers = Preprocessor.GetSourceLineNumbers(node);
             id = null;
             version = null;
             win64 = YesNoType.NotSet;
+            arpUseUninstallString = YesNoType.NotSet;
+            uninstallArguments = null;
 
             foreach (var attrib in node.Attributes())
             {
@@ -2990,6 +2983,12 @@ namespace WixToolset.Core
                         case "Version":
                             version = this.Core.GetAttributeValue(sourceLineNumbers, attrib);
                             break;
+                        case "AdditionalUninstallArguments":
+                            uninstallArguments = this.Core.GetAttributeValue(sourceLineNumbers, attrib);
+                            break;
+                        case "UseUninstallString":
+                            arpUseUninstallString = this.Core.GetAttributeYesNoValue(sourceLineNumbers, attrib);
+                            break;
                         case "Win64":
                             win64 = this.Core.GetAttributeYesNoValue(sourceLineNumbers, attrib);
                             break;