Permalink
Browse files

Modifying GetUpdates to validate input

  • Loading branch information...
1 parent f534860 commit 30640888d48f92f85731cd710e499761d35bd5bc @pranavkm pranavkm committed Jun 7, 2012
Showing with 77 additions and 13 deletions.
  1. +53 −0 Facts/Services/FeedServiceFacts.cs
  2. +24 −12 Website/DataServices/V2Feed.svc.cs
  3. +0 −1 Website/Website.csproj
@@ -3,6 +3,7 @@
using System.Linq;
using Moq;
using Xunit;
+using Xunit.Extensions;
namespace NuGetGallery.Services
{
@@ -141,6 +142,58 @@ public void V2FeedFindPackagesByIdReturnsUnlistedAndPrereleasePackages()
Assert.Equal("1.0.1-a", result.Last().Version);
}
+ [Theory]
+ [InlineData(null, "1.0.0|0.9")]
+ [InlineData("", "1.0.0|0.9")]
+ [InlineData(" ", "1.0.0|0.9")]
+ [InlineData("| ", "1.0.0|0.9")]
+ [InlineData("A", null)]
+ [InlineData("A", "")]
+ [InlineData("A", " |")]
+ [InlineData("A", "| ")]
+ [InlineData("A|B", "1.0|")]
+ public void V2FeedGetUpdatesReturnsEmptyResultsIfInputIsMalformed(string id, string version)
+ {
+ // Arrange
+ var repo = Mock.Of<IEntityRepository<Package>>();
+ var configuration = Mock.Of<IConfiguration>();
+ var v2Service = new TestableV2Feed(repo, configuration, null);
+
+ // Act
+ var result = v2Service.GetUpdates(id, version, includePrerelease: false, includeAllVersions: true, targetFrameworks: null)
+ .ToList();
+
+ // Assert
+ Assert.Empty(result);
+ }
+
+ [Fact]
+ public void V2FeedGetUpdatesIgnoresItemsWithMalformedVersions()
+ {
+ // Arrange
+ var packageRegistrationA = new PackageRegistration { Id = "Foo" };
+ var packageRegistrationB = new PackageRegistration { Id = "Qux" };
+ var repo = new Mock<IEntityRepository<Package>>(MockBehavior.Strict);
+ repo.Setup(r => r.GetAll()).Returns(new[] {
+ new Package { PackageRegistration = packageRegistrationA, Version = "1.0.0", IsPrerelease = false, Listed = true },
+ new Package { PackageRegistration = packageRegistrationA, Version = "1.1.0", IsPrerelease = false, Listed = true },
+ new Package { PackageRegistration = packageRegistrationA, Version = "1.2.0-alpha", IsPrerelease = true, Listed = true },
+ new Package { PackageRegistration = packageRegistrationA, Version = "1.2.0", IsPrerelease = false, Listed = true },
+ new Package { PackageRegistration = packageRegistrationB, Version = "2.0", IsPrerelease = false, Listed = true },
+ }.AsQueryable());
+ var configuration = new Mock<IConfiguration>(MockBehavior.Strict);
+ configuration.Setup(c => c.GetSiteRoot(false)).Returns("https://localhost:8081/");
+ var v2Service = new TestableV2Feed(repo.Object, configuration.Object, null);
+
+ // Act
+ var result = v2Service.GetUpdates("Foo|Qux", "1.0.0|abcd", includePrerelease: false, includeAllVersions: false, targetFrameworks: null)
+ .ToList();
+
+ // Assert
+ Assert.Equal(1, result.Count);
+ AssertPackage(new { Id = "Foo", Version = "1.2.0" }, result[0]);
+ }
+
[Fact]
public void V2FeedGetUpdatesReturnsVersionsNewerThanListedVersion()
{
@@ -66,23 +66,31 @@ public IQueryable<V2FeedPackage> FindPackagesById(string id)
[WebGet]
public IQueryable<V2FeedPackage> GetUpdates(string packageIds, string versions, bool includePrerelease, bool includeAllVersions, string targetFrameworks)
{
- var idValues = packageIds.Split('|');
- var versionValues = versions.Split('|');
+ if (String.IsNullOrEmpty(packageIds) || String.IsNullOrEmpty(versions))
+ {
+ return Enumerable.Empty<V2FeedPackage>().AsQueryable();
+ }
+
+ var idValues = packageIds.Trim().Split(new[] { '|' }, StringSplitOptions.RemoveEmptyEntries);
+ var versionValues = versions.Trim().Split(new[] { '|' }, StringSplitOptions.RemoveEmptyEntries);
var targetFrameworkValues = String.IsNullOrEmpty(targetFrameworks) ? null :
targetFrameworks.Split('|').Select(VersionUtility.ParseFrameworkName).ToList();
- if (!idValues.Any() || !idValues.Count().Equals(versionValues.Count()))
+ if ((idValues.Length == 0) || (idValues.Length != versionValues.Length))
{
- throw new InvalidOperationException("Number of packages do not match versions");
+ // Exit early if the request looks invalid
+ return Enumerable.Empty<V2FeedPackage>().AsQueryable();
}
- Dictionary<string, SemanticVersion> versionLookup = new Dictionary<string, SemanticVersion>(idValues.Count(), StringComparer.OrdinalIgnoreCase);
+ Dictionary<string, SemanticVersion> versionLookup = new Dictionary<string, SemanticVersion>(idValues.Length, StringComparer.OrdinalIgnoreCase);
for (int i = 0; i < idValues.Length; i++)
{
var id = idValues[i];
SemanticVersion version;
- SemanticVersion currentVersion = new SemanticVersion(versionValues[i]);
- if (!versionLookup.TryGetValue(id, out version) || (currentVersion > version))
+ SemanticVersion currentVersion;
+
+ if (SemanticVersion.TryParse(versionValues[i], out currentVersion) &&
+ (!versionLookup.TryGetValue(id, out version) || (currentVersion > version)))
{
// If we've never added the package to lookup or we encounter the same id but with a higher version, then choose the higher version.
versionLookup[id] = currentVersion;
@@ -110,11 +118,15 @@ public IQueryable<V2FeedPackage> GetUpdates(string packageIds, string versions,
// TODO: We could optimize for the includeAllVersions case here by short circuiting the operation once we've encountered the highest version
// for a given Id
var version = SemanticVersion.Parse(p.Version);
- var clientVersion = versionLookup[p.PackageRegistration.Id];
- var supportedPackageFrameworks = p.SupportedFrameworks.Select(f => f.FrameworkName);
-
- return (version > clientVersion) &&
- (targetFrameworkValues == null || targetFrameworkValues.Any(s => VersionUtility.IsCompatible(s, supportedPackageFrameworks)));
+ SemanticVersion clientVersion;
+ if (versionLookup.TryGetValue(p.PackageRegistration.Id, out clientVersion))
+ {
+ var supportedPackageFrameworks = p.SupportedFrameworks.Select(f => f.FrameworkName);
+
+ return (version > clientVersion) &&
+ (targetFrameworkValues == null || targetFrameworkValues.Any(s => VersionUtility.IsCompatible(s, supportedPackageFrameworks)));
+ }
+ return false;
});
if (!includeAllVersions)
View
@@ -13,7 +13,6 @@
<RootNamespace>NuGetGallery</RootNamespace>
<AssemblyName>NuGetGallery.Website</AssemblyName>
<TargetFrameworkVersion>v4.0</TargetFrameworkVersion>
- <MvcBuildViews>true</MvcBuildViews>
<UseIISExpress>true</UseIISExpress>
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\</SolutionDir>
<RestorePackages>true</RestorePackages>

0 comments on commit 3064088

Please sign in to comment.