From 376ac06e6e07d4ee8d1e28f6b2346e3891487496 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Sharma?= <737941+loic-sharma@users.noreply.github.com> Date: Mon, 15 Mar 2021 15:49:28 -0700 Subject: [PATCH] Update package ID regex (#968) See: https://nuget.visualstudio.com/NuGetMicrosoft/_git/NuGet.Jobs/pullrequest/1681 --- .../UpdateDownloadsCommand.cs | 3 +- .../PackageIdValidator.cs | 29 +++ .../SearchService/IndexOperationBuilder.cs | 4 +- .../SearchService/SearchParametersBuilder.cs | 2 +- .../SearchService/SearchTextBuilder.cs | 2 +- .../NuGet.Services.AzureSearch.Tests.csproj | 1 + .../PackageIdValidatorFacts.cs | 168 ++++++++++++++++++ 7 files changed, 204 insertions(+), 5 deletions(-) create mode 100644 src/NuGet.Services.AzureSearch/PackageIdValidator.cs create mode 100644 tests/NuGet.Services.AzureSearch.Tests/PackageIdValidatorFacts.cs diff --git a/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/UpdateDownloadsCommand.cs b/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/UpdateDownloadsCommand.cs index e6391711b..ba78bad2e 100644 --- a/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/UpdateDownloadsCommand.cs +++ b/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/UpdateDownloadsCommand.cs @@ -390,7 +390,8 @@ private void CleanDownloadData(DownloadData data) foreach (var id in data.Keys.ToList()) { - var isValidId = id.Length <= PackageIdValidator.MaxPackageIdLength && PackageIdValidator.IsValidPackageId(id); + var isValidId = id.Length <= PackageIdValidator.MaxPackageIdLength + && PackageIdValidator.IsValidPackageIdWithTimeout(id); if (!isValidId) { invalidIdCount++; diff --git a/src/NuGet.Services.AzureSearch/PackageIdValidator.cs b/src/NuGet.Services.AzureSearch/PackageIdValidator.cs new file mode 100644 index 000000000..7be01789c --- /dev/null +++ b/src/NuGet.Services.AzureSearch/PackageIdValidator.cs @@ -0,0 +1,29 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Globalization; +using System.Text.RegularExpressions; + +namespace NuGet.Services.AzureSearch +{ + // TODO: Delete this copy of the PackageIdValidator. + // Tracked by: https://github.com/NuGet/Engineering/issues/3669 + // Forked from: https://github.com/NuGet/NuGet.Client/blob/18863da5be3dc8c7315f4416df1bc9ef96cb7446/src/NuGet.Core/NuGet.Packaging/PackageCreation/Utility/PackageIdValidator.cs + public static class PackageIdValidator + { + public const int MaxPackageIdLength = 100; + private static readonly Regex IdRegex = new Regex(pattern: @"^\w+([.-]\w+)*$", + options: RegexOptions.IgnoreCase | RegexOptions.ExplicitCapture | RegexOptions.CultureInvariant, + matchTimeout: TimeSpan.FromSeconds(15)); + + public static bool IsValidPackageIdWithTimeout(string packageId) + { + if (packageId == null) + { + throw new ArgumentNullException(nameof(packageId)); + } + return IdRegex.IsMatch(packageId); + } + } +} diff --git a/src/NuGet.Services.AzureSearch/SearchService/IndexOperationBuilder.cs b/src/NuGet.Services.AzureSearch/SearchService/IndexOperationBuilder.cs index 4e897e425..c739233a2 100644 --- a/src/NuGet.Services.AzureSearch/SearchService/IndexOperationBuilder.cs +++ b/src/NuGet.Services.AzureSearch/SearchService/IndexOperationBuilder.cs @@ -150,7 +150,7 @@ private bool TryGetSinglePackageId( { packageId = terms.First(); if (packageId.Length <= PackageIdValidator.MaxPackageIdLength - && PackageIdValidator.IsValidPackageId(packageId)) + && PackageIdValidator.IsValidPackageIdWithTimeout(packageId)) { return true; } @@ -183,7 +183,7 @@ private static bool HasInvalidParameters(SearchRequest request, string packageTy // Requests with bad parameters yield no results. For the package type case, by specification a package type // valid characters are the same as a package ID. return request.Skip > MaximumSkip - || (packageType != null && !PackageIdValidator.IsValidPackageId(packageType)); + || (packageType != null && !PackageIdValidator.IsValidPackageIdWithTimeout(packageType)); } private static bool PagedToFirstItem(SearchRequest request) diff --git a/src/NuGet.Services.AzureSearch/SearchService/SearchParametersBuilder.cs b/src/NuGet.Services.AzureSearch/SearchService/SearchParametersBuilder.cs index 1901616fa..3fe91d36b 100644 --- a/src/NuGet.Services.AzureSearch/SearchService/SearchParametersBuilder.cs +++ b/src/NuGet.Services.AzureSearch/SearchService/SearchParametersBuilder.cs @@ -153,7 +153,7 @@ private void ApplySearchIndexFilter( // Verify that the package type only has valid package ID characters so we don't need to worry about // escaping quotes and such. - if (packageType != null && PackageIdValidator.IsValidPackageId(packageType)) + if (packageType != null && PackageIdValidator.IsValidPackageIdWithTimeout(packageType)) { filterString += $" and {IndexFields.Search.FilterablePackageTypes}/any(p: p eq '{packageType.ToLowerInvariant()}')"; } diff --git a/src/NuGet.Services.AzureSearch/SearchService/SearchTextBuilder.cs b/src/NuGet.Services.AzureSearch/SearchService/SearchTextBuilder.cs index f7b1a68b9..145eaa5b3 100644 --- a/src/NuGet.Services.AzureSearch/SearchService/SearchTextBuilder.cs +++ b/src/NuGet.Services.AzureSearch/SearchService/SearchTextBuilder.cs @@ -314,7 +314,7 @@ private static IEnumerable ProcessFieldValues(QueryField field, IEnumera private static bool IsId(string query) { return query.Length <= PackageIdValidator.MaxPackageIdLength - && PackageIdValidator.IsValidPackageId(query); + && PackageIdValidator.IsValidPackageIdWithTimeout(query); } private static bool IsIdWithSeparator(string query) diff --git a/tests/NuGet.Services.AzureSearch.Tests/NuGet.Services.AzureSearch.Tests.csproj b/tests/NuGet.Services.AzureSearch.Tests/NuGet.Services.AzureSearch.Tests.csproj index b55fefaa8..fe31d850e 100644 --- a/tests/NuGet.Services.AzureSearch.Tests/NuGet.Services.AzureSearch.Tests.csproj +++ b/tests/NuGet.Services.AzureSearch.Tests/NuGet.Services.AzureSearch.Tests.csproj @@ -69,6 +69,7 @@ + diff --git a/tests/NuGet.Services.AzureSearch.Tests/PackageIdValidatorFacts.cs b/tests/NuGet.Services.AzureSearch.Tests/PackageIdValidatorFacts.cs new file mode 100644 index 000000000..5eeced260 --- /dev/null +++ b/tests/NuGet.Services.AzureSearch.Tests/PackageIdValidatorFacts.cs @@ -0,0 +1,168 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Xunit; + +namespace NuGet.Services.AzureSearch +{ + // TODO: Delete this copy of the PackageIdValidator. + // Tracked by: https://github.com/NuGet/Engineering/issues/3669 + // Forked from: https://github.com/NuGet/NuGet.Client/blob/18863da5be3dc8c7315f4416df1bc9ef96cb7446/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageIdValidatorTest.cs#L8 + public class PackageIdValidatorTest + { + [Fact] + public void EmptyIsNotValid() + { + // Arrange + string packageId = ""; + + // Act + bool isValid = PackageIdValidator.IsValidPackageIdWithTimeout(packageId); + + // Assert + Assert.False(isValid); + } + + [Fact] + public void NullThrowsException() + { + // Arrange + string packageId = null; + + // Act & Assert + Assert.Throws(paramName: "packageId", + testCode: () => PackageIdValidator.IsValidPackageIdWithTimeout(packageId)); + } + + [Fact] + public void AlphaNumericIsValid() + { + // Arrange + string packageId = "42This1Is4You"; + + // Act + bool isValid = PackageIdValidator.IsValidPackageIdWithTimeout(packageId); + + // Assert + Assert.True(isValid); + } + + [Fact] + public void MultipleDotSeparatorsAllowed() + { + // Arrange + string packageId = "I.Like.Writing.Unit.Tests"; + + // Act + bool isValid = PackageIdValidator.IsValidPackageIdWithTimeout(packageId); + + // Assert + Assert.True(isValid); + } + + [Fact] + public void NumbersAndWordsDotSeparatedAllowd() + { + // Arrange + string packageId = "1.2.3.4.Uno.Dos.Tres.Cuatro"; + + // Act + bool isValid = PackageIdValidator.IsValidPackageIdWithTimeout(packageId); + + // Assert + Assert.True(isValid); + } + + [Fact] + public void UnderscoreDotAndDashSeparatorsAreValid() + { + // Arrange + string packageId = "Nu_Get.Core-IsCool"; + + // Act + bool isValid = PackageIdValidator.IsValidPackageIdWithTimeout(packageId); + + // Assert + Assert.True(isValid); + } + + [Fact] + public void NonAlphaNumericUnderscoreDotDashIsInvalid() + { + // Arrange + string packageId = "ILike*Asterisks"; + + // Act + bool isValid = PackageIdValidator.IsValidPackageIdWithTimeout(packageId); + + // Assert + Assert.False(isValid); + } + + [Fact] + public void ConsecutiveSeparatorsNotAllowed() + { + // Arrange + string packageId = "I_.Like.-Separators"; + + // Act + bool isValid = PackageIdValidator.IsValidPackageIdWithTimeout(packageId); + + // Assert + Assert.False(isValid); + } + + [Fact] + public void StartingWithSeparatorsNotAllowed() + { + // Arrange + string packageId = "-StartWithSeparator"; + + // Act + bool isValid = PackageIdValidator.IsValidPackageIdWithTimeout(packageId); + + // Assert + Assert.False(isValid); + } + + [Fact] + public void EndingWithSeparatorsNotAllowed() + { + // Arrange + string packageId = "StartWithSeparator."; + + // Act + bool isValid = PackageIdValidator.IsValidPackageIdWithTimeout(packageId); + + // Assert + Assert.False(isValid); + } + + [Fact] + public void DotToolsIsNotAllowed() + { + // Arrange + string packageId = ".tools"; + + // Act + bool isValid = PackageIdValidator.IsValidPackageIdWithTimeout(packageId); + + // Assert + Assert.False(isValid); + } + + [Fact] + public void IsValidPackageId_PackageIdWithTwoUnderscores_Success() + { + // Arrange + string packageId = "Hello__World"; + + // Act + bool isValid = PackageIdValidator.IsValidPackageIdWithTimeout(packageId); + + // Assert + Assert.True(isValid); + } + } +}