Skip to content

Commit fd420f7

Browse files
author
Daniel Jacinto
authored
Packages with duplicated entries in zip file should be blocked on upload. (#7979)
* Added duplicated entries check on ValidateBeforeGeneratePackageAsync * case sensitive and path validation Added * nit * improvements on path normalization * Change ToLowerInvariant to be ToLower * Use client PathUtility for normalization * Usage of FileNameHelper for normalization * Removed unecessary added package reference * Added duplicated entries validation symbols workflow * Removed unecessary ToLower method.
1 parent e75c8ae commit fd420f7

11 files changed

Lines changed: 276 additions & 7 deletions

File tree

‎src/NuGetGallery/Helpers/ValidationHelper.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
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.Linq;
6+
using NuGet.Packaging;
7+
8+
namespace NuGetGallery.Helpers
9+
{
10+
public class ValidationHelper
11+
{
12+
public static bool HasDuplicatedEntries(PackageArchiveReader nuGetPackage)
13+
{
14+
// Normalize paths and ensures case sensitivity is also considered
15+
var packageFiles = nuGetPackage.GetFiles().Select(packageFile => FileNameHelper.GetZipEntryPath(packageFile));
16+
17+
return packageFiles.Count() != packageFiles.Distinct(StringComparer.OrdinalIgnoreCase).Count();
18+
}
19+
}
20+
}

‎src/NuGetGallery/NuGetGallery.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,7 @@
213213
<Compile Include="Controllers\ManageDeprecationJsonApiController.cs" />
214214
<Compile Include="ExtensionMethods.cs" />
215215
<Compile Include="Extensions\ImageExtensions.cs" />
216+
<Compile Include="Helpers\ValidationHelper.cs" />
216217
<Compile Include="Helpers\ViewModelExtensions\DeleteAccountListPackageItemViewModelFactory.cs" />
217218
<Compile Include="Helpers\ViewModelExtensions\DeletePackageViewModelFactory.cs" />
218219
<Compile Include="Helpers\ViewModelExtensions\DisplayLicenseViewModelFactory.cs" />

‎src/NuGetGallery/Services/PackageUploadService.cs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,13 @@ public async Task<PackageValidationResult> ValidateBeforeGeneratePackageAsync(
118118
return result;
119119
}
120120

121+
result = CheckPackageDuplicatedEntries(nuGetPackage);
122+
123+
if (result != null)
124+
{
125+
return result;
126+
}
127+
121128
var nuspecFileEntry = nuGetPackage.GetEntry(nuGetPackage.GetNuspecFile());
122129
using (var nuspecFileStream = await nuGetPackage.GetNuspecAsync(CancellationToken.None))
123130
{
@@ -599,6 +606,16 @@ private async Task<PackageValidationResult> CheckPackageEntryCountAsync(
599606
return null;
600607
}
601608

609+
private PackageValidationResult CheckPackageDuplicatedEntries(PackageArchiveReader nuGetPackage)
610+
{
611+
if (ValidationHelper.HasDuplicatedEntries(nuGetPackage))
612+
{
613+
return PackageValidationResult.Invalid(Strings.UploadPackage_PackageContainsDuplicatedEntries);
614+
}
615+
616+
return null;
617+
}
618+
602619
/// <summary>
603620
/// Validate repository metadata:
604621
/// 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.

‎src/NuGetGallery/Services/SymbolPackageUploadService.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
using NuGet.Frameworks;
1111
using NuGet.Packaging;
1212
using NuGet.Services.Entities;
13+
using NuGetGallery.Helpers;
1314
using NuGetGallery.Packaging;
1415

1516
namespace NuGetGallery
@@ -88,6 +89,12 @@ public async Task<SymbolPackageValidationResult> ValidateUploadedSymbolsPackage(
8889
normalizedVersion));
8990
}
9091

92+
// Check for duplicated entries in symbols package
93+
if (ValidationHelper.HasDuplicatedEntries(packageToPush))
94+
{
95+
return SymbolPackageValidationResult.Invalid(Strings.UploadPackage_PackageContainsDuplicatedEntries);
96+
}
97+
9198
// Do not allow to upload a snupkg to a package which has symbols package pending validations.
9299
if (package.SymbolPackages.Any(sp => sp.StatusKey == PackageStatus.Validating))
93100
{

‎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
@@ -957,6 +957,9 @@ Policy violations: {0}</value>
957957
<data name="UploadPackage_PackageContainsTooManyEntries" xml:space="preserve">
958958
<value>The package contains too many files and/or folders.</value>
959959
</data>
960+
<data name="UploadPackage_PackageContainsDuplicatedEntries" xml:space="preserve">
961+
<value>The package contains one or more duplicated files in the same folder.</value>
962+
</data>
960963
<data name="TyposquattingCheckFails" xml:space="preserve">
961964
<value>The uploaded package's id is too similar to the already existing packages: {0} </value>
962965
<comment>{0} is the existing collision Ids compared with the uploaded package Id.</comment>

‎tests/NuGetGallery.Core.Facts/TestUtils/PackageServiceUtility.cs‎

Lines changed: 23 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 Moq;
89
using NuGet.Frameworks;
910
using NuGet.Packaging;
@@ -130,7 +131,8 @@ public static MemoryStream CreateNuGetPackageStream(string id = "theId",
130131
string licenseFilename = null,
131132
byte[] licenseFileContents = null,
132133
string iconFilename = null,
133-
byte[] iconFileBinaryContents = null)
134+
byte[] iconFileBinaryContents = null,
135+
IReadOnlyList<string> entryNames = null)
134136
{
135137
if (packageDependencyGroups == null)
136138
{
@@ -189,7 +191,16 @@ public static MemoryStream CreateNuGetPackageStream(string id = "theId",
189191
writer.Write("Fake signature file.");
190192
}
191193
}
192-
}, desiredTotalEntryCount: desiredTotalEntryCount,
194+
195+
if (entryNames != null)
196+
{
197+
foreach(var entryName in entryNames)
198+
{
199+
WriteEntry(archive, entryName);
200+
}
201+
}
202+
},
203+
desiredTotalEntryCount: desiredTotalEntryCount,
193204
getCustomNuspecNodes: getCustomNuspecNodes,
194205
licenseExpression: licenseExpression,
195206
licenseFilename: licenseFilename,
@@ -198,6 +209,16 @@ public static MemoryStream CreateNuGetPackageStream(string id = "theId",
198209
iconFileContents: iconFileBinaryContents);
199210
}
200211

212+
private static void WriteEntry(ZipArchive archive, string entryName)
213+
{
214+
var entry = archive.CreateEntry(entryName);
215+
using (var stream = entry.Open())
216+
using (var writer = new StreamWriter(stream))
217+
{
218+
writer.Write(entryName);
219+
}
220+
}
221+
201222
public static PackageArchiveReader CreateArchiveReader(Stream stream)
202223
{
203224
if (stream == null)

‎tests/NuGetGallery.Facts/Helpers/ValidationHelperFacts.cs‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
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.Collections.Generic;
5+
using Moq;
6+
using NuGetGallery.TestUtils;
7+
using Xunit;
8+
9+
namespace NuGetGallery.Helpers
10+
{
11+
public class ValidationHelperFacts
12+
{
13+
public class HasDuplicatedEntriesMethod
14+
{
15+
[Theory]
16+
[InlineData("duplicatedFile.txt", "duplicatedFile.txt")]
17+
[InlineData("./temp/duplicatedFile.txt", "./temp/duplicatedFile.txt")]
18+
[InlineData("./temp/duplicatedFile.txt", "./temp\\duplicatedFile.txt")]
19+
[InlineData("./temp\\duplicatedFile.txt", "./temp\\duplicatedFile.txt")]
20+
[InlineData("duplicatedFile.txt", "duplicatedFile.TXT")]
21+
[InlineData("./duplicatedFile.txt", "duplicatedFile.txt")]
22+
[InlineData("./duplicatedFile.txt", "/duplicatedFile.txt")]
23+
[InlineData("/duplicatedFile.txt", "duplicatedFile.txt")]
24+
[InlineData(".\\duplicatedFile.txt", "./duplicatedFile.txt")]
25+
public void WithDuplicatedEntries_ReturnsFalse(params string[] entryNames)
26+
{
27+
// Arrange
28+
var package = GeneratePackage(entryNames: entryNames);
29+
30+
// Act
31+
var hasDuplicatedEntries = ValidationHelper.HasDuplicatedEntries(package.Object);
32+
33+
// Assert
34+
Assert.True(hasDuplicatedEntries);
35+
}
36+
37+
[Theory]
38+
[InlineData("noDuplicatedFile.txt", "./temp/noDuplicatedFile.txt")]
39+
[InlineData("./temp1/noDuplicatedFile.txt", "./temp2/noDuplicatedFile.txt")]
40+
[InlineData("./temp1/noDuplicatedFile.txt", "./temp1/noDuplicatedFile.css")]
41+
[InlineData("./temp1/noDuplicatedFile.txt", "./temp1\\noDuplicatedFile.css")]
42+
public void WithNoDuplicatedEntries_ReturnsTrue(params string[] entryNames)
43+
{
44+
// Arrange
45+
var package = GeneratePackage(entryNames: entryNames);
46+
47+
// Act
48+
var hasDuplicatedEntries = ValidationHelper.HasDuplicatedEntries(package.Object);
49+
50+
// Assert
51+
Assert.False(hasDuplicatedEntries);
52+
}
53+
54+
private Mock<TestPackageReader> GeneratePackage(IReadOnlyList<string> entryNames)
55+
{
56+
var packageStream = PackageServiceUtility.CreateNuGetPackageStream(entryNames: entryNames);
57+
return PackageServiceUtility.CreateNuGetPackage(packageStream);
58+
}
59+
}
60+
}
61+
}

‎tests/NuGetGallery.Facts/NuGetGallery.Facts.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@
7979
<Compile Include="Authentication\AuthenticationServiceFacts.cs" />
8080
<Compile Include="Authentication\Providers\CommonAuth\AzureActiveDirectoryV2AuthenticatorFacts.cs" />
8181
<Compile Include="Authentication\TestCredentialHelper.cs" />
82+
<Compile Include="Helpers\ValidationHelperFacts.cs" />
8283
<Compile Include="Infrastructure\ODataCacheOutputAttributeFacts.cs" />
8384
<Compile Include="Queries\AutocompleteDatabasePackageIdsQueryFacts.cs" />
8485
<Compile Include="Queries\AutocompleteDatabasePackageVersionsQueryFacts.cs" />

‎tests/NuGetGallery.Facts/Services/PackageUploadServiceFacts.cs‎

Lines changed: 60 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,55 @@ public async Task WithNotTooManyPackageEntries_WhenRejectPackagesWithTooManyPack
509509
Assert.Empty(result.Warnings);
510510
}
511511

512+
[Theory]
513+
[InlineData("duplicatedFile.txt", "duplicatedFile.txt")]
514+
[InlineData("./temp/duplicatedFile.txt", "./temp/duplicatedFile.txt")]
515+
[InlineData("./temp/duplicatedFile.txt", "./temp\\duplicatedFile.txt")]
516+
[InlineData("./temp\\duplicatedFile.txt", "./temp\\duplicatedFile.txt")]
517+
[InlineData("duplicatedFile.txt", "duplicatedFile.TXT")]
518+
[InlineData("./duplicatedFile.txt", "duplicatedFile.txt")]
519+
[InlineData("./duplicatedFile.txt", "/duplicatedFile.txt")]
520+
[InlineData("/duplicatedFile.txt", "duplicatedFile.txt")]
521+
[InlineData(".\\duplicatedFile.txt", "./duplicatedFile.txt")]
522+
public async Task WithDuplicatedEntries_ReturnsInvalidPackage(params string[] entryNames)
523+
{
524+
// Arrange
525+
_nuGetPackage = GeneratePackage(entryNames: entryNames);
526+
527+
// Act
528+
var result = await _target.ValidateBeforeGeneratePackageAsync(
529+
_nuGetPackage.Object,
530+
GetPackageMetadata(_nuGetPackage),
531+
_currentUser);
532+
533+
// Assert
534+
Assert.Equal(PackageValidationResultType.Invalid, result.Type);
535+
Assert.Equal("The package contains one or more duplicated files in the same folder.", result.Message.PlainTextMessage);
536+
Assert.Empty(result.Warnings);
537+
}
538+
539+
[Theory]
540+
[InlineData("noDuplicatedFile.txt", "./temp/noDuplicatedFile.txt")]
541+
[InlineData("./temp1/noDuplicatedFile.txt", "./temp2/noDuplicatedFile.txt")]
542+
[InlineData("./temp1/noDuplicatedFile.txt", "./temp1/noDuplicatedFile.css")]
543+
[InlineData("./temp1/noDuplicatedFile.txt", "./temp1\\noDuplicatedFile.css")]
544+
public async Task WithNoDuplicatedEntries_ReturnsAcceptedPackage(params string[] entryNames)
545+
{
546+
// Arrange
547+
_nuGetPackage = GeneratePackage(entryNames: entryNames);
548+
549+
// Act
550+
var result = await _target.ValidateBeforeGeneratePackageAsync(
551+
_nuGetPackage.Object,
552+
GetPackageMetadata(_nuGetPackage),
553+
_currentUser);
554+
555+
// Assert
556+
Assert.Equal(PackageValidationResultType.Accepted, result.Type);
557+
Assert.Null(result.Message);
558+
Assert.Empty(result.Warnings);
559+
}
560+
512561
[Theory]
513562
[InlineData(false, false)]
514563
[InlineData(true, true)]
@@ -2309,6 +2358,7 @@ protected static Mock<TestPackageReader> GeneratePackage(
23092358
RepositoryMetadata repositoryMetadata = null,
23102359
bool isSigned = true,
23112360
int? desiredTotalEntryCount = null,
2361+
IReadOnlyList<string> entryNames = null,
23122362
Func<string> getCustomNuspecNodes = null)
23132363
=> GeneratePackageWithUserContent(
23142364
version: version,
@@ -2320,7 +2370,8 @@ protected static Mock<TestPackageReader> GeneratePackage(
23202370
licenseExpression: "MIT",
23212371
licenseFilename: null,
23222372
licenseFileContents: null,
2323-
licenseFileBinaryContents: null);
2373+
licenseFileBinaryContents: null,
2374+
entryNames: entryNames);
23242375

23252376
protected static Mock<TestPackageReader> GeneratePackageWithUserContent(
23262377
string version = "1.2.3-alpha.0",
@@ -2335,7 +2386,8 @@ protected static Mock<TestPackageReader> GeneratePackageWithUserContent(
23352386
string licenseFileContents = null,
23362387
byte[] licenseFileBinaryContents = null,
23372388
string iconFilename = null,
2338-
byte[] iconFileBinaryContents = null)
2389+
byte[] iconFileBinaryContents = null,
2390+
IReadOnlyList<string> entryNames = null)
23392391
{
23402392
var packageStream = GeneratePackageStream(
23412393
version: version,
@@ -2350,7 +2402,8 @@ protected static Mock<TestPackageReader> GeneratePackageWithUserContent(
23502402
licenseFileContents: licenseFileContents,
23512403
licenseFileBinaryContents: licenseFileBinaryContents,
23522404
iconFilename: iconFilename,
2353-
iconFileBinaryContents: iconFileBinaryContents);
2405+
iconFileBinaryContents: iconFileBinaryContents,
2406+
entryNames: entryNames);
23542407

23552408
return PackageServiceUtility.CreateNuGetPackage(packageStream);
23562409
}
@@ -2368,7 +2421,8 @@ protected static MemoryStream GeneratePackageStream(
23682421
string licenseFileContents = null,
23692422
byte[] licenseFileBinaryContents = null,
23702423
string iconFilename = null,
2371-
byte[] iconFileBinaryContents = null)
2424+
byte[] iconFileBinaryContents = null,
2425+
IReadOnlyList<string> entryNames = null)
23722426
{
23732427
return PackageServiceUtility.CreateNuGetPackageStream(
23742428
id: PackageId,
@@ -2383,7 +2437,8 @@ protected static MemoryStream GeneratePackageStream(
23832437
licenseFilename: licenseFilename,
23842438
licenseFileContents: GetBinaryLicenseFileContents(licenseFileBinaryContents, licenseFileContents),
23852439
iconFilename: iconFilename,
2386-
iconFileBinaryContents: iconFileBinaryContents);
2440+
iconFileBinaryContents: iconFileBinaryContents,
2441+
entryNames: entryNames);
23872442
}
23882443

23892444
private static byte[] GetBinaryLicenseFileContents(byte[] binaryContents, string stringContents)

0 commit comments

Comments
 (0)