diff --git a/src/NuGetGallery.Core/Packaging/ManifestValidator.cs b/src/NuGetGallery.Core/Packaging/ManifestValidator.cs index 024bdca0fd..167f03ee17 100644 --- a/src/NuGetGallery.Core/Packaging/ManifestValidator.cs +++ b/src/NuGetGallery.Core/Packaging/ManifestValidator.cs @@ -22,7 +22,7 @@ public static IEnumerable Validate(Stream nuspecStream, out Nu var rawMetadata = nuspecReader.GetMetadata(); if (rawMetadata != null && rawMetadata.Any()) { - return ValidateCore(PackageMetadata.FromNuspecReader(nuspecReader)); + return ValidateCore(PackageMetadata.FromNuspecReader(nuspecReader, strict: true)); } } catch (Exception ex) diff --git a/src/NuGetGallery.Core/Packaging/PackageMetadata.cs b/src/NuGetGallery.Core/Packaging/PackageMetadata.cs index dacf48be29..b39f457400 100644 --- a/src/NuGetGallery.Core/Packaging/PackageMetadata.cs +++ b/src/NuGetGallery.Core/Packaging/PackageMetadata.cs @@ -147,15 +147,18 @@ private Uri GetValue(string key, Uri alternateValue) /// Gets package metadata from a the provided instance. /// /// The instance used to read the + /// + /// Whether or not to be strict when reading the . This should be true + /// on initial ingestion but false when a package that has already been accepted is being processed. /// /// We default to use a strict version-check on dependency groups. /// When an invalid dependency version range is detected, a will be thrown. /// - public static PackageMetadata FromNuspecReader(NuspecReader nuspecReader) + public static PackageMetadata FromNuspecReader(NuspecReader nuspecReader, bool strict) { return new PackageMetadata( nuspecReader.GetMetadata().ToDictionary(kvp => kvp.Key, kvp => kvp.Value), - nuspecReader.GetDependencyGroups(useStrictVersionCheck: true), + nuspecReader.GetDependencyGroups(useStrictVersionCheck: strict), nuspecReader.GetFrameworkReferenceGroups(), nuspecReader.GetPackageTypes(), nuspecReader.GetMinClientVersion() diff --git a/src/NuGetGallery/Controllers/PackagesController.cs b/src/NuGetGallery/Controllers/PackagesController.cs index 9391ac7f7d..adabfa170f 100644 --- a/src/NuGetGallery/Controllers/PackagesController.cs +++ b/src/NuGetGallery/Controllers/PackagesController.cs @@ -169,7 +169,8 @@ public async virtual Task UploadPackage() try { packageMetadata = PackageMetadata.FromNuspecReader( - package.GetNuspecReader()); + package.GetNuspecReader(), + strict: true); } catch (Exception ex) { @@ -407,7 +408,8 @@ public virtual async Task UploadPackage(HttpPostedFileBase uploadFil try { packageMetadata = PackageMetadata.FromNuspecReader( - package.GetNuspecReader()); + package.GetNuspecReader(), + strict: true); } catch (Exception ex) { @@ -1545,7 +1547,8 @@ public virtual async Task VerifyPackage(VerifyPackageRequest formDat Debug.Assert(nugetPackage != null); var packageMetadata = PackageMetadata.FromNuspecReader( - nugetPackage.GetNuspecReader()); + nugetPackage.GetNuspecReader(), + strict: true); // Rule out problem scenario with multiple tabs - verification request (possibly with edits) was submitted by user // viewing a different package to what was actually most recently uploaded diff --git a/src/NuGetGallery/Services/PackageService.cs b/src/NuGetGallery/Services/PackageService.cs index 4b992a452a..e6cb346922 100644 --- a/src/NuGetGallery/Services/PackageService.cs +++ b/src/NuGetGallery/Services/PackageService.cs @@ -45,7 +45,9 @@ public void EnsureValid(PackageArchiveReader packageArchiveReader) { try { - var packageMetadata = PackageMetadata.FromNuspecReader(packageArchiveReader.GetNuspecReader()); + var packageMetadata = PackageMetadata.FromNuspecReader( + packageArchiveReader.GetNuspecReader(), + strict: true); ValidateNuGetPackageMetadata(packageMetadata); @@ -82,7 +84,9 @@ public async Task CreatePackageAsync(PackageArchiveReader nugetPackage, try { - packageMetadata = PackageMetadata.FromNuspecReader(nugetPackage.GetNuspecReader()); + packageMetadata = PackageMetadata.FromNuspecReader( + nugetPackage.GetNuspecReader(), + strict: true); ValidateNuGetPackageMetadata(packageMetadata); diff --git a/src/NuGetGallery/Services/ReflowPackageService.cs b/src/NuGetGallery/Services/ReflowPackageService.cs index 87c3113aab..44c9acec1e 100644 --- a/src/NuGetGallery/Services/ReflowPackageService.cs +++ b/src/NuGetGallery/Services/ReflowPackageService.cs @@ -50,7 +50,9 @@ public async Task ReflowAsync(string id, string version) Size = packageStream.Length, }; - var packageMetadata = PackageMetadata.FromNuspecReader(packageArchive.GetNuspecReader()); + var packageMetadata = PackageMetadata.FromNuspecReader( + packageArchive.GetNuspecReader(), + strict: false); // 3) Clear referenced objects that will be reflowed ClearSupportedFrameworks(package); diff --git a/tests/NuGetGallery.Core.Facts/Packaging/PackageMetadataFacts.cs b/tests/NuGetGallery.Core.Facts/Packaging/PackageMetadataFacts.cs index 5bdca379d8..84f2cbebcd 100644 --- a/tests/NuGetGallery.Core.Facts/Packaging/PackageMetadataFacts.cs +++ b/tests/NuGetGallery.Core.Facts/Packaging/PackageMetadataFacts.cs @@ -23,7 +23,9 @@ public static void CanReadBasicMetadataProperties() var nuspec = nupkg.GetNuspecReader(); // Act - var packageMetadata = PackageMetadata.FromNuspecReader(nuspec); + var packageMetadata = PackageMetadata.FromNuspecReader( + nuspec, + strict: true); // Assert Assert.Equal("TestPackage", packageMetadata.Id); @@ -47,7 +49,39 @@ public static void ThrowsPackagingExceptionWhenInvalidDepencencyVersionRangeDete var nuspec = nupkg.GetNuspecReader(); // Act & Assert - Assert.Throws(() => PackageMetadata.FromNuspecReader(nuspec)); + Assert.Throws(() => PackageMetadata.FromNuspecReader( + nuspec, + strict: true)); + } + + [Fact] + public static void DoesNotThrowInvalidDepencencyVersionRangeDetectedAndParsingIsNotStrict() + { + var packageStream = CreateTestPackageStreamWithInvalidDependencyVersion(); + var nupkg = new PackageArchiveReader(packageStream, leaveStreamOpen: false); + var nuspec = nupkg.GetNuspecReader(); + + // Act + var packageMetadata = PackageMetadata.FromNuspecReader( + nuspec, + strict: false); + + // Assert + Assert.Equal("TestPackage", packageMetadata.Id); + Assert.Equal(NuGetVersion.Parse("0.0.0.1"), packageMetadata.Version); + Assert.Equal("Package A", packageMetadata.Title); + Assert.Equal(2, packageMetadata.Authors.Count); + Assert.Equal("ownera, ownerb", packageMetadata.Owners); + Assert.False(packageMetadata.RequireLicenseAcceptance); + Assert.Equal("package A description.", packageMetadata.Description); + Assert.Equal("en-US", packageMetadata.Language); + Assert.Equal("http://www.nuget.org/", packageMetadata.ProjectUrl.ToString()); + Assert.Equal("http://www.nuget.org/", packageMetadata.IconUrl.ToString()); + Assert.Equal("http://www.nuget.org/", packageMetadata.LicenseUrl.ToString()); + var dependencyGroup = Assert.Single(packageMetadata.GetDependencyGroups()); + var dependency = Assert.Single(dependencyGroup.Packages); + Assert.Equal("SampleDependency", dependency.Id); + Assert.Equal(VersionRange.All, dependency.VersionRange); } private static Stream CreateTestPackageStream() diff --git a/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs index dad3a64e66..93add14028 100644 --- a/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs @@ -96,7 +96,9 @@ public TestableApiController( MockPackageUploadService.Setup(x => x.GeneratePackageAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Returns((string id, PackageArchiveReader nugetPackage, PackageStreamMetadata packageStreamMetadata, User owner, User currentUser) => { - var packageMetadata = PackageMetadata.FromNuspecReader(nugetPackage.GetNuspecReader()); + var packageMetadata = PackageMetadata.FromNuspecReader( + nugetPackage.GetNuspecReader(), + strict: true); var package = new Package(); package.PackageRegistration = new PackageRegistration { Id = packageMetadata.Id, IsVerified = false }; diff --git a/tests/NuGetGallery.Facts/Services/PackageUploadServiceFacts.cs b/tests/NuGetGallery.Facts/Services/PackageUploadServiceFacts.cs index 9f99dcc6ef..a9329ddfbf 100644 --- a/tests/NuGetGallery.Facts/Services/PackageUploadServiceFacts.cs +++ b/tests/NuGetGallery.Facts/Services/PackageUploadServiceFacts.cs @@ -36,7 +36,9 @@ private static PackageUploadService CreateService( .CreatePackageAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Returns((PackageArchiveReader packageArchiveReader, PackageStreamMetadata packageStreamMetadata, User owner, User currentUser, bool isVerified) => { - var packageMetadata = PackageMetadata.FromNuspecReader(packageArchiveReader.GetNuspecReader()); + var packageMetadata = PackageMetadata.FromNuspecReader( + packageArchiveReader.GetNuspecReader(), + strict: true); var newPackage = new Package(); newPackage.PackageRegistration = new PackageRegistration { Id = packageMetadata.Id, IsVerified = isVerified }; diff --git a/tests/NuGetGallery.Facts/Services/ReflowPackageServiceFacts.cs b/tests/NuGetGallery.Facts/Services/ReflowPackageServiceFacts.cs index ce437df4b3..5d0ce1e86b 100644 --- a/tests/NuGetGallery.Facts/Services/ReflowPackageServiceFacts.cs +++ b/tests/NuGetGallery.Facts/Services/ReflowPackageServiceFacts.cs @@ -262,6 +262,46 @@ public async Task CallsUpdateIsLatestAsync() // Assert packageService.Verify(s => s.UpdateIsLatestAsync(package.PackageRegistration, false), Times.Once); } + + [Fact] + public async Task AllowsInvalidPackageDependencyVersion() + { + // Arrange + var package = PackageServiceUtility.CreateTestPackage(); + + var packageService = SetupPackageService(package); + var entitiesContext = SetupEntitiesContext(); + var packageFileService = SetupPackageFileService( + package, + CreateInvalidDependencyVersionTestPackageStream()); + + var service = CreateService( + packageService: packageService, + entitiesContext: entitiesContext, + packageFileService: packageFileService); + + // Act + var result = await service.ReflowAsync("test", "1.0.0"); + + // Assert + Assert.Equal("test", result.PackageRegistration.Id); + Assert.Equal("1.0.0", result.NormalizedVersion); + + Assert.True(result.Dependencies.Any(d => + d.Id == "WebActivator" + && d.VersionSpec == "(, )" + && d.TargetFramework == "net40")); + + Assert.True(result.Dependencies.Any(d => + d.Id == "PackageC" + && d.VersionSpec == "[1.1.0, 2.0.1)" + && d.TargetFramework == "net40")); + + Assert.True(result.Dependencies.Any(d => + d.Id == "jQuery" + && d.VersionSpec == "(, )" + && d.TargetFramework == "net451")); + } } private static Mock SetupPackageService(Package package) @@ -325,27 +365,50 @@ private static Mock SetupEntitiesContext() return entitiesContext; } - private static Mock SetupPackageFileService(Package package) + private static Mock SetupPackageFileService(Package package, Stream packageStream = null) { var packageFileService = new Mock(); packageFileService .Setup(s => s.DownloadPackageFileAsync(package)) - .Returns(Task.FromResult(CreateTestPackageStream())) + .Returns(Task.FromResult(packageStream ?? CreateTestPackageStream())) .Verifiable(); return packageFileService; } + private static Stream CreateInvalidDependencyVersionTestPackageStream() + { + return CreateTestPackageStream(@" + + + test + 1.0.0 + Test package + authora, authorb + ownera + false + package A description. + en-US + http://www.nuget.org/ + http://www.nuget.org/ + http://www.nuget.org/ + + + + + + + + + + + "); + } + private static Stream CreateTestPackageStream() { - var packageStream = new MemoryStream(); - using (var packageArchive = new ZipArchive(packageStream, ZipArchiveMode.Create, true)) - { - var nuspecEntry = packageArchive.CreateEntry("TestPackage.nuspec", CompressionLevel.Fastest); - using (var streamWriter = new StreamWriter(nuspecEntry.Open())) - { - streamWriter.WriteLine(@" + return CreateTestPackageStream(@" test @@ -370,6 +433,17 @@ private static Stream CreateTestPackageStream() "); + } + + private static Stream CreateTestPackageStream(string nuspec) + { + var packageStream = new MemoryStream(); + using (var packageArchive = new ZipArchive(packageStream, ZipArchiveMode.Create, true)) + { + var nuspecEntry = packageArchive.CreateEntry("TestPackage.nuspec", CompressionLevel.Fastest); + using (var streamWriter = new StreamWriter(nuspecEntry.Open())) + { + streamWriter.WriteLine(nuspec); } packageArchive.CreateEntry("content\\HelloWorld.cs", CompressionLevel.Fastest);