NuGet pack "The DateTimeOffset specified cannot be converted into a Z…

…ip file timestamp" (#3793) * Fix NuGet pack "The DateTimeOffset specified cannot be converted into a Zip file timestamp" * Add unit test for Zip file date before 1980 case * Address PR review comment by Damon and Nikolche * Log that zip file modified dates are set to 1/4/1980 if it's before year 1980. * Fix failing unit test * Make modified datetime passed to ZipArchive UTC so any file modified before 1/1/1980 00:00:00 UTC will be corrected, also any file created after 12/31/2107 23:59:58 UTC will be corrected. * Fix typos. * Added Zip file 2 second precision limitation comment. * Address code review comments by Andy for deterministic packing. * Change to local Datetime for timestamping. Change back to UTC time after discussing with Andy. * Clean up * Revert accidental changes. * Cleanup comments. * Address Nikolche's code review. * Address PR comment by Andy. * Address comment about string.Fomatting. * Fix typo. * Fix typo. * Address more comments for string resouces. * Address additional review comments. * Address string.Format with string.Concat(+) for better performance. * Address comment by Nikolche. * Check if warningMessage has something before trimming new line.
NuGet · Feb 3, 2021 · 0461335 · 0461335
1 parent ebd82e1
commit 0461335
Show file tree

Hide file tree

Showing 18 changed files with 407 additions and 46 deletions.
diff --git a/src/NuGet.Clients/NuGet.CommandLine/Commands/ProjectFactory.cs b/src/NuGet.Clients/NuGet.CommandLine/Commands/ProjectFactory.cs
@@ -228,7 +228,7 @@ public Packaging.PackageBuilder CreateBuilder(string basePath, NuGetVersion vers
                         Path.GetFullPath(Path.GetDirectoryName(TargetPath))), LogLevel.Minimal));
             }
 
-            builder = new Packaging.PackageBuilder();
+            builder = new PackageBuilder(false, Logger);
 
             try
             {

diff --git a/src/NuGet.Core/NuGet.Build.Tasks.Pack/PackTaskLogic.cs b/src/NuGet.Core/NuGet.Build.Tasks.Pack/PackTaskLogic.cs
@@ -114,7 +114,7 @@ public PackageBuilder GetPackageBuilder(IPackTaskRequest<IMSBuildItem> request)
                     assetsFilePath));
             }
 
-            var builder = new PackageBuilder(request.Deterministic)
+            var builder = new PackageBuilder(request.Deterministic, request.Logger)
             {
                 Id = request.PackageId,
                 Description = request.Description,

diff --git a/src/NuGet.Core/NuGet.Commands/CommandRunners/PackCommandRunner.cs b/src/NuGet.Core/NuGet.Commands/CommandRunners/PackCommandRunner.cs
@@ -719,15 +719,17 @@ private PackageBuilder CreatePackageBuilderFromNuspec(string path)
                     path,
                     _packArgs.GetPropertyValue,
                     !_packArgs.ExcludeEmptyDirectories,
-                    _packArgs.Deterministic);
+                    _packArgs.Deterministic,
+                    _packArgs.Logger);
             }
 
             return new PackageBuilder(
                 path,
                 _packArgs.BasePath,
                 _packArgs.GetPropertyValue,
                 !_packArgs.ExcludeEmptyDirectories,
-                _packArgs.Deterministic);
+                _packArgs.Deterministic,
+                _packArgs.Logger);
         }
 
         private bool BuildFromProjectFile(string path)

diff --git a/src/NuGet.Core/NuGet.Common/Errors/NuGetLogCode.cs b/src/NuGet.Core/NuGet.Common/Errors/NuGetLogCode.cs
@@ -872,6 +872,11 @@ public enum NuGetLogCode
         /// </summary>
         NU5131 = 5131,
 
+        /// <summary>
+        /// File last write timestamp is out of range of what the zip format supports warning
+        /// </summary>
+        NU5132 = 5132,
+
         /// <summary>
         /// Undefined package warning
         /// </summary>

diff --git a/src/NuGet.Core/NuGet.Common/PublicAPI/net45/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Common/PublicAPI/net45/PublicAPI.Unshipped.txt
@@ -1,3 +1,4 @@
+NuGet.Common.NuGetLogCode.NU5132 = 5132 -> NuGet.Common.NuGetLogCode
 NuGet.Common.NuGetLogCode.NU5501 = 5501 -> NuGet.Common.NuGetLogCode
 NuGet.Common.NuGetLogCode.NU1012 = 1012 -> NuGet.Common.NuGetLogCode
 NuGet.Common.NuGetLogCode.NU5045 = 5045 -> NuGet.Common.NuGetLogCode

diff --git a/src/NuGet.Core/NuGet.Common/PublicAPI/net472/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Common/PublicAPI/net472/PublicAPI.Unshipped.txt
@@ -1,3 +1,4 @@
+NuGet.Common.NuGetLogCode.NU5132 = 5132 -> NuGet.Common.NuGetLogCode
 NuGet.Common.NuGetLogCode.NU5501 = 5501 -> NuGet.Common.NuGetLogCode
 NuGet.Common.NuGetLogCode.NU1012 = 1012 -> NuGet.Common.NuGetLogCode
 NuGet.Common.NuGetLogCode.NU5045 = 5045 -> NuGet.Common.NuGetLogCode

diff --git a/src/NuGet.Core/NuGet.Common/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Common/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt
@@ -1,3 +1,4 @@
+NuGet.Common.NuGetLogCode.NU5132 = 5132 -> NuGet.Common.NuGetLogCode
 NuGet.Common.NuGetLogCode.NU5501 = 5501 -> NuGet.Common.NuGetLogCode
 NuGet.Common.NuGetLogCode.NU1012 = 1012 -> NuGet.Common.NuGetLogCode
 NuGet.Common.NuGetLogCode.NU5045 = 5045 -> NuGet.Common.NuGetLogCode

diff --git a/src/NuGet.Core/NuGet.Packaging.Extraction/Strings.Designer.cs b/src/NuGet.Core/NuGet.Packaging.Extraction/Strings.Designer.cs
diff --git a/src/NuGet.Core/NuGet.Packaging.Extraction/Strings.resx b/src/NuGet.Core/NuGet.Packaging.Extraction/Strings.resx
@@ -865,4 +865,13 @@ Valid from:</comment>
     <value>'{0}' cannot be null.</value>
     <comment>0 - property name</comment>
   </data>
+  <data name="ZipFileLastWriteTimeStampModifiedMessage" xml:space="preserve">
+    <value>'{0}' changed from '{1}' to '{2}'</value>
+    <comment>0 - File name
+1 - Original last write timestamp
+2 - Updated last write timestamp</comment>
+  </data>
+  <data name="ZipFileTimeStampModifiedWarning" xml:space="preserve">
+    <value>The zip format supports a limited date range. The following files are outside the supported range:</value>
+  </data>
 </root>
diff --git a/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs b/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs
@@ -10,15 +10,14 @@
 using System.IO.Compression;
 using System.Linq;
 using System.Reflection;
-using System.Reflection.Emit;
+using System.Text;
 using System.Xml.Linq;
 using NuGet.Client;
 using NuGet.Common;
 using NuGet.ContentModel;
 using NuGet.Frameworks;
 using NuGet.Packaging.Core;
 using NuGet.Packaging.PackageCreation.Resources;
-using NuGet.Packaging.Rules;
 using NuGet.RuntimeModel;
 using NuGet.Versioning;
 
@@ -27,10 +26,12 @@ namespace NuGet.Packaging
     public class PackageBuilder : IPackageMetadata
     {
         private static readonly Uri DefaultUri = new Uri("http://defaultcontainer/");
+        private static readonly DateTime ZipFormatMinDate = new DateTime(1980, 1, 1, 0, 0, 0, DateTimeKind.Utc);
+        private static readonly DateTime ZipFormatMaxDate = new DateTime(2107, 12, 31, 23, 59, 58, DateTimeKind.Utc);
         internal const string ManifestRelationType = "manifest";
         private readonly bool _includeEmptyDirectories;
         private readonly bool _deterministic;
-        private static readonly DateTime ZipFormatMinDate = new DateTime(1980, 1, 1, 0, 0, 0, DateTimeKind.Utc);
+        private readonly ILogger _logger;
 
         /// <summary>
         /// Maximum Icon file size: 1 megabyte
@@ -47,11 +48,22 @@ public PackageBuilder(string path, Func<string, string> propertyProvider, bool i
         {
         }
 
+        public PackageBuilder(string path, Func<string, string> propertyProvider, bool includeEmptyDirectories, bool deterministic, ILogger logger)
+            : this(path, Path.GetDirectoryName(path), propertyProvider, includeEmptyDirectories, deterministic, logger)
+        {
+        }
+
         public PackageBuilder(string path, string basePath, Func<string, string> propertyProvider, bool includeEmptyDirectories)
             : this(path, basePath, propertyProvider, includeEmptyDirectories, deterministic: false)
         {
         }
 
+        public PackageBuilder(string path, string basePath, Func<string, string> propertyProvider, bool includeEmptyDirectories, bool deterministic, ILogger logger)
+            : this(path, basePath, propertyProvider, includeEmptyDirectories, deterministic)
+        {
+            _logger = logger;
+        }
+
         public PackageBuilder(string path, string basePath, Func<string, string> propertyProvider, bool includeEmptyDirectories, bool deterministic)
             : this(includeEmptyDirectories, deterministic)
         {
@@ -90,10 +102,21 @@ public PackageBuilder()
         {
         }
 
+        public PackageBuilder(bool deterministic, ILogger logger)
+            : this(includeEmptyDirectories: false, deterministic: deterministic, logger)
+        {
+        }
+
         private PackageBuilder(bool includeEmptyDirectories, bool deterministic)
+            : this(includeEmptyDirectories: false, deterministic: deterministic, logger: NullLogger.Instance)
+        {
+        }
+
+        private PackageBuilder(bool includeEmptyDirectories, bool deterministic, ILogger logger)
         {
             _includeEmptyDirectories = includeEmptyDirectories;
             _deterministic = false; // fix in https://github.com/NuGet/Home/issues/8601
+            _logger = logger;
             Files = new Collection<IPackageFile>();
             DependencyGroups = new Collection<PackageDependencyGroup>();
             FrameworkReferences = new Collection<FrameworkAssemblyReference>();
@@ -896,10 +919,25 @@ private ZipArchiveEntry CreateEntry(ZipArchive package, string entryName, Compre
             return entry;
         }
 
-        private ZipArchiveEntry CreatePackageFileEntry(ZipArchive package, string entryName, DateTimeOffset timeOffset, CompressionLevel compressionLevel)
+        private ZipArchiveEntry CreatePackageFileEntry(ZipArchive package, string entryName, DateTimeOffset timeOffset, CompressionLevel compressionLevel, StringBuilder warningMessage)
         {
             var entry = package.CreateEntry(entryName, compressionLevel);
-            entry.LastWriteTime = timeOffset;
+
+            if (timeOffset.UtcDateTime < ZipFormatMinDate)
+            {
+                warningMessage.AppendLine(StringFormatter.ZipFileTimeStampModifiedMessage(entryName, timeOffset.DateTime.ToShortDateString(), ZipFormatMinDate.ToShortDateString()));
+                entry.LastWriteTime = ZipFormatMinDate;
+            }
+            else if (timeOffset.UtcDateTime > ZipFormatMaxDate)
+            {
+                warningMessage.AppendLine(StringFormatter.ZipFileTimeStampModifiedMessage(entryName, timeOffset.DateTime.ToShortDateString(), ZipFormatMaxDate.ToShortDateString()));
+                entry.LastWriteTime = ZipFormatMaxDate;
+            }
+            else
+            {
+                entry.LastWriteTime = timeOffset.UtcDateTime;
+            }
+
             return entry;
         }
 
@@ -921,6 +959,7 @@ private void WriteManifest(ZipArchive package, int minimumManifestVersion, strin
         private HashSet<string> WriteFiles(ZipArchive package, HashSet<string> filesWithoutExtensions)
         {
             var extensions = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
+            var warningMessage = new StringBuilder();
 
             // Add files that might not come from expanding files on disk
             foreach (IPackageFile file in new HashSet<IPackageFile>(Files))
@@ -933,7 +972,8 @@ private HashSet<string> WriteFiles(ZipArchive package, HashSet<string> filesWith
                             package,
                             file.Path,
                             stream,
-                            lastWriteTime: _deterministic ? ZipFormatMinDate : file.LastWriteTime);
+                            lastWriteTime: _deterministic ? ZipFormatMinDate : file.LastWriteTime,
+                            warningMessage);
                         var fileExtension = Path.GetExtension(file.Path);
 
                         // We have files without extension (e.g. the executables for Nix)
@@ -957,6 +997,18 @@ private HashSet<string> WriteFiles(ZipArchive package, HashSet<string> filesWith
                 }
             }
 
+            if (warningMessage.Length > Environment.NewLine.Length)
+            {
+                warningMessage.Length -= Environment.NewLine.Length;
+            }
+
+            var warningMessageString = warningMessage.ToString();
+
+            if (!string.IsNullOrEmpty(warningMessageString))
+            {
+                _logger?.Log(PackagingLogMessage.CreateWarning(StringFormatter.ZipFileTimeStampModifiedWarning(warningMessageString), NuGetLogCode.NU5132));
+            }
+
             return extensions;
         }
 
@@ -1079,15 +1131,15 @@ private static void ExcludeFiles(List<PhysicalPackageFile> searchFiles, string b
             }
         }
 
-        private void CreatePart(ZipArchive package, string path, Stream sourceStream, DateTimeOffset lastWriteTime)
+        private void CreatePart(ZipArchive package, string path, Stream sourceStream, DateTimeOffset lastWriteTime, StringBuilder warningMessage)
         {
             if (PackageHelper.IsNuspec(path))
             {
                 return;
             }
 
             string entryName = CreatePartEntryName(path);
-            var entry = CreatePackageFileEntry(package, entryName, lastWriteTime, CompressionLevel.Optimal);
+            var entry = CreatePackageFileEntry(package, entryName, lastWriteTime, CompressionLevel.Optimal, warningMessage);
 
             using (var stream = entry.Open())
             {

diff --git a/src/NuGet.Core/NuGet.Packaging/PackageExtraction/ZipArchiveExtensions.cs b/src/NuGet.Core/NuGet.Packaging/PackageExtraction/ZipArchiveExtensions.cs
@@ -75,7 +75,7 @@ public static void UpdateFileTimeFromEntry(this ZipArchiveEntry entry, string fi
             {
                 try
                 {
-                    File.SetLastWriteTimeUtc(fileFullPath, entry.LastWriteTime.UtcDateTime);
+                    File.SetLastWriteTimeUtc(fileFullPath, entry.LastWriteTime.Add(entry.LastWriteTime.Offset).UtcDateTime);
                 }
                 catch (ArgumentOutOfRangeException ex)
                 {

diff --git a/src/NuGet.Core/NuGet.Packaging/PublicAPI/net472/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Packaging/PublicAPI/net472/PublicAPI.Unshipped.txt
@@ -4,6 +4,9 @@ NuGet.Packaging.NupkgMetadataFile.Source.get -> string
 NuGet.Packaging.NupkgMetadataFile.Source.set -> void
 NuGet.Packaging.PackageBuilder.EmitRequireLicenseAcceptance.get -> bool
 NuGet.Packaging.PackageBuilder.EmitRequireLicenseAcceptance.set -> void
+NuGet.Packaging.PackageBuilder.PackageBuilder(bool deterministic, NuGet.Common.ILogger logger) -> void
+NuGet.Packaging.PackageBuilder.PackageBuilder(string path, System.Func<string, string> propertyProvider, bool includeEmptyDirectories, bool deterministic, NuGet.Common.ILogger logger) -> void
+NuGet.Packaging.PackageBuilder.PackageBuilder(string path, string basePath, System.Func<string, string> propertyProvider, bool includeEmptyDirectories, bool deterministic, NuGet.Common.ILogger logger) -> void
 static NuGet.Packaging.Rules.AnalysisResources.InvalidUndottedFrameworkInDependencyGroupsWarning.get -> string
 static NuGet.Packaging.Rules.AnalysisResources.InvalidUndottedFrameworkInFilesWarning.get -> string
 static NuGet.Packaging.Rules.AnalysisResources.InvalidUndottedFrameworkInFrameworkAssemblyGroupsWarning.get -> string

diff --git a/src/NuGet.Core/NuGet.Packaging/PublicAPI/netcoreapp5.0/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Packaging/PublicAPI/netcoreapp5.0/PublicAPI.Unshipped.txt
@@ -4,6 +4,9 @@ NuGet.Packaging.NupkgMetadataFile.Source.get -> string
 NuGet.Packaging.NupkgMetadataFile.Source.set -> void
 NuGet.Packaging.PackageBuilder.EmitRequireLicenseAcceptance.get -> bool
 NuGet.Packaging.PackageBuilder.EmitRequireLicenseAcceptance.set -> void
+NuGet.Packaging.PackageBuilder.PackageBuilder(bool deterministic, NuGet.Common.ILogger logger) -> void
+NuGet.Packaging.PackageBuilder.PackageBuilder(string path, System.Func<string, string> propertyProvider, bool includeEmptyDirectories, bool deterministic, NuGet.Common.ILogger logger) -> void
+NuGet.Packaging.PackageBuilder.PackageBuilder(string path, string basePath, System.Func<string, string> propertyProvider, bool includeEmptyDirectories, bool deterministic, NuGet.Common.ILogger logger) -> void
 static NuGet.Packaging.Rules.AnalysisResources.InvalidUndottedFrameworkInDependencyGroupsWarning.get -> string
 static NuGet.Packaging.Rules.AnalysisResources.InvalidUndottedFrameworkInFilesWarning.get -> string
 static NuGet.Packaging.Rules.AnalysisResources.InvalidUndottedFrameworkInFrameworkAssemblyGroupsWarning.get -> string

diff --git a/src/NuGet.Core/NuGet.Packaging/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Packaging/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt
@@ -4,6 +4,9 @@ NuGet.Packaging.NupkgMetadataFile.Source.get -> string
 NuGet.Packaging.NupkgMetadataFile.Source.set -> void
 NuGet.Packaging.PackageBuilder.EmitRequireLicenseAcceptance.get -> bool
 NuGet.Packaging.PackageBuilder.EmitRequireLicenseAcceptance.set -> void
+NuGet.Packaging.PackageBuilder.PackageBuilder(bool deterministic, NuGet.Common.ILogger logger) -> void
+NuGet.Packaging.PackageBuilder.PackageBuilder(string path, System.Func<string, string> propertyProvider, bool includeEmptyDirectories, bool deterministic, NuGet.Common.ILogger logger) -> void
+NuGet.Packaging.PackageBuilder.PackageBuilder(string path, string basePath, System.Func<string, string> propertyProvider, bool includeEmptyDirectories, bool deterministic, NuGet.Common.ILogger logger) -> void
 static NuGet.Packaging.Rules.AnalysisResources.InvalidUndottedFrameworkInDependencyGroupsWarning.get -> string
 static NuGet.Packaging.Rules.AnalysisResources.InvalidUndottedFrameworkInFilesWarning.get -> string
 static NuGet.Packaging.Rules.AnalysisResources.InvalidUndottedFrameworkInFrameworkAssemblyGroupsWarning.get -> string

diff --git a/src/NuGet.Core/NuGet.Packaging/StringFormatter.cs b/src/NuGet.Core/NuGet.Packaging/StringFormatter.cs
@@ -1,6 +1,7 @@
 // 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.Globalization;
 
 namespace NuGet.Packaging
@@ -20,5 +21,23 @@ internal static class StringFormatter
                 source,
                 contentHash);
         }
+
+        internal static string ZipFileTimeStampModifiedMessage(
+            string filePath,
+            string originalLastWriteTimeStamp,
+            string updatedLastWriteTimeStamp)
+        {
+            return string.Format(CultureInfo.CurrentCulture,
+                Strings.ZipFileLastWriteTimeStampModifiedMessage,
+                filePath,
+                originalLastWriteTimeStamp,
+                updatedLastWriteTimeStamp);
+        }
+
+        internal static string ZipFileTimeStampModifiedWarning(
+            string listOfFileTimeStampModifiedMessages)
+        {
+            return Strings.ZipFileTimeStampModifiedWarning + Environment.NewLine + listOfFileTimeStampModifiedMessages;
+        }
     }
 }
diff --git a/src/NuGet.Core/NuGet.Packaging/Strings.Designer.cs b/src/NuGet.Core/NuGet.Packaging/Strings.Designer.cs
diff --git a/src/NuGet.Core/NuGet.Packaging/Strings.resx b/src/NuGet.Core/NuGet.Packaging/Strings.resx
@@ -865,4 +865,13 @@ Valid from:</comment>
     <value>'{0}' cannot be null.</value>
     <comment>0 - property name</comment>
   </data>
+  <data name="ZipFileLastWriteTimeStampModifiedMessage" xml:space="preserve">
+    <value>'{0}' changed from '{1}' to '{2}'</value>
+    <comment>0 - File name
+1 - Original last write timestamp
+2 - Updated last write timestamp</comment>
+  </data>
+  <data name="ZipFileTimeStampModifiedWarning" xml:space="preserve">
+    <value>The zip format supports a limited date range. The following files are outside the supported range:</value>
+  </data>
 </root>