Skip to content

Commit 4d1e437

Browse files
xavierdecosterxavierdecoster
authored
Verify nuspec file size (#7038)
1 parent f69f547 commit 4d1e437

7 files changed

Lines changed: 148 additions & 44 deletions

File tree

‎src/NuGetGallery/Services/PackageUploadService.cs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,15 @@ public async Task<PackageValidationResult> ValidateBeforeGeneratePackageAsync(Pa
100100
return result;
101101
}
102102

103+
var nuspecFileEntry = nuGetPackage.GetEntry(nuGetPackage.GetNuspecFile());
104+
using (var nuspecFileStream = await nuGetPackage.GetNuspecAsync(CancellationToken.None))
105+
{
106+
if (!await IsStreamLengthMatchesReportedAsync(nuspecFileStream, nuspecFileEntry.Length))
107+
{
108+
return PackageValidationResult.Invalid(Strings.UploadPackage_CorruptNupkg);
109+
}
110+
}
111+
103112
result = await CheckForUnsignedPushAfterAuthorSignedAsync(
104113
nuGetPackage,
105114
warnings);
@@ -225,7 +234,7 @@ private async Task<PackageValidationResult> CheckLicenseMetadataAsync(PackageArc
225234
{
226235
return PackageValidationResult.Invalid(new InvalidUrlEncodingForLicenseUrlValidationMessage());
227236
}
228-
237+
229238
if (licenseMetadata.Type == LicenseType.File)
230239
{
231240
return PackageValidationResult.Invalid(

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,9 @@ private static string WriteRepositoryMetadata(RepositoryMetadata repositoryMetad
106106
{
107107
return repositoryMetadata == null
108108
? string.Empty
109-
: "<repository type=\"" + repositoryMetadata.Type + "\" " +
110-
"url =\"" + repositoryMetadata.Url + "\" " +
111-
"commit=\"" + repositoryMetadata.Commit + "\" " +
109+
: "<repository type=\"" + repositoryMetadata.Type + "\" " +
110+
"url =\"" + repositoryMetadata.Url + "\" " +
111+
"commit=\"" + repositoryMetadata.Commit + "\" " +
112112
"branch=\"" + repositoryMetadata.Branch + "\"/>";
113113
}
114114

@@ -120,7 +120,7 @@ private static string WritePackageTypes(IEnumerable<NuGet.Packaging.Core.Package
120120
}
121121

122122
var output = new StringBuilder();
123-
foreach(var packageType in packageTypes)
123+
foreach (var packageType in packageTypes)
124124
{
125125
output.Append("<packageType");
126126
if (packageType.Name != null)
@@ -233,10 +233,10 @@ public static MemoryStream CreateTestSymbolPackageStream(string id = "theId", st
233233
{
234234
var packageTypes = new List<ClientPackageType>();
235235
packageTypes.Add(new ClientPackageType(name: "SymbolsPackage", version: ClientPackageType.EmptyVersion));
236-
return CreateTestPackageStream(id,
237-
version,
238-
packageTypes: packageTypes,
239-
populatePackage: populatePackage,
236+
return CreateTestPackageStream(id,
237+
version,
238+
packageTypes: packageTypes,
239+
populatePackage: populatePackage,
240240
isSymbolPackage: true);
241241
}
242242

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@
9292
<Compile Include="Helpers\StreamHelperFacts.cs" />
9393
<Compile Include="Queries\AutocompleteCweIdsQueryFacts.cs" />
9494
<Compile Include="Services\PackageDeprecationServiceFacts.cs" />
95+
<Compile Include="TestData\TestDataResourceUtility.cs" />
9596
<Compile Include="UsernameValidationRegex.cs" />
9697
<Compile Include="Extensions\NumberExtensionsFacts.cs" />
9798
<Compile Include="Extensions\RouteExtensionsFacts.cs" />
@@ -297,6 +298,7 @@
297298
<SubType>Designer</SubType>
298299
</None>
299300
<EmbeddedResource Include="TestData\certificate.cer" />
301+
<EmbeddedResource Include="TestData\theId.nuspec" />
300302
</ItemGroup>
301303
<ItemGroup>
302304
<ProjectReference Include="..\..\src\NuGet.Services.Search.Client\NuGet.Services.Search.Client.csproj">

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

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -183,19 +183,9 @@ public void Validate_WhenStreamIsDerEncodedCertificate_Succeeds(string fileName)
183183
}
184184
}
185185

186-
private byte[] GetResourceBytes(string name)
187-
{
188-
var resourceName = $"NuGetGallery.TestData.{name}";
189-
190-
using (var reader = new BinaryReader(GetType().Assembly.GetManifestResourceStream(resourceName)))
191-
{
192-
return reader.ReadBytes((int)reader.BaseStream.Length);
193-
}
194-
}
195-
196186
private X509Certificate2 GetCertificate()
197187
{
198-
return new X509Certificate2(GetResourceBytes("certificate.cer"));
188+
return new X509Certificate2(TestDataResourceUtility.GetResourceBytes("certificate.cer"));
199189
}
200190

201191
private MemoryStream GetDerEncodedCertificateStream()

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

Lines changed: 55 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -118,7 +118,8 @@ public async Task WillMarkPackageRegistrationVerifiedFlagCorrectly(bool shouldMa
118118
var matchingNamepsaces = testNamespaces
119119
.Where(rn => prefixes.Any(pr => id.StartsWith(pr, StringComparison.OrdinalIgnoreCase)))
120120
.ToList();
121-
prefixes.ForEach(p => {
121+
prefixes.ForEach(p =>
122+
{
122123
var existingNamespace = testNamespaces.FirstOrDefault(rn => rn.Value.Equals(p, StringComparison.OrdinalIgnoreCase));
123124
existingNamespace.Owners.Add(firstUser);
124125
});
@@ -149,7 +150,8 @@ public async Task WillMarkPackageRegistrationNotVerifiedIfIdMatchesNonOwnedShare
149150
var testUsers = ReservedNamespaceServiceTestData.GetTestUsers();
150151
var firstUser = testUsers.First();
151152
var lastUser = testUsers.Last();
152-
prefixes.ForEach(p => {
153+
prefixes.ForEach(p =>
154+
{
153155
var existingNamespace = testNamespaces.FirstOrDefault(rn => rn.Value.Equals(p, StringComparison.OrdinalIgnoreCase));
154156
existingNamespace.IsSharedNamespace = true;
155157
existingNamespace.Owners.Add(firstUser);
@@ -257,7 +259,7 @@ public async Task AcceptsUnsignedPackageAfterUnsignedPackage()
257259
});
258260

259261
var result = await _target.ValidateBeforeGeneratePackageAsync(
260-
_nuGetPackage.Object,
262+
_nuGetPackage.Object,
261263
GetPackageMetadata(_nuGetPackage));
262264

263265
Assert.Equal(PackageValidationResultType.Accepted, result.Type);
@@ -320,7 +322,7 @@ public async Task AcceptsUnsignedPackagesWithNoPackageRegistration()
320322
Times.Once);
321323
}
322324

323-
public static IEnumerable<object[]> WarnsOnMalformedRepositoryMetadata_Data = new []
325+
public static IEnumerable<object[]> WarnsOnMalformedRepositoryMetadata_Data = new[]
324326
{
325327
new object[] { null, null, null },
326328
new object[] { "git", null, null },
@@ -983,7 +985,7 @@ public async Task RejectsLongLicenseNodeValues(string licenseNodeValue)
983985
}
984986

985987
[Fact]
986-
public async Task RejectsNupkgsReportingIncorrectFileLength()
988+
public async Task RejectsNupkgsReportingIncorrectFileLengthForLicenseFile()
987989
{
988990
const string licenseFilename = "license.txt";
989991
const string licenseFileContents = "abcdefghijklnopqrstuvwxyz";
@@ -994,44 +996,73 @@ public async Task RejectsNupkgsReportingIncorrectFileLength()
994996
licenseFilename: licenseFilename,
995997
licenseFileContents: licenseFileContents);
996998

999+
PatchFileSizeInPackageStream(licenseFilename, licenseFileContents, packageStream);
1000+
1001+
_nuGetPackage = PackageServiceUtility.CreateNuGetPackage(packageStream);
1002+
1003+
// Act
1004+
var result = await _target.ValidateBeforeGeneratePackageAsync(
1005+
_nuGetPackage.Object,
1006+
GetPackageMetadata(_nuGetPackage));
1007+
1008+
// Assert
1009+
Assert.Equal(PackageValidationResultType.Invalid, result.Type);
1010+
Assert.Contains("corrupt", result.Message.PlainTextMessage);
1011+
Assert.Empty(result.Warnings);
1012+
}
1013+
1014+
[Fact]
1015+
public async Task RejectsNupkgsReportingIncorrectFileLengthForNuspecFile()
1016+
{
1017+
const string nuspecFilename = "theId.nuspec";
1018+
var nuspecFileContents = TestDataResourceUtility.GetResourceString(nuspecFilename);
1019+
1020+
// Arrange
1021+
var packageStream = GeneratePackageStream();
1022+
1023+
PatchFileSizeInPackageStream(nuspecFilename, nuspecFileContents, packageStream);
1024+
1025+
_nuGetPackage = PackageServiceUtility.CreateNuGetPackage(packageStream);
1026+
1027+
// Act
1028+
var result = await _target.ValidateBeforeGeneratePackageAsync(
1029+
_nuGetPackage.Object,
1030+
GetPackageMetadata(_nuGetPackage));
1031+
1032+
// Assert
1033+
Assert.Equal(PackageValidationResultType.Invalid, result.Type);
1034+
Assert.Contains("corrupt", result.Message.PlainTextMessage);
1035+
Assert.Empty(result.Warnings);
1036+
}
1037+
1038+
private static void PatchFileSizeInPackageStream(string fileName, string fileContents, MemoryStream packageStream)
1039+
{
9971040
var buffer = packageStream.GetBuffer();
9981041

999-
var licenseFilenameBytes = Encoding.ASCII.GetBytes(licenseFilename);
1042+
var fileNameInBytes = Encoding.ASCII.GetBytes(fileName);
10001043

10011044
// the file name should appear twice in the zip stream:
10021045
// 1. where the compressed stream is saved.
10031046
// 2. in the central directory
10041047
// we'll need to patch stream length in both places
10051048

1006-
var firstInstanceOffset = FindSequenceIndex(licenseFilenameBytes, buffer);
1049+
var firstInstanceOffset = FindSequenceIndex(fileNameInBytes, buffer);
10071050
Assert.True(firstInstanceOffset > 0);
10081051
var firstSizeOffset = firstInstanceOffset - 8;
10091052
Assert.True(firstSizeOffset > 0);
10101053
var firstLength = BitConverter.ToInt32(buffer, firstSizeOffset);
1011-
Assert.Equal(licenseFileContents.Length, firstLength);
1054+
Assert.Equal(fileContents.Length, firstLength);
10121055

1013-
var secondInstanceOffset = FindSequenceIndex(licenseFilenameBytes, buffer, firstInstanceOffset + licenseFilename.Length);
1056+
var secondInstanceOffset = FindSequenceIndex(fileNameInBytes, buffer, firstInstanceOffset + fileName.Length);
10141057
Assert.True(secondInstanceOffset > 0);
10151058
var secondSizeOffset = secondInstanceOffset - 22;
10161059
Assert.True(secondSizeOffset > 0);
10171060
var secondLength = BitConverter.ToInt32(buffer, secondSizeOffset);
1018-
Assert.Equal(licenseFileContents.Length, secondLength);
1061+
Assert.Equal(fileContents.Length, secondLength);
10191062

10201063
// now that we have offsets, we'll just patch them
10211064
buffer[firstSizeOffset] = 1;
10221065
buffer[secondSizeOffset] = 1;
1023-
1024-
_nuGetPackage = PackageServiceUtility.CreateNuGetPackage(packageStream);
1025-
1026-
// Act
1027-
var result = await _target.ValidateBeforeGeneratePackageAsync(
1028-
_nuGetPackage.Object,
1029-
GetPackageMetadata(_nuGetPackage));
1030-
1031-
// Assert
1032-
Assert.Equal(PackageValidationResultType.Invalid, result.Type);
1033-
Assert.Contains("corrupt", result.Message.PlainTextMessage);
1034-
Assert.Empty(result.Warnings);
10351066
}
10361067

10371068
[Theory]
@@ -1418,7 +1449,7 @@ public async Task AcceptNotTyposquattingNotNewVersion()
14181449
public async Task RejectIsTyposquattingNewVersion()
14191450
{
14201451
_isNewPackageRegistration = true;
1421-
_typosquattingCheckCollisionIds = new List<string>{ "typosquatting_package_Id" };
1452+
_typosquattingCheckCollisionIds = new List<string> { "typosquatting_package_Id" };
14221453
_typosquattingService
14231454
.Setup(x => x.IsUploadedPackageIdTyposquatting(It.IsAny<string>(), It.IsAny<User>(), out _typosquattingCheckCollisionIds))
14241455
.Returns(true);

‎tests/NuGetGallery.Facts/TestData/TestDataResourceUtility.cs‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
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.Globalization;
5+
using System.IO;
6+
using System.Reflection;
7+
8+
namespace NuGetGallery
9+
{
10+
internal static class TestDataResourceUtility
11+
{
12+
private static readonly string ResourceNameFormat = "NuGetGallery.TestData.{0}";
13+
private static readonly Assembly CurrentAssembly = typeof(TestDataResourceUtility).Assembly;
14+
15+
internal static byte[] GetResourceBytes(string name)
16+
{
17+
using (var reader = new BinaryReader(GetManifestResourceStream(name)))
18+
{
19+
return reader.ReadBytes((int)reader.BaseStream.Length);
20+
}
21+
}
22+
23+
internal static string GetResourceString(string name)
24+
{
25+
using (var resourceStream = GetManifestResourceStream(name))
26+
using (var streamReader = new StreamReader(resourceStream))
27+
{
28+
return streamReader.ReadToEnd();
29+
}
30+
}
31+
32+
private static Stream GetManifestResourceStream(string name)
33+
{
34+
var resourceName = GetResourceName(name);
35+
36+
return CurrentAssembly.GetManifestResourceStream(resourceName);
37+
}
38+
39+
private static string GetResourceName(string name)
40+
{
41+
return string.Format(
42+
CultureInfo.InvariantCulture,
43+
ResourceNameFormat,
44+
name);
45+
}
46+
}
47+
}

‎tests/NuGetGallery.Facts/TestData/theId.nuspec‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?xml version="1.0"?>
2+
<package xmlns="http://schemas.microsoft.com/packaging/2011/08/nuspec.xsd">
3+
<metadata>
4+
<id>theId</id>
5+
<version>1.2.3-alpha.0</version>
6+
<title>theTitle</title>
7+
<summary>theSummary</summary>
8+
<description>theDescription</description>
9+
<tags>theTags</tags>
10+
<requireLicenseAcceptance>True</requireLicenseAcceptance>
11+
<authors>theFirstAuthor, theSecondAuthor</authors>
12+
<owners>Package owners</owners>
13+
<language></language>
14+
<copyright>theCopyright</copyright>
15+
<releaseNotes>theReleaseNotes</releaseNotes>
16+
<licenseUrl></licenseUrl>
17+
18+
<projectUrl></projectUrl>
19+
<iconUrl></iconUrl>
20+
<packageTypes><packageType name="dependency" version="1.0.0"/><packageType name="DotNetCliTool" version="2.1.1"/></packageTypes>
21+
<dependencies><group targetFramework="net40"><dependency id="theFirstDependency" version="[1.0.0, 2.0.0)"></dependency><dependency id="theSecondDependency" version="[1.0.0, 1.0.0]"></dependency><dependency id="theThirdDependency" version="(, )"></dependency></group><group targetFramework="net35"><dependency id="theFourthDependency" version="[1.0.0, 1.0.0]"></dependency></group></dependencies>
22+
23+
24+
</metadata>
25+
</package>

0 commit comments

Comments
 (0)