Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Hotfix]change sql get vulnerabilities by package id #8540

Merged
merged 6 commits into from
Apr 23, 2021
Merged

[Hotfix]change sql get vulnerabilities by package id #8540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7819767
change sql get vulunerabilities by package id
Apr 22, 2021
25efbae
Adjusted logic to provide correct groupings, adjusted UTs
drewgillies Apr 23, 2021
f50f4cf
Addressed feedback
drewgillies Apr 23, 2021
71c5506
Added a 30 minute cache for vulnerability dictionaries
drewgillies Apr 23, 2021
706fa86
Revert "Added a 30 minute cache for vulnerability dictionaries"
drewgillies Apr 23, 2021
ae9560b
Addressed feedback
drewgillies Apr 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 10 additions & 21 deletions src/NuGetGallery/Services/PackageVulnerabilitiesService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Data.Entity;
using System.IO;
using System.Linq;
using NuGet.Services.Entities;

Expand All @@ -21,26 +21,15 @@ public PackageVulnerabilitiesService(IEntitiesContext entitiesContext)

public IReadOnlyDictionary<int, IReadOnlyList<PackageVulnerability>> GetVulnerabilitiesById(string id)
{
var result = new Dictionary<int, List<PackageVulnerability>>();
var packagesMatchingId = _entitiesContext.Packages
.Where(p => p.PackageRegistration != null && p.PackageRegistration.Id == id)
.Include($"{nameof(Package.VulnerablePackageRanges)}.{nameof(VulnerablePackageVersionRange.Vulnerability)}");
foreach (var package in packagesMatchingId)
{
if (package.VulnerablePackageRanges == null)
{
continue;
}

if (package.VulnerablePackageRanges.Any())
{
result.Add(package.Key,
package.VulnerablePackageRanges.Select(vr => vr.Vulnerability).ToList());
}
}

return !result.Any() ? null :
result.ToDictionary(kv => kv.Key, kv => kv.Value as IReadOnlyList<PackageVulnerability>);
var packageKeyAndVulnerability = _entitiesContext.VulnerableRanges
.Include(x => x.Vulnerability)
.Where(x => x.PackageId == id)
.SelectMany(x => x.Packages.Select(p => new {PackageKey = p.Key, x.Vulnerability}))
.GroupBy(pv => pv.PackageKey, pv => pv.Vulnerability)
.ToDictionary(pv => pv.Key, pv => pv.ToList().AsReadOnly() as IReadOnlyList<PackageVulnerability>);

return !packageKeyAndVulnerability.Any() ? null
: new ReadOnlyDictionary<int, IReadOnlyList<PackageVulnerability>>(packageKeyAndVulnerability);
}

public bool IsPackageVulnerable(Package package)
Expand Down
29 changes: 9 additions & 20 deletions tests/NuGetGallery.Facts/Services/PackageVulnerabilitiesServiceFacts.cs
View file
Open in desktop
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.Collections.Generic;
using System.Linq;
using NuGet.Services.Entities;
using NuGetGallery.Framework;
using Xunit;
Expand All @@ -22,7 +21,6 @@ public class PackageVulnerabilitiesServiceFacts : TestContainer
private Package _packageVulnerable100;
private Package _packageVulnerable110;
private Package _packageVulnerable111;
private Package _packageVulnerable112;

private Package _packageNotVulnerable;

Expand All @@ -31,16 +29,9 @@ public void GetsVulnerabilitiesOfPackage()
{
// Arrange
SetUp();
var packages = new[]
{
_packageVulnerable100,
_packageVulnerable110,
_packageVulnerable111,
_packageVulnerable112,
_packageNotVulnerable
};
var vulnerableRanges = new[] {_versionRangeModerate, _versionRangeCritical};
var context = GetFakeContext();
context.Packages.AddRange(packages);
context.VulnerableRanges.AddRange(vulnerableRanges);
var target = Get<PackageVulnerabilitiesService>();

// Act
Expand Down Expand Up @@ -96,12 +87,14 @@ private void SetUp()

_versionRangeCritical = new VulnerablePackageVersionRange
{
PackageId = "Vulnerable",
Vulnerability = _vulnerabilityCritical,
PackageVersionRange = "1.1.1",
FirstPatchedPackageVersion = "1.1.2"
};
_versionRangeModerate = new VulnerablePackageVersionRange
{
PackageId = "Vulnerable",
Vulnerability = _vulnerabilityModerate,
PackageVersionRange = "<=1.1.1",
FirstPatchedPackageVersion = "1.1.2"
Expand Down Expand Up @@ -129,7 +122,7 @@ private void SetUp()
};
_packageVulnerable111 = new Package
{
Key = 3, // simulate a different order in db - create a non-contiguous range of rows, even if the range is contiguous
Key = 2,
PackageRegistration = _registrationVulnerable,
Version = "1.1.1",
VulnerablePackageRanges = new List<VulnerablePackageVersionRange>
Expand All @@ -138,19 +131,15 @@ private void SetUp()
_versionRangeCritical
}
};
_packageVulnerable112 = new Package
{
Key = 2, // simulate a different order in db - create a non-contiguous range of rows, even if the range is contiguous
PackageRegistration = _registrationVulnerable,
Version = "1.1.2",
VulnerablePackageRanges = new List<VulnerablePackageVersionRange>()
};
_packageNotVulnerable = new Package
{
Key = 4,
Key = 3,
PackageRegistration = new PackageRegistration { Id = "NotVulnerable" },
VulnerablePackageRanges = new List<VulnerablePackageVersionRange>()
};

_versionRangeCritical.Packages = new List<Package> { _packageVulnerable111 };
_versionRangeModerate.Packages = new List<Package> { _packageVulnerable100, _packageVulnerable110, _packageVulnerable111 };
}
}
}