Resolve container and payload DownloadUrls

Fixes wixtoolset/issues#6405

4 files changed, 138 insertions, 48 deletions

diff --git a/src/WixToolset.Core.Burn/Bind/BindBundleCommand.cs b/src/WixToolset.Core.Burn/Bind/BindBundleCommand.cs
index ea8d33d0..fd847e05 100644
--- a/src/WixToolset.Core.Burn/Bind/BindBundleCommand.cs
+++ b/src/WixToolset.Core.Burn/Bind/BindBundleCommand.cs
@@ -459,12 +459,18 @@ namespace WixToolset.Core.Burn
                 containers = command.Containers;
             }
 
+            // Resolve the download URLs now that we have all of the containers and payloads calculated.
+            {
+                var command = new ResolveDownloadUrlsCommand(this.Messaging, this.BackendExtensions, containers, payloadSymbols);
+                command.Execute();
+            }
+
             // Create the bundle manifest.
             string manifestPath;
             {
                 var executableName = Path.GetFileName(this.OutputPath);
 
-                var command = new CreateBurnManifestCommand(this.Messaging, this.BackendExtensions, executableName, section, bundleSymbol, containers, chainSymbol, orderedFacades, boundaries, uxPayloads, payloadSymbols, packagesPayloads, orderedSearches, this.IntermediateFolder);
+                var command = new CreateBurnManifestCommand(executableName, section, bundleSymbol, containers, chainSymbol, orderedFacades, boundaries, uxPayloads, payloadSymbols, packagesPayloads, orderedSearches, this.IntermediateFolder);
                 command.Execute();
 
                 manifestPath = command.OutputPath;
diff --git a/src/WixToolset.Core.Burn/Bind/ResolveDownloadUrlsCommand.cs b/src/WixToolset.Core.Burn/Bind/ResolveDownloadUrlsCommand.cs
new file mode 100644
index 00000000..e41c1058
--- /dev/null
+++ b/src/WixToolset.Core.Burn/Bind/ResolveDownloadUrlsCommand.cs
@@ -0,0 +1,127 @@
+// 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.
+
+namespace WixToolset.Core.Burn.Bind
+{
+    using System;
+    using System.Collections.Generic;
+    using WixToolset.Data;
+    using WixToolset.Data.Symbols;
+    using WixToolset.Extensibility;
+    using WixToolset.Extensibility.Services;
+
+    internal class ResolveDownloadUrlsCommand
+    {
+        public ResolveDownloadUrlsCommand(IMessaging messaging, IEnumerable<IBurnBackendBinderExtension> backendExtensions, IEnumerable<WixBundleContainerSymbol> containers, Dictionary<string, WixBundlePayloadSymbol> payloadsById)
+        {
+            this.Messaging = messaging;
+            this.BackendExtensions = backendExtensions;
+            this.Containers = containers;
+            this.PayloadsById = payloadsById;
+        }
+
+        private IMessaging Messaging { get; }
+
+        private IEnumerable<IBurnBackendBinderExtension> BackendExtensions { get; }
+
+        private IEnumerable<WixBundleContainerSymbol> Containers { get; }
+
+        private Dictionary<string, WixBundlePayloadSymbol> PayloadsById { get; }
+
+        public void Execute()
+        {
+            this.ResolveContainerUrls();
+
+            this.ResolvePayloadUrls();
+        }
+
+        private void ResolveContainerUrls()
+        {
+            foreach (var container in this.Containers)
+            {
+                if (container.Type == ContainerType.Detached)
+                {
+                    var resolvedUrl = this.ResolveUrl(container.DownloadUrl, null, null, container.Id.Id, container.Name);
+                    if (!String.IsNullOrEmpty(resolvedUrl))
+                    {
+                        container.DownloadUrl = resolvedUrl;
+                    }
+                }
+                else if (container.Type == ContainerType.Attached)
+                {
+                    if (!String.IsNullOrEmpty(container.DownloadUrl))
+                    {
+                        this.Messaging.Write(WarningMessages.DownloadUrlNotSupportedForAttachedContainers(container.SourceLineNumbers, container.Id.Id));
+                    }
+                }
+            }
+        }
+
+        private void ResolvePayloadUrls()
+        {
+            foreach (var payload in this.PayloadsById.Values)
+            {
+                if (payload.Packaging == PackagingType.External)
+                {
+                    var packageId = payload.ParentPackagePayloadRef;
+                    var parentUrl = payload.ParentPackagePayloadRef == null ? null : this.PayloadsById[payload.ParentPackagePayloadRef].DownloadUrl;
+                    var resolvedUrl = this.ResolveUrl(payload.DownloadUrl, parentUrl, packageId, payload.Id.Id, payload.Name);
+                    if (!String.IsNullOrEmpty(resolvedUrl))
+                    {
+                        payload.DownloadUrl = resolvedUrl;
+                    }
+                }
+                else if (payload.Packaging == PackagingType.Embedded)
+                {
+                    if (!String.IsNullOrEmpty(payload.DownloadUrl))
+                    {
+                        this.Messaging.Write(WarningMessages.DownloadUrlNotSupportedForEmbeddedPayloads(payload.SourceLineNumbers, payload.Id.Id));
+                    }
+                }
+            }
+        }
+
+        private string ResolveUrl(string url, string fallbackUrl, string packageId, string payloadId, string fileName)
+        {
+            string resolvedUrl = null;
+
+            foreach (var extension in this.BackendExtensions)
+            {
+                resolvedUrl = extension.ResolveUrl(url, fallbackUrl, packageId, payloadId, fileName);
+                if (!String.IsNullOrEmpty(resolvedUrl))
+                {
+                    break;
+                }
+            }
+
+            if (String.IsNullOrEmpty(resolvedUrl))
+            {
+                // If a URL was not specified but there is a fallback URL that has a format specifier in it
+                // then use the fallback URL formatter for this URL.
+                if (String.IsNullOrEmpty(url) && !String.IsNullOrEmpty(fallbackUrl))
+                {
+                    var formattedFallbackUrl = String.Format(fallbackUrl, packageId, payloadId, fileName);
+                    if (!String.Equals(fallbackUrl, formattedFallbackUrl, StringComparison.OrdinalIgnoreCase))
+                    {
+                        url = fallbackUrl;
+                    }
+                }
+
+                if (!String.IsNullOrEmpty(url))
+                {
+                    var formattedUrl = String.Format(url, packageId, payloadId, fileName);
+
+                    if (Uri.TryCreate(formattedUrl, UriKind.Absolute, out var canonicalUri))
+                    {
+                        resolvedUrl = canonicalUri.AbsoluteUri;
+                    }
+                    else
+                    {
+                        resolvedUrl = null;
+                    }
+                }
+            }
+
+            return resolvedUrl;
+        }
+    }
+}
diff --git a/src/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs b/src/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs
index 128c7a5f..a8a12c28 100644
--- a/src/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs
+++ b/src/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs
@@ -18,10 +18,8 @@ namespace WixToolset.Core.Burn.Bundles
 
     internal class CreateBurnManifestCommand
     {
-        public CreateBurnManifestCommand(IMessaging messaging, IEnumerable<IBurnBackendBinderExtension> backendExtensions, string executableName, IntermediateSection section, WixBundleSymbol bundleSymbol, IEnumerable<WixBundleContainerSymbol> containers, WixChainSymbol chainSymbol, IEnumerable<PackageFacade> orderedPackages, IEnumerable<WixBundleRollbackBoundarySymbol> boundaries, IEnumerable<WixBundlePayloadSymbol> uxPayloads, Dictionary<string, WixBundlePayloadSymbol> allPayloadsById, Dictionary<string, Dictionary<string, WixBundlePayloadSymbol>> packagesPayloads, IEnumerable<ISearchFacade> orderedSearches, string intermediateFolder)
+        public CreateBurnManifestCommand(string executableName, IntermediateSection section, WixBundleSymbol bundleSymbol, IEnumerable<WixBundleContainerSymbol> containers, WixChainSymbol chainSymbol, IEnumerable<PackageFacade> orderedPackages, IEnumerable<WixBundleRollbackBoundarySymbol> boundaries, IEnumerable<WixBundlePayloadSymbol> uxPayloads, Dictionary<string, WixBundlePayloadSymbol> allPayloadsById, Dictionary<string, Dictionary<string, WixBundlePayloadSymbol>> packagesPayloads, IEnumerable<ISearchFacade> orderedSearches, string intermediateFolder)
         {
-            this.Messaging = messaging;
-            this.BackendExtensions = backendExtensions;
             this.ExecutableName = executableName;
             this.Section = section;
             this.BundleSymbol = bundleSymbol;
@@ -38,10 +36,6 @@ namespace WixToolset.Core.Burn.Bundles
 
         public string OutputPath { get; private set; }
 
-        private IMessaging Messaging { get; }
-
-        private IEnumerable<IBurnBackendBinderExtension> BackendExtensions { get; }
-
         private string ExecutableName { get; }
 
         private IntermediateSection Section { get; }
@@ -657,12 +651,7 @@ namespace WixToolset.Core.Burn.Bundles
 
             if (ContainerType.Detached == container.Type)
             {
-                string resolvedUrl = this.ResolveUrl(container.DownloadUrl, null, null, container.Id.Id, container.Name);
-                if (!String.IsNullOrEmpty(resolvedUrl))
-                {
-                    writer.WriteAttributeString("DownloadUrl", resolvedUrl);
-                }
-                else if (!String.IsNullOrEmpty(container.DownloadUrl))
+                if (!String.IsNullOrEmpty(container.DownloadUrl))
                 {
                     writer.WriteAttributeString("DownloadUrl", container.DownloadUrl);
                 }
@@ -671,11 +660,6 @@ namespace WixToolset.Core.Burn.Bundles
             }
             else if (ContainerType.Attached == container.Type)
             {
-                if (!String.IsNullOrEmpty(container.DownloadUrl))
-                {
-                    this.Messaging.Write(WarningMessages.DownloadUrlNotSupportedForAttachedContainers(container.SourceLineNumbers, container.Id.Id));
-                }
-
                 writer.WriteAttributeString("FilePath", executableName); // attached containers use the name of the bundle since they are attached to the executable.
                 writer.WriteAttributeString("AttachedIndex", container.AttachedContainerIndex.Value.ToString(CultureInfo.InvariantCulture));
                 writer.WriteAttributeString("Attached", "yes");
@@ -700,11 +684,6 @@ namespace WixToolset.Core.Burn.Bundles
             switch (payload.Packaging)
             {
                 case PackagingType.Embedded: // this means it's in a container.
-                    if (!String.IsNullOrEmpty(payload.DownloadUrl))
-                    {
-                        this.Messaging.Write(WarningMessages.DownloadUrlNotSupportedForEmbeddedPayloads(payload.SourceLineNumbers, payload.Id.Id));
-                    }
-
                     writer.WriteAttributeString("Packaging", "embedded");
                     writer.WriteAttributeString("SourcePath", payload.EmbeddedId);
 
@@ -715,14 +694,7 @@ namespace WixToolset.Core.Burn.Bundles
                     break;
 
                 case PackagingType.External:
-                    var packageId = payload.ParentPackagePayloadRef;
-                    var parentUrl = payload.ParentPackagePayloadRef == null ? null : allPayloads[payload.ParentPackagePayloadRef].DownloadUrl;
-                    var resolvedUrl = this.ResolveUrl(payload.DownloadUrl, parentUrl, packageId, payload.Id.Id, payload.Name);
-                    if (!String.IsNullOrEmpty(resolvedUrl))
-                    {
-                        writer.WriteAttributeString("DownloadUrl", resolvedUrl);
-                    }
-                    else if (!String.IsNullOrEmpty(payload.DownloadUrl))
+                    if (!String.IsNullOrEmpty(payload.DownloadUrl))
                     {
                         writer.WriteAttributeString("DownloadUrl", payload.DownloadUrl);
                     }
@@ -732,20 +704,5 @@ namespace WixToolset.Core.Burn.Bundles
                     break;
             }
         }
-
-        private string ResolveUrl(string url, string fallbackUrl, string packageId, string payloadId, string fileName)
-        {
-            string resolved = null;
-            foreach (var extension in this.BackendExtensions)
-            {
-                resolved = extension.ResolveUrl(url, fallbackUrl, packageId, payloadId, fileName);
-                if (!String.IsNullOrEmpty(resolved))
-                {
-                    break;
-                }
-            }
-
-            return resolved;
-        }
     }
 }
diff --git a/src/test/WixToolsetTest.CoreIntegration/PayloadFixture.cs b/src/test/WixToolsetTest.CoreIntegration/PayloadFixture.cs
index 9bd33eac..e9e59b9e 100644
--- a/src/test/WixToolsetTest.CoreIntegration/PayloadFixture.cs
+++ b/src/test/WixToolsetTest.CoreIntegration/PayloadFixture.cs
@@ -143,7 +143,7 @@ namespace WixToolsetTest.CoreIntegration
             }
         }
 
-        [Fact(Skip = "Test demonstrates failure")]
+        [Fact]
         public void ReplacesDownloadUrlPlaceholders()
         {
             var folder = TestData.Get(@"TestData");