Skip to content

Commit

Permalink
(chocolatey#508) Do case insensitive comparison on package ids
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
AdmiringWorm authored and gep13 committed May 15, 2023
1 parent 2175d3e commit 952d6e2
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 8 deletions.
5 changes: 4 additions & 1 deletion src/chocolatey/BannedSymbols.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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.
2 changes: 1 addition & 1 deletion src/chocolatey/EnumExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion src/chocolatey/TypeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
8 changes: 4 additions & 4 deletions src/chocolatey/infrastructure.app/services/NugetService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
{
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/chocolatey/infrastructure.app/services/RuleService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 20 additions & 0 deletions tests/chocolatey-tests/commands/choco-install.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -1758,6 +1758,26 @@ To install a local, or remote file, you may use:
}
}

Context "Installing a package when the user specifies a non-conforming 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
Expand Down
22 changes: 22 additions & 0 deletions tests/chocolatey-tests/commands/choco-uninstall.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
18 changes: 18 additions & 0 deletions tests/chocolatey-tests/commands/choco-upgrade.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 952d6e2

Please sign in to comment.