From 42fbab6519e033d667a4fe3ae013b0a9918754d7 Mon Sep 17 00:00:00 2001 From: AdmiringWorm Date: Mon, 15 May 2023 11:12:48 +0200 Subject: [PATCH] (#508) Do case insensitive comparison on package ids This commit updates the Equals comparisons in the nuget service to be case insensitive. This is done as without this change the user will always have to pass in the identifier in the exact casing that is returned from the nuget server, causing issues especially during upgrade scenarios. Additionally, Equals and Compare methods has been added to the banned API list to prevent the use of these without specifying a StringComparison. --- src/chocolatey/BannedSymbols.txt | 5 ++++- src/chocolatey/EnumExtensions.cs | 2 +- src/chocolatey/TypeExtensions.cs | 2 +- .../services/NugetService.cs | 8 +++---- .../services/RuleService.cs | 2 +- .../commands/choco-install.Tests.ps1 | 20 +++++++++++++++++ .../commands/choco-uninstall.Tests.ps1 | 22 +++++++++++++++++++ .../commands/choco-upgrade.Tests.ps1 | 18 +++++++++++++++ 8 files changed, 71 insertions(+), 8 deletions(-) diff --git a/src/chocolatey/BannedSymbols.txt b/src/chocolatey/BannedSymbols.txt index e22a635d3a..80cc711ec0 100644 --- a/src/chocolatey/BannedSymbols.txt +++ b/src/chocolatey/BannedSymbols.txt @@ -4,4 +4,7 @@ M:NuGet.Versioning.SemanticVersion.ToNormalizedString;Use ToNormalizedStringChec M:Chocolatey.NuGet.Versioning.SemanticVersion.ToNormalizedString();Use ToNormalizedStringChecked() extension method instead. M:System.StringComparer.InvariantCultureIgnoreCase;Use OrdinalIgnoreCase comparer instead. M:NuGet.Protocol.Core.Types.SourceRepository.GetResource`1;Get the NuGet cached endpoint. -M:NuGet.Protocol.Core.Types.SourceRepository.GetResourceAsync`1;Get the NuGet cached endpoint. \ No newline at end of file +M:NuGet.Protocol.Core.Types.SourceRepository.GetResourceAsync`1;Get the NuGet cached endpoint. +M:System.String.Equals(System.String); Use extension method IsEqualTo to make case insensitive comparison. +M:System.String.Equals(System.String,System.String); Use overload to pass in StringComparison, or use IsEqualTo overload for case insensitive comparison. +M:System.String.Compare(System.String,System.String); Use overload to pass in StringComparison. diff --git a/src/chocolatey/EnumExtensions.cs b/src/chocolatey/EnumExtensions.cs index 2c93b39aa2..de8d879604 100644 --- a/src/chocolatey/EnumExtensions.cs +++ b/src/chocolatey/EnumExtensions.cs @@ -57,7 +57,7 @@ public static TEnum ParseEnumDescription(this string description) foreach (var fieldInfo in type.GetFields()) { var attr = fieldInfo.GetCustomAttributes(typeof (DescriptionAttribute), false).Cast().SingleOrDefault(); - if (attr != null && attr.Description.Equals(description)) + if (attr != null && attr.Description.Equals(description, StringComparison.Ordinal)) { return (TEnum) fieldInfo.GetValue(null); } diff --git a/src/chocolatey/TypeExtensions.cs b/src/chocolatey/TypeExtensions.cs index 5858c41ca0..29457b3165 100644 --- a/src/chocolatey/TypeExtensions.cs +++ b/src/chocolatey/TypeExtensions.cs @@ -36,7 +36,7 @@ public static bool IsBuiltinType(this Type type) return type.IsPrimitive || type.IsValueType || (type == typeof (string)) - || type.Namespace.Equals("System"); + || type.Namespace.Equals("System", StringComparison.Ordinal); } /// diff --git a/src/chocolatey/infrastructure.app/services/NugetService.cs b/src/chocolatey/infrastructure.app/services/NugetService.cs index 5ad6c45b36..0349148085 100644 --- a/src/chocolatey/infrastructure.app/services/NugetService.cs +++ b/src/chocolatey/infrastructure.app/services/NugetService.cs @@ -549,7 +549,7 @@ public virtual ConcurrentDictionary Install(ChocolateyCon var sourcePackageDependencyInfos = new HashSet(PackageIdentityComparer.Default); var localPackageToRemoveDependencyInfos = new HashSet(PackageIdentityComparer.Default); - var installedPackage = allLocalPackages.FirstOrDefault(p => p.Name.Equals(packageName)); + var installedPackage = allLocalPackages.FirstOrDefault(p => p.Name.IsEqualTo(packageName)); if (Platform.GetPlatform() != PlatformType.Windows && !packageName.EndsWith(".template")) { @@ -981,7 +981,7 @@ public virtual ConcurrentDictionary Upgrade(ChocolateyCon config.RevertChanges(); var allLocalPackages = GetInstalledPackages(config).ToList(); - var installedPackage = allLocalPackages.FirstOrDefault(p => p.Name.Equals(packageName)); + var installedPackage = allLocalPackages.FirstOrDefault(p => p.Name.IsEqualTo(packageName)); var packagesToInstall = new List(); var packagesToUninstall = new HashSet(); var sourcePackageDependencyInfos = new HashSet(PackageIdentityComparer.Default); @@ -1666,14 +1666,14 @@ private static void RemoveInvalidDependenciesAndParents( if (removedAvailablePackage == null) { - removedAvailablePackage = packagesToRemove.FirstOrDefault(p => p.Id.Equals(availablePackage.Identity.Id) && p.Version == availablePackage.Identity.Version); + removedAvailablePackage = packagesToRemove.FirstOrDefault(p => p.Id.IsEqualTo(availablePackage.Identity.Id) && p.Version == availablePackage.Identity.Version); } sourcePackageDependencyInfos.RemoveWhere(s => packagesToRemove.Contains(s)); removedSources.AddRange(packagesToRemove); } - if (removedAvailablePackage != null && !sourcePackageDependencyInfos.Any(s => s.Id.Equals(removedAvailablePackage.Id))) + if (removedAvailablePackage != null && !sourcePackageDependencyInfos.Any(s => s.Id.IsEqualTo(removedAvailablePackage.Id))) { removedSources.Remove(removedAvailablePackage); sourcePackageDependencyInfos.Add(removedAvailablePackage); diff --git a/src/chocolatey/infrastructure.app/services/RuleService.cs b/src/chocolatey/infrastructure.app/services/RuleService.cs index 2bdb5b4eb0..8478e83875 100644 --- a/src/chocolatey/infrastructure.app/services/RuleService.cs +++ b/src/chocolatey/infrastructure.app/services/RuleService.cs @@ -104,7 +104,7 @@ private class RuleIdEqualityComparer : IEqualityComparer { public bool Equals(ImmutableRule x, ImmutableRule y) { - return ReferenceEquals(x, y) || string.Equals(x.Id, x.Id); + return ReferenceEquals(x, y) || x.Id.IsEqualTo(x.Id); } public int GetHashCode(ImmutableRule obj) diff --git a/tests/chocolatey-tests/commands/choco-install.Tests.ps1 b/tests/chocolatey-tests/commands/choco-install.Tests.ps1 index dad52fda4b..82f6d907a0 100644 --- a/tests/chocolatey-tests/commands/choco-install.Tests.ps1 +++ b/tests/chocolatey-tests/commands/choco-install.Tests.ps1 @@ -1758,6 +1758,26 @@ To install a local, or remote file, you may use: } } + Context "Installing a package when the user specifies a non-comforming casing of the id" { + BeforeAll { + Restore-ChocolateyInstallSnapshot + + $Output = Invoke-Choco install InstAlLpAckaGe --confirm + } + + It 'Exits with Success' { + $Output.ExitCode | Should -Be 0 -Because $Output.String + } + + It 'Outputs successful installation of single package' { + $Output.Lines | Should -Contain 'Chocolatey installed 1/1 packages.' -Because $Output.String + } + + It 'Installed package to expected location' { + "$env:ChocolateyInstall\lib\installpackage" | Should -Exist + } + } + # This needs to be the last test in this block, to ensure NuGet configurations aren't being created. # Any tests after this block are expected to generate the configuration as they're explicitly using the NuGet CLI Test-NuGetPaths diff --git a/tests/chocolatey-tests/commands/choco-uninstall.Tests.ps1 b/tests/chocolatey-tests/commands/choco-uninstall.Tests.ps1 index c1b372da63..1e2a0531d4 100644 --- a/tests/chocolatey-tests/commands/choco-uninstall.Tests.ps1 +++ b/tests/chocolatey-tests/commands/choco-uninstall.Tests.ps1 @@ -399,6 +399,28 @@ Describe "choco uninstall" -Tag Chocolatey, UninstallCommand { } } + Context "Uninstalling package when user specifies non-confirming package id" { + BeforeAll { + Restore-ChocolateyInstallSnapshot + + $null = Invoke-Choco install isdependency --confirm + + $Output = Invoke-Choco uninstall IsDePeNDency --confirm + } + + It "Exits with Success (0)" { + $Output.ExitCode | Should -Be 0 + } + + It "Uninstall package successfully" { + $Output.Lines | Should -Contain "isdependency 2.1.0 Uninstalled" -Because $Output.String + } + + It "Removed package successfully from lib directory" { + "$env:ChocolateyInstall\lib\isdependency" | Should -Not -Exist + } + } + # This needs to be the last test in this block, to ensure NuGet configurations aren't being created. Test-NuGetPaths } diff --git a/tests/chocolatey-tests/commands/choco-upgrade.Tests.ps1 b/tests/chocolatey-tests/commands/choco-upgrade.Tests.ps1 index f5afcea132..66d54a340b 100644 --- a/tests/chocolatey-tests/commands/choco-upgrade.Tests.ps1 +++ b/tests/chocolatey-tests/commands/choco-upgrade.Tests.ps1 @@ -505,6 +505,24 @@ To upgrade a local, or remote file, you may use: } } + Context "Upgrading a package when user specifies non-conforming case and is latest available version (no-op)" { + BeforeAll { + Restore-ChocolateyInstallSnapshot + + $null = Invoke-Choco install isdependency --confirm + + $Output = Invoke-Choco upgrade IsDePeNDency --noop -r + } + + It "Exits with Success (0)" { + $Output.ExitCode | Should -Be 0 + } + + It "Outputs line with package name version and old version" { + $Output.String | Should -MatchExactly "isdependency\|2\.1\.0\|2\.1\.0\|false" + } + } + # This needs to be (almost) the last test in this block, to ensure NuGet configurations aren't being created. # Any tests after this block are expected to generate the configuration as they're explicitly using the NuGet CLI Test-NuGetPaths