From a310f9e4b1b7b371b36444c7eae75672346d3a0a Mon Sep 17 00:00:00 2001 From: alinpahontu2912 Date: Tue, 4 Aug 2026 15:55:31 +0200 Subject: [PATCH 1/5] Fix Uri equality contract violation in PackUriHelper.ValidatedPartUri PR #118574 added object.Equals/GetHashCode overrides to ValidatedPartUri (which derives from System.Uri) to support case-insensitive part-name matching. This broke the inherited Uri equality contract: Equals became asymmetric against a plain System.Uri of the same value, and GetHashCode became inconsistent with System.Uri.GetHashCode(). Consumers mixing ValidatedPartUri with plain System.Uri in HashSet/Dictionary (e.g. the Open XML SDK) got incorrect lookup results. - Revert the Equals(object?)/GetHashCode() overrides, restoring the original CA1067 suppression. ValidatedPartUri once again inherits Uri's value-equality, matching plain System.Uri semantics. - Add PackUriHelper.ValidatedPartUriEqualityComparer, an internal IEqualityComparer providing case-insensitive equality/hashing, and use it explicitly in ZipPackage's content-type override dictionary. Without this, removing GetHashCode() would leave that Dictionary with a hash/equals mismatch (case-insensitive Equals via IEquatable, case-sensitive hash via inherited Uri.GetHashCode()), silently dropping parts whose zip entry name casing differs from their [Content_Types].xml Override PartName. - Add regression tests covering the Uri equality contract and the content-type override case-insensitivity behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f1ae4251-6f11-4001-afb3-7cfdea3b64b5 --- .../src/System/IO/Packaging/PackUriHelper.cs | 51 ++++++++++----- .../src/System/IO/Packaging/ZipPackage.cs | 4 +- .../tests/PartPieceTests.cs | 64 +++++++++++++++++++ 3 files changed, 102 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs index 19ed7644e1cecc..aa9213a11650e6 100644 --- a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs +++ b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Text; @@ -601,7 +602,9 @@ private static bool IsPartNameEmpty(string partName) /// to reduce the parsing and number of allocations for Strings and Uris /// we cache the results after parsing. /// +#pragma warning disable CA1067 // Override Equals because it implements IEquatable; not overriding to avoid possible regressions in code that's working internal sealed class ValidatedPartUri : Uri, IComparable, IEquatable +#pragma warning restore CA1067 { //------------------------------------------------------ // @@ -651,22 +654,6 @@ bool IEquatable.Equals(ValidatedPartUri? otherPartUri) #endregion IEquatable Methods - #region Overrides - - public override bool Equals(object? obj) - { - if (obj is ValidatedPartUri other) - return Compare(other) == 0; - return false; - } - - public override int GetHashCode() - { - return StringComparer.OrdinalIgnoreCase.GetHashCode(NormalizedPartUriString); - } - - #endregion Overrides - #region Internal Properties //------------------------------------------------------ @@ -893,6 +880,38 @@ private int Compare(ValidatedPartUri? otherPartUri) //------------------------------------------------------ } + /// + /// Compares instances for equality using the same + /// case-insensitive, normalized comparison as + /// via . + /// This is used explicitly by internal collections that need case-insensitive part-name + /// semantics (e.g. 's content-type override dictionary), without + /// relying on overriding + /// or , which would break the equality contract inherited + /// from when a is compared against, or + /// co-located in a hash collection with, a plain of the same value. + /// + internal sealed class ValidatedPartUriEqualityComparer : IEqualityComparer + { + internal static readonly ValidatedPartUriEqualityComparer Instance = new(); + + private ValidatedPartUriEqualityComparer() { } + + public bool Equals(ValidatedPartUri? x, ValidatedPartUri? y) + { + if (ReferenceEquals(x, y)) + return true; + + if (x is null || y is null) + return false; + + return ((IEquatable)x).Equals(y); + } + + public int GetHashCode(ValidatedPartUri obj) + => StringComparer.OrdinalIgnoreCase.GetHashCode(obj.NormalizedPartUriString); + } + #endregion Private Class } } diff --git a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs index f3e1a416e360b6..6189375cd4a6bb 100644 --- a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs +++ b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs @@ -947,7 +947,9 @@ private void EnsureOverrideDictionary() { // The part Uris are stored in the Override Dictionary in their original form , but they are compared // in a normalized manner using the PartUriComparer - _overrideDictionary ??= new Dictionary(OverrideDictionaryInitialSize); + _overrideDictionary ??= new Dictionary( + OverrideDictionaryInitialSize, + PackUriHelper.ValidatedPartUriEqualityComparer.Instance); } private void ParseContentTypesFile(System.Collections.ObjectModel.ReadOnlyCollection zipFiles) diff --git a/src/libraries/System.IO.Packaging/tests/PartPieceTests.cs b/src/libraries/System.IO.Packaging/tests/PartPieceTests.cs index 37736f4d6d7f6f..210e2a01157126 100644 --- a/src/libraries/System.IO.Packaging/tests/PartPieceTests.cs +++ b/src/libraries/System.IO.Packaging/tests/PartPieceTests.cs @@ -361,6 +361,70 @@ public void DuplicatePartsDifferingOnlyByCaseAreNotAllowed() } } + [Fact] + public void PartUriHonorsSystemUriEqualityContract() + { + // PackUriHelper.CreatePartUri returns an internal Uri subclass (ValidatedPartUri) that must + // preserve System.Uri's object.Equals/GetHashCode contract so it can be safely mixed with + // plain System.Uri instances in hash-based collections such as HashSet/Dictionary. + Uri plain = new Uri("/foo.xml", UriKind.Relative); + Uri validated = PackUriHelper.CreatePartUri(plain); + + object a = validated; + object b = plain; + + // object.Equals must be symmetric. + Assert.Equal(a.Equals(b), b.Equals(a)); + + // GetHashCode must be consistent with a value-equal plain System.Uri so both types can + // coexist as keys in the same hash-based collection. + Assert.Equal(plain.GetHashCode(), validated.GetHashCode()); + + var set = new HashSet { plain }; + Assert.Contains(validated, set); + + var set2 = new HashSet { validated }; + Assert.Contains(plain, set2); + } + + [Fact] + public void ContentTypeOverrideLookupIsCaseInsensitive() + { + // Regression test: a package whose [Content_Types].xml Override PartName differs only + // by case from the actual zip entry name must still resolve the part's content type. + // This exercises ZipPackage's internal ValidatedPartUri-keyed override dictionary, which + // must remain case-insensitive independent of whether ValidatedPartUri overrides + // object.Equals/GetHashCode. + using var ms = new MemoryStream(); + using (var zipArchive = new ZipArchive(ms, ZipArchiveMode.Create, leaveOpen: true)) + { + var contentTypesEntry = zipArchive.CreateEntry("[Content_Types].xml"); + using (var writer = new StreamWriter(contentTypesEntry.Open())) + { + writer.Write( + """ + + + + + """); + } + + var partEntry = zipArchive.CreateEntry("test.xml"); + using (var writer = new StreamWriter(partEntry.Open())) + { + writer.Write(""); + } + } + + ms.Position = 0; + using var package = Package.Open(ms, FileMode.Open, FileAccess.Read); + PackagePart[] parts = package.GetParts().ToArray(); + + Assert.Single(parts); + Assert.Equal("application/foo", parts[0].ContentType); + } + [Fact] public void CanCreateAtomicPart() { From c729c14534c5d8cb278da1e263d7dd11707c8cf2 Mon Sep 17 00:00:00 2001 From: alinpahontu2912 Date: Tue, 4 Aug 2026 15:56:01 +0200 Subject: [PATCH 2/5] Revert redundant OrdinalIgnoreCase comparison in ValidatedPartUri.Compare PR #118574 changed Compare() to use StringComparison.OrdinalIgnoreCase instead of string.CompareOrdinal. This is redundant: both operands are NormalizedPartUriString, which is already case-folded via ToUpperInvariant() before the comparison runs, so Ordinal and OrdinalIgnoreCase comparisons produce identical results for realistic part-name characters. Reverting to string.CompareOrdinal restores the original code, minimizes the diff, and removes any residual risk from divergent Unicode case-folding tables between ToUpperInvariant and OrdinalIgnoreCase for non-ASCII edge cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f1ae4251-6f11-4001-afb3-7cfdea3b64b5 --- .../src/System/IO/Packaging/PackUriHelper.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs index aa9213a11650e6..72118c56c3ad38 100644 --- a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs +++ b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs @@ -842,11 +842,7 @@ private int Compare(ValidatedPartUri? otherPartUri) return 1; //Compare the normalized uri strings for the two part uris. - return string.Compare( - NormalizedPartUriString, - otherPartUri.NormalizedPartUriString, - StringComparison.OrdinalIgnoreCase - ); + return string.CompareOrdinal(NormalizedPartUriString, otherPartUri.NormalizedPartUriString); } //------------------------------------------------------ From 5aab320865644eedc3036c15e33c4d7b1f0acaaa Mon Sep 17 00:00:00 2001 From: Stefan-Alin Pahontu <56953855+alinpahontu2912@users.noreply.github.com> Date: Wed, 5 Aug 2026 14:50:15 +0200 Subject: [PATCH 3/5] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../src/System/IO/Packaging/PackUriHelper.cs | 5 ++--- .../src/System/IO/Packaging/ZipPackage.cs | 4 ++-- src/libraries/System.IO.Packaging/tests/PartPieceTests.cs | 5 +++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs index 72118c56c3ad38..121675bc962501 100644 --- a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs +++ b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs @@ -877,9 +877,8 @@ private int Compare(ValidatedPartUri? otherPartUri) } /// - /// Compares instances for equality using the same - /// case-insensitive, normalized comparison as - /// via . + /// Compares instances for equality using the normalized (upper-cased) part URI + /// string comparison implemented by . /// This is used explicitly by internal collections that need case-insensitive part-name /// semantics (e.g. 's content-type override dictionary), without /// relying on overriding diff --git a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs index 6189375cd4a6bb..e3caa3d2102a14 100644 --- a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs +++ b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs @@ -945,8 +945,8 @@ internal void SaveToFile() [MemberNotNull(nameof(_overrideDictionary))] private void EnsureOverrideDictionary() { - // The part Uris are stored in the Override Dictionary in their original form , but they are compared - // in a normalized manner using the PartUriComparer + // The part Uris are stored in the Override Dictionary in their original form, but they are compared + // in a normalized, case-insensitive manner using ValidatedPartUriEqualityComparer. _overrideDictionary ??= new Dictionary( OverrideDictionaryInitialSize, PackUriHelper.ValidatedPartUriEqualityComparer.Instance); diff --git a/src/libraries/System.IO.Packaging/tests/PartPieceTests.cs b/src/libraries/System.IO.Packaging/tests/PartPieceTests.cs index 210e2a01157126..d7011327ad1105 100644 --- a/src/libraries/System.IO.Packaging/tests/PartPieceTests.cs +++ b/src/libraries/System.IO.Packaging/tests/PartPieceTests.cs @@ -373,8 +373,9 @@ public void PartUriHonorsSystemUriEqualityContract() object a = validated; object b = plain; - // object.Equals must be symmetric. - Assert.Equal(a.Equals(b), b.Equals(a)); + // object.Equals must be symmetric and treat a value-equal plain System.Uri as equal. + Assert.True(a.Equals(b)); + Assert.True(b.Equals(a)); // GetHashCode must be consistent with a value-equal plain System.Uri so both types can // coexist as keys in the same hash-based collection. From f2d60181b7b57a78c5d5bf4f415c2f2a879ca3d4 Mon Sep 17 00:00:00 2001 From: Stefan-Alin Pahontu <56953855+alinpahontu2912@users.noreply.github.com> Date: Thu, 6 Aug 2026 14:06:54 +0200 Subject: [PATCH 4/5] update misleading comment --- .../src/System/IO/Packaging/PackUriHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs index 121675bc962501..80350a7a05ed67 100644 --- a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs +++ b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs @@ -602,7 +602,7 @@ private static bool IsPartNameEmpty(string partName) /// to reduce the parsing and number of allocations for Strings and Uris /// we cache the results after parsing. /// -#pragma warning disable CA1067 // Override Equals because it implements IEquatable; not overriding to avoid possible regressions in code that's working +#pragma warning disable CA1067 // Not overriding Equals(object); changing equality behavior could affect existing case-sensitive comparisons. internal sealed class ValidatedPartUri : Uri, IComparable, IEquatable #pragma warning restore CA1067 { From bcda8d03c82aa9d87b2e004ff8543898360aeec5 Mon Sep 17 00:00:00 2001 From: alinpahontu2912 Date: Fri, 7 Aug 2026 14:58:34 +0200 Subject: [PATCH 5/5] update valdiateparturi comparer --- .../src/System/IO/Packaging/PackUriHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs index 72118c56c3ad38..61162f3b524dce 100644 --- a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs +++ b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/PackUriHelper.cs @@ -905,7 +905,7 @@ public bool Equals(ValidatedPartUri? x, ValidatedPartUri? y) } public int GetHashCode(ValidatedPartUri obj) - => StringComparer.OrdinalIgnoreCase.GetHashCode(obj.NormalizedPartUriString); + => StringComparer.Ordinal.GetHashCode(obj.NormalizedPartUriString); } #endregion Private Class