diff --git a/src/StaticWebAssetsSdk/Tasks/Compression/DiscoverPrecompressedAssets.cs b/src/StaticWebAssetsSdk/Tasks/Compression/DiscoverPrecompressedAssets.cs index 8c3ed69a038d..4358fbf26bf0 100644 --- a/src/StaticWebAssetsSdk/Tasks/Compression/DiscoverPrecompressedAssets.cs +++ b/src/StaticWebAssetsSdk/Tasks/Compression/DiscoverPrecompressedAssets.cs @@ -32,7 +32,36 @@ public override bool Execute() var candidates = StaticWebAsset.FromTaskItemGroup(CandidateAssets); var assetsToUpdate = new List(); - var candidatesByIdentity = candidates.ToDictionary(asset => asset.Identity, OSPath.PathComparer); + var candidatesByIdentity = new Dictionary(OSPath.PathComparer); + foreach (var asset in candidates) + { + if (candidatesByIdentity.TryGetValue(asset.Identity, out var existing)) + { + if (existing.SourceId != asset.SourceId || + existing.RelativePath != asset.RelativePath) + { + Log.LogWarning( + "Duplicate candidate asset '{0}' with differing metadata (SourceId='{1}' vs '{2}', RelativePath='{3}' vs '{4}'). Keeping first occurrence.", + asset.Identity, + existing.SourceId, + asset.SourceId, + existing.RelativePath, + asset.RelativePath); + } + else + { + Log.LogMessage( + MessageImportance.Low, + "Skipping duplicate candidate asset '{0}' (SourceId='{1}'). Assets are identical.", + asset.Identity, + asset.SourceId); + } + } + else + { + candidatesByIdentity[asset.Identity] = asset; + } + } foreach (var candidate in candidates) { diff --git a/src/StaticWebAssetsSdk/Tasks/Data/StaticWebAsset.cs b/src/StaticWebAssetsSdk/Tasks/Data/StaticWebAsset.cs index 60058355f0b6..073dd6537cf6 100644 --- a/src/StaticWebAssetsSdk/Tasks/Data/StaticWebAsset.cs +++ b/src/StaticWebAssetsSdk/Tasks/Data/StaticWebAsset.cs @@ -9,6 +9,7 @@ using System.Security.Cryptography; using Microsoft.AspNetCore.StaticWebAssets.Tasks.Utils; using Microsoft.Build.Framework; +using Microsoft.Build.Utilities; namespace Microsoft.AspNetCore.StaticWebAssets.Tasks; @@ -1228,13 +1229,41 @@ internal static FileInfo ResolveFile(string identity, string originalItemSpec) throw new InvalidOperationException($"No file exists for the asset at either location '{identity}' or '{originalItemSpec}'."); } - internal static Dictionary ToAssetDictionary(ITaskItem[] candidateAssets, bool validate = false) + internal static Dictionary ToAssetDictionary( + ITaskItem[] candidateAssets, + bool validate = false, + TaskLoggingHelper log = null) { var dictionary = new Dictionary(candidateAssets.Length); for (var i = 0; i < candidateAssets.Length; i++) { var candidateAsset = FromTaskItem(candidateAssets[i], validate); - dictionary.Add(candidateAsset.Identity, candidateAsset); + if (dictionary.TryGetValue(candidateAsset.Identity, out var existing)) + { + if (existing.SourceId != candidateAsset.SourceId || + existing.RelativePath != candidateAsset.RelativePath) + { + log?.LogWarning( + "Duplicate static web asset '{0}' with differing metadata (SourceId='{1}' vs '{2}', RelativePath='{3}' vs '{4}'). Keeping first occurrence.", + candidateAsset.Identity, + existing.SourceId, + candidateAsset.SourceId, + existing.RelativePath, + candidateAsset.RelativePath); + } + else + { + log?.LogMessage( + MessageImportance.Low, + "Skipping duplicate static web asset '{0}' (SourceId='{1}'). Assets are identical.", + candidateAsset.Identity, + candidateAsset.SourceId); + } + } + else + { + dictionary.Add(candidateAsset.Identity, candidateAsset); + } } return dictionary; diff --git a/src/StaticWebAssetsSdk/Tasks/GenerateStaticWebAssetEndpointsManifest.cs b/src/StaticWebAssetsSdk/Tasks/GenerateStaticWebAssetEndpointsManifest.cs index f996a75b7168..3f6751f26f06 100644 --- a/src/StaticWebAssetsSdk/Tasks/GenerateStaticWebAssetEndpointsManifest.cs +++ b/src/StaticWebAssetsSdk/Tasks/GenerateStaticWebAssetEndpointsManifest.cs @@ -64,8 +64,37 @@ public override bool Execute() // Get the list of the asset that need to be part of the manifest (this is similar to GenerateStaticWebAssetsDevelopmentManifest) var assets = StaticWebAsset.FromTaskItemGroup(Assets); - var manifestAssets = ComputeManifestAssets(assets, ManifestType) - .ToDictionary(a => a.ResolvedAsset.Identity, a => a, OSPath.PathComparer); + var manifestAssetsList = ComputeManifestAssets(assets, ManifestType); + var manifestAssets = new Dictionary(OSPath.PathComparer); + foreach (var a in manifestAssetsList) + { + if (manifestAssets.TryGetValue(a.ResolvedAsset.Identity, out var existing)) + { + if (existing.TargetPath != a.TargetPath || + existing.ResolvedAsset.SourceId != a.ResolvedAsset.SourceId) + { + Log.LogWarning( + "Duplicate static web asset '{0}' with differing metadata (TargetPath='{1}' vs '{2}', SourceId='{3}' vs '{4}'). Keeping first occurrence.", + a.ResolvedAsset.Identity, + existing.TargetPath, + a.TargetPath, + existing.ResolvedAsset.SourceId, + a.ResolvedAsset.SourceId); + } + else + { + Log.LogMessage( + MessageImportance.Low, + "Skipping duplicate static web asset '{0}' (SourceId='{1}'). Assets are identical.", + a.ResolvedAsset.Identity, + a.ResolvedAsset.SourceId); + } + } + else + { + manifestAssets[a.ResolvedAsset.Identity] = a; + } + } // Build exclusion matcher if patterns are provided StaticWebAssetGlobMatcher exclusionMatcher = null; diff --git a/src/StaticWebAssetsSdk/Tasks/GenerateStaticWebAssetsManifest.cs b/src/StaticWebAssetsSdk/Tasks/GenerateStaticWebAssetsManifest.cs index af3d46ff6caf..80e840099968 100644 --- a/src/StaticWebAssetsSdk/Tasks/GenerateStaticWebAssetsManifest.cs +++ b/src/StaticWebAssetsSdk/Tasks/GenerateStaticWebAssetsManifest.cs @@ -99,7 +99,36 @@ private StaticWebAssetEndpoint[] FilterPublishEndpointsIfNeeded(StaticWebAsset[] // inside the manifest because its cumbersome to do it in MSBuild directly. if (StaticWebAssetsManifest.ManifestTypes.IsPublish(ManifestType)) { - var assetsByIdentity = assets.ToDictionary(a => a.Identity, a => a, OSPath.PathComparer); + var assetsByIdentity = new Dictionary(OSPath.PathComparer); + foreach (var a in assets) + { + if (assetsByIdentity.TryGetValue(a.Identity, out var existing)) + { + if (existing.SourceId != a.SourceId || + existing.RelativePath != a.RelativePath) + { + Log.LogWarning( + "Duplicate static web asset '{0}' with differing metadata (SourceId='{1}' vs '{2}', RelativePath='{3}' vs '{4}'). Keeping first occurrence.", + a.Identity, + existing.SourceId, + a.SourceId, + existing.RelativePath, + a.RelativePath); + } + else + { + Log.LogMessage( + MessageImportance.Low, + "Skipping duplicate static web asset '{0}' (SourceId='{1}'). Assets are identical.", + a.Identity, + a.SourceId); + } + } + else + { + assetsByIdentity[a.Identity] = a; + } + } var filteredEndpoints = new List(); foreach (var endpoint in Endpoints.Select(StaticWebAssetEndpoint.FromTaskItem)) diff --git a/test/Microsoft.NET.Sdk.StaticWebAssets.Tests/StaticWebAssets/DuplicateAssetIdentityHandlingTest.cs b/test/Microsoft.NET.Sdk.StaticWebAssets.Tests/StaticWebAssets/DuplicateAssetIdentityHandlingTest.cs new file mode 100644 index 000000000000..988b5dfc761f --- /dev/null +++ b/test/Microsoft.NET.Sdk.StaticWebAssets.Tests/StaticWebAssets/DuplicateAssetIdentityHandlingTest.cs @@ -0,0 +1,271 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#nullable disable + +using Microsoft.AspNetCore.StaticWebAssets.Tasks; +using Microsoft.Build.Framework; +using Moq; + +namespace Microsoft.NET.Sdk.StaticWebAssets.Tests; + +/// +/// Tests that tasks handle duplicate asset identities gracefully (first occurrence wins) +/// instead of throwing ArgumentException from Dictionary operations. +/// Regression tests for https://github.com/dotnet/sdk/issues/52089 +/// +public class DuplicateAssetIdentityHandlingTest +{ + [Fact] + public void ToAssetDictionary_WithDuplicateIdentities_KeepsFirstOccurrence() + { + // Arrange — two assets with the same Identity but different SourceId + var firstAsset = CreateAsset("wwwroot/dotnet.js.map", sourceId: "WasmProject1"); + var secondAsset = CreateAsset("wwwroot/dotnet.js.map", sourceId: "WasmProject2"); + + var taskItems = new ITaskItem[] { firstAsset.ToTaskItem(), secondAsset.ToTaskItem() }; + + // Act — should not throw ArgumentException + var dictionary = StaticWebAsset.ToAssetDictionary(taskItems); + + // Assert — first occurrence wins + dictionary.Should().ContainSingle(); + dictionary.Values.Single().SourceId.Should().Be("WasmProject1"); + } + + [Fact] + public void ToAssetDictionary_WithUniqueIdentities_ReturnsAll() + { + // Arrange + var asset1 = CreateAsset("wwwroot/app.js", sourceId: "Project1"); + var asset2 = CreateAsset("wwwroot/site.css", sourceId: "Project2"); + + var taskItems = new ITaskItem[] { asset1.ToTaskItem(), asset2.ToTaskItem() }; + + // Act + var dictionary = StaticWebAsset.ToAssetDictionary(taskItems); + + // Assert + dictionary.Should().HaveCount(2); + } + + [Fact] + public void DiscoverPrecompressedAssets_WithIdenticalDuplicates_LogsLowMessage() + { + // Arrange — truly identical duplicates (same SourceId) → Low-priority message + var messages = new List(); + var buildEngine = new Mock(); + buildEngine.Setup(e => e.LogMessageEvent(It.IsAny())) + .Callback(args => messages.Add(args.Message)); + + var asset1 = CreateAsset("wwwroot/dotnet.js.map", sourceId: "WasmProject1"); + var asset2 = CreateAsset("wwwroot/dotnet.js.map", sourceId: "WasmProject1"); + + var task = new DiscoverPrecompressedAssets + { + CandidateAssets = [asset1.ToTaskItem(), asset2.ToTaskItem()], + BuildEngine = buildEngine.Object + }; + + // Act + var result = task.Execute(); + + // Assert — task succeeds, logs at low importance + result.Should().BeTrue(); + messages.Should().Contain(m => m.Contains("Assets are identical")); + } + + [Fact] + public void DiscoverPrecompressedAssets_WithMismatchedDuplicates_LogsWarning() + { + // Arrange — duplicates with different SourceId → Warning + var warnings = new List(); + var buildEngine = new Mock(); + buildEngine.Setup(e => e.LogWarningEvent(It.IsAny())) + .Callback(args => warnings.Add(args.Message)); + + var asset1 = CreateAsset("wwwroot/dotnet.js.map", sourceId: "WasmProject1"); + var asset2 = CreateAsset("wwwroot/dotnet.js.map", sourceId: "WasmProject2"); + + var task = new DiscoverPrecompressedAssets + { + CandidateAssets = [asset1.ToTaskItem(), asset2.ToTaskItem()], + BuildEngine = buildEngine.Object + }; + + // Act + var result = task.Execute(); + + // Assert — task succeeds but emits a warning about mismatched metadata + result.Should().BeTrue(); + warnings.Should().Contain(m => m.Contains("differing metadata")); + } + + [Fact] + public void DiscoverPrecompressedAssets_WithDuplicateCandidates_StillFindsCompressedPairs() + { + // Arrange — duplicate uncompressed + one compressed + var buildEngine = new Mock(); + + var uncompressed1 = CreateAsset("wwwroot/site.js", sourceId: "Project1", + relativePath: "site#[.{fingerprint}]?.js"); + var uncompressed2 = CreateAsset("wwwroot/site.js", sourceId: "Project1", + relativePath: "site#[.{fingerprint}]?.js"); + var compressed = CreateAsset("wwwroot/site.js.gz", sourceId: "Project1", + relativePath: "site.js#[.{fingerprint}]?.gz"); + + var task = new DiscoverPrecompressedAssets + { + CandidateAssets = [ + uncompressed1.ToTaskItem(), + uncompressed2.ToTaskItem(), + compressed.ToTaskItem() + ], + BuildEngine = buildEngine.Object + }; + + // Act + var result = task.Execute(); + + // Assert — task succeeds and compressed asset is discovered + result.Should().BeTrue(); + task.DiscoveredCompressedAssets.Should().ContainSingle(); + } + + [Fact] + public void GenerateStaticWebAssetsManifest_PublishMode_WithDuplicateIdentities_DoesNotThrow() + { + // Arrange — the Publish path in FilterPublishEndpointsIfNeeded builds an identity dictionary + var errorMessages = new List(); + var buildEngine = new Mock(); + buildEngine.Setup(e => e.LogErrorEvent(It.IsAny())) + .Callback(args => errorMessages.Add(args.Message)); + + var tempPath = Path.Combine( + AppContext.BaseDirectory, + Guid.NewGuid().ToString("N") + ".json"); + + var asset1 = CreateAsset("wwwroot/dotnet.js.map", sourceId: "WasmProject1"); + var asset2 = CreateAsset("wwwroot/dotnet.js.map", sourceId: "WasmProject2"); + var endpoint = CreateEndpoint(asset1); + + var task = new GenerateStaticWebAssetsManifest + { + BuildEngine = buildEngine.Object, + Assets = [asset1.ToTaskItem(), asset2.ToTaskItem()], + Endpoints = [endpoint.ToTaskItem()], + ReferencedProjectsConfigurations = [], + DiscoveryPatterns = [], + BasePath = "/", + Source = "TestProject", + ManifestType = "Publish", + Mode = "Default", + ManifestPath = tempPath, + }; + + // Act — should not throw ArgumentException + var result = task.Execute(); + + // Assert — task completes (may still report "conflicting assets" but no crash) + // The key assertion is that we don't get an unhandled ArgumentException. + // Whether result is true/false depends on downstream validation, not on our fix. + errorMessages.Should().NotContain(m => m.Contains("An item with the same key has already been added")); + } + + [Fact] + public void GenerateStaticWebAssetEndpointsManifest_WithDuplicateIdentities_DoesNotThrow() + { + // Arrange + var buildEngine = new Mock(); + + var tempPath = Path.Combine( + AppContext.BaseDirectory, + Guid.NewGuid().ToString("N") + "endpoints.json"); + + var asset1 = CreateAsset("wwwroot/dotnet.js.map", sourceId: "WasmProject1"); + var asset2 = CreateAsset("wwwroot/dotnet.js.map", sourceId: "WasmProject2"); + var endpoint = CreateEndpoint(asset1); + + var task = new GenerateStaticWebAssetEndpointsManifest + { + Assets = [asset1.ToTaskItem(), asset2.ToTaskItem()], + Endpoints = [endpoint.ToTaskItem()], + ManifestType = "Build", + Source = "TestProject", + ManifestPath = tempPath, + BuildEngine = buildEngine.Object + }; + + // Act — should not throw ArgumentException + var act = () => task.Execute(); + + // Assert — task completes without ArgumentException crash. + // The return value may be false due to downstream validation (e.g., conflicting assets), + // but the critical thing is no unhandled dictionary exception. + act.Should().NotThrow(); + } + + private static StaticWebAssetEndpoint CreateEndpoint(StaticWebAsset asset) + { + return new StaticWebAssetEndpoint + { + Route = asset.ComputeTargetPath("", '/'), + AssetFile = asset.Identity, + Selectors = [], + EndpointProperties = [], + ResponseHeaders = + [ + new() { Name = "Content-Type", Value = "application/javascript" }, + new() { Name = "Content-Length", Value = "10" }, + new() { Name = "ETag", Value = "\"integrity\"" }, + new() { Name = "Last-Modified", Value = "Sat, 01 Jan 2000 00:00:01 GMT" } + ] + }; + } + + private static StaticWebAsset CreateAsset( + string itemSpec, + string sourceId = "TestProject", + string sourceType = "Discovered", + string relativePath = null, + string assetKind = "All", + string assetMode = "All", + string basePath = "base", + string assetRole = "Primary", + string relatedAsset = "", + string assetTraitName = "", + string assetTraitValue = "", + string copyToOutputDirectory = "Never", + string copyToPublishDirectory = "PreserveNewest") + { + var result = new StaticWebAsset() + { + Identity = Path.GetFullPath(itemSpec), + SourceId = sourceId, + SourceType = sourceType, + ContentRoot = Directory.GetCurrentDirectory(), + BasePath = basePath, + RelativePath = relativePath ?? itemSpec, + AssetKind = assetKind, + AssetMode = assetMode, + AssetRole = assetRole, + AssetMergeBehavior = StaticWebAsset.MergeBehaviors.PreferTarget, + AssetMergeSource = "", + RelatedAsset = relatedAsset, + AssetTraitName = assetTraitName, + AssetTraitValue = assetTraitValue, + CopyToOutputDirectory = copyToOutputDirectory, + CopyToPublishDirectory = copyToPublishDirectory, + OriginalItemSpec = itemSpec, + Integrity = "integrity", + Fingerprint = "fingerprint", + FileLength = 10, + LastWriteTime = new DateTime(2000, 1, 1, 0, 0, 1) + }; + + result.ApplyDefaults(); + result.Normalize(); + + return result; + } +}