Skip to content

Commit

Permalink
Remove validation rules that block uploading valid semver2 versions #…
Browse files Browse the repository at this point in the history
  • Loading branch information
xavierdecoster committed May 31, 2017
1 parent 65bfb66 commit 77b08a6
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 118 deletions.
18 changes: 9 additions & 9 deletions src/NuGetGallery.Core/CoreStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/NuGetGallery.Core/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,6 @@
<data name="Manifest_TargetFrameworkNotSupported" xml:space="preserve">
<value>The target framework {0} is not supported.</value>
</data>
<data name="Manifest_InvalidVersionSemVer200" xml:space="preserve">
<value>The version '{0}' is not supported. The NuGet Gallery currently does not currently support Semantic Version 2.0 as it would break older NuGet clients.</value>
</data>
<data name="Http404NotFound" xml:space="preserve">
<value>(404) Error - Not Found</value>
</data>
Expand All @@ -180,4 +177,7 @@
<data name="NegativeIndexesAreInvalid" xml:space="preserve">
<value>Negative indexes are invalid.</value>
</data>
<data name="Manifest_InvalidDependencyVersion" xml:space="preserve">
<value>The package manifest contains an invalid Dependency Version: '{0}'</value>
</data>
</root>
11 changes: 0 additions & 11 deletions src/NuGetGallery.Core/NuGetVersionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Text.RegularExpressions;
using NuGet.Versioning;

namespace NuGetGallery
Expand All @@ -23,19 +22,9 @@ public static string Normalize(string version)

public static class NuGetVersionExtensions
{
private const RegexOptions Flags = RegexOptions.Compiled | RegexOptions.IgnoreCase | RegexOptions.ExplicitCapture;
private static readonly Regex SemanticVersionRegex = new Regex(@"^(?<Version>\d+(\s*\.\s*\d+){0,3})(?<Release>-[a-z][0-9a-z-]*)?$", Flags);

public static string ToNormalizedStringSafe(this NuGetVersion self)
{
return self != null ? self.ToNormalizedString() : String.Empty;
}

public static bool IsValidVersionForLegacyClients(this NuGetVersion self)
{
var match = SemanticVersionRegex.Match(self.ToString().Trim());

return match.Success;
}
}
}
31 changes: 10 additions & 21 deletions src/NuGetGallery.Core/Packaging/ManifestValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public static IEnumerable<ValidationResult> Validate(Stream nuspecStream, out Nu
catch (Exception ex)
{
nuspecReader = null;
return new [] { new ValidationResult(ex.Message) };
return new[] { new ValidationResult(ex.Message) };
}

return Enumerable.Empty<ValidationResult>();
Expand Down Expand Up @@ -59,7 +59,7 @@ private static IEnumerable<ValidationResult> ValidateCore(PackageMetadata packag
// Check and validate URL properties
foreach (var result in CheckUrls(
packageMetadata.GetValueFromMetadata("IconUrl"),
packageMetadata.GetValueFromMetadata("ProjectUrl"),
packageMetadata.GetValueFromMetadata("ProjectUrl"),
packageMetadata.GetValueFromMetadata("LicenseUrl")))
{
yield return result;
Expand All @@ -76,12 +76,6 @@ private static IEnumerable<ValidationResult> ValidateCore(PackageMetadata packag
version));
}

var versionValidationResult = ValidateVersion(packageMetadata.Version);
if (versionValidationResult != null)
{
yield return versionValidationResult;
}

// Check framework reference groups
var frameworkReferenceGroups = packageMetadata.GetFrameworkReferenceGroups();
if (frameworkReferenceGroups != null)
Expand Down Expand Up @@ -143,17 +137,19 @@ private static IEnumerable<ValidationResult> ValidateCore(PackageMetadata packag
// Versions
if (dependency.VersionRange.MinVersion != null)
{
var versionRangeValidationResult = ValidateVersion(dependency.VersionRange.MinVersion);
var versionRangeValidationResult =
ValidateDependencyVersion(dependency.VersionRange.MinVersion);
if (versionRangeValidationResult != null)
{
yield return versionRangeValidationResult;
}
}

if (dependency.VersionRange.MaxVersion != null
if (dependency.VersionRange.MaxVersion != null
&& dependency.VersionRange.MaxVersion != dependency.VersionRange.MinVersion)
{
var versionRangeValidationResult = ValidateVersion(dependency.VersionRange.MaxVersion);
var versionRangeValidationResult =
ValidateDependencyVersion(dependency.VersionRange.MaxVersion);
if (versionRangeValidationResult != null)
{
yield return versionRangeValidationResult;
Expand All @@ -164,22 +160,15 @@ private static IEnumerable<ValidationResult> ValidateCore(PackageMetadata packag
}
}

private static ValidationResult ValidateVersion(NuGetVersion version)
private static ValidationResult ValidateDependencyVersion(NuGetVersion version)
{
if (version.IsSemVer2)
if (version.HasMetadata)
{
return new ValidationResult(string.Format(
CultureInfo.CurrentCulture,
CoreStrings.Manifest_InvalidVersionSemVer200,
CoreStrings.Manifest_InvalidDependencyVersion,
version.ToFullString()));
}
else if (!version.IsValidVersionForLegacyClients())
{
return new ValidationResult(string.Format(
CultureInfo.CurrentCulture,
CoreStrings.Manifest_InvalidVersion,
version));
}

return null;
}
Expand Down
15 changes: 0 additions & 15 deletions src/NuGetGallery/Services/PackageService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -626,21 +626,6 @@ private static void ValidateNuGetPackageMetadata(PackageMetadata packageMetadata
{
throw new EntityException(Strings.NuGetPackagePropertyTooLong, "Id", CoreConstants.MaxPackageIdLength);
}
if (packageMetadata.Version.IsPrerelease)
{
var release = packageMetadata.Version.Release;

if (release.Contains("."))
{
throw new EntityException(Strings.NuGetPackageReleaseVersionWithDot, "Version");
}

long temp;
if (long.TryParse(release, out temp))
{
throw new EntityException(Strings.NuGetPackageReleaseVersionContainsOnlyNumerics, "Version");
}
}
if (packageMetadata.Authors != null && packageMetadata.Authors.Flatten().Length > 4000)
{
throw new EntityException(Strings.NuGetPackagePropertyTooLong, "Authors", "4000");
Expand Down
18 changes: 0 additions & 18 deletions src/NuGetGallery/Strings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 0 additions & 6 deletions src/NuGetGallery/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -314,12 +314,6 @@ The {1} Team</value>
<data name="UploadPackage_InvalidNuspec" xml:space="preserve">
<value>The NuGet package contains an invalid .nuspec file. The error encountered was: '{0}'. Correct the error and try again.</value>
</data>
<data name="NuGetPackageReleaseVersionWithDot" xml:space="preserve">
<value>Package {0} invalid: no '.' allowed in the release label.</value>
</data>
<data name="NuGetPackageReleaseVersionContainsOnlyNumerics" xml:space="preserve">
<value>Package {0} invalid: the release label can not only contain numerics.</value>
</data>
<data name="CopyExternalPackages_PackageFileBlobAlreadyExists" xml:space="preserve">
<value>SKIPPED! Package file blob {0} already exists</value>
</data>
Expand Down
39 changes: 12 additions & 27 deletions tests/NuGetGallery.Core.Facts/Packaging/ManifestValidatorFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -424,44 +424,29 @@ public void ReturnsErrorIfVersionInvalid2()
}

[Fact]
public void ReturnsErrorIfVersionIsSemVer200()
public void ReturnsNoErrorIfVersionIsSemVer200()
{
var nuspecStream = CreateNuspecStream(NuSpecSemVer200);

Assert.Equal(new[] { String.Format(CoreStrings.Manifest_InvalidVersionSemVer200, "2.0.0+123") }, GetErrors(nuspecStream));
Assert.Empty(GetErrors(nuspecStream));
}

[Theory]
[InlineData("1.0.0-beta.1")]
[InlineData("3.0.0-beta+12")]
public void ReturnsErrorIfDependencyVersionIsSemVer200(string version)
[Fact]
public void ReturnsNoErrorIfDependencyVersionIsSemVer200WithoutMetadataPart()
{
const string version = "1.0.0-beta.1";
var nuspecStream = CreateNuspecStream(string.Format(NuSpecDependencyVersionPlaceholder, version));

Assert.Equal(new[] { String.Format(CoreStrings.Manifest_InvalidVersionSemVer200, version) }, GetErrors(nuspecStream));
}

[Theory]
[InlineData("1.0.0-10")]
[InlineData("1.0.0--")]
public void ReturnsErrorIfVersionIsInvalid(string version)
{
// https://github.com/NuGet/NuGetGallery/issues/3226

var nuspecStream = CreateNuspecStream(string.Format(NuSpecPlaceholderVersion, version));

Assert.Equal(new[] { String.Format(CoreStrings.Manifest_InvalidVersion, version) }, GetErrors(nuspecStream));

Assert.Empty(GetErrors(nuspecStream));
}


[Theory]
[InlineData("1.0.0-10")]
[InlineData("1.0.0--")]
public void ReturnsErrorIfDependencyVersionIsInvalid(string version)
[Fact]
public void ReturnsErrorIfDependencyVersionIsSemVer200WithMetadataPart()
{
const string version = "3.0.0-beta+12";
var nuspecStream = CreateNuspecStream(string.Format(NuSpecDependencyVersionPlaceholder, version));

Assert.Equal(new[] { String.Format(CoreStrings.Manifest_InvalidVersion, version) }, GetErrors(nuspecStream));
Assert.Equal(new[] { String.Format(CoreStrings.Manifest_InvalidDependencyVersion, version) }, GetErrors(nuspecStream));
}

[Fact]
Expand Down
12 changes: 4 additions & 8 deletions tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -702,25 +702,21 @@ private async Task WillThrowIfTheNuGetPackageIdIsLongerThanMaxPackageIdLength()
}

[Fact]
private async Task WillThrowIfTheNuGetPackageSpecialVersionContainsADot()
private async Task DoesNotThrowIfTheNuGetPackageSpecialVersionContainsADot()
{
var service = CreateService();
var nugetPackage = CreateNuGetPackage(id: "theId", version: "1.2.3-alpha.0");

var ex = await Assert.ThrowsAsync<InvalidPackageException>(async () => await service.CreatePackageAsync(nugetPackage.Object, new PackageStreamMetadata(), null));

Assert.Equal(String.Format(Strings.NuGetPackageReleaseVersionWithDot, "Version"), ex.Message);
await service.CreatePackageAsync(nugetPackage.Object, new PackageStreamMetadata(), null);
}

[Fact]
private async Task WillThrowIfTheNuGetPackageSpecialVersionContainsOnlyNumbers()
private async Task DoesNotThrowIfTheNuGetPackageSpecialVersionContainsOnlyNumbers()
{
var service = CreateService();
var nugetPackage = CreateNuGetPackage(id: "theId", version: "1.2.3-12345");

var ex = await Assert.ThrowsAsync<InvalidPackageException>(async () => await service.CreatePackageAsync(nugetPackage.Object, new PackageStreamMetadata(), null));

Assert.Equal(String.Format(Strings.NuGetPackageReleaseVersionContainsOnlyNumerics, "Version"), ex.Message);
await service.CreatePackageAsync(nugetPackage.Object, new PackageStreamMetadata(), null);
}

[Fact]
Expand Down

0 comments on commit 77b08a6

Please sign in to comment.