From 2cd6e15868f46d75602481411a07f04c88b50ae4 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 20 May 2025 13:32:28 +0200 Subject: [PATCH 1/5] System.Formats.Tar: default to no ctime/atime. atime and ctime are not well supported on non-PAX formats. Even for PAX formats, tools do not include these timestamps by default because they are of limited use. This makes .NET to also default to not include these timestamps. --- .../src/System/Formats/Tar/GnuTarEntry.cs | 24 +--- .../PaxGlobalExtendedAttributesTarEntry.cs | 2 +- .../src/System/Formats/Tar/PaxTarEntry.cs | 103 ++-------------- .../src/System/Formats/Tar/TarHeader.Read.cs | 33 +++--- .../src/System/Formats/Tar/TarHeader.Write.cs | 21 ++-- .../src/System/Formats/Tar/TarHeader.cs | 6 +- .../TarEntry/PaxTarEntry.Conversion.Tests.cs | 26 ++-- .../TarEntry.Conversion.Tests.Base.cs | 112 ++++++------------ .../tests/TarReader/TarReader.Tests.cs | 2 +- .../tests/TarTestsBase.Pax.cs | 13 +- .../TarWriter.WriteEntry.Entry.Pax.Tests.cs | 20 ++-- ...rWriter.WriteEntryAsync.Entry.Pax.Tests.cs | 18 +-- 12 files changed, 112 insertions(+), 268 deletions(-) diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/GnuTarEntry.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/GnuTarEntry.cs index 3a2a416704eb8d..04587f698823ad 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/GnuTarEntry.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/GnuTarEntry.cs @@ -34,10 +34,7 @@ internal GnuTarEntry(TarHeader header, TarReader readerOfOrigin) /// is not supported in the specified format. public GnuTarEntry(TarEntryType entryType, string entryName) : base(entryType, entryName, TarEntryFormat.Gnu, isGea: false) - { - _header._aTime = default; - _header._cTime = default; - } + { } /// /// Initializes a new instance by converting the specified entry into the GNU format. @@ -50,24 +47,7 @@ public GnuTarEntry(TarEntryType entryType, string entryName) /// The entry type of is not supported for conversion to the GNU format. public GnuTarEntry(TarEntry other) : base(other, TarEntryFormat.Gnu) - { - if (other is GnuTarEntry gnuOther) - { - _header._aTime = gnuOther.AccessTime; - _header._cTime = gnuOther.ChangeTime; - _header._gnuUnusedBytes = other._header._gnuUnusedBytes; - } - else - { - // '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; - } - } + { } /// /// 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. diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/PaxGlobalExtendedAttributesTarEntry.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/PaxGlobalExtendedAttributesTarEntry.cs index 94ba60167f7212..9dc572f8c1c1e4 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/PaxGlobalExtendedAttributesTarEntry.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/PaxGlobalExtendedAttributesTarEntry.cs @@ -28,7 +28,7 @@ public PaxGlobalExtendedAttributesTarEntry(IEnumerable diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/PaxTarEntry.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/PaxTarEntry.cs index a062993fef0473..4226f180479c96 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/PaxTarEntry.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/PaxTarEntry.cs @@ -21,7 +21,7 @@ internal PaxTarEntry(TarHeader header, TarReader readerOfOrigin) } /// - /// Initializes a new instance with the specified entry type, entry name, and the default extended attributes. + /// Initializes a new instance with the specified entry type and entry name. /// /// The type of the entry. /// A string with the path and file name of this entry. @@ -30,20 +30,7 @@ internal PaxTarEntry(TarHeader header, TarReader readerOfOrigin) /// In all platforms: , , , . /// In Unix platforms only: , and . /// - /// Use the constructor to include additional extended attributes when creating the entry. - /// The following entries are always found in the Extended Attributes dictionary of any PAX entry: - /// - /// Modification time, under the name mtime, as a number. - /// Access time, under the name atime, as a number. - /// Change time, under the name ctime, as a number. - /// Path, under the name path, as a string. - /// - /// The following entries are only found in the Extended Attributes dictionary of a PAX entry if certain conditions are met: - /// - /// Group name, under the name gname, as a string, if it is larger than 32 bytes. - /// User name, under the name uname, as a string, if it is larger than 32 bytes. - /// File length, under the name size, as an , if the string representation of the number is larger than 12 bytes. - /// + /// Use the constructor to include extended attributes when creating the entry. /// /// is . /// is empty. @@ -53,13 +40,10 @@ public PaxTarEntry(TarEntryType entryType, string entryName) : base(entryType, entryName, TarEntryFormat.Pax, isGea: false) { _header._prefix = string.Empty; - - Debug.Assert(_header._mTime != default); - AddNewAccessAndChangeTimestampsIfNotExist(useMTime: true); } /// - /// Initializes a new instance with the specified entry type, entry name and Extended Attributes enumeration. + /// Initializes a new instance with the specified entry type, entry name and extended attributes. /// /// The type of the entry. /// A string with the path and file name of this entry. @@ -69,20 +53,6 @@ public PaxTarEntry(TarEntryType entryType, string entryName) /// In all platforms: , , , . /// In Unix platforms only: , and . /// - /// The specified get appended to the default attributes, unless the specified enumeration overrides any of them. - /// The following entries are always found in the Extended Attributes dictionary of any PAX entry: - /// - /// Modification time, under the name mtime, as a number. - /// Access time, under the name atime, as a number. - /// Change time, under the name ctime, as a number. - /// Path, under the name path, as a string. - /// - /// The following entries are only found in the Extended Attributes dictionary of a PAX entry if certain conditions are met: - /// - /// Group name, under the name gname, as a string, if it is larger than 32 bytes. - /// User name, under the name uname, as a string, if it is larger than 32 bytes. - /// File length, under the name size, as an , if the string representation of the number is larger than 12 bytes. - /// /// /// or is . /// is empty. @@ -94,10 +64,7 @@ public PaxTarEntry(TarEntryType entryType, string entryName, IEnumerable @@ -119,72 +86,28 @@ public PaxTarEntry(TarEntry other) if (other is PaxTarEntry paxOther) { - _header.InitializeExtendedAttributesWithExisting(paxOther.ExtendedAttributes); + _header.AddExtendedAttributes(paxOther.ExtendedAttributes); } - else + else if (other is GnuTarEntry gnuOther) { - if (other is GnuTarEntry gnuOther) + if (gnuOther.AccessTime != default) + { + _header.ExtendedAttributes[TarHeader.PaxEaATime] = TarHelpers.GetTimestampStringFromDateTimeOffset(gnuOther.AccessTime); + } + if (gnuOther.ChangeTime != default) { - if (gnuOther.AccessTime != default) - { - _header.ExtendedAttributes[TarHeader.PaxEaATime] = TarHelpers.GetTimestampStringFromDateTimeOffset(gnuOther.AccessTime); - } - if (gnuOther.ChangeTime != default) - { - _header.ExtendedAttributes[TarHeader.PaxEaCTime] = TarHelpers.GetTimestampStringFromDateTimeOffset(gnuOther.ChangeTime); - } + _header.ExtendedAttributes[TarHeader.PaxEaCTime] = TarHelpers.GetTimestampStringFromDateTimeOffset(gnuOther.ChangeTime); } } - - AddNewAccessAndChangeTimestampsIfNotExist(useMTime: false); } /// /// Returns the extended attributes for this entry. /// - /// The extended attributes are specified when constructing an entry. Use to append your own enumeration of extended attributes to the current entry on top of the default ones. Use to only use the default extended attributes. - /// The following entries are always found in the Extended Attributes dictionary of any PAX entry: - /// - /// Modification time, under the name mtime, as a number. - /// Access time, under the name atime, as a number. - /// Change time, under the name ctime, as a number. - /// Path, under the name path, as a string. - /// - /// The following entries are only found in the Extended Attributes dictionary of a PAX entry if certain conditions are met: - /// - /// Group name, under the name gname, as a string, if it is larger than 32 bytes. - /// User name, under the name uname, as a string, if it is larger than 32 bytes. - /// File length, under the name size, as an , if the string representation of the number is larger than 12 bytes. - /// - /// + /// The extended attributes are specified when constructing an entry and updated with additional attributes when the entry is written. Use to append custom extended attributes. public IReadOnlyDictionary ExtendedAttributes => _readOnlyExtendedAttributes ??= _header.ExtendedAttributes.AsReadOnly(); // Determines if the current instance's entry type supports setting a data stream. internal override bool IsDataStreamSetterSupported() => EntryType == TarEntryType.RegularFile; - - // Checks if the extended attributes dictionary contains 'atime' and 'ctime'. - // If any of them is not found, it is added with the value of either the current entry's 'mtime', - // or 'DateTimeOffset.UtcNow', depending on the value of 'useMTime'. - private void AddNewAccessAndChangeTimestampsIfNotExist(bool useMTime) - { - Debug.Assert(!useMTime || (useMTime && _header._mTime != default)); - bool containsATime = _header.ExtendedAttributes.ContainsKey(TarHeader.PaxEaATime); - bool containsCTime = _header.ExtendedAttributes.ContainsKey(TarHeader.PaxEaCTime); - - if (!containsATime || !containsCTime) - { - string secondsFromEpochString = TarHelpers.GetTimestampStringFromDateTimeOffset(useMTime ? _header._mTime : DateTimeOffset.UtcNow); - - if (!containsATime) - { - _header.ExtendedAttributes[TarHeader.PaxEaATime] = secondsFromEpochString; - } - - if (!containsCTime) - { - _header.ExtendedAttributes[TarHeader.PaxEaCTime] = secondsFromEpochString; - } - } - } } } diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs index e15d7a1add2dbf..37570d0d33a5ea 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs @@ -15,8 +15,6 @@ 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. @@ -106,7 +104,7 @@ internal void ReplaceNormalAttributesWithExtended(Dictionary? di return; } - InitializeExtendedAttributesWithExisting(dictionaryFromExtendedAttributesHeader); + AddExtendedAttributes(dictionaryFromExtendedAttributesHeader); // Find all the extended attributes with known names and save them in the expected standard attribute. @@ -388,7 +386,7 @@ private async Task ProcessDataBlockAsync(Stream archiveStream, bool copyData, Ca TarHeader header = new(initialFormat, name: TarHelpers.GetTrimmedUtf8String(buffer.Slice(FieldLocations.Name, FieldLengths.Name)), mode: TarHelpers.ParseNumeric(buffer.Slice(FieldLocations.Mode, FieldLengths.Mode)), - mTime: TarHelpers.GetDateTimeOffsetFromSecondsSinceEpoch(TarHelpers.ParseNumeric(buffer.Slice(FieldLocations.MTime, FieldLengths.MTime))), + mTime: ParseAsTimestamp(buffer.Slice(FieldLocations.MTime, FieldLengths.MTime)), typeFlag: (TarEntryType)buffer[FieldLocations.TypeFlag]) { _checksum = checksum, @@ -538,21 +536,24 @@ private void ReadPosixAndGnuSharedAttributes(ReadOnlySpan buffer) // Throws if any conversion fails. private void ReadGnuAttributes(ReadOnlySpan buffer) { - // Convert byte arrays - ReadOnlySpan aTimeBuffer = buffer.Slice(FieldLocations.ATime, FieldLengths.ATime); - if (!aTimeBuffer.SequenceEqual(ArrayOf12NullBytes)) // null values are ignored - { - long aTime = TarHelpers.ParseNumeric(aTimeBuffer); - _aTime = TarHelpers.GetDateTimeOffsetFromSecondsSinceEpoch(aTime); - } - ReadOnlySpan cTimeBuffer = buffer.Slice(FieldLocations.CTime, FieldLengths.CTime); - if (!cTimeBuffer.SequenceEqual(ArrayOf12NullBytes)) // An all nulls buffer is interpreted as MinValue + _aTime = ParseAsTimestamp(buffer.Slice(FieldLocations.ATime, FieldLengths.ATime)); + + _cTime = ParseAsTimestamp(buffer.Slice(FieldLocations.CTime, FieldLengths.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 + } + + private static DateTimeOffset ParseAsTimestamp(ReadOnlySpan buffer) + { + // When all bytes are zero, the timestamp is not initialized, and we map it to default. + bool allZeros = !buffer.ContainsAnyExcept((byte)0); + if (allZeros) { - long cTime = TarHelpers.ParseNumeric(cTimeBuffer); - _cTime = TarHelpers.GetDateTimeOffsetFromSecondsSinceEpoch(cTime); + return default(DateTimeOffset); } - // 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 + long time = TarHelpers.ParseNumeric(buffer); + return TarHelpers.GetDateTimeOffsetFromSecondsSinceEpoch(time); } // Reads the ustar prefix attribute. diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs index 421cf210efdd08..df33b14d35a94d 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs @@ -790,19 +790,8 @@ private int WriteGnuFields(Span buffer) if (_typeFlag is not TarEntryType.LongLink and not TarEntryType.LongPath) { - if (_aTime != default) - { - checksum += WriteAsTimestamp(_aTime, buffer.Slice(FieldLocations.ATime, FieldLengths.ATime)); - } - if (_cTime != default) - { - checksum += WriteAsTimestamp(_cTime, buffer.Slice(FieldLocations.CTime, FieldLengths.CTime)); - } - } - - if (_gnuUnusedBytes != null) - { - checksum += WriteLeftAlignedBytesAndGetChecksum(_gnuUnusedBytes, buffer.Slice(FieldLocations.GnuUnused, FieldLengths.AllGnuUnused)); + checksum += WriteAsTimestamp(_aTime, buffer.Slice(FieldLocations.ATime, FieldLengths.ATime)); + checksum += WriteAsTimestamp(_cTime, buffer.Slice(FieldLocations.CTime, FieldLengths.CTime)); } return checksum; @@ -1179,6 +1168,12 @@ private static int FormatOctal(long value, Span destination) // Writes the specified DateTimeOffset's Unix time seconds, and returns its checksum. private int WriteAsTimestamp(DateTimeOffset timestamp, Span destination) { + // For 'default' we leave the buffer zero-ed to indicate: "no timestamp". + if (timestamp == default) + { + return 0; + } + long unixTimeSeconds = timestamp.ToUnixTimeSeconds(); return FormatNumeric(unixTimeSeconds, destination); } diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.cs index 9306ef1997c888..35da5b566ac37f 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.cs @@ -82,10 +82,6 @@ internal sealed partial class TarHeader internal DateTimeOffset _aTime; internal DateTimeOffset _cTime; - // If the archive is GNU and the offset, longnames, unused, sparse, isextended and realsize - // fields have data, we store it to avoid data loss, but we don't yet expose it publicly. - internal byte[]? _gnuUnusedBytes; - // Constructor called when creating an entry with default common fields. internal TarHeader(TarEntryFormat format, string name = "", int mode = 0, DateTimeOffset mTime = default, TarEntryType typeFlag = TarEntryType.RegularFile) { @@ -112,7 +108,7 @@ internal TarHeader(TarEntryFormat format, TarEntryType typeFlag, TarHeader other _dataStream = other._dataStream; } - internal void InitializeExtendedAttributesWithExisting(IEnumerable> existing) + internal void AddExtendedAttributes(IEnumerable> existing) { Debug.Assert(_ea == null); Debug.Assert(existing != null); diff --git a/src/libraries/System.Formats.Tar/tests/TarEntry/PaxTarEntry.Conversion.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarEntry/PaxTarEntry.Conversion.Tests.cs index eb80726dbe063c..07f4c6b538ca0b 100644 --- a/src/libraries/System.Formats.Tar/tests/TarEntry/PaxTarEntry.Conversion.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarEntry/PaxTarEntry.Conversion.Tests.cs @@ -92,8 +92,8 @@ public void Constructor_ConversionFromV7_Write(TarEntryFormat originalEntryForma dataStream.Position = 0; originalEntry.DataStream = dataStream; - DateTimeOffset expectedATime = default; - DateTimeOffset expectedCTime = default; + DateTimeOffset? expectedATime = null; + DateTimeOffset? expectedCTime = null; if (originalEntry is GnuTarEntry gnuEntry) { @@ -108,11 +108,9 @@ public void Constructor_ConversionFromV7_Write(TarEntryFormat originalEntryForma } else if (originalEntry is PaxTarEntry paxEntry) { - expectedATime = GetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes, PaxEaATime); - expectedCTime = GetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes, PaxEaCTime); + expectedATime = TryGetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes, PaxEaATime); + expectedCTime = TryGetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes, PaxEaCTime); - Assert.Equal(paxEntry.ModificationTime, expectedATime); - Assert.Equal(paxEntry.ModificationTime, expectedCTime); // Can't change them, it's a read-only dictionary } @@ -140,19 +138,11 @@ public void Constructor_ConversionFromV7_Write(TarEntryFormat originalEntryForma Assert.Equal(contents, streamReader.ReadLine()); } - // atime and ctime should've been added automatically in the conversion constructor - // and should not be equal to the value of mtime, which was set on the original entry constructor + DateTimeOffset? atime = TryGetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes, PaxEaATime); + DateTimeOffset? ctime = TryGetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes, PaxEaCTime); - Assert.Contains(PaxEaATime, paxEntry.ExtendedAttributes); - Assert.Contains(PaxEaCTime, paxEntry.ExtendedAttributes); - DateTimeOffset atime = GetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes[PaxEaATime]); - DateTimeOffset ctime = GetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes[PaxEaCTime]); - - if (originalEntryFormat is TarEntryFormat.Pax or TarEntryFormat.Gnu) - { - Assert.Equal(expectedATime, atime); - Assert.Equal(expectedCTime, ctime); - } + Assert.Equal(expectedATime, atime); + Assert.Equal(expectedCTime, ctime); } } diff --git a/src/libraries/System.Formats.Tar/tests/TarEntry/TarEntry.Conversion.Tests.Base.cs b/src/libraries/System.Formats.Tar/tests/TarEntry/TarEntry.Conversion.Tests.Base.cs index fef2da357e8234..74b8b91982d508 100644 --- a/src/libraries/System.Formats.Tar/tests/TarEntry/TarEntry.Conversion.Tests.Base.cs +++ b/src/libraries/System.Formats.Tar/tests/TarEntry/TarEntry.Conversion.Tests.Base.cs @@ -69,10 +69,8 @@ private TarEntry GetFirstEntry(MemoryStream dataStream, TarEntryType entryType, if (format is TarEntryFormat.Pax) { PaxTarEntry paxEntry = firstEntry as PaxTarEntry; - Assert.Contains("atime", paxEntry.ExtendedAttributes); - Assert.Contains("ctime", paxEntry.ExtendedAttributes); - Assert.Equal(firstEntry.ModificationTime, GetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes, "atime")); - Assert.Equal(firstEntry.ModificationTime, GetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes, "ctime")); + Assert.DoesNotContain("atime", paxEntry.ExtendedAttributes); + Assert.DoesNotContain("ctime", paxEntry.ExtendedAttributes); } else if (format is TarEntryFormat.Gnu) { @@ -115,98 +113,64 @@ private TarEntry ConvertAndVerifyEntry(TarEntry originalEntry, TarEntryType entr Assert.Equal(originalPosixTarEntry.DeviceMinor, convertedPosixTarEntry.DeviceMinor); } + // 'null' indicates the originalEntry did not include a timestamp. + DateTimeOffset? expectedATime = null, expectedCTime = null; + if (originalEntry.Format is TarEntryFormat.Pax or TarEntryFormat.Gnu) + { + GetExpectedTimestampsFromOriginalPaxOrGnu(originalEntry, formatToConvert, out expectedATime, out expectedCTime); + } + if (formatToConvert is TarEntryFormat.Pax) { PaxTarEntry paxEntry = convertedEntry as PaxTarEntry; - if (originalEntry.Format is TarEntryFormat.Gnu) - { - GnuTarEntry gnuEntry = originalEntry as GnuTarEntry; - - DateTimeOffset expectedATime = gnuEntry.AccessTime; - DateTimeOffset expectedCTime = gnuEntry.ChangeTime; - - DateTimeOffset actualAccessTime = GetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes, PaxEaATime); - DateTimeOffset actualChangeTime = GetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes, PaxEaCTime); - - if (expectedATime == default) - { - AssertExtensions.GreaterThanOrEqualTo(actualAccessTime, paxEntry.ModificationTime); - } - else - { - expectedATime = expectedATime - _oneSecond; - AssertExtensions.GreaterThanOrEqualTo(expectedATime, actualAccessTime); - } - - if (expectedCTime == default) - { - AssertExtensions.GreaterThanOrEqualTo(actualChangeTime, paxEntry.ModificationTime); - } - else - { - expectedCTime = expectedCTime - _oneSecond; - AssertExtensions.GreaterThanOrEqualTo(expectedCTime, actualChangeTime); - } - } - else if (originalEntry.Format is TarEntryFormat.Pax) - { - PaxTarEntry originalPaxEntry = originalEntry as PaxTarEntry; - - DateTimeOffset expectedATime = GetDateTimeOffsetFromTimestampString(originalPaxEntry.ExtendedAttributes, PaxEaATime) - _oneSecond; - DateTimeOffset expectedCTime = GetDateTimeOffsetFromTimestampString(originalPaxEntry.ExtendedAttributes, PaxEaCTime) - _oneSecond; - - DateTimeOffset actualAccessTime = GetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes, PaxEaATime); - DateTimeOffset actualChangeTime = GetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes, PaxEaCTime); - - AssertExtensions.GreaterThanOrEqualTo(actualAccessTime, expectedATime); - AssertExtensions.GreaterThanOrEqualTo(actualChangeTime, expectedCTime); - } - else if (originalEntry.Format is TarEntryFormat.Ustar or TarEntryFormat.V7) - { - DateTimeOffset actualAccessTime = GetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes, PaxEaATime); - DateTimeOffset actualChangeTime = GetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes, PaxEaCTime); - - AssertExtensions.GreaterThanOrEqualTo(actualAccessTime, initialNow); - AssertExtensions.GreaterThanOrEqualTo(actualChangeTime, initialNow); - } + DateTimeOffset? actualAccessTime = TryGetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes, PaxEaATime); + DateTimeOffset? actualChangeTime = TryGetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes, PaxEaCTime); + Assert.Equal(expectedATime, actualAccessTime); + Assert.Equal(expectedCTime, actualChangeTime); } - - if (formatToConvert is TarEntryFormat.Gnu) + else if (formatToConvert is TarEntryFormat.Gnu) { GnuTarEntry gnuEntry = convertedEntry as GnuTarEntry; - if (originalEntry.Format is TarEntryFormat.Pax or TarEntryFormat.Gnu) - { - GetExpectedTimestampsFromOriginalPaxOrGnu(originalEntry, formatToConvert, out DateTimeOffset expectedATime, out DateTimeOffset expectedCTime); - AssertExtensions.GreaterThanOrEqualTo(gnuEntry.AccessTime, expectedATime); - AssertExtensions.GreaterThanOrEqualTo(gnuEntry.ChangeTime, expectedCTime); - } - else if (originalEntry.Format is TarEntryFormat.Ustar or TarEntryFormat.V7) - { - Assert.Equal(default, gnuEntry.AccessTime); - Assert.Equal(default, gnuEntry.ChangeTime); - } + Assert.Equal(expectedATime ?? default, gnuEntry.AccessTime); + Assert.Equal(expectedCTime ?? default, gnuEntry.ChangeTime); } return convertedEntry; } - private void GetExpectedTimestampsFromOriginalPaxOrGnu(TarEntry originalEntry, TarEntryFormat formatToConvert, out DateTimeOffset expectedATime, out DateTimeOffset expectedCTime) + private void GetExpectedTimestampsFromOriginalPaxOrGnu(TarEntry originalEntry, TarEntryFormat formatToConvert, out DateTimeOffset? expectedATime, out DateTimeOffset? expectedCTime) { Assert.True(originalEntry.Format is TarEntryFormat.Gnu or TarEntryFormat.Pax); if (originalEntry.Format is TarEntryFormat.Pax) { PaxTarEntry originalPaxEntry = originalEntry as PaxTarEntry; - Assert.Contains("atime", originalPaxEntry.ExtendedAttributes); // We are verifying that the original had an atime and ctime set - Assert.Contains("ctime", originalPaxEntry.ExtendedAttributes); // and that when converting to GNU we are _not_ preserving them - // And that instead, we are setting them to MinValue - expectedATime = formatToConvert is TarEntryFormat.Gnu ? default : GetDateTimeOffsetFromTimestampString(originalPaxEntry.ExtendedAttributes, "atime"); - expectedCTime = formatToConvert is TarEntryFormat.Gnu ? default : GetDateTimeOffsetFromTimestampString(originalPaxEntry.ExtendedAttributes, "ctime"); + + expectedATime = null; + if (originalPaxEntry.ExtendedAttributes.ContainsKey("atime")) + { + expectedATime = GetDateTimeOffsetFromTimestampString(originalPaxEntry.ExtendedAttributes, "atime"); + } + + expectedCTime = null; + if (originalPaxEntry.ExtendedAttributes.ContainsKey("ctime")) + { + expectedCTime = GetDateTimeOffsetFromTimestampString(originalPaxEntry.ExtendedAttributes, "ctime"); + } } else { GnuTarEntry originalGnuEntry = originalEntry as GnuTarEntry; expectedATime = originalGnuEntry.AccessTime; + // default means: no timestamp. + if (expectedATime == default(DateTimeOffset)) + { + expectedATime = null; + } expectedCTime = originalGnuEntry.ChangeTime; + if (expectedCTime == default(DateTimeOffset)) + { + expectedCTime = null; + } } } diff --git a/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.Tests.cs index f95a22335a1418..38a94515afa9af 100644 --- a/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.Tests.cs @@ -113,7 +113,7 @@ public void PaxExtendedAttribute_Roundtrips(string key, string value) using (var reader = new TarReader(stream)) { PaxTarEntry entry = Assert.IsType(reader.GetNextEntry()); - Assert.Equal(5, entry.ExtendedAttributes.Count); + Assert.Equal(3, entry.ExtendedAttributes.Count); Assert.Contains(KeyValuePair.Create(key, value), entry.ExtendedAttributes); Assert.Null(reader.GetNextEntry()); } diff --git a/src/libraries/System.Formats.Tar/tests/TarTestsBase.Pax.cs b/src/libraries/System.Formats.Tar/tests/TarTestsBase.Pax.cs index adfbea8fdaa02b..9426c36bd53943 100644 --- a/src/libraries/System.Formats.Tar/tests/TarTestsBase.Pax.cs +++ b/src/libraries/System.Formats.Tar/tests/TarTestsBase.Pax.cs @@ -90,6 +90,15 @@ private DateTimeOffset GetDateTimeOffsetFromSecondsSinceEpoch(decimal secondsSin private decimal GetSecondsSinceEpochFromDateTimeOffset(DateTimeOffset value) => ((decimal)(value.UtcDateTime - DateTime.UnixEpoch).Ticks) / TimeSpan.TicksPerSecond; + protected DateTimeOffset? TryGetDateTimeOffsetFromTimestampString(IReadOnlyDictionary ea, string fieldName) + { + if (!ea.ContainsKey(fieldName)) + { + return default; + } + return GetDateTimeOffsetFromTimestampString(ea[fieldName]); + } + protected DateTimeOffset GetDateTimeOffsetFromTimestampString(IReadOnlyDictionary ea, string fieldName) { Assert.Contains(fieldName, ea); @@ -117,11 +126,9 @@ protected void VerifyExtendedAttributeTimestamp(PaxTarEntry paxEntry, string fie protected void VerifyExtendedAttributeTimestamps(PaxTarEntry pax) { Assert.NotNull(pax.ExtendedAttributes); - AssertExtensions.GreaterThanOrEqualTo(pax.ExtendedAttributes.Count, 3); // Expect to at least collect mtime, ctime and atime + AssertExtensions.GreaterThanOrEqualTo(pax.ExtendedAttributes.Count, 1); // Expect to at least collect mtime VerifyExtendedAttributeTimestamp(pax, PaxEaMTime, MinimumTime); - VerifyExtendedAttributeTimestamp(pax, PaxEaATime, MinimumTime); - VerifyExtendedAttributeTimestamp(pax, PaxEaCTime, MinimumTime); } public static IEnumerable GetPaxExtendedAttributesRoundtripTestData() diff --git a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Entry.Pax.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Entry.Pax.Tests.cs index 006aace683716c..840cc8946ea3b7 100644 --- a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Entry.Pax.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Entry.Pax.Tests.cs @@ -182,12 +182,10 @@ public void WritePaxAttributes_CustomAttribute() Assert.NotNull(regularFile.ExtendedAttributes); // path, mtime, atime and ctime are always collected by default - AssertExtensions.GreaterThanOrEqualTo(regularFile.ExtendedAttributes.Count, 5); + AssertExtensions.GreaterThanOrEqualTo(regularFile.ExtendedAttributes.Count, 3); Assert.Contains(PaxEaName, regularFile.ExtendedAttributes); Assert.Contains(PaxEaMTime, regularFile.ExtendedAttributes); - Assert.Contains(PaxEaATime, regularFile.ExtendedAttributes); - Assert.Contains(PaxEaCTime, regularFile.ExtendedAttributes); Assert.Contains(expectedKey, regularFile.ExtendedAttributes); Assert.Equal(expectedValue, regularFile.ExtendedAttributes[expectedKey]); @@ -210,10 +208,8 @@ public void WritePaxAttributes_Timestamps_AutomaticallyAdded() { PaxTarEntry regularFile = reader.GetNextEntry() as PaxTarEntry; - AssertExtensions.GreaterThanOrEqualTo(regularFile.ExtendedAttributes.Count, 4); + AssertExtensions.GreaterThanOrEqualTo(regularFile.ExtendedAttributes.Count, 2); VerifyExtendedAttributeTimestamp(regularFile, PaxEaMTime, minimumTime); - VerifyExtendedAttributeTimestamp(regularFile, PaxEaATime, minimumTime); - VerifyExtendedAttributeTimestamp(regularFile, PaxEaCTime, minimumTime); } } @@ -269,13 +265,11 @@ public void WritePaxAttributes_LongGroupName_LongUserName() Assert.NotNull(regularFile.ExtendedAttributes); - // path, mtime, atime and ctime are always collected by default - AssertExtensions.GreaterThanOrEqualTo(regularFile.ExtendedAttributes.Count, 6); + // path, mtime are always collected by default + AssertExtensions.GreaterThanOrEqualTo(regularFile.ExtendedAttributes.Count, 4); Assert.Contains(PaxEaName, regularFile.ExtendedAttributes); Assert.Contains(PaxEaMTime, regularFile.ExtendedAttributes); - Assert.Contains(PaxEaATime, regularFile.ExtendedAttributes); - Assert.Contains(PaxEaCTime, regularFile.ExtendedAttributes); Assert.Contains(PaxEaUName, regularFile.ExtendedAttributes); Assert.Equal(userName, regularFile.ExtendedAttributes[PaxEaUName]); @@ -304,7 +298,7 @@ public void WritePaxAttributes_Name_AutomaticallyAdded() { PaxTarEntry regularFile = reader.GetNextEntry() as PaxTarEntry; - AssertExtensions.GreaterThanOrEqualTo(regularFile.ExtendedAttributes.Count, 4); + AssertExtensions.GreaterThanOrEqualTo(regularFile.ExtendedAttributes.Count, 2); Assert.Contains(PaxEaName, regularFile.ExtendedAttributes); } } @@ -332,7 +326,7 @@ public void WritePaxAttributes_LongLinkName_AutomaticallyAdded() { PaxTarEntry symlink = reader.GetNextEntry() as PaxTarEntry; - AssertExtensions.GreaterThanOrEqualTo(symlink.ExtendedAttributes.Count, 5); + AssertExtensions.GreaterThanOrEqualTo(symlink.ExtendedAttributes.Count, 3); Assert.Contains(PaxEaName, symlink.ExtendedAttributes); Assert.Equal("symlink", symlink.ExtendedAttributes[PaxEaName]); @@ -341,7 +335,7 @@ public void WritePaxAttributes_LongLinkName_AutomaticallyAdded() PaxTarEntry hardlink = reader.GetNextEntry() as PaxTarEntry; - AssertExtensions.GreaterThanOrEqualTo(hardlink.ExtendedAttributes.Count, 5); + AssertExtensions.GreaterThanOrEqualTo(hardlink.ExtendedAttributes.Count, 3); Assert.Contains(PaxEaName, hardlink.ExtendedAttributes); Assert.Equal("hardlink", hardlink.ExtendedAttributes[PaxEaName]); diff --git a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Entry.Pax.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Entry.Pax.Tests.cs index b20d1c78e1aa06..160444743a0882 100644 --- a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Entry.Pax.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Entry.Pax.Tests.cs @@ -198,12 +198,10 @@ public async Task WritePaxAttributes_CustomAttribute_Async() Assert.NotNull(regularFile.ExtendedAttributes); // path, mtime, atime and ctime are always collected by default - AssertExtensions.GreaterThanOrEqualTo(regularFile.ExtendedAttributes.Count, 5); + AssertExtensions.GreaterThanOrEqualTo(regularFile.ExtendedAttributes.Count, 3); Assert.Contains(PaxEaName, regularFile.ExtendedAttributes); Assert.Contains(PaxEaMTime, regularFile.ExtendedAttributes); - Assert.Contains(PaxEaATime, regularFile.ExtendedAttributes); - Assert.Contains(PaxEaCTime, regularFile.ExtendedAttributes); Assert.Contains(expectedKey, regularFile.ExtendedAttributes); Assert.Equal(expectedValue, regularFile.ExtendedAttributes[expectedKey]); @@ -228,10 +226,8 @@ public async Task WritePaxAttributes_Timestamps_AutomaticallyAdded_Async() { PaxTarEntry regularFile = await reader.GetNextEntryAsync() as PaxTarEntry; - AssertExtensions.GreaterThanOrEqualTo(regularFile.ExtendedAttributes.Count, 4); + AssertExtensions.GreaterThanOrEqualTo(regularFile.ExtendedAttributes.Count, 2); VerifyExtendedAttributeTimestamp(regularFile, PaxEaMTime, minimumTime); - VerifyExtendedAttributeTimestamp(regularFile, PaxEaATime, minimumTime); - VerifyExtendedAttributeTimestamp(regularFile, PaxEaCTime, minimumTime); } } } @@ -291,12 +287,10 @@ public async Task WritePaxAttributes_LongGroupName_LongUserName_Async() Assert.NotNull(regularFile.ExtendedAttributes); // path, mtime, atime and ctime are always collected by default - AssertExtensions.GreaterThanOrEqualTo(regularFile.ExtendedAttributes.Count, 6); + AssertExtensions.GreaterThanOrEqualTo(regularFile.ExtendedAttributes.Count, 4); Assert.Contains(PaxEaName, regularFile.ExtendedAttributes); Assert.Contains(PaxEaMTime, regularFile.ExtendedAttributes); - Assert.Contains(PaxEaATime, regularFile.ExtendedAttributes); - Assert.Contains(PaxEaCTime, regularFile.ExtendedAttributes); Assert.Contains(PaxEaUName, regularFile.ExtendedAttributes); Assert.Equal(userName, regularFile.ExtendedAttributes[PaxEaUName]); @@ -325,7 +319,7 @@ public async Task WritePaxAttributes_Name_AutomaticallyAdded_Async() { PaxTarEntry regularFile = await reader.GetNextEntryAsync() as PaxTarEntry; - AssertExtensions.GreaterThanOrEqualTo(regularFile.ExtendedAttributes.Count, 4); + AssertExtensions.GreaterThanOrEqualTo(regularFile.ExtendedAttributes.Count, 2); Assert.Contains(PaxEaName, regularFile.ExtendedAttributes); } } @@ -353,7 +347,7 @@ public async Task WritePaxAttributes_LongLinkName_AutomaticallyAdded_Async() { PaxTarEntry symlink = await reader.GetNextEntryAsync() as PaxTarEntry; - AssertExtensions.GreaterThanOrEqualTo(symlink.ExtendedAttributes.Count, 5); + AssertExtensions.GreaterThanOrEqualTo(symlink.ExtendedAttributes.Count, 3); Assert.Contains(PaxEaName, symlink.ExtendedAttributes); Assert.Equal("symlink", symlink.ExtendedAttributes[PaxEaName]); @@ -362,7 +356,7 @@ public async Task WritePaxAttributes_LongLinkName_AutomaticallyAdded_Async() PaxTarEntry hardlink = await reader.GetNextEntryAsync() as PaxTarEntry; - AssertExtensions.GreaterThanOrEqualTo(hardlink.ExtendedAttributes.Count, 5); + AssertExtensions.GreaterThanOrEqualTo(hardlink.ExtendedAttributes.Count, 3); Assert.Contains(PaxEaName, hardlink.ExtendedAttributes); Assert.Equal("hardlink", hardlink.ExtendedAttributes[PaxEaName]); From 925092b0fd3e8c6151adcf7dd70f6c3781bd0230 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 20 May 2025 14:38:09 +0200 Subject: [PATCH 2/5] Add back attribute lists in the XML docs. --- .../src/System/Formats/Tar/PaxTarEntry.cs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/PaxTarEntry.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/PaxTarEntry.cs index 4226f180479c96..30397b03b27ad0 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/PaxTarEntry.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/PaxTarEntry.cs @@ -53,6 +53,12 @@ public PaxTarEntry(TarEntryType entryType, string entryName) /// In all platforms: , , , . /// In Unix platforms only: , and . /// + /// The specified are additional attributes to be used for the entry. + /// It may include PAX attributes like: + /// + /// Access time, under the name atime, as a number. + /// Change time, under the name ctime, as a number. + /// /// /// or is . /// is empty. @@ -104,7 +110,18 @@ public PaxTarEntry(TarEntry other) /// /// Returns the extended attributes for this entry. /// - /// The extended attributes are specified when constructing an entry and updated with additional attributes when the entry is written. Use to append custom extended attributes. + /// The extended attributes are specified when constructing an entry and updated with additional attributes when the entry is written. Use to append custom extended attributes. + /// The following common PAX attributes may be included: + /// + /// Modification time, under the name mtime, as a number. + /// Access time, under the name atime, as a number. + /// Change time, under the name ctime, as a number. + /// Path, under the name path, as a string. + /// Group name, under the name gname, as a string. + /// User name, under the name uname, as a string. + /// File length, under the name size, as an . + /// + /// public IReadOnlyDictionary ExtendedAttributes => _readOnlyExtendedAttributes ??= _header.ExtendedAttributes.AsReadOnly(); // Determines if the current instance's entry type supports setting a data stream. From 2b4f6c32bac39b36576bb9e577fd4191c03cf043 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 20 May 2025 20:03:31 +0200 Subject: [PATCH 3/5] Copy atime/ctime. --- .../System.Formats.Tar/src/System/Formats/Tar/TarHeader.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.cs index 35da5b566ac37f..bdf2012ae181d9 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.cs @@ -106,6 +106,8 @@ internal TarHeader(TarEntryFormat format, TarEntryType typeFlag, TarHeader other _checksum = other._checksum; _linkName = other._linkName; _dataStream = other._dataStream; + _aTime = other._aTime; + _cTime = other._cTime; } internal void AddExtendedAttributes(IEnumerable> existing) From 48e68300828b31066dd20b0be8333b6d524bc1ce Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 20 May 2025 21:00:47 +0200 Subject: [PATCH 4/5] Move atime/ctime copy back to GnuTarEntry ctor. --- .../src/System/Formats/Tar/GnuTarEntry.cs | 17 ++++++++++++++++- .../src/System/Formats/Tar/TarHeader.cs | 2 -- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/GnuTarEntry.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/GnuTarEntry.cs index 04587f698823ad..c1bb06cb8e7646 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/GnuTarEntry.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/GnuTarEntry.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics; + namespace System.Formats.Tar { /// @@ -47,7 +49,20 @@ public GnuTarEntry(TarEntryType entryType, string entryName) /// The entry type of is not supported for conversion to the GNU format. public GnuTarEntry(TarEntry other) : base(other, TarEntryFormat.Gnu) - { } + { + // Some tools don't accept GNU entries that have an atime/ctime. + // We only copy for round-tripping between GnuTarEntries. + if (other is GnuTarEntry gnuOther) + { + _header._aTime = gnuOther.AccessTime; + _header._cTime = gnuOther.ChangeTime; + } + else + { + Debug.Assert(_header._aTime == default); + Debug.Assert(_header._cTime == default); + } + } /// /// 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. diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.cs index bdf2012ae181d9..35da5b566ac37f 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.cs @@ -106,8 +106,6 @@ internal TarHeader(TarEntryFormat format, TarEntryType typeFlag, TarHeader other _checksum = other._checksum; _linkName = other._linkName; _dataStream = other._dataStream; - _aTime = other._aTime; - _cTime = other._cTime; } internal void AddExtendedAttributes(IEnumerable> existing) From d4accde7effedeae52de52828bd71a0564572df7 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 21 May 2025 08:48:05 +0200 Subject: [PATCH 5/5] Test conversion with atime/ctime set. --- .../src/System/Formats/Tar/GnuTarEntry.cs | 4 +- .../TarEntry/GnuTarEntry.Conversion.Tests.cs | 2 + .../TarEntry/PaxTarEntry.Conversion.Tests.cs | 2 + .../TarEntry.Conversion.Tests.Base.cs | 52 ++++++++------- .../UstarTarEntry.Conversion.Tests.cs | 2 + .../TarEntry/V7TarEntry.Conversion.Tests.cs | 2 + .../tests/TarTestsBase.Pax.cs | 9 --- .../System.Formats.Tar/tests/TarTestsBase.cs | 65 +++++++++++++++++-- 8 files changed, 95 insertions(+), 43 deletions(-) diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/GnuTarEntry.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/GnuTarEntry.cs index c1bb06cb8e7646..89806cb98828a3 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/GnuTarEntry.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/GnuTarEntry.cs @@ -50,8 +50,8 @@ public GnuTarEntry(TarEntryType entryType, string entryName) public GnuTarEntry(TarEntry other) : base(other, TarEntryFormat.Gnu) { - // Some tools don't accept GNU entries that have an atime/ctime. - // We only copy for round-tripping between GnuTarEntries. + // Some tools don't accept Gnu entries that have an atime/ctime. + // We only copy atime/ctime for round-tripping between GnuTarEntries and clear it for other formats. if (other is GnuTarEntry gnuOther) { _header._aTime = gnuOther.AccessTime; diff --git a/src/libraries/System.Formats.Tar/tests/TarEntry/GnuTarEntry.Conversion.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarEntry/GnuTarEntry.Conversion.Tests.cs index c1ba5c62bc698c..ee5c1e31bcb6b5 100644 --- a/src/libraries/System.Formats.Tar/tests/TarEntry/GnuTarEntry.Conversion.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarEntry/GnuTarEntry.Conversion.Tests.cs @@ -13,6 +13,8 @@ namespace System.Formats.Tar.Tests { public class GnuTarEntry_Conversion_Tests : TarTestsConversionBase { + protected override TarEntryFormat FormatUnderTest => TarEntryFormat.Gnu; + [Fact] public void Constructor_ConversionFromV7_RegularFile() => TestConstructionConversion(TarEntryType.RegularFile, TarEntryFormat.V7, TarEntryFormat.Gnu); diff --git a/src/libraries/System.Formats.Tar/tests/TarEntry/PaxTarEntry.Conversion.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarEntry/PaxTarEntry.Conversion.Tests.cs index 07f4c6b538ca0b..ba4eec334d858c 100644 --- a/src/libraries/System.Formats.Tar/tests/TarEntry/PaxTarEntry.Conversion.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarEntry/PaxTarEntry.Conversion.Tests.cs @@ -13,6 +13,8 @@ namespace System.Formats.Tar.Tests { public class PaxTarEntry_Conversion_Tests : TarTestsConversionBase { + protected override TarEntryFormat FormatUnderTest => TarEntryFormat.Pax; + [Fact] public void Constructor_ConversionFromV7_RegularFile() => TestConstructionConversion(TarEntryType.RegularFile, TarEntryFormat.V7, TarEntryFormat.Pax); diff --git a/src/libraries/System.Formats.Tar/tests/TarEntry/TarEntry.Conversion.Tests.Base.cs b/src/libraries/System.Formats.Tar/tests/TarEntry/TarEntry.Conversion.Tests.Base.cs index 74b8b91982d508..f35052d453a163 100644 --- a/src/libraries/System.Formats.Tar/tests/TarEntry/TarEntry.Conversion.Tests.Base.cs +++ b/src/libraries/System.Formats.Tar/tests/TarEntry/TarEntry.Conversion.Tests.Base.cs @@ -6,14 +6,17 @@ namespace System.Formats.Tar.Tests { - public class TarTestsConversionBase : TarTestsBase + public abstract class TarTestsConversionBase : TarTestsBase { + protected abstract TarEntryFormat FormatUnderTest { get; } + private readonly TimeSpan _oneSecond = TimeSpan.FromSeconds(1); protected void TestConstructionConversion( TarEntryType originalEntryType, TarEntryFormat firstFormat, - TarEntryFormat formatToConvert) + TarEntryFormat formatToConvert, + bool setATimeCTime = false) { DateTimeOffset now = DateTimeOffset.UtcNow; @@ -21,7 +24,7 @@ protected void TestConstructionConversion( TarEntryType actualEntryType = GetTarEntryTypeForTarEntryFormat(originalEntryType, firstFormat); - TarEntry firstEntry = GetFirstEntry(dataStream, actualEntryType, firstFormat); + TarEntry firstEntry = GetFirstEntry(dataStream, actualEntryType, firstFormat, setATimeCTime); TarEntry otherEntry = ConvertAndVerifyEntry(firstEntry, originalEntryType, formatToConvert, now); } @@ -42,9 +45,9 @@ protected void TestConstructionConversionBackAndForth( ConvertAndVerifyEntry(otherEntry, firstAndLastEntryType, firstAndLastFormat, secondNow); // Convert back to original format } - private TarEntry GetFirstEntry(MemoryStream dataStream, TarEntryType entryType, TarEntryFormat format) + private TarEntry GetFirstEntry(MemoryStream dataStream, TarEntryType entryType, TarEntryFormat format, bool setATimeCTime = false) { - TarEntry firstEntry = InvokeTarEntryCreationConstructor(format, entryType, "file.txt"); + TarEntry firstEntry = InvokeTarEntryCreationConstructor(format, entryType, "file.txt", setATimeCTime); firstEntry.Gid = TestGid; firstEntry.Uid = TestUid; @@ -66,19 +69,6 @@ private TarEntry GetFirstEntry(MemoryStream dataStream, TarEntryType entryType, posixTarEntry.DeviceMinor = entryType is TarEntryType.BlockDevice ? TestBlockDeviceMinor : TestCharacterDeviceMinor; } - if (format is TarEntryFormat.Pax) - { - PaxTarEntry paxEntry = firstEntry as PaxTarEntry; - Assert.DoesNotContain("atime", paxEntry.ExtendedAttributes); - Assert.DoesNotContain("ctime", paxEntry.ExtendedAttributes); - } - else if (format is TarEntryFormat.Gnu) - { - GnuTarEntry gnuEntry = firstEntry as GnuTarEntry; - Assert.Equal(default, gnuEntry.AccessTime); - Assert.Equal(default, gnuEntry.ChangeTime); - } - return firstEntry; } @@ -114,10 +104,12 @@ private TarEntry ConvertAndVerifyEntry(TarEntry originalEntry, TarEntryType entr } // 'null' indicates the originalEntry did not include a timestamp. - DateTimeOffset? expectedATime = null, expectedCTime = null; - if (originalEntry.Format is TarEntryFormat.Pax or TarEntryFormat.Gnu) + GetTimestampsFromEntry(originalEntry, out DateTimeOffset? expectedATime, out DateTimeOffset? expectedCTime); + // Converting to Gnu only preserves timestamps when the original is also Gnu. + if (formatToConvert == TarEntryFormat.Gnu && originalEntry.Format != TarEntryFormat.Gnu) { - GetExpectedTimestampsFromOriginalPaxOrGnu(originalEntry, formatToConvert, out expectedATime, out expectedCTime); + expectedATime = null; + expectedCTime = null; } if (formatToConvert is TarEntryFormat.Pax) @@ -138,9 +130,8 @@ private TarEntry ConvertAndVerifyEntry(TarEntry originalEntry, TarEntryType entr return convertedEntry; } - private void GetExpectedTimestampsFromOriginalPaxOrGnu(TarEntry originalEntry, TarEntryFormat formatToConvert, out DateTimeOffset? expectedATime, out DateTimeOffset? expectedCTime) + private void GetTimestampsFromEntry(TarEntry originalEntry, out DateTimeOffset? expectedATime, out DateTimeOffset? expectedCTime) { - Assert.True(originalEntry.Format is TarEntryFormat.Gnu or TarEntryFormat.Pax); if (originalEntry.Format is TarEntryFormat.Pax) { PaxTarEntry originalPaxEntry = originalEntry as PaxTarEntry; @@ -157,7 +148,7 @@ private void GetExpectedTimestampsFromOriginalPaxOrGnu(TarEntry originalEntry, T expectedCTime = GetDateTimeOffsetFromTimestampString(originalPaxEntry.ExtendedAttributes, "ctime"); } } - else + else if (originalEntry.Format is TarEntryFormat.Gnu) { GnuTarEntry originalGnuEntry = originalEntry as GnuTarEntry; expectedATime = originalGnuEntry.AccessTime; @@ -172,7 +163,12 @@ private void GetExpectedTimestampsFromOriginalPaxOrGnu(TarEntry originalEntry, T expectedCTime = null; } } - + else + { + // Format has no timestamps. + expectedATime = null; + expectedCTime = null; + } } protected TarEntry InvokeTarEntryConversionConstructor(TarEntryFormat targetFormat, TarEntry other) @@ -184,5 +180,11 @@ protected TarEntry InvokeTarEntryConversionConstructor(TarEntryFormat targetForm TarEntryFormat.Gnu => new GnuTarEntry(other), _ => throw new InvalidDataException($"Unexpected format: {targetFormat}") }; + + [Fact] + public void Constructor_ConversionFromGnu_ATimeCTime() => TestConstructionConversion(TarEntryType.RegularFile, TarEntryFormat.Gnu, FormatUnderTest, setATimeCTime: true); + + [Fact] + public void Constructor_ConversionFromPax_ATimeCTime() => TestConstructionConversion(TarEntryType.RegularFile, TarEntryFormat.Pax, FormatUnderTest, setATimeCTime: true); } } diff --git a/src/libraries/System.Formats.Tar/tests/TarEntry/UstarTarEntry.Conversion.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarEntry/UstarTarEntry.Conversion.Tests.cs index 4cec52322be4d8..ef366d415f68c2 100644 --- a/src/libraries/System.Formats.Tar/tests/TarEntry/UstarTarEntry.Conversion.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarEntry/UstarTarEntry.Conversion.Tests.cs @@ -13,6 +13,8 @@ namespace System.Formats.Tar.Tests { public class UstarTarEntry_Conversion_Tests : TarTestsConversionBase { + protected override TarEntryFormat FormatUnderTest => TarEntryFormat.Ustar; + [Fact] public void Constructor_ConversionFromV7_RegularFile() => TestConstructionConversion(TarEntryType.RegularFile, TarEntryFormat.V7, TarEntryFormat.Ustar); diff --git a/src/libraries/System.Formats.Tar/tests/TarEntry/V7TarEntry.Conversion.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarEntry/V7TarEntry.Conversion.Tests.cs index 8856ec4d54809a..e6293999af0140 100644 --- a/src/libraries/System.Formats.Tar/tests/TarEntry/V7TarEntry.Conversion.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarEntry/V7TarEntry.Conversion.Tests.cs @@ -13,6 +13,8 @@ namespace System.Formats.Tar.Tests { public class V7TarEntry_Conversion_Tests : TarTestsConversionBase { + protected override TarEntryFormat FormatUnderTest => TarEntryFormat.V7; + [Fact] public void Constructor_Conversion_UnsupportedEntryTypes_Ustar() { diff --git a/src/libraries/System.Formats.Tar/tests/TarTestsBase.Pax.cs b/src/libraries/System.Formats.Tar/tests/TarTestsBase.Pax.cs index 9426c36bd53943..9946c6ce437004 100644 --- a/src/libraries/System.Formats.Tar/tests/TarTestsBase.Pax.cs +++ b/src/libraries/System.Formats.Tar/tests/TarTestsBase.Pax.cs @@ -87,9 +87,6 @@ protected void VerifyFifo(PaxTarEntry fifo) private DateTimeOffset GetDateTimeOffsetFromSecondsSinceEpoch(decimal secondsSinceUnixEpoch) => new DateTimeOffset((long)(secondsSinceUnixEpoch * TimeSpan.TicksPerSecond) + DateTime.UnixEpoch.Ticks, TimeSpan.Zero); - private decimal GetSecondsSinceEpochFromDateTimeOffset(DateTimeOffset value) => - ((decimal)(value.UtcDateTime - DateTime.UnixEpoch).Ticks) / TimeSpan.TicksPerSecond; - protected DateTimeOffset? TryGetDateTimeOffsetFromTimestampString(IReadOnlyDictionary ea, string fieldName) { if (!ea.ContainsKey(fieldName)) @@ -111,12 +108,6 @@ protected DateTimeOffset GetDateTimeOffsetFromTimestampString(string strNumber) return GetDateTimeOffsetFromSecondsSinceEpoch(secondsSinceEpoch); } - protected string GetTimestampStringFromDateTimeOffset(DateTimeOffset timestamp) - { - decimal secondsSinceEpoch = GetSecondsSinceEpochFromDateTimeOffset(timestamp); - return secondsSinceEpoch.ToString("G", CultureInfo.InvariantCulture); - } - protected void VerifyExtendedAttributeTimestamp(PaxTarEntry paxEntry, string fieldName, DateTimeOffset minimumTime) { DateTimeOffset converted = GetDateTimeOffsetFromTimestampString(paxEntry.ExtendedAttributes, fieldName); diff --git a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs index e8df0e4a9a3482..b68493996a095d 100644 --- a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs +++ b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Globalization; using System.IO; using System.Linq; using System.Runtime.CompilerServices; @@ -222,6 +223,15 @@ protected TarTestsBase() 8); } + private static decimal GetSecondsSinceEpochFromDateTimeOffset(DateTimeOffset value) => + ((decimal)(value.UtcDateTime - DateTime.UnixEpoch).Ticks) / TimeSpan.TicksPerSecond; + + protected static string GetTimestampStringFromDateTimeOffset(DateTimeOffset timestamp) + { + decimal secondsSinceEpoch = GetSecondsSinceEpochFromDateTimeOffset(timestamp); + return secondsSinceEpoch.ToString("G", CultureInfo.InvariantCulture); + } + protected static string GetTestCaseUnarchivedFolderPath(string testCaseName) => Path.Join(Directory.GetCurrentDirectory(), "unarchived", testCaseName); @@ -493,16 +503,57 @@ protected static TarEntryType GetTarEntryTypeForTarEntryFormat(TarEntryType entr return entryType; } - protected static TarEntry InvokeTarEntryCreationConstructor(TarEntryFormat targetFormat, TarEntryType entryType, string entryName) - => targetFormat switch + protected TarEntry InvokeTarEntryCreationConstructor(TarEntryFormat targetFormat, TarEntryType entryType, string entryName, bool setATimeCTime = false) + { + TarEntry entry = (targetFormat, setATimeCTime) switch { - TarEntryFormat.V7 => new V7TarEntry(entryType, entryName), - TarEntryFormat.Ustar => new UstarTarEntry(entryType, entryName), - TarEntryFormat.Pax => new PaxTarEntry(entryType, entryName), - TarEntryFormat.Gnu => new GnuTarEntry(entryType, entryName), + (TarEntryFormat.V7, _) => new V7TarEntry(entryType, entryName), + (TarEntryFormat.Ustar, _) => new UstarTarEntry(entryType, entryName), + (TarEntryFormat.Pax, true) => new PaxTarEntry(entryType, entryName, CreateATimeCTimeExtendedAttributes()), + (TarEntryFormat.Pax, false) => new PaxTarEntry(entryType, entryName), + (TarEntryFormat.Gnu, _) => new GnuTarEntry(entryType, entryName), _ => throw new InvalidDataException($"Unexpected format: {targetFormat}") }; + if (entry is GnuTarEntry gnuTarEntry) + { + if (setATimeCTime) + { + gnuTarEntry.AccessTime = TestAccessTime; + gnuTarEntry.ChangeTime = TestChangeTime; + } + else + { + Assert.Equal(default, gnuTarEntry.AccessTime); + Assert.Equal(default, gnuTarEntry.ChangeTime); + } + } + else if (entry is PaxTarEntry paxTarEntry) + { + if (setATimeCTime) + { + Assert.Contains("ctime", paxTarEntry.ExtendedAttributes); + Assert.Contains("atime", paxTarEntry.ExtendedAttributes); + } + else + { + Assert.DoesNotContain("ctime", paxTarEntry.ExtendedAttributes); + Assert.DoesNotContain("atime", paxTarEntry.ExtendedAttributes); + } + } + + return entry; + } + + private Dictionary CreateATimeCTimeExtendedAttributes() + { + return new Dictionary() + { + { "atime", GetTimestampStringFromDateTimeOffset(TestAccessTime) }, + { "ctime", GetTimestampStringFromDateTimeOffset(TestChangeTime) }, + }; + } + public static IEnumerable GetTestTarFormats() { foreach (TestTarFormat testFormat in Enum.GetValues()) @@ -837,7 +888,7 @@ internal enum NameCapabilities Unlimited } - internal static void WriteTarArchiveWithOneEntry(Stream s, TarEntryFormat entryFormat, TarEntryType entryType) + internal void WriteTarArchiveWithOneEntry(Stream s, TarEntryFormat entryFormat, TarEntryType entryType) { using TarWriter writer = new(s, leaveOpen: true);