Use bundle's default CacheId instead of hash in certificate CacheId.

9 files changed, 188 insertions, 50 deletions

diff --git a/src/wix/WixToolset.Core.Burn/Bundles/CacheIdGenerator.cs b/src/wix/WixToolset.Core.Burn/Bundles/CacheIdGenerator.cs
index 2efa748b..389eed2d 100644
--- a/src/wix/WixToolset.Core.Burn/Bundles/CacheIdGenerator.cs
+++ b/src/wix/WixToolset.Core.Burn/Bundles/CacheIdGenerator.cs
@@ -14,32 +14,80 @@ namespace WixToolset.Core.Burn.Bundles
         private const int ReasonableCountOfCharsFromCertificateThumbprint = 20;
         private const int ReasonableUpperLimitForCacheId = 64;
 
-        public static string GenerateCacheIdFromPayloadHashAndThumbprint(WixBundlePayloadSymbol payloadSymbol)
+        public static string GenerateLocalCacheId(IMessaging messaging, IntermediateSymbol harvestedPackageSymbol, WixBundlePayloadSymbol payloadSymbol, SourceLineNumber sourceLineNumbers, string elementName)
         {
+            string cacheId = null;
+
+            // If we are validating the package via certificate,
+            // the CacheId must be specified in source code.
+            if (!String.IsNullOrEmpty(payloadSymbol.CertificatePublicKey) || !String.IsNullOrEmpty(payloadSymbol.CertificateThumbprint))
+            {
+                var oneOfCertificateAttributeNames = !String.IsNullOrEmpty(payloadSymbol.CertificatePublicKey) ? "CertificatePublicKey" : "CertificateThumbprint";
+
+                messaging.Write(ErrorMessages.ExpectedAttribute(sourceLineNumbers, elementName, "CacheId", oneOfCertificateAttributeNames));
+            }
+            else
+            {
+                cacheId = GenerateDefaultCacheId(harvestedPackageSymbol, payloadSymbol, out _);
+            }
+
+            return cacheId;
+        }
+
+        public static string GenerateRemoteCacheId(IntermediateSymbol harvestedPackageSymbol, WixBundlePayloadSymbol payloadSymbol)
+        {
+            // If we are not validating the package via certificate,
+            // the CacheId can be generated at build time.
+            if (String.IsNullOrEmpty(payloadSymbol.CertificateThumbprint))
+            {
+                return null;
+            }
+
+            var defaultCacheId = GenerateDefaultCacheId(harvestedPackageSymbol, payloadSymbol, out var canTruncate);
             var takeFromThumbprint = Math.Min(ReasonableCountOfCharsFromCertificateThumbprint, payloadSymbol.CertificateThumbprint.Length);
-            var takeFromHash = Math.Min(ReasonableUpperLimitForCacheId - takeFromThumbprint, payloadSymbol.Hash.Length);
+            var takeFromDefault = Math.Min(ReasonableUpperLimitForCacheId - takeFromThumbprint, defaultCacheId.Length);
 
-            return payloadSymbol.Hash.Substring(0, takeFromHash) + payloadSymbol.CertificateThumbprint.Substring(0, takeFromThumbprint);
+            var defaultPart = !canTruncate ? defaultCacheId : defaultCacheId.Substring(0, takeFromDefault);
+            var certificatePart = payloadSymbol.CertificateThumbprint.Substring(0, takeFromThumbprint);
+            return defaultPart + certificatePart;
         }
 
-        public static string GenerateCacheIdFromPackagePayloadHash(IMessaging messaging, WixBundlePayloadSymbol packagePayload, string elementName)
+        public static string GenerateDefaultCacheId(IntermediateSymbol harvestedPackageSymbol, WixBundlePayloadSymbol payloadSymbol, out bool canTruncate)
         {
-            string cacheId = null;
+            string cacheId;
+            canTruncate = false;
 
-            // If we are validating the package via certificate, the CacheId must be specified
-            // in source code.
-            if (!String.IsNullOrEmpty(packagePayload.CertificatePublicKey) || !String.IsNullOrEmpty(packagePayload.CertificateThumbprint))
+            if (harvestedPackageSymbol is WixBundleHarvestedBundlePackageSymbol harvestedBundlePackageSymbol)
             {
-                var oneOfCertificateAttributeNames = !String.IsNullOrEmpty(packagePayload.CertificatePublicKey) ? "CertificatePublicKey" : "CertificateThumbprint";
-
-                messaging.Write(ErrorMessages.ExpectedAttribute(packagePayload.SourceLineNumbers, elementName, "CacheId", oneOfCertificateAttributeNames));
+                cacheId = GenerateCacheIdFromGuidAndVersion(harvestedBundlePackageSymbol.BundleId, harvestedBundlePackageSymbol.Version);
+            }
+            else if (harvestedPackageSymbol is WixBundleHarvestedMsiPackageSymbol harvestedMsiPackageSymbol)
+            {
+                cacheId = GenerateCacheIdFromGuidAndVersion(harvestedMsiPackageSymbol.ProductCode, harvestedMsiPackageSymbol.ProductVersion);
+            }
+            else if (harvestedPackageSymbol is WixBundleHarvestedMspPackageSymbol harvestedMspPackageSymbol)
+            {
+                cacheId = harvestedMspPackageSymbol.PatchCode;
             }
-            else // validating package by hashing, so the CacheId can be defaulted to the hash with a "reasonable" upper limit since the CacheId is in the cached file path.
+            else
             {
-                cacheId = packagePayload.Hash.Length > ReasonableUpperLimitForCacheId ? packagePayload.Hash.Substring(0, ReasonableUpperLimitForCacheId) : packagePayload.Hash;
+                // No inherent id is available, so use the hash.
+                cacheId = GenerateCacheIdFromHash(payloadSymbol.Hash);
+                canTruncate = true;
             }
 
             return cacheId;
         }
+
+        public static string GenerateCacheIdFromGuidAndVersion(string guid, string version)
+        {
+            return String.Format("{0}v{1}", guid, version);
+        }
+
+        public static string GenerateCacheIdFromHash(string hash)
+        {
+            // The hash needs to be truncated to a "reasonable" upper limit since the CacheId is in the cached file path.
+            return hash.Length > ReasonableUpperLimitForCacheId ? hash.Substring(0, ReasonableUpperLimitForCacheId) : hash;
+        }
     }
 }
diff --git a/src/wix/WixToolset.Core.Burn/Bundles/ProcessBundlePackageCommand.cs b/src/wix/WixToolset.Core.Burn/Bundles/ProcessBundlePackageCommand.cs
index 36602886..83ec10b3 100644
--- a/src/wix/WixToolset.Core.Burn/Bundles/ProcessBundlePackageCommand.cs
+++ b/src/wix/WixToolset.Core.Burn/Bundles/ProcessBundlePackageCommand.cs
@@ -94,7 +94,7 @@ namespace WixToolset.Core.Burn.Bundles
 
             if (String.IsNullOrEmpty(this.ChainPackage.CacheId))
             {
-                this.ChainPackage.CacheId = String.Format("{0}v{1}", this.BundlePackage.BundleId, this.BundlePackage.Version);
+                this.ChainPackage.CacheId = CacheIdGenerator.GenerateLocalCacheId(this.Messaging, harvestedBundlePackage, this.PackagePayload, this.BundlePackage.SourceLineNumbers, "BundlePackage");
             }
 
             if (String.IsNullOrEmpty(this.ChainPackage.DisplayName))
diff --git a/src/wix/WixToolset.Core.Burn/Bundles/ProcessExePackageCommand.cs b/src/wix/WixToolset.Core.Burn/Bundles/ProcessExePackageCommand.cs
index 3307db47..5e0de101 100644
--- a/src/wix/WixToolset.Core.Burn/Bundles/ProcessExePackageCommand.cs
+++ b/src/wix/WixToolset.Core.Burn/Bundles/ProcessExePackageCommand.cs
@@ -35,7 +35,7 @@ namespace WixToolset.Core.Burn.Bundles
 
             if (String.IsNullOrEmpty(this.Facade.PackageSymbol.CacheId))
             {
-                this.Facade.PackageSymbol.CacheId = CacheIdGenerator.GenerateCacheIdFromPackagePayloadHash(this.Messaging, packagePayload, "ExePackage");
+                this.Facade.PackageSymbol.CacheId = CacheIdGenerator.GenerateLocalCacheId(this.Messaging, null, packagePayload, this.Facade.PackageSymbol.SourceLineNumbers, "ExePackage");
             }
 
             this.Facade.PackageSymbol.Version = packagePayload.Version;
diff --git a/src/wix/WixToolset.Core.Burn/Bundles/ProcessMsiPackageCommand.cs b/src/wix/WixToolset.Core.Burn/Bundles/ProcessMsiPackageCommand.cs
index 87528a48..72da0c0b 100644
--- a/src/wix/WixToolset.Core.Burn/Bundles/ProcessMsiPackageCommand.cs
+++ b/src/wix/WixToolset.Core.Burn/Bundles/ProcessMsiPackageCommand.cs
@@ -100,7 +100,7 @@ namespace WixToolset.Core.Burn.Bundles
 
             if (String.IsNullOrEmpty(this.ChainPackage.CacheId))
             {
-                this.ChainPackage.CacheId = String.Format("{0}v{1}", this.MsiPackage.ProductCode, this.MsiPackage.ProductVersion);
+                this.ChainPackage.CacheId = CacheIdGenerator.GenerateLocalCacheId(this.Messaging, harvestedMsiPackage, this.PackagePayload, this.MsiPackage.SourceLineNumbers, "MsiPackage");
             }
 
             if (String.IsNullOrEmpty(this.ChainPackage.DisplayName))
diff --git a/src/wix/WixToolset.Core.Burn/Bundles/ProcessMspPackageCommand.cs b/src/wix/WixToolset.Core.Burn/Bundles/ProcessMspPackageCommand.cs
index 5b43e614..b889c2ce 100644
--- a/src/wix/WixToolset.Core.Burn/Bundles/ProcessMspPackageCommand.cs
+++ b/src/wix/WixToolset.Core.Burn/Bundles/ProcessMspPackageCommand.cs
@@ -82,7 +82,7 @@ namespace WixToolset.Core.Burn.Bundles
 
             if (String.IsNullOrEmpty(this.ChainPackage.CacheId))
             {
-                this.ChainPackage.CacheId = this.MspPackage.PatchCode;
+                this.ChainPackage.CacheId = CacheIdGenerator.GenerateLocalCacheId(this.Messaging, harvestedMspPackage, this.PackagePayload, this.MspPackage.SourceLineNumbers, "MspPackage");
             }
         }
 
diff --git a/src/wix/WixToolset.Core.Burn/Bundles/ProcessMsuPackageCommand.cs b/src/wix/WixToolset.Core.Burn/Bundles/ProcessMsuPackageCommand.cs
index b61956a2..47e89616 100644
--- a/src/wix/WixToolset.Core.Burn/Bundles/ProcessMsuPackageCommand.cs
+++ b/src/wix/WixToolset.Core.Burn/Bundles/ProcessMsuPackageCommand.cs
@@ -32,7 +32,7 @@ namespace WixToolset.Core.Burn.Bundles
 
             if (String.IsNullOrEmpty(this.Facade.PackageSymbol.CacheId))
             {
-                this.Facade.PackageSymbol.CacheId = CacheIdGenerator.GenerateCacheIdFromPackagePayloadHash(this.Messaging, packagePayload, "MsuPackage");
+                this.Facade.PackageSymbol.CacheId = CacheIdGenerator.GenerateLocalCacheId(this.Messaging, null, packagePayload, this.Facade.PackageSymbol.SourceLineNumbers, "MsuPackage");
             }
 
             this.Facade.PackageSymbol.PerMachine = true; // MSUs are always per-machine.
diff --git a/src/wix/WixToolset.Core.Burn/CommandLine/RemotePayloadSubcommand.cs b/src/wix/WixToolset.Core.Burn/CommandLine/RemotePayloadSubcommand.cs
index 83b10c7c..d152504d 100644
--- a/src/wix/WixToolset.Core.Burn/CommandLine/RemotePayloadSubcommand.cs
+++ b/src/wix/WixToolset.Core.Burn/CommandLine/RemotePayloadSubcommand.cs
@@ -26,6 +26,7 @@ namespace WixToolset.Core.Burn.CommandLine
         private static readonly XName ExePackagePayloadName = "ExePackagePayload";
         private static readonly XName MsuPackagePayloadName = "MsuPackagePayload";
         private static readonly XName PayloadName = "Payload";
+        private static readonly XName PayloadGroupName = "PayloadGroup";
         private static readonly XName RemoteBundleName = "RemoteBundle";
         private static readonly XName RemoteRelatedBundleName = "RemoteRelatedBundle";
 
@@ -192,42 +193,31 @@ namespace WixToolset.Core.Burn.CommandLine
         private XElement HarvestPackageElement(IEnumerable<string> paths)
         {
             var harvestedFiles = this.HarvestRemotePayloads(paths).ToList();
+            var firstFile = harvestedFiles.FirstOrDefault();
 
-            XElement element;
-
-            switch (harvestedFiles[0].PackageType)
+            if (firstFile == null)
             {
-                case WixBundlePackageType.Bundle:
-                    element = new XElement(BundlePackageName);
-                    break;
-
-                case WixBundlePackageType.Exe:
-                    element = new XElement(ExePackageName);
-                    break;
-
-                case WixBundlePackageType.Msu:
-                    element = new XElement(MsuPackageName);
-                    break;
-
-                default:
-                    return null;
+                return null;
             }
 
-            var packagePayloadFile = harvestedFiles.FirstOrDefault();
+            var containerElement = firstFile.PackageElement;
 
-            if (packagePayloadFile != null)
+            if (containerElement == null)
+            {
+                containerElement = new XElement(PayloadGroupName);
+            }
+            else
             {
-                if (packagePayloadFile.Element.Attribute("CertificateThumbprint") != null)
+                var cacheId = CacheIdGenerator.GenerateRemoteCacheId(firstFile.HarvestedPackageSymbol, firstFile.PayloadSymbol);
+                if (cacheId != null)
                 {
-                    var cacheId = CacheIdGenerator.GenerateCacheIdFromPayloadHashAndThumbprint(packagePayloadFile.PayloadSymbol);
-
-                    element.Add(new XAttribute("CacheId", cacheId));
+                    containerElement.Add(new XAttribute("CacheId", cacheId));
                 }
-
-                element.Add(harvestedFiles.Select(h => h.Element));
             }
 
-            return element;
+            containerElement.Add(harvestedFiles.Select(h => h.Element));
+
+            return containerElement;
         }
 
         private IEnumerable<HarvestedFile> HarvestRemotePayloads(IEnumerable<string> paths)
@@ -363,14 +353,19 @@ namespace WixToolset.Core.Burn.CommandLine
             var harvestedFile = new HarvestedFile
             {
                 Element = element,
-                PackageType = packageType,
                 PayloadSymbol = payloadSymbol,
             };
 
             switch (packageType)
             {
                 case WixBundlePackageType.Bundle:
-                    this.HarvestBundle(harvestedFile);
+                    this.HarvestBundlePackage(harvestedFile);
+                    break;
+                case WixBundlePackageType.Exe:
+                    this.HarvestExePackage(harvestedFile);
+                    break;
+                case WixBundlePackageType.Msu:
+                    this.HarvestMsuPackage(harvestedFile);
                     break;
             }
 
@@ -390,7 +385,7 @@ namespace WixToolset.Core.Burn.CommandLine
             return Path.GetFileName(path); 
         }
 
-        private void HarvestBundle(HarvestedFile harvestedFile)
+        private void HarvestBundlePackage(HarvestedFile harvestedFile)
         {
             var packagePayloadSymbol = new WixBundleBundlePackagePayloadSymbol(null, new Identifier(AccessModifier.Section, harvestedFile.PayloadSymbol.Id.Id))
             {
@@ -448,16 +443,34 @@ namespace WixToolset.Core.Burn.CommandLine
                 }
 
                 harvestedFile.PackagePayloads.AddRange(command.Payloads);
-
+                harvestedFile.HarvestedPackageSymbol = command.HarvestedBundlePackage;
                 harvestedFile.Element.Add(bundleElement);
+
+                harvestedFile.PackageElement = new XElement(BundlePackageName);
+                if (BurnCommon.BurnV3Namespace == command.HarvestedBundlePackage.ManifestNamespace)
+                {
+                    harvestedFile.PackageElement.Add(new XAttribute("Visible", "yes"));
+                }
             }
         }
 
+        private void HarvestExePackage(HarvestedFile harvestedFile)
+        {
+            harvestedFile.PackageElement = new XElement(ExePackageName);
+        }
+
+        private void HarvestMsuPackage(HarvestedFile harvestedFile)
+        {
+            harvestedFile.PackageElement = new XElement(MsuPackageName);
+        }
+
         private class HarvestedFile
         {
             public XElement Element { get; set; }
 
-            public WixBundlePackageType? PackageType { get; internal set; }
+            public XElement PackageElement { get; set; }
+
+            public IntermediateSymbol HarvestedPackageSymbol { get; set; }
 
             public WixBundlePayloadSymbol PayloadSymbol { get; set; }
 
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/BurnRemotePayloadSubcommandFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/BurnRemotePayloadSubcommandFixture.cs
index 7dae1ee9..f9059db0 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/BurnRemotePayloadSubcommandFixture.cs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/BurnRemotePayloadSubcommandFixture.cs
@@ -149,6 +149,83 @@ namespace WixToolsetTest.CoreIntegration
         }
 
         [Fact]
+        public void CanGetRemoteBundlePayloadWithCertificate()
+        {
+            var folder = TestData.Get(@"TestData");
+
+            using (var fs = new DisposableFileSystem())
+            {
+                var outputFolder = fs.GetFolder();
+                var outFile = Path.Combine(outputFolder, "out.xml");
+                var remotePayloadSourceFile = Path.Combine(outputFolder, "remotePayload.wxs");
+                var intermediateFolder = Path.Combine(outputFolder, "obj");
+                var bundleFile = Path.Combine(intermediateFolder, "out.exe");
+                var baFolderPath = Path.Combine(outputFolder, "ba");
+                var extractFolderPath = Path.Combine(outputFolder, "extract");
+
+                var result = WixRunner.Execute(new[]
+                {
+                    "burn", "remotepayload",
+                    "-usecertificate",
+                    "-downloadUrl", "http://wixtoolset.org/{0}",
+                    Path.Combine(folder, ".Data", "signed_wix314_4118_engine.exe"),
+                    "-o", outFile,
+                    "-packagetype", "bundle",
+                });
+
+                result.AssertSuccess();
+
+                var elements = File.ReadAllLines(outFile);
+                elements = elements.Select(s => s.Replace("\"", "'")).ToArray();
+
+                WixAssert.CompareLineByLine(new[]
+                {
+                    @"<BundlePackage Visible='yes' CacheId='{C0BA713B-9CFE-42DF-B92C-883F6846B4BA}v3.14.0.4118C95FC39334E667F3DD3D'>",
+                    @"  <BundlePackagePayload Name='signed_wix314_4118_engine.exe' ProductName='WiX Toolset v3.14.0.4118' Description='WiX Toolset v3.14.0.4118' DownloadUrl='http://wixtoolset.org/{0}' CertificatePublicKey='03169B5A32E602D436FC14EC14C435D7309945D4' CertificateThumbprint='C95FC39334E667F3DD3D82AF382E05719B88F7C1' Size='1088640' Version='3.14.0.4118'>",
+                    @"    <RemoteBundle BundleId='{C0BA713B-9CFE-42DF-B92C-883F6846B4BA}' DisplayName='WiX Toolset v3.14.0.4118' InstallSize='188426175' ManifestNamespace='http://schemas.microsoft.com/wix/2008/Burn' PerMachine='yes' ProviderKey='{c0ba713b-9cfe-42df-b92c-883f6846b4ba}' ProtocolVersion='1' Version='3.14.0.4118' Win64='no' UpgradeCode='{65E893AD-EDD5-4E7D-80CA-F0F50F383532}' />",
+                    @"  </BundlePackagePayload>",
+                    @"</BundlePackage>",
+                }, elements);
+
+                var remotePayloadSourceText = "<Wix xmlns='http://wixtoolset.org/schemas/v4/wxs'>" +
+                    "  <Fragment>" +
+                    "    <PackageGroup Id='BundlePackages'>" +
+                    String.Join(Environment.NewLine, elements) +
+                    "    </PackageGroup>" +
+                    "  </Fragment>" +
+                    "</Wix>";
+
+                File.WriteAllText(remotePayloadSourceFile, remotePayloadSourceText);
+
+                result = WixRunner.Execute(new[]
+                {
+                    "build",
+                    Path.Combine(folder, "BundleWithPackageGroupRef", "Bundle.wxs"),
+                    remotePayloadSourceFile,
+                    "-bindpath", Path.Combine(folder, "SimpleBundle", "data"),
+                    "-bindpath", Path.Combine(folder, ".Data"),
+                    "-intermediateFolder", intermediateFolder,
+                     "-o", bundleFile
+                });
+
+                result.AssertSuccess();
+
+                var extractResult = BundleExtractor.ExtractBAContainer(null, bundleFile, baFolderPath, extractFolderPath);
+                extractResult.AssertSuccess();
+
+                var msuPackages = extractResult.GetManifestTestXmlLines("/burn:BurnManifest/burn:Chain/burn:BundlePackage");
+                WixAssert.CompareLineByLine(new string[]
+                {
+                    "<BundlePackage Id='signed_wix314_4118_engine.exe' Cache='keep' CacheId='{C0BA713B-9CFE-42DF-B92C-883F6846B4BA}v3.14.0.4118C95FC39334E667F3DD3D' InstallSize='188426175' Size='1088640' PerMachine='yes' Permanent='no' Vital='yes' RollbackBoundaryForward='WixDefaultBoundary' RollbackBoundaryBackward='WixDefaultBoundary' LogPathVariable='WixBundleLog_signed_wix314_4118_engine.exe' RollbackLogPathVariable='WixBundleRollbackLog_signed_wix314_4118_engine.exe' BundleId='{C0BA713B-9CFE-42DF-B92C-883F6846B4BA}' Version='3.14.0.4118' InstallArguments='' UninstallArguments='' RepairArguments='' SupportsBurnProtocol='yes' Win64='no'>" +
+                    "<Provides Key='{c0ba713b-9cfe-42df-b92c-883f6846b4ba}' Version='3.14.0.4118' DisplayName='WiX Toolset v3.14.0.4118' Imported='yes' />" +
+                    "<RelatedBundle Id='{65E893AD-EDD5-4E7D-80CA-F0F50F383532}' Action='Upgrade' />" +
+                    "<PayloadRef Id='signed_wix314_4118_engine.exe' />" +
+                    "</BundlePackage>",
+                }, msuPackages);
+            }
+        }
+
+        [Fact]
         public void CanGetRemoteV3BundlePayload()
         {
             var folder = TestData.Get(@"TestData");
@@ -174,7 +251,7 @@ namespace WixToolsetTest.CoreIntegration
 
                 WixAssert.CompareLineByLine(new[]
                 {
-                    "<BundlePackage>",
+                    "<BundlePackage Visible='yes'>",
                     "  <BundlePackagePayload Name='v3bundle.exe' ProductName='CustomV3Theme' Description='CustomV3Theme' DownloadUrl='https://www.example.com/files/{0}' Hash='80739E7B8C31D75B4CDC48D60D74F5E481CB904212A3AE3FB0920365A163FBF32B0C5C175AB516D4124F107923E96200605DE1D560D362FEB47350FA727823B4' Size='648397' Version='1.0.0.0'>",
                     "    <RemoteBundle BundleId='{215A70DB-AB35-48C7-BE51-D66EAAC87177}' DisplayName='CustomV3Theme' InstallSize='1135' ManifestNamespace='http://schemas.microsoft.com/wix/2008/Burn' PerMachine='yes' ProviderKey='{215a70db-ab35-48c7-be51-d66eaac87177}' ProtocolVersion='1' Version='1.0.0.0' Win64='no' UpgradeCode='{2BF4C01F-C132-4E70-97AB-2BC68C7CCD10}' />",
                     "  </BundlePackagePayload>",
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/.Data/signed_wix314_4118_engine.exe b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/.Data/signed_wix314_4118_engine.exe
new file mode 100644
index 00000000..a20d7f50
--- /dev/null
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/.Data/signed_wix314_4118_engine.exe