Skip to content

Commit

Permalink
Block packages with illegal package dependency version ranges (#3728)
Browse files Browse the repository at this point in the history
* Upgrade NuGet.Packaging dependency

* Use strict version checks when calling NuspecReader.GetDependencyGroups
  • Loading branch information
xavierdecoster authored Apr 5, 2017
1 parent 2c584e7 commit f6dc9e9
Show file tree
Hide file tree
Showing 20 changed files with 242 additions and 147 deletions.
23 changes: 10 additions & 13 deletions src/NuGetGallery.Core/NuGetGallery.Core.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -84,23 +84,20 @@
<Reference Include="Newtonsoft.Json, Version=9.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed, processorArchitecture=MSIL">
<HintPath>..\..\packages\Newtonsoft.Json.9.0.1\lib\net45\Newtonsoft.Json.dll</HintPath>
</Reference>
<Reference Include="NuGet.Common, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Common.4.0.0\lib\net45\NuGet.Common.dll</HintPath>
<Reference Include="NuGet.Common, Version=4.3.0.2462, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Common.4.3.0-preview1-2462\lib\net45\NuGet.Common.dll</HintPath>
</Reference>
<Reference Include="NuGet.Frameworks, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Frameworks.4.0.0\lib\net45\NuGet.Frameworks.dll</HintPath>
<Reference Include="NuGet.Frameworks, Version=4.3.0.2462, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Frameworks.4.3.0-preview1-2462\lib\net45\NuGet.Frameworks.dll</HintPath>
</Reference>
<Reference Include="NuGet.Packaging, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Packaging.4.0.0\lib\net45\NuGet.Packaging.dll</HintPath>
<Reference Include="NuGet.Packaging, Version=4.3.0.2462, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Packaging.4.3.0-preview1-2462\lib\net45\NuGet.Packaging.dll</HintPath>
</Reference>
<Reference Include="NuGet.Packaging.Core, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Packaging.Core.4.0.0\lib\net45\NuGet.Packaging.Core.dll</HintPath>
<Reference Include="NuGet.Packaging.Core, Version=4.3.0.2462, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Packaging.Core.4.3.0-preview1-2462\lib\net45\NuGet.Packaging.Core.dll</HintPath>
</Reference>
<Reference Include="NuGet.Packaging.Core.Types, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Packaging.Core.Types.4.0.0\lib\net45\NuGet.Packaging.Core.Types.dll</HintPath>
</Reference>
<Reference Include="NuGet.Versioning, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Versioning.4.0.0\lib\net45\NuGet.Versioning.dll</HintPath>
<Reference Include="NuGet.Versioning, Version=4.3.0.2462, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Versioning.4.3.0-preview1-2462\lib\net45\NuGet.Versioning.dll</HintPath>
</Reference>
<Reference Include="System" />
<Reference Include="System.ComponentModel.DataAnnotations" />
Expand Down
40 changes: 24 additions & 16 deletions src/NuGetGallery.Core/Packaging/PackageMetadata.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public class PackageMetadata

public PackageMetadata(
Dictionary<string, string> metadata,
IEnumerable<PackageDependencyGroup> dependencyGroups,
IEnumerable<PackageDependencyGroup> dependencyGroups,
IEnumerable<FrameworkSpecificGroup> frameworkGroups,
IEnumerable<NuGet.Packaging.Core.PackageType> packageTypes,
NuGetVersion minClientVersion)
Expand All @@ -42,26 +42,26 @@ private void SetPropertiesFromMetadata()
{
throw new FormatException(string.Format(CoreStrings.PackageMetadata_VersionStringInvalid, versionString));
}

NuGetVersion nugetVersion;
if (NuGetVersion.TryParse(versionString, out nugetVersion))
{
Version = nugetVersion;
}

IconUrl = GetValue(PackageMetadataStrings.IconUrl, (Uri) null);
ProjectUrl = GetValue(PackageMetadataStrings.ProjectUrl, (Uri) null);
LicenseUrl = GetValue(PackageMetadataStrings.LicenseUrl, (Uri) null);
Copyright = GetValue(PackageMetadataStrings.Copyright, (string) null);
Description = GetValue(PackageMetadataStrings.Description, (string) null);
ReleaseNotes = GetValue(PackageMetadataStrings.ReleaseNotes, (string) null);
IconUrl = GetValue(PackageMetadataStrings.IconUrl, (Uri)null);
ProjectUrl = GetValue(PackageMetadataStrings.ProjectUrl, (Uri)null);
LicenseUrl = GetValue(PackageMetadataStrings.LicenseUrl, (Uri)null);
Copyright = GetValue(PackageMetadataStrings.Copyright, (string)null);
Description = GetValue(PackageMetadataStrings.Description, (string)null);
ReleaseNotes = GetValue(PackageMetadataStrings.ReleaseNotes, (string)null);
RequireLicenseAcceptance = GetValue(PackageMetadataStrings.RequireLicenseAcceptance, false);
Summary = GetValue(PackageMetadataStrings.Summary, (string) null);
Title = GetValue(PackageMetadataStrings.Title, (string) null);
Tags = GetValue(PackageMetadataStrings.Tags, (string) null);
Language = GetValue(PackageMetadataStrings.Language, (string) null);
Summary = GetValue(PackageMetadataStrings.Summary, (string)null);
Title = GetValue(PackageMetadataStrings.Title, (string)null);
Tags = GetValue(PackageMetadataStrings.Tags, (string)null);
Language = GetValue(PackageMetadataStrings.Language, (string)null);

Owners = GetValue(PackageMetadataStrings.Owners, (string) null);
Owners = GetValue(PackageMetadataStrings.Owners, (string)null);

var authorsString = GetValue(PackageMetadataStrings.Authors, Owners ?? string.Empty);
Authors = new List<string>(authorsString.Split(',').Select(author => author.Trim()));
Expand All @@ -87,7 +87,7 @@ private void SetPropertiesFromMetadata()

public string GetValueFromMetadata(string key)
{
return GetValue(key, (string) null);
return GetValue(key, (string)null);
}

public IReadOnlyCollection<PackageDependencyGroup> GetDependencyGroups()
Expand Down Expand Up @@ -131,7 +131,7 @@ private bool GetValue(string key, bool alternateValue)

private Uri GetValue(string key, Uri alternateValue)
{
var value = GetValue(key, (string) null);
var value = GetValue(key, (string)null);
if (!string.IsNullOrEmpty(value))
{
Uri result;
Expand All @@ -144,11 +144,19 @@ private Uri GetValue(string key, Uri alternateValue)
return alternateValue;
}

/// <summary>
/// Gets package metadata from a the provided <see cref="NuspecReader"/> instance.
/// </summary>
/// <param name="nuspecReader">The <see cref="NuspecReader"/> instance used to read the <see cref="PackageMetadata"/></param>
/// <exception cref="PackagingException">
/// We default to use a strict version-check on dependency groups.
/// When an invalid dependency version range is detected, a <see cref="PackagingException"/> will be thrown.
/// </exception>
public static PackageMetadata FromNuspecReader(NuspecReader nuspecReader)
{
return new PackageMetadata(
nuspecReader.GetMetadata().ToDictionary(kvp => kvp.Key, kvp => kvp.Value),
nuspecReader.GetDependencyGroups(),
nuspecReader.GetDependencyGroups(useStrictVersionCheck: true),
nuspecReader.GetFrameworkReferenceGroups(),
nuspecReader.GetPackageTypes(),
nuspecReader.GetMinClientVersion()
Expand Down
4 changes: 2 additions & 2 deletions src/NuGetGallery.Core/app.config
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="NuGet.Frameworks" publicKeyToken="31bf3856ad364e35" culture="neutral" />
<bindingRedirect oldVersion="0.0.0.0-4.0.0.0" newVersion="4.0.0.0" />
<bindingRedirect oldVersion="0.0.0.0-4.3.0.2462" newVersion="4.3.0.2462" />
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="NuGet.Versioning" publicKeyToken="31bf3856ad364e35" culture="neutral" />
<bindingRedirect oldVersion="0.0.0.0-4.0.0.0" newVersion="4.0.0.0" />
<bindingRedirect oldVersion="0.0.0.0-4.3.0.2462" newVersion="4.3.0.2462" />
</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="NuGet.Logging" publicKeyToken="31bf3856ad364e35" culture="neutral" />
Expand Down
11 changes: 5 additions & 6 deletions src/NuGetGallery.Core/packages.config
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@
<package id="Microsoft.Web.Xdt" version="2.1.1" targetFramework="net452" />
<package id="Microsoft.WindowsAzure.ConfigurationManager" version="3.1.0" targetFramework="net452" />
<package id="Newtonsoft.Json" version="9.0.1" targetFramework="net452" />
<package id="NuGet.Common" version="4.0.0" targetFramework="net452" />
<package id="NuGet.Frameworks" version="4.0.0" targetFramework="net452" />
<package id="NuGet.Packaging" version="4.0.0" targetFramework="net452" />
<package id="NuGet.Packaging.Core" version="4.0.0" targetFramework="net452" />
<package id="NuGet.Packaging.Core.Types" version="4.0.0" targetFramework="net452" />
<package id="NuGet.Versioning" version="4.0.0" targetFramework="net452" />
<package id="NuGet.Common" version="4.3.0-preview1-2462" targetFramework="net452" />
<package id="NuGet.Frameworks" version="4.3.0-preview1-2462" targetFramework="net452" />
<package id="NuGet.Packaging" version="4.3.0-preview1-2462" targetFramework="net452" />
<package id="NuGet.Packaging.Core" version="4.3.0-preview1-2462" targetFramework="net452" />
<package id="NuGet.Versioning" version="4.3.0-preview1-2462" targetFramework="net452" />
<package id="System.Spatial" version="5.6.5-beta" targetFramework="net452" />
<package id="WindowsAzure.Storage" version="7.0.0" targetFramework="net452" />
</packages>
10 changes: 5 additions & 5 deletions src/NuGetGallery/Controllers/PackagesController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,7 @@ public virtual ActionResult ManagePackageOwners(string id)

return View(model);
}

[HttpGet]
[Authorize]
[RequiresAccountConfirmation("delete a package")]
Expand Down Expand Up @@ -772,10 +772,10 @@ public virtual async Task<ActionResult> Reflow(string id, string version)
{
return HttpNotFound();
}

var reflowPackageService = new ReflowPackageService(
_entitiesContext,
(PackageService) _packageService,
_entitiesContext,
(PackageService)_packageService,
_packageFileService);

try
Expand Down Expand Up @@ -1157,7 +1157,7 @@ public virtual async Task<ActionResult> VerifyPackage(VerifyPackageRequest formD
package = await _packageService.CreatePackageAsync(nugetPackage, packageStreamMetadata, currentUser, commitChanges: false);
Debug.Assert(package.PackageRegistration != null);
}
catch (EntityException ex)
catch (InvalidPackageException ex)
{
TempData["Message"] = ex.Message;
return Redirect(Url.UploadPackage());
Expand Down
23 changes: 10 additions & 13 deletions src/NuGetGallery/NuGetGallery.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -423,25 +423,22 @@
<Reference Include="Newtonsoft.Json, Version=9.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed, processorArchitecture=MSIL">
<HintPath>..\..\packages\Newtonsoft.Json.9.0.1\lib\net45\Newtonsoft.Json.dll</HintPath>
</Reference>
<Reference Include="NuGet.Common, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Common.4.0.0\lib\net45\NuGet.Common.dll</HintPath>
<Reference Include="NuGet.Common, Version=4.3.0.2462, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Common.4.3.0-preview1-2462\lib\net45\NuGet.Common.dll</HintPath>
</Reference>
<Reference Include="NuGet.Frameworks, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Frameworks.4.0.0\lib\net45\NuGet.Frameworks.dll</HintPath>
<Reference Include="NuGet.Frameworks, Version=4.3.0.2462, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Frameworks.4.3.0-preview1-2462\lib\net45\NuGet.Frameworks.dll</HintPath>
</Reference>
<Reference Include="NuGet.Logging, Version=3.5.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<SpecificVersion>False</SpecificVersion>
<HintPath>..\..\packages\NuGet.Logging.3.5.0-beta-1160\lib\net45\NuGet.Logging.dll</HintPath>
<Private>True</Private>
</Reference>
<Reference Include="NuGet.Packaging, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Packaging.4.0.0\lib\net45\NuGet.Packaging.dll</HintPath>
<Reference Include="NuGet.Packaging, Version=4.3.0.2462, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Packaging.4.3.0-preview1-2462\lib\net45\NuGet.Packaging.dll</HintPath>
</Reference>
<Reference Include="NuGet.Packaging.Core, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Packaging.Core.4.0.0\lib\net45\NuGet.Packaging.Core.dll</HintPath>
</Reference>
<Reference Include="NuGet.Packaging.Core.Types, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Packaging.Core.Types.4.0.0\lib\net45\NuGet.Packaging.Core.Types.dll</HintPath>
<Reference Include="NuGet.Packaging.Core, Version=4.3.0.2462, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Packaging.Core.4.3.0-preview1-2462\lib\net45\NuGet.Packaging.Core.dll</HintPath>
</Reference>
<Reference Include="NuGet.Services.KeyVault, Version=1.0.0.0, Culture=neutral, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Services.KeyVault.1.0.0.0\lib\net45\NuGet.Services.KeyVault.dll</HintPath>
Expand All @@ -452,8 +449,8 @@
<HintPath>..\..\packages\NuGet.Services.Platform.Client.3.0.29-r-master\lib\portable-net45+wp80+win\NuGet.Services.Platform.Client.dll</HintPath>
<Private>True</Private>
</Reference>
<Reference Include="NuGet.Versioning, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Versioning.4.0.0\lib\net45\NuGet.Versioning.dll</HintPath>
<Reference Include="NuGet.Versioning, Version=4.3.0.2462, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<HintPath>..\..\packages\NuGet.Versioning.4.3.0-preview1-2462\lib\net45\NuGet.Versioning.dll</HintPath>
</Reference>
<Reference Include="ODataNullPropagationVisitor, Version=0.5.4237.2641, Culture=neutral, processorArchitecture=MSIL">
<SpecificVersion>False</SpecificVersion>
Expand Down
2 changes: 1 addition & 1 deletion src/NuGetGallery/Scripts/nugetgallery.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Global utility script for NuGetGallery
/// <reference path="jquery-1.6.4.js" />
/// <reference path="jquery-1.11.0.js" />
(function (window, $, undefined) {
$(function () {
// Export an object with global config data
Expand Down
57 changes: 48 additions & 9 deletions src/NuGetGallery/Services/PackageService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Threading.Tasks;
using NuGet.Frameworks;
using NuGet.Packaging;
using NuGet.Packaging.Core;
using NuGet.Versioning;
using NuGetGallery.Auditing;
using NuGetGallery.Packaging;
Expand Down Expand Up @@ -69,28 +70,66 @@ public PackageService(
_auditingService = auditingService;
}

/// <summary>
/// When no exceptions thrown, this method ensures the package metadata is valid.
/// </summary>
/// <param name="packageArchiveReader">
/// The <see cref="PackageArchiveReader"/> instance providing the package metadata.
/// </param>
/// <exception cref="InvalidPackageException">
/// This exception will be thrown when a package metadata property violates a data validation constraint.
/// </exception>
public void EnsureValid(PackageArchiveReader packageArchiveReader)
{
var packageMetadata = PackageMetadata.FromNuspecReader(packageArchiveReader.GetNuspecReader());
try
{
var packageMetadata = PackageMetadata.FromNuspecReader(packageArchiveReader.GetNuspecReader());

ValidateNuGetPackageMetadata(packageMetadata);
ValidateNuGetPackageMetadata(packageMetadata);

ValidatePackageTitle(packageMetadata);
ValidatePackageTitle(packageMetadata);

var supportedFrameworks = GetSupportedFrameworks(packageArchiveReader).Select(fn => fn.ToShortNameOrNull()).ToArray();
if (!supportedFrameworks.AnySafe(sf => sf == null))
var supportedFrameworks = GetSupportedFrameworks(packageArchiveReader).Select(fn => fn.ToShortNameOrNull()).ToArray();
if (!supportedFrameworks.AnySafe(sf => sf == null))
{
ValidateSupportedFrameworks(supportedFrameworks);
}
}
catch (Exception exception) when (exception is EntityException || exception is PackagingException)
{
ValidateSupportedFrameworks(supportedFrameworks);
// Wrap the exception for consistency of this API.
throw new InvalidPackageException(exception.Message, exception);
}
}

/// <summary>
/// Validates and creates a <see cref="Package"/> entity from a NuGet package archive.
/// </summary>
/// <param name="nugetPackage">A <see cref="PackageArchiveReader"/> instance from which package metadata can be read.</param>
/// <param name="packageStreamMetadata">The <see cref="PackageStreamMetadata"/> instance providing metadata about the package stream.</param>
/// <param name="user">The <see cref="User"/> creating the package.</param>
/// <param name="commitChanges"><c>True</c> to commit the changes to the data store and notify the indexing service; otherwise <c>false</c>.</param>
/// <returns>Returns the created <see cref="Package"/> entity.</returns>
/// <exception cref="InvalidPackageException">
/// This exception will be thrown when a package metadata property violates a data validation constraint.
/// </exception>
public async Task<Package> CreatePackageAsync(PackageArchiveReader nugetPackage, PackageStreamMetadata packageStreamMetadata, User user, bool commitChanges = true)
{
var packageMetadata = PackageMetadata.FromNuspecReader(nugetPackage.GetNuspecReader());
PackageMetadata packageMetadata;

ValidateNuGetPackageMetadata(packageMetadata);
try
{
packageMetadata = PackageMetadata.FromNuspecReader(nugetPackage.GetNuspecReader());

ValidatePackageTitle(packageMetadata);
ValidateNuGetPackageMetadata(packageMetadata);

ValidatePackageTitle(packageMetadata);
}
catch (Exception exception) when (exception is EntityException || exception is PackagingException)
{
// Wrap the exception for consistency of this API.
throw new InvalidPackageException(exception.Message, exception);
}

var packageRegistration = CreateOrGetPackageRegistration(user, packageMetadata);

Expand Down
Loading

0 comments on commit f6dc9e9

Please sign in to comment.