Tar: Refine GNU timestamp handling

src/libraries/System.Formats.Tar/System.Formats.Tar.sln

@@ -1,4 +1,8 @@
-Microsoft Visual Studio Solution File, Format Version 12.00
+
+Microsoft Visual Studio Solution File, Format Version 12.00
+# Visual Studio Version 17
+VisualStudioVersion = 17.14.36105.17
+MinimumVisualStudioVersion = 10.0.40219.1
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "StreamConformanceTests", "..\Common\tests\StreamConformanceTests\StreamConformanceTests.csproj", "{BE259E6E-B4F5-47DC-93D5-204297098A8C}"
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "TestUtilities", "..\Common\tests\TestUtilities\TestUtilities.csproj", "{45972587-B4BF-4F09-94DC-20E2D460FAA8}"
@@ -39,11 +43,11 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{55A8C7E4-925
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "gen", "gen", "{0345BAA8-92BC-4499-B550-21AC44910FD2}"
 EndProject
-Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "gen", "tools\gen", "{07E13495-DC86-43BF-9E64-2CEA381D892D}"
+Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "gen", "gen", "{07E13495-DC86-43BF-9E64-2CEA381D892D}"
 EndProject
-Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "tools\src", "{3CB7A441-325E-41C9-B0D3-30D29CC21E82}"
+Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{3CB7A441-325E-41C9-B0D3-30D29CC21E82}"
 EndProject
-Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "ref", "tools\ref", "{F845BA28-9AFC-4B52-8ED1-A4302AEB5C11}"
+Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "ref", "ref", "{F845BA28-9AFC-4B52-8ED1-A4302AEB5C11}"
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "tools", "tools", "{4A77449F-D456-4E19-B31C-7E0E3702680B}"
 EndProject
@@ -124,25 +128,29 @@ Global
 	GlobalSection(NestedProjects) = preSolution
 		{BE259E6E-B4F5-47DC-93D5-204297098A8C} = {6CF0D830-3EE9-44B1-B548-EA8750AD7B3E}
 		{45972587-B4BF-4F09-94DC-20E2D460FAA8} = {6CF0D830-3EE9-44B1-B548-EA8750AD7B3E}
-		{D2788A26-CDAE-4388-AE4B-A36B0E6DFF9D} = {6CF0D830-3EE9-44B1-B548-EA8750AD7B3E}
-		{6FD1E284-7B50-4077-B73A-5B31CB0E3577} = {6CF0D830-3EE9-44B1-B548-EA8750AD7B3E}
 		{E0B882C6-2082-45F2-806E-568461A61975} = {9BE8AFF4-D37B-49AF-AFD3-A15E514AC8AE}
-		{A00011A0-E609-4A49-B893-EBFC72C98707} = {9BE8AFF4-D37B-49AF-AFD3-A15E514AC8AE}
 		{9F751C2B-56DD-4604-A3F3-568627F8C006} = {55A8C7E4-925C-4F21-B68B-CEFC19137A4B}
+		{D2788A26-CDAE-4388-AE4B-A36B0E6DFF9D} = {6CF0D830-3EE9-44B1-B548-EA8750AD7B3E}
+		{6FD1E284-7B50-4077-B73A-5B31CB0E3577} = {6CF0D830-3EE9-44B1-B548-EA8750AD7B3E}
 		{00477EA4-C3E5-48A9-8CA8-8CCF689E0DB4} = {0345BAA8-92BC-4499-B550-21AC44910FD2}
 		{CD3A1327-8C67-4370-AC34-033065330F3F} = {0345BAA8-92BC-4499-B550-21AC44910FD2}
 		{E89FEF3E-E0B9-41C4-A51C-9759AD1A3B69} = {0345BAA8-92BC-4499-B550-21AC44910FD2}
 		{50E6D5FD-0E06-4D07-966E-C28E5448A1D3} = {0345BAA8-92BC-4499-B550-21AC44910FD2}
+		{A00011A0-E609-4A49-B893-EBFC72C98707} = {9BE8AFF4-D37B-49AF-AFD3-A15E514AC8AE}
 		{AFEE875F-22C7-46AE-B28F-AF5C05CA0BA5} = {07E13495-DC86-43BF-9E64-2CEA381D892D}
 		{AABA2E7B-6B45-4DD7-9C33-5F0FCDA1193F} = {07E13495-DC86-43BF-9E64-2CEA381D892D}
-		{07E13495-DC86-43BF-9E64-2CEA381D892D} = {4A77449F-D456-4E19-B31C-7E0E3702680B}
 		{1720175E-27DA-4004-ABF2-DD47A338A3DB} = {3CB7A441-325E-41C9-B0D3-30D29CC21E82}
 		{EB826FA8-035F-4DEF-8767-CBF94446916B} = {3CB7A441-325E-41C9-B0D3-30D29CC21E82}
-		{3CB7A441-325E-41C9-B0D3-30D29CC21E82} = {4A77449F-D456-4E19-B31C-7E0E3702680B}
 		{B2F7C18F-2333-432B-AC5E-9AD672582DF3} = {F845BA28-9AFC-4B52-8ED1-A4302AEB5C11}
+		{07E13495-DC86-43BF-9E64-2CEA381D892D} = {4A77449F-D456-4E19-B31C-7E0E3702680B}
+		{3CB7A441-325E-41C9-B0D3-30D29CC21E82} = {4A77449F-D456-4E19-B31C-7E0E3702680B}
 		{F845BA28-9AFC-4B52-8ED1-A4302AEB5C11} = {4A77449F-D456-4E19-B31C-7E0E3702680B}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {F9B8DA67-C83B-466D-907C-9541CDBDCFEF}
 	EndGlobalSection
+	GlobalSection(SharedMSBuildProjectFiles) = preSolution
+		..\..\tools\illink\src\ILLink.Shared\ILLink.Shared.projitems*{aaba2e7b-6b45-4dd7-9c33-5f0fcda1193f}*SharedItemsImports = 5
+		..\..\tools\illink\src\ILLink.Shared\ILLink.Shared.projitems*{eb826fa8-035f-4def-8767-cbf94446916b}*SharedItemsImports = 5
+	EndGlobalSection
 EndGlobal

src/libraries/System.Formats.Tar/src/System/Formats/Tar/GnuTarEntry.cs

@@ -20,7 +20,9 @@ internal GnuTarEntry(TarHeader header, TarReader readerOfOrigin)
         /// </summary>
         /// <param name="entryType">The type of the entry.</param>
         /// <param name="entryName">A string with the path and file name of this entry.</param>
-        /// <remarks>When creating an instance using the <see cref="GnuTarEntry(TarEntryType, string)"/> constructor, only the following entry types are supported:
+        /// <remarks>
+        /// <para>When creating an instance of <see cref="GnuTarEntry"/> using this constructor, the <see cref="AccessTime"/> and <see cref="ChangeTime"/> properties are set to <see langword="default" />, which in the entry header <c>atime</c> and <c>ctime</c> fields is written as null bytes. This ensures compatibility with other tools that are unable to read the <c>atime</c> and <c>ctime</c> in <see cref="TarEntryFormat.Gnu"/> entries, as these two fields are not POSIX compatible because other formats expect the <c>prefix</c> field in the same header location where <see cref="TarEntryFormat.Gnu"/> writes <c>atime</c> and <c>ctime</c>.</para>
+        /// <para>When creating an instance using the <see cref="GnuTarEntry(TarEntryType, string)"/> constructor, only the following entry types are supported:</para>
         /// <list type="bullet">
         /// <item>In all platforms: <see cref="TarEntryType.Directory"/>, <see cref="TarEntryType.HardLink"/>, <see cref="TarEntryType.SymbolicLink"/>, <see cref="TarEntryType.RegularFile"/>.</item>
         /// <item>In Unix platforms only: <see cref="TarEntryType.BlockDevice"/>, <see cref="TarEntryType.CharacterDevice"/> and <see cref="TarEntryType.Fifo"/>.</item>
@@ -33,13 +35,16 @@ internal GnuTarEntry(TarHeader header, TarReader readerOfOrigin)
         public GnuTarEntry(TarEntryType entryType, string entryName)
             : base(entryType, entryName, TarEntryFormat.Gnu, isGea: false)
         {
-            _header._aTime = _header._mTime; // mtime was set in base constructor
-            _header._cTime = _header._mTime;
+            _header._aTime = default;
+            _header._cTime = default;
         }
 
         /// <summary>
         /// Initializes a new <see cref="GnuTarEntry"/> instance by converting the specified <paramref name="other"/> entry into the GNU format.
         /// </summary>
+        /// <remarks>
+        /// <para>When creating an instance of <see cref="GnuTarEntry"/> using this constructor, if <paramref name="other"/> is <see cref="TarEntryFormat.Gnu"/> or <see cref="TarEntryFormat.Pax"/>, then the <see cref="AccessTime"/> and <see cref="ChangeTime"/> properties are set to the same values set in <paramref name="other"/>. But if <paramref name="other"/> is of any other format, then <see cref="AccessTime"/> and <see cref="ChangeTime"/> are set to <see langword="default" />, which in the entry header <c>atime</c> and <c>ctime</c> fields is written as null bytes. This ensures compatibility with other tools that are unable to read the <c>atime</c> and <c>ctime</c> in <see cref="TarEntryFormat.Gnu"/> entries, as these two fields are not POSIX compatible because other formats expect the <c>prefix</c> field in the same header location where <see cref="TarEntryFormat.Gnu"/> writes <c>atime</c> and <c>ctime</c>.</para>
+        /// </remarks>
         /// <exception cref="ArgumentException"><para><paramref name="other"/> is a <see cref="PaxGlobalExtendedAttributesTarEntry"/> and cannot be converted.</para>
         /// <para>-or-</para>
         /// <para>The entry type of <paramref name="other"/> is not supported for conversion to the GNU format.</para></exception>
@@ -54,45 +59,23 @@ public GnuTarEntry(TarEntry other)
             }
             else
             {
-                bool changedATime = false;
-                bool changedCTime = false;
-
-                if (other is PaxTarEntry paxOther)
-                {
-                    changedATime = TarHelpers.TryGetDateTimeOffsetFromTimestampString(paxOther._header.ExtendedAttributes, TarHeader.PaxEaATime, out DateTimeOffset aTime);
-                    if (changedATime)
-                    {
-                        _header._aTime = aTime;
-                    }
-
-                    changedCTime = TarHelpers.TryGetDateTimeOffsetFromTimestampString(paxOther._header.ExtendedAttributes, TarHeader.PaxEaCTime, out DateTimeOffset cTime);
-                    if (changedCTime)
-                    {
-                        _header._cTime = cTime;
-                    }
-                }
-
-                // Either 'other' was V7 or Ustar (those formats do not have atime or ctime),
-                // or 'other' was PAX and at least one of the timestamps was not found in the extended attributes
-                if (!changedATime || !changedCTime)
-                {
-                    DateTimeOffset now = DateTimeOffset.UtcNow;
-                    if (!changedATime)
-                    {
-                        _header._aTime = now;
-                    }
-                    if (!changedCTime)
-                    {
-                        _header._cTime = now;
-                    }
-                }
+                // 'other' was V7, Ustar (those formats do not have atime or ctime),
+                // or even PAX (which could contain atime and ctime in the ExtendedAttributes), but
+                // to avoid creating a GNU entry that might be incompatible with other tools,
+                // we avoid setting the atime and ctime fields. The user would have to set them manually
+                // if they are really needed.
+                _header._aTime = default;
+                _header._cTime = default;
             }
         }
 
         /// <summary>
-        /// A timestamp that represents the last time the file represented by this entry was accessed.
+        /// A timestamp that represents the last time the file represented by this entry was accessed. Setting a value for this property is not recommended because most TAR reading tools do not support it.
         /// </summary>
-        /// <remarks>In Unix platforms, this timestamp is commonly known as <c>atime</c>.</remarks>
+        /// <remarks>
+        /// <para>In Unix platforms, this timestamp is commonly known as <c>atime</c>.</para>
+        /// <para>Setting the value of this property to a value other than <see langword="default"/> may cause problems with other tools that read TAR files, because the <see cref="TarEntryFormat.Gnu"/> format writes the <c>atime</c> field where other formats would normally read and write the <c>prefix</c> field in the header. You should only set this property to something other than <see langword="default"/> if this entry will be read by tools that know how to correctly interpret the <c>atime</c> field of the <see cref="TarEntryFormat.Gnu"/> format.</para>
+        /// </remarks>
         public DateTimeOffset AccessTime
         {
             get => _header._aTime;
@@ -103,9 +86,12 @@ public DateTimeOffset AccessTime
         }
 
         /// <summary>
-        /// A timestamp that represents the last time the metadata of the file represented by this entry was changed.
+        /// A timestamp that represents the last time the metadata of the file represented by this entry was changed. Setting a value for this property is not recommended because most TAR reading tools do not support it.
         /// </summary>
-        /// <remarks>In Unix platforms, this timestamp is commonly known as <c>ctime</c>.</remarks>
+        /// <remarks>
+        /// <para>In Unix platforms, this timestamp is commonly known as <c>ctime</c>.</para>
+        /// <para>Setting the value of this property to a value other than <see langword="default"/> may cause problems with other tools that read TAR files, because the <see cref="TarEntryFormat.Gnu"/> format writes the <c>ctime</c> field where other formats would normally read and write the <c>prefix</c> field in the header. You should only set this property to something other than <see langword="default"/> if this entry will be read by tools that know how to correctly interpret the <c>ctime</c> field of the <see cref="TarEntryFormat.Gnu"/> format.</para>
+        /// </remarks>
         public DateTimeOffset ChangeTime
         {
             get => _header._cTime;

src/libraries/System.Formats.Tar/src/System/Formats/Tar/PaxTarEntry.cs

@@ -103,9 +103,12 @@ public PaxTarEntry(TarEntryType entryType, string entryName, IEnumerable<KeyValu
         /// <summary>
         /// Initializes a new <see cref="PaxTarEntry"/> instance by converting the specified <paramref name="other"/> entry into the PAX format.
         /// </summary>
-        /// <exception cref="ArgumentException"><para><paramref name="other"/> is a <see cref="PaxGlobalExtendedAttributesTarEntry"/> and cannot be converted.</para>
+        /// <exception cref="ArgumentException">
+        /// <para><paramref name="other"/> is a <see cref="PaxGlobalExtendedAttributesTarEntry"/> and cannot be converted.</para>
         /// <para>-or-</para>
-        /// <para>The entry type of <paramref name="other"/> is not supported for conversion to the PAX format.</para></exception>
+        /// <para>The entry type of <paramref name="other"/> is not supported for conversion to the PAX format.</para>
+        /// </exception>
+        /// <remarks>When converting a <see cref="GnuTarEntry"/> to <see cref="PaxTarEntry"/> using this constructor, the <see cref="GnuTarEntry.AccessTime"/> and <see cref="GnuTarEntry.ChangeTime"/> values will get transfered to the <see cref="ExtendedAttributes" /> dictionary only if their values are not <see langword="default"/> (which is <see cref="DateTimeOffset.MinValue"/>).</remarks>
         public PaxTarEntry(TarEntry other)
             : base(other, TarEntryFormat.Pax)
         {
@@ -122,8 +125,14 @@ public PaxTarEntry(TarEntry other)
             {
                 if (other is GnuTarEntry gnuOther)
                 {
-                    _header.ExtendedAttributes[TarHeader.PaxEaATime] = TarHelpers.GetTimestampStringFromDateTimeOffset(gnuOther.AccessTime);
-                    _header.ExtendedAttributes[TarHeader.PaxEaCTime] = TarHelpers.GetTimestampStringFromDateTimeOffset(gnuOther.ChangeTime);
+                    if (gnuOther.AccessTime != default)
+                    {
+                        _header.ExtendedAttributes[TarHeader.PaxEaATime] = TarHelpers.GetTimestampStringFromDateTimeOffset(gnuOther.AccessTime);
+                    }
+                    if (gnuOther.ChangeTime != default)
+                    {
+                        _header.ExtendedAttributes[TarHeader.PaxEaCTime] = TarHelpers.GetTimestampStringFromDateTimeOffset(gnuOther.ChangeTime);
+                    }
                 }
             }
 

src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs

@@ -15,6 +15,8 @@ namespace System.Formats.Tar
     // Reads the header attributes from a tar archive entry.
     internal sealed partial class TarHeader
     {
+        private readonly byte[] ArrayOf12NullBytes = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0];
+
         // Attempts to retrieve the next header from the specified tar archive stream.
         // Throws if end of stream is reached or if any data type conversion fails.
         // Returns a valid TarHeader object if the attributes were read successfully, null otherwise.
@@ -537,11 +539,18 @@ private void ReadPosixAndGnuSharedAttributes(ReadOnlySpan<byte> buffer)
         private void ReadGnuAttributes(ReadOnlySpan<byte> buffer)
         {
             // Convert byte arrays
-            long aTime = TarHelpers.ParseNumeric<long>(buffer.Slice(FieldLocations.ATime, FieldLengths.ATime));
-            _aTime = TarHelpers.GetDateTimeOffsetFromSecondsSinceEpoch(aTime);
-
-            long cTime = TarHelpers.ParseNumeric<long>(buffer.Slice(FieldLocations.CTime, FieldLengths.CTime));
-            _cTime = TarHelpers.GetDateTimeOffsetFromSecondsSinceEpoch(cTime);
+            ReadOnlySpan<byte> aTimeBuffer = buffer.Slice(FieldLocations.ATime, FieldLengths.ATime);
+            if (!aTimeBuffer.SequenceEqual(ArrayOf12NullBytes)) // null values are ignored
+            {
+                long aTime = TarHelpers.ParseNumeric<long>(aTimeBuffer);
+                _aTime = TarHelpers.GetDateTimeOffsetFromSecondsSinceEpoch(aTime);
+            }
+            ReadOnlySpan<byte> cTimeBuffer = buffer.Slice(FieldLocations.CTime, FieldLengths.CTime);
+            if (!cTimeBuffer.SequenceEqual(ArrayOf12NullBytes)) // An all nulls buffer is interpreted as MinValue
+            {
+                long cTime = TarHelpers.ParseNumeric<long>(cTimeBuffer);
+                _cTime = TarHelpers.GetDateTimeOffsetFromSecondsSinceEpoch(cTime);
+            }
 
             // TODO: Read the bytes of the currently unsupported GNU fields, in case user wants to write this entry into another GNU archive, they need to be preserved. https://github.com/dotnet/runtime/issues/68230
         }