(#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.
chocolatey · May 15, 2023 · 42fbab6 · 42fbab6
1 parent 2175d3e
commit 42fbab6
Show file tree

Hide file tree

Showing 8 changed files with 71 additions and 8 deletions.
diff --git 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.
+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
@@ -57,7 +57,7 @@ public static TEnum ParseEnumDescription<TEnum>(this string description)
             foreach (var fieldInfo in type.GetFields())
             {
                 var attr = fieldInfo.GetCustomAttributes(typeof (DescriptionAttribute), false).Cast<DescriptionAttribute>().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
@@ -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);
         }
 
         /// <summary>

diff --git a/src/chocolatey/infrastructure.app/services/NugetService.cs b/src/chocolatey/infrastructure.app/services/NugetService.cs
@@ -549,7 +549,7 @@ public virtual ConcurrentDictionary<string, PackageResult> Install(ChocolateyCon
                 var sourcePackageDependencyInfos = new HashSet<SourcePackageDependencyInfo>(PackageIdentityComparer.Default);
                 var localPackageToRemoveDependencyInfos = new HashSet<SourcePackageDependencyInfo>(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<string, PackageResult> 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<IPackageSearchMetadata>();
                 var packagesToUninstall = new HashSet<PackageResult>();
                 var sourcePackageDependencyInfos = new HashSet<SourcePackageDependencyInfo>(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
@@ -104,7 +104,7 @@ private class RuleIdEqualityComparer : IEqualityComparer<ImmutableRule>
         {
             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
@@ -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
@@ -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
@@ -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