Skip to content

Commit 2533703

Browse files
authored
Gallery: enable blocking packages with too many package entries (#6358)
Progress on #6357.
1 parent 42dd5a9 commit 2533703

9 files changed

Lines changed: 182 additions & 24 deletions

File tree

src/NuGetGallery/Configuration/AppConfiguration.cs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -313,8 +313,10 @@ public class AppConfiguration : IAppConfiguration
313313
/// Gets/sets a string that is brand string to display in the footer, this also
314314
/// accepts a single {0} string format token which is replaced by the UTC year
315315
/// </summary>
316-
public string ExternalBrandingMessage {
317-
get {
316+
public string ExternalBrandingMessage
317+
{
318+
get
319+
{
318320
return _ExternalBrandingMessage;
319321
}
320322

@@ -338,5 +340,7 @@ public string ExternalBrandingMessage {
338340
public bool IsHosted { get; set; }
339341

340342
public bool RejectSignedPackagesWithNoRegisteredCertificate { get; set; }
343+
344+
public bool RejectPackagesWithTooManyPackageEntries { get; set; }
341345
}
342-
}
346+
}

src/NuGetGallery/Configuration/IAppConfiguration.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,5 +339,10 @@ public interface IAppConfiguration : ICoreMessageServiceConfiguration
339339
/// by the owner.
340340
/// </summary>
341341
bool RejectSignedPackagesWithNoRegisteredCertificate { get; set; }
342+
343+
/// <summary>
344+
/// Whether or not to synchronously reject packages on push/upload that have too many package entries.
345+
/// </summary>
346+
bool RejectPackagesWithTooManyPackageEntries { get; set; }
342347
}
343-
}
348+
}

src/NuGetGallery/Services/PackageUploadService.cs

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,14 @@ public async Task<PackageValidationResult> ValidateBeforeGeneratePackageAsync(Pa
4444
{
4545
var warnings = new List<string>();
4646

47-
var result = await CheckForUnsignedPushAfterAuthorSignedAsync(
47+
var result = await CheckPackageEntryCountAsync(nuGetPackage, warnings);
48+
49+
if (result != null)
50+
{
51+
return result;
52+
}
53+
54+
result = await CheckForUnsignedPushAfterAuthorSignedAsync(
4855
nuGetPackage,
4956
warnings);
5057

@@ -63,6 +70,34 @@ public async Task<PackageValidationResult> ValidateBeforeGeneratePackageAsync(Pa
6370
return PackageValidationResult.AcceptedWithWarnings(warnings);
6471
}
6572

73+
private async Task<PackageValidationResult> CheckPackageEntryCountAsync(
74+
PackageArchiveReader nuGetPackage,
75+
List<string> warnings)
76+
{
77+
if (!_config.RejectPackagesWithTooManyPackageEntries)
78+
{
79+
return null;
80+
}
81+
82+
const ushort maxPackageEntryCount = ushort.MaxValue - 1;
83+
84+
var packageEntryCount = nuGetPackage.GetFiles().Count();
85+
86+
if (await nuGetPackage.IsSignedAsync(CancellationToken.None))
87+
{
88+
if (packageEntryCount > maxPackageEntryCount)
89+
{
90+
return PackageValidationResult.Invalid(Strings.UploadPackage_PackageContainsTooManyEntries);
91+
}
92+
}
93+
else if (packageEntryCount >= maxPackageEntryCount)
94+
{
95+
return PackageValidationResult.Invalid(Strings.UploadPackage_PackageContainsTooManyEntries);
96+
}
97+
98+
return null;
99+
}
100+
66101
/// <summary>
67102
/// Validate repository metadata:
68103
/// 1. If the type is "git" - allow the URL scheme "git://" or "https://". We will translate "git://" to "https://" at display time for known domains.
@@ -212,10 +247,10 @@ private async Task<PackageValidationResult> ValidateSignatureFilePresenceAsync(
212247
else
213248
{
214249
return new PackageValidationResult(
215-
PackageValidationResultType.PackageShouldNotBeSigned,
216-
string.Format(
217-
Strings.UploadPackage_PackageIsSignedButMissingCertificate_RequiredSigner,
218-
owner.Username));
250+
PackageValidationResultType.PackageShouldNotBeSigned,
251+
string.Format(
252+
Strings.UploadPackage_PackageIsSignedButMissingCertificate_RequiredSigner,
253+
owner.Username));
219254
}
220255
}
221256
}

src/NuGetGallery/Strings.Designer.cs

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/NuGetGallery/Strings.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -986,4 +986,7 @@ Policy violations: {0}</value>
986986
<data name="WarningNotHttpsRepositoryUrlScheme" xml:space="preserve">
987987
<value>Repository URL scheme should be 'https'. The provided URL will not be displayed.</value>
988988
</data>
989+
<data name="UploadPackage_PackageContainsTooManyEntries" xml:space="preserve">
990+
<value>The package contains too many files and/or folders.</value>
991+
</data>
989992
</root>

src/NuGetGallery/Web.config

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@
146146
<add key="Gallery.TrademarksUrl" value=""/>
147147
<add key="Gallery.EnforceDefaultSecurityPolicies" value="false"/>
148148
<add key="Gallery.RejectSignedPackagesWithNoRegisteredCertificate" value="false"/>
149+
<add key="Gallery.RejectPackagesWithTooManyPackageEntries" value="false"/>
149150
<!-- This is also the default so you can remove this setting if you really want. -->
150151
<!-- Set this to false if you want to disable search indexing in the background. -->
151152
<add key="Gallery.AutoUpdateSearchIndex" value="true"/>

tests/NuGetGallery.Facts/Services/PackageUploadServiceFacts.cs

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Collections.Generic;
66
using System.IO;
7+
using System.IO.Compression;
78
using System.Linq;
89
using System.Threading;
910
using System.Threading.Tasks;
@@ -376,6 +377,69 @@ public async Task AggregatesWarnings()
376377
Assert.Equal(2, result.Warnings.Count());
377378
}
378379

380+
[Theory]
381+
[InlineData(true)]
382+
[InlineData(false)]
383+
public async Task WithTooManyPackageEntries_WhenRejectPackagesWithTooManyPackageEntriesIsFalse_AcceptsPackage(bool isSigned)
384+
{
385+
var desiredTotalEntryCount = isSigned ? ushort.MaxValue : ushort.MaxValue - 1;
386+
387+
_nuGetPackage = GeneratePackage(isSigned: isSigned, desiredTotalEntryCount: desiredTotalEntryCount);
388+
_config
389+
.Setup(x => x.RejectPackagesWithTooManyPackageEntries)
390+
.Returns(false);
391+
392+
var result = await _target.ValidateBeforeGeneratePackageAsync(
393+
_nuGetPackage.Object,
394+
GetPackageMetadata(_nuGetPackage));
395+
396+
Assert.Equal(PackageValidationResultType.Accepted, result.Type);
397+
Assert.Null(result.Message);
398+
Assert.Empty(result.Warnings);
399+
}
400+
401+
[Theory]
402+
[InlineData(true)]
403+
[InlineData(false)]
404+
public async Task WithTooManyPackageEntries_WhenRejectPackagesWithTooManyPackageEntriesIsTrue_RejectsPackage(bool isSigned)
405+
{
406+
var desiredTotalEntryCount = isSigned ? ushort.MaxValue : ushort.MaxValue - 1;
407+
408+
_nuGetPackage = GeneratePackage(isSigned: isSigned, desiredTotalEntryCount: desiredTotalEntryCount);
409+
_config
410+
.Setup(x => x.RejectPackagesWithTooManyPackageEntries)
411+
.Returns(true);
412+
413+
var result = await _target.ValidateBeforeGeneratePackageAsync(
414+
_nuGetPackage.Object,
415+
GetPackageMetadata(_nuGetPackage));
416+
417+
Assert.Equal(PackageValidationResultType.Invalid, result.Type);
418+
Assert.Equal("The package contains too many files and/or folders.", result.Message);
419+
Assert.Empty(result.Warnings);
420+
}
421+
422+
[Theory]
423+
[InlineData(true)]
424+
[InlineData(false)]
425+
public async Task WithNotTooManyPackageEntries_WhenRejectPackagesWithTooManyPackageEntriesIsTrue_AcceptsPackage(bool isSigned)
426+
{
427+
var desiredTotalEntryCount = (isSigned ? ushort.MaxValue : ushort.MaxValue - 1) - 1;
428+
429+
_nuGetPackage = GeneratePackage(isSigned: isSigned, desiredTotalEntryCount: desiredTotalEntryCount);
430+
_config
431+
.Setup(x => x.RejectPackagesWithTooManyPackageEntries)
432+
.Returns(true);
433+
434+
var result = await _target.ValidateBeforeGeneratePackageAsync(
435+
_nuGetPackage.Object,
436+
GetPackageMetadata(_nuGetPackage));
437+
438+
Assert.Equal(PackageValidationResultType.Accepted, result.Type);
439+
Assert.Null(result.Message);
440+
Assert.Empty(result.Warnings);
441+
}
442+
379443
private PackageMetadata GetPackageMetadata(Mock<TestPackageReader> mockPackage)
380444
{
381445
return PackageMetadata.FromNuspecReader(mockPackage.Object.GetNuspecReader(), strict: true);
@@ -934,13 +998,18 @@ public FactsBase()
934998
_config.Object);
935999
}
9361000

937-
protected static Mock<TestPackageReader> GeneratePackage(string version = "1.2.3-alpha.0", RepositoryMetadata repositoryMetadata = null, bool isSigned = true)
1001+
protected static Mock<TestPackageReader> GeneratePackage(
1002+
string version = "1.2.3-alpha.0",
1003+
RepositoryMetadata repositoryMetadata = null,
1004+
bool isSigned = true,
1005+
int? desiredTotalEntryCount = null)
9381006
{
9391007
return PackageServiceUtility.CreateNuGetPackage(
9401008
id: "theId",
9411009
version: version,
9421010
repositoryMetadata: repositoryMetadata,
943-
isSigned: isSigned);
1011+
isSigned: isSigned,
1012+
desiredTotalEntryCount: desiredTotalEntryCount);
9441013
}
9451014
}
9461015
}

tests/NuGetGallery.Facts/TestPackage.cs

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,8 @@ public static Stream CreateTestPackageStream(
171171
IEnumerable<ClientPackageType> packageTypes = null,
172172
RepositoryMetadata repositoryMetadata = null,
173173
Action<ZipArchive> populatePackage = null,
174-
bool isSymbolPackage = false)
174+
bool isSymbolPackage = false,
175+
int? desiredTotalEntryCount = null)
175176
{
176177
return CreateTestPackageStream(packageArchive =>
177178
{
@@ -187,7 +188,7 @@ public static Stream CreateTestPackageStream(
187188
{
188189
populatePackage(packageArchive);
189190
}
190-
});
191+
}, desiredTotalEntryCount);
191192
}
192193

193194
public static Stream CreateTestSymbolPackageStream(string id, string version, Action<ZipArchive> populatePackage = null)
@@ -218,17 +219,44 @@ public static Stream CreateTestPackageStreamFromNuspec(string id, string nuspec,
218219
});
219220
}
220221

221-
public static Stream CreateTestPackageStream(Action<ZipArchive> populatePackage)
222+
public static Stream CreateTestPackageStream(Action<ZipArchive> populatePackage, int? desiredTotalEntryCount = null)
222223
{
223224
var packageStream = new MemoryStream();
224-
using (var packageArchive = new ZipArchive(packageStream, ZipArchiveMode.Create, true))
225+
using (var packageArchive = new ZipArchive(packageStream, ZipArchiveMode.Create, leaveOpen: true))
225226
{
226227
if (populatePackage != null)
227228
{
228229
populatePackage(packageArchive);
229230
}
230231
}
231232

233+
if (desiredTotalEntryCount.HasValue)
234+
{
235+
int packageEntryCount;
236+
237+
using (var packageArchive = new ZipArchive(packageStream, ZipArchiveMode.Read, leaveOpen: true))
238+
{
239+
packageEntryCount = packageArchive.Entries.Count;
240+
}
241+
242+
if (desiredTotalEntryCount.Value < packageEntryCount)
243+
{
244+
throw new ArgumentException(
245+
$"The desired count ({desiredTotalEntryCount.Value}) of package entries is less than the actual count ({packageEntryCount}) of package entries.",
246+
nameof(desiredTotalEntryCount));
247+
}
248+
249+
using (var packageArchive = new ZipArchive(packageStream, ZipArchiveMode.Update, leaveOpen: true))
250+
{
251+
while (packageEntryCount < desiredTotalEntryCount.Value)
252+
{
253+
packageArchive.CreateEntry(Guid.NewGuid().ToString());
254+
255+
++packageEntryCount;
256+
}
257+
}
258+
}
259+
232260
packageStream.Position = 0;
233261

234262
return packageStream;

tests/NuGetGallery.Facts/TestUtils/TestServiceUtility.cs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
1-
using Moq;
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Data.Entity;
7+
using System.IO;
8+
using System.Linq;
9+
using System.Linq.Expressions;
10+
using Moq;
211
using NuGet.Frameworks;
312
using NuGet.Packaging;
413
using NuGet.Packaging.Core;
@@ -7,12 +16,6 @@
716
using NuGetGallery.Configuration;
817
using NuGetGallery.Framework;
918
using NuGetGallery.Security;
10-
using System;
11-
using System.Collections.Generic;
12-
using System.Data.Entity;
13-
using System.IO;
14-
using System.Linq;
15-
using System.Linq.Expressions;
1619

1720
namespace NuGetGallery.TestUtils
1821
{
@@ -78,7 +81,8 @@ public static Mock<TestPackageReader> CreateNuGetPackage(
7881
IEnumerable<PackageDependencyGroup> packageDependencyGroups = null,
7982
IEnumerable<NuGet.Packaging.Core.PackageType> packageTypes = null,
8083
RepositoryMetadata repositoryMetadata = null,
81-
bool isSigned = false)
84+
bool isSigned = false,
85+
int? desiredTotalEntryCount = null)
8286
{
8387
if (packageDependencyGroups == null)
8488
{
@@ -136,7 +140,7 @@ public static Mock<TestPackageReader> CreateNuGetPackage(
136140
writer.Write("Fake signature file.");
137141
}
138142
}
139-
});
143+
}, desiredTotalEntryCount: desiredTotalEntryCount);
140144

141145
var mock = new Mock<TestPackageReader>(testPackage);
142146
mock.CallBase = true;

0 commit comments

Comments
 (0)