[NuGet Symbol Server] Gallery upload - Refactoring (#6478)

Author: shishirx34

src/NuGetGallery.Core/NuGetGallery.Core.csproj

@@ -168,6 +168,7 @@
     <Compile Include="Infrastructure\ElmahException.cs" />
     <Compile Include="Infrastructure\TableErrorLog.cs" />
     <Compile Include="Authentication\NuGetClaims.cs" />
+    <Compile Include="PackageMetadataExtensions.cs" />
     <Compile Include="PackageReaderCoreExtensions.cs" />
     <Compile Include="PackageStatus.cs" />
     <Compile Include="Packaging\InvalidPackageException.cs" />

src/NuGetGallery.Core/PackageMetadataExtensions.cs

@@ -0,0 +1,32 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using NuGetGallery.Packaging;
+using System.Linq;
+using ClientPackageType = NuGet.Packaging.Core.PackageType;
+
+namespace NuGetGallery
+{
+    /// <summary>
+    /// Utilities extending PackageMetadata
+    /// </summary>
+    public static class PackageMetadataExtensions
+    {
+        private const string SymbolPackageTypeName = "SymbolsPackage";
+        private static readonly ClientPackageType SymbolPackageType = new ClientPackageType(SymbolPackageTypeName, ClientPackageType.EmptyVersion);
+
+        /// <summary>
+        /// The package is a symbol package, if and only if it has metadata 
+        /// element of type <see cref="SymbolPackageTypeName"/> and only that element in package types.
+        /// </summary>
+        /// <param name="metadata"><see cref="PackageMetadata"/> for package</param>
+        /// <returns>True if the package is a symbols package</returns>
+        public static bool IsSymbolPackage(this PackageMetadata metadata)
+        {
+            var packageTypes = metadata.GetPackageTypes();
+            return packageTypes.Any()
+                && packageTypes.Count() == 1
+                && packageTypes.First() == SymbolPackageType;
+        }
+    }
+}

src/NuGetGallery.Core/StreamExtensions.cs

@@ -2,7 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System.IO;
-using System.Threading.Tasks;
 
 namespace NuGetGallery
 {

src/NuGetGallery/Controllers/ApiController.cs

@@ -0,0 +1,42 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.IO;
+using System.IO.Compression;
+using System.Linq;
+
+namespace NuGetGallery
+{
+    public static class ZipArchiveHelpers
+    {
+        /// <summary>
+        /// This method checks all the <see cref="ZipArchiveEntry"/> in a given 
+        /// <see cref="Stream"/> if it has the entry in the future. It will return
+        /// the first entry found in the future.
+        /// </summary>
+        /// <param name="stream"><see cref="Stream"/> object to verify</param>
+        /// <param name="entry"><see cref="ZipArchiveEntry"/> found with future entry.</param>
+        /// <returns>True if <see cref="Stream"/> contains an entry in future, false otherwise.</returns>
+        public static bool FoundEntryInFuture(Stream stream, out ZipArchiveEntry entry)
+        {
+            entry = null;
+
+            using (var archive = new ZipArchive(stream, ZipArchiveMode.Read, leaveOpen: true))
+            {
+                var reference = DateTime.UtcNow.AddDays(1); // allow "some" clock skew
+
+                var entryInTheFuture = archive.Entries.FirstOrDefault(
+                    e => e.LastWriteTime.UtcDateTime > reference);
+
+                if (entryInTheFuture != null)
+                {
+                    entry = entryInTheFuture;
+                    return true;
+                }
+            }
+
+            return false;
+        }
+    }
+}

src/NuGetGallery/Controllers/PackagesController.cs

@@ -292,6 +292,7 @@
     <Compile Include="GlobalSuppressions.cs" />
     <Compile Include="Helpers\GravatarHelper.cs" />
     <Compile Include="Helpers\ObfuscationHelper.cs" />
+    <Compile Include="Helpers\ZipArchiveHelpers.cs" />
     <Compile Include="Helpers\PermissionsHelpers.cs" />
     <Compile Include="Helpers\RegexEx.cs" />
     <Compile Include="Helpers\RouteUrlTemplate.cs" />
@@ -400,6 +401,7 @@
     <Compile Include="Services\ITyposquattingConfiguration.cs" />
     <Compile Include="Services\ISymbolsConfiguration.cs" />
     <Compile Include="Services\ITyposquattingService.cs" />
+    <Compile Include="Services\SymbolPackageValidationResult.cs" />
     <Compile Include="Services\SymbolPackageUploadService.cs" />
     <Compile Include="Services\SymbolPackageFileService.cs" />
     <Compile Include="Services\SymbolPackageService.cs" />

src/NuGetGallery/Helpers/ZipArchiveHelpers.cs

@@ -20,9 +20,19 @@ public interface ISymbolPackageUploadService
         /// cases (such as database failures). This method does not dispose the provided stream but does read it.
         /// </summary>
         /// <param name="package">The package for which the symbols are to be uploaded.</param>
-        /// <param name="packageStreamMetadata">The metadata object for the uploaded snupkg.</param>
-        /// <param name="symbolPackageFile">The seekable stream containing the symbol package content (.snupkg).</param>
+        /// <param name="symbolPackageStream">The Stream object for the uploaded snupkg.</param>
         /// <returns>Awaitable task with <see cref="PackageCommitResult"/></returns>
-        Task<PackageCommitResult> CreateAndUploadSymbolsPackage(Package package, PackageStreamMetadata packageStreamMetadata, Stream symbolPackageFile);
+        Task<PackageCommitResult> CreateAndUploadSymbolsPackage(Package package, Stream symbolPackageStream);
+
+        /// <summary>
+        /// Validate the uploaded symbols package. This method will perform all required validations for symbols, including
+        /// synchronous package validations for fail fast scenario. This method will not perform the ownership validations
+        /// for the symbols package. It is the responsibility of the caller to perform ownership validations. This method
+        /// does not dispose the provided stream but does read it.
+        /// </summary>
+        /// <param name="uploadStream">The <see cref="Stream"/> object for the uploaded snupkg.</param>
+        /// <param name="currentUser">The user performing the uploads.</param>
+        /// <returns>Awaitable task with <see cref="SymbolPackageValidationResult"/></returns>
+        Task<SymbolPackageValidationResult> ValidateUploadedSymbolsPackage(Stream uploadStream, User currentUser);
     }
 }

src/NuGetGallery/NuGetGallery.csproj

@@ -11,7 +11,6 @@
 using NuGet.Packaging;
 using NuGet.Packaging.Core;
 using NuGetGallery.Packaging;
-using ClientPackageType = NuGet.Packaging.Core.PackageType;
 
 namespace NuGetGallery
 {
@@ -26,8 +25,6 @@ public class SymbolPackageService : CoreSymbolPackageService, ISymbolPackageServ
             ".rels",
             ".p7s"
         };
-        private const string SymbolPackageTypeName = "SymbolsPackage";
-        private static readonly ClientPackageType SymbolPackageType = new ClientPackageType(SymbolPackageTypeName, ClientPackageType.EmptyVersion);
 
         public SymbolPackageService(
             IEntityRepository<SymbolPackage> symbolPackageRepository,
@@ -62,7 +59,7 @@ public async Task EnsureValidAsync(PackageArchiveReader symbolPackageArchiveRead
                     symbolPackageArchiveReader.GetNuspecReader(),
                     strict: true);
 
-                if (!IsSymbolPackage(packageMetadata))
+                if (!packageMetadata.IsSymbolPackage())
                 {
                     throw new InvalidPackageException(Strings.SymbolsPackage_NotSymbolPackage);
                 }
@@ -173,14 +170,6 @@ private static bool CheckForAllowedFiles(PackageArchiveReader symbolPackage)
             return true;
         }
 
-        private static bool IsSymbolPackage(PackageMetadata metadata)
-        {
-            var packageTypes = metadata.GetPackageTypes();
-            return packageTypes.Any()
-                && packageTypes.Count() == 1
-                && packageTypes.First() == SymbolPackageType;
-        }
-
         private static bool IsPortable(string pdbFile)
         {
             byte[] currentPDBStamp = new byte[4];

src/NuGetGallery/Services/ISymbolPackageUploadService.cs

@@ -2,9 +2,13 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Globalization;
 using System.IO;
+using System.IO.Compression;
 using System.Linq;
 using System.Threading.Tasks;
+using NuGet.Frameworks;
+using NuGet.Packaging;
 using NuGetGallery.Packaging;
 
 namespace NuGetGallery
@@ -15,17 +19,109 @@ public class SymbolPackageUploadService : ISymbolPackageUploadService
         private readonly IValidationService _validationService;
         private readonly ISymbolPackageService _symbolPackageService;
         private readonly ISymbolPackageFileService _symbolPackageFileService;
+        private readonly IPackageService _packageService;
+        private readonly ITelemetryService _telemetryService;
+        private readonly IContentObjectService _contentObjectService;
 
         public SymbolPackageUploadService(
             ISymbolPackageService symbolPackageService,
             ISymbolPackageFileService symbolPackageFileService,
             IEntitiesContext entitiesContext,
-            IValidationService validationService)
+            IValidationService validationService,
+            IPackageService packageService,
+            ITelemetryService telemetryService,
+            IContentObjectService contentObjectService)
         {
             _symbolPackageService = symbolPackageService ?? throw new ArgumentNullException(nameof(symbolPackageService));
             _symbolPackageFileService = symbolPackageFileService ?? throw new ArgumentNullException(nameof(symbolPackageFileService));
             _entitiesContext = entitiesContext ?? throw new ArgumentNullException(nameof(entitiesContext));
             _validationService = validationService ?? throw new ArgumentNullException(nameof(validationService));
+            _packageService = packageService ?? throw new ArgumentNullException(nameof(packageService));
+            _telemetryService = telemetryService ?? throw new ArgumentNullException(nameof(telemetryService));
+            _contentObjectService = contentObjectService ?? throw new ArgumentNullException(nameof(contentObjectService));
+        }
+
+        /// <summary>
+        /// This method does not perform the ownership validations for the symbols package. It is the responsibility
+        /// of the caller to do it. Also, this method does not dispose the <see cref="Stream"/> object, the caller 
+        /// should take care of it.
+        /// </summary>
+        /// <param name="symbolPackageStream"><see cref="Stream"/> object for the symbols package.</param>
+        /// <param name="currentUser">The user performing the uploads.</param>
+        /// <returns>Awaitable task for <see cref="SymbolPackageValidationResult"/></returns>
+        public async Task<SymbolPackageValidationResult> ValidateUploadedSymbolsPackage(Stream symbolPackageStream, User currentUser)
+        {
+            Package package = null;
+
+            // Check if symbol package upload is allowed for this user.
+            if (!_contentObjectService.SymbolsConfiguration.IsSymbolsUploadEnabledForUser(currentUser))
+            {
+                return SymbolPackageValidationResult.UserNotAllowedToUpload(Strings.SymbolsPackage_UploadNotAllowed);
+            }
+
+            try
+            {
+                if (ZipArchiveHelpers.FoundEntryInFuture(symbolPackageStream, out ZipArchiveEntry entryInTheFuture))
+                {
+                    return SymbolPackageValidationResult.Invalid(string.Format(
+                        CultureInfo.CurrentCulture,
+                        Strings.PackageEntryFromTheFuture,
+                        entryInTheFuture.Name));
+                }
+
+                using (var packageToPush = new PackageArchiveReader(symbolPackageStream, leaveStreamOpen: true))
+                {
+                    var nuspec = packageToPush.GetNuspecReader();
+                    var id = nuspec.GetId();
+                    var version = nuspec.GetVersion();
+                    var normalizedVersion = version.ToNormalizedStringSafe();
+
+                    // Ensure the corresponding package exists before pushing a snupkg.
+                    package = _packageService.FindPackageByIdAndVersionStrict(id, version.ToStringSafe());
+                    if (package == null || package.PackageStatusKey == PackageStatus.Deleted)
+                    {
+                        return SymbolPackageValidationResult.MissingPackage(string.Format(
+                            CultureInfo.CurrentCulture,
+                            Strings.SymbolsPackage_PackageIdAndVersionNotFound,
+                            id,
+                            normalizedVersion));
+                    }
+
+                    // Do not allow to upload a snupkg to a package which has symbols package pending validations.
+                    if (package.SymbolPackages.Any(sp => sp.StatusKey == PackageStatus.Validating))
+                    {
+                        return SymbolPackageValidationResult.SymbolsPackageExists(Strings.SymbolsPackage_ConflictValidating);
+                    }
+
+                    try
+                    {
+                        await _symbolPackageService.EnsureValidAsync(packageToPush);
+                    }
+                    catch (Exception ex)
+                    {
+                        ex.Log();
+
+                        var message = Strings.SymbolsPackage_FailedToReadPackage;
+                        if (ex is InvalidPackageException || ex is InvalidDataException || ex is EntityException)
+                        {
+                            message = ex.Message;
+                        }
+
+                        _telemetryService.TrackSymbolPackageFailedGalleryValidationEvent(id, normalizedVersion);
+                        return SymbolPackageValidationResult.Invalid(message);
+                    }
+                }
+
+                return SymbolPackageValidationResult.AcceptedForPackage(package);
+            }
+            catch (Exception ex) when (ex is InvalidPackageException
+                || ex is InvalidDataException
+                || ex is EntityException
+                || ex is FrameworkException)
+            {
+                return SymbolPackageValidationResult.Invalid(
+                    string.Format(CultureInfo.CurrentCulture, Strings.UploadPackage_InvalidPackage, ex.Message));
+            }
         }
 
         /// <summary>
@@ -34,11 +130,21 @@ public SymbolPackageUploadService(
         /// based on the result. It will then update the references in the database for persistence with appropriate status.
         /// </summary>
         /// <param name="package">The package for which symbols package is to be uplloaded</param>
-        /// <param name="packageStreamMetadata">The package stream metadata for the uploaded symbols package file.</param>
-        /// <param name="symbolPackageFile">The symbol package file stream.</param>
-        /// <returns>The <see cref="PackageCommitResult"/> for the symbol package upload flow.</returns>
-        public async Task<PackageCommitResult> CreateAndUploadSymbolsPackage(Package package, PackageStreamMetadata packageStreamMetadata, Stream symbolPackageFile)
+        /// <param name="symbolPackageStream">The symbols package stream metadata for the uploaded symbols package file.</param>
+        /// <returns>The <see cref="PackageCommitResult"/> for the create and upload symbol package flow.</returns>
+        public async Task<PackageCommitResult> CreateAndUploadSymbolsPackage(Package package, Stream symbolPackageStream)
         {
+            var packageStreamMetadata = new PackageStreamMetadata
+            {
+                HashAlgorithm = CoreConstants.Sha512HashAlgorithmId,
+                Hash = CryptographyService.GenerateHash(
+                    symbolPackageStream.AsSeekableStream(),
+                    CoreConstants.Sha512HashAlgorithmId),
+                Size = symbolPackageStream.Length
+            };
+
+            Stream symbolPackageFile = symbolPackageStream.AsSeekableStream();
+
             var symbolPackage = _symbolPackageService.CreateSymbolPackage(package, packageStreamMetadata);
 
             await _validationService.StartValidationAsync(symbolPackage);
@@ -66,7 +172,7 @@ public async Task<PackageCommitResult> CreateAndUploadSymbolsPackage(Package pac
                     // Mark any other associated available symbol packages for deletion.
                     var availableSymbolPackages = package
                         .SymbolPackages
-                        .Where(sp => sp.StatusKey == PackageStatus.Available 
+                        .Where(sp => sp.StatusKey == PackageStatus.Available
                             && sp != symbolPackage);
 
                     var overwrite = false;
@@ -120,6 +226,8 @@ await _symbolPackageFileService.DeletePackageFileAsync(
                 return PackageCommitResult.Conflict;
             }
 
+            _telemetryService.TrackSymbolPackagePushEvent(package.Id, package.NormalizedVersion);
+
             return PackageCommitResult.Success;
         }
     }