-
Notifications
You must be signed in to change notification settings - Fork 5.1k
System.Formats.Tar: default to no ctime/atime. #115778
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
2cd6e15
925092b
2b4f6c3
48e6830
d4accde
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,7 @@ internal PaxTarEntry(TarHeader header, TarReader readerOfOrigin) | |
} | ||
|
||
/// <summary> | ||
/// Initializes a new <see cref="PaxTarEntry"/> instance with the specified entry type, entry name, and the default extended attributes. | ||
/// Initializes a new <see cref="PaxTarEntry"/> instance with the specified entry type and entry name. | ||
/// </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> | ||
|
@@ -30,20 +30,7 @@ internal PaxTarEntry(TarHeader header, TarReader readerOfOrigin) | |
/// <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> | ||
/// </list> | ||
/// <para>Use the <see cref="PaxTarEntry(TarEntryType, string, IEnumerable{KeyValuePair{string, string}})"/> constructor to include additional extended attributes when creating the entry.</para> | ||
/// <para>The following entries are always found in the Extended Attributes dictionary of any PAX entry:</para> | ||
/// <list type="bullet"> | ||
/// <item>Modification time, under the name <c>mtime</c>, as a <see cref="double"/> number.</item> | ||
/// <item>Access time, under the name <c>atime</c>, as a <see cref="double"/> number.</item> | ||
/// <item>Change time, under the name <c>ctime</c>, as a <see cref="double"/> number.</item> | ||
/// <item>Path, under the name <c>path</c>, as a string.</item> | ||
/// </list> | ||
/// <para>The following entries are only found in the Extended Attributes dictionary of a PAX entry if certain conditions are met:</para> | ||
/// <list type="bullet"> | ||
/// <item>Group name, under the name <c>gname</c>, as a string, if it is larger than 32 bytes.</item> | ||
/// <item>User name, under the name <c>uname</c>, as a string, if it is larger than 32 bytes.</item> | ||
/// <item>File length, under the name <c>size</c>, as an <see cref="int"/>, if the string representation of the number is larger than 12 bytes.</item> | ||
/// </list> | ||
/// <para>Use the <see cref="PaxTarEntry(TarEntryType, string, IEnumerable{KeyValuePair{string, string}})"/> constructor to include extended attributes when creating the entry.</para> | ||
/// </remarks> | ||
/// <exception cref="ArgumentNullException"><paramref name="entryName"/> is <see langword="null"/>.</exception> | ||
/// <exception cref="ArgumentException"><para><paramref name="entryName"/> is empty.</para> | ||
|
@@ -53,13 +40,10 @@ public PaxTarEntry(TarEntryType entryType, string entryName) | |
: base(entryType, entryName, TarEntryFormat.Pax, isGea: false) | ||
{ | ||
_header._prefix = string.Empty; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to mark this as a breaking change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not including atime/ctime is default behavior of other tools and we're now following that behavior. I don't think it's likely we'll break someone by no longer setting these timestamps to UtcNow by default. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ericstj what do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't hurt to document the behavior changes, why we did them, and why we think they are not going to be breaking. We can use a breaking change notice to do so. Let me get that started and tag you to have your input. |
||
|
||
Debug.Assert(_header._mTime != default); | ||
AddNewAccessAndChangeTimestampsIfNotExist(useMTime: true); | ||
tmds marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/// <summary> | ||
/// Initializes a new <see cref="PaxTarEntry"/> instance with the specified entry type, entry name and Extended Attributes enumeration. | ||
/// Initializes a new <see cref="PaxTarEntry"/> instance with the specified entry type, entry name and extended attributes. | ||
/// </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> | ||
|
@@ -69,19 +53,11 @@ public PaxTarEntry(TarEntryType entryType, string entryName) | |
/// <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> | ||
/// </list> | ||
/// The specified <paramref name="extendedAttributes"/> get appended to the default attributes, unless the specified enumeration overrides any of them. | ||
/// <para>The following entries are always found in the Extended Attributes dictionary of any PAX entry:</para> | ||
/// The specified <paramref name="extendedAttributes"/> are additional attributes to be used for the entry. | ||
/// <para>It may include PAX attributes like:</para> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've updated this list to the things I think a user may want to set. |
||
/// <list type="bullet"> | ||
/// <item>Modification time, under the name <c>mtime</c>, as a <see cref="double"/> number.</item> | ||
/// <item>Access time, under the name <c>atime</c>, as a <see cref="double"/> number.</item> | ||
/// <item>Change time, under the name <c>ctime</c>, as a <see cref="double"/> number.</item> | ||
/// <item>Path, under the name <c>path</c>, as a string.</item> | ||
/// </list> | ||
/// <para>The following entries are only found in the Extended Attributes dictionary of a PAX entry if certain conditions are met:</para> | ||
/// <list type="bullet"> | ||
/// <item>Group name, under the name <c>gname</c>, as a string, if it is larger than 32 bytes.</item> | ||
/// <item>User name, under the name <c>uname</c>, as a string, if it is larger than 32 bytes.</item> | ||
/// <item>File length, under the name <c>size</c>, as an <see cref="int"/>, if the string representation of the number is larger than 12 bytes.</item> | ||
/// </list> | ||
/// </remarks> | ||
/// <exception cref="ArgumentNullException"><paramref name="extendedAttributes"/> or <paramref name="entryName"/> is <see langword="null"/>.</exception> | ||
|
@@ -94,10 +70,7 @@ public PaxTarEntry(TarEntryType entryType, string entryName, IEnumerable<KeyValu | |
ArgumentNullException.ThrowIfNull(extendedAttributes); | ||
|
||
_header._prefix = string.Empty; | ||
_header.InitializeExtendedAttributesWithExisting(extendedAttributes); | ||
|
||
Debug.Assert(_header._mTime != default); | ||
AddNewAccessAndChangeTimestampsIfNotExist(useMTime: true); | ||
_header.AddExtendedAttributes(extendedAttributes); | ||
} | ||
|
||
/// <summary> | ||
|
@@ -119,72 +92,39 @@ 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); | ||
} | ||
|
||
/// <summary> | ||
/// Returns the extended attributes for this entry. | ||
/// </summary> | ||
/// <remarks>The extended attributes are specified when constructing an entry. Use <see cref="PaxTarEntry(TarEntryType, string, IEnumerable{KeyValuePair{string, string}})"/> to append your own enumeration of extended attributes to the current entry on top of the default ones. Use <see cref="PaxTarEntry(TarEntryType, string)"/> to only use the default extended attributes. | ||
/// <para>The following entries are always found in the Extended Attributes dictionary of any PAX entry:</para> | ||
/// <remarks>The extended attributes are specified when constructing an entry and updated with additional attributes when the entry is written. Use <see cref="PaxTarEntry(TarEntryType, string, IEnumerable{KeyValuePair{string, string}})"/> to append custom extended attributes. | ||
/// <para>The following common PAX attributes may be included:</para> | ||
/// <list type="bullet"> | ||
/// <item>Modification time, under the name <c>mtime</c>, as a <see cref="double"/> number.</item> | ||
/// <item>Access time, under the name <c>atime</c>, as a <see cref="double"/> number.</item> | ||
/// <item>Change time, under the name <c>ctime</c>, as a <see cref="double"/> number.</item> | ||
/// <item>Path, under the name <c>path</c>, as a string.</item> | ||
/// </list> | ||
/// <para>The following entries are only found in the Extended Attributes dictionary of a PAX entry if certain conditions are met:</para> | ||
/// <list type="bullet"> | ||
/// <item>Group name, under the name <c>gname</c>, as a string, if it is larger than 32 bytes.</item> | ||
/// <item>User name, under the name <c>uname</c>, as a string, if it is larger than 32 bytes.</item> | ||
/// <item>File length, under the name <c>size</c>, as an <see cref="int"/>, if the string representation of the number is larger than 12 bytes.</item> | ||
/// <item>Group name, under the name <c>gname</c>, as a string.</item> | ||
/// <item>User name, under the name <c>uname</c>, as a string.</item> | ||
/// <item>File length, under the name <c>size</c>, as an <see cref="int"/>.</item> | ||
/// </list> | ||
/// </remarks> | ||
public IReadOnlyDictionary<string, string> 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; | ||
} | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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<string, string>? 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<int>(buffer.Slice(FieldLocations.Mode, FieldLengths.Mode)), | ||
mTime: TarHelpers.GetDateTimeOffsetFromSecondsSinceEpoch(TarHelpers.ParseNumeric<long>(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<byte> buffer) | |
// Throws if any conversion fails. | ||
private void ReadGnuAttributes(ReadOnlySpan<byte> buffer) | ||
{ | ||
// Convert byte arrays | ||
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 | ||
_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 | ||
ericstj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
private static DateTimeOffset ParseAsTimestamp(ReadOnlySpan<byte> buffer) | ||
{ | ||
// When all bytes are zero, the timestamp is not initialized, and we map it to default. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Technically is an all-zero timestamp a valid timestamp? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, a valid timestamp is never serialized as all zero bytes. |
||
bool allZeros = !buffer.ContainsAnyExcept((byte)0); | ||
if (allZeros) | ||
{ | ||
long cTime = TarHelpers.ParseNumeric<long>(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<long>(buffer); | ||
return TarHelpers.GetDateTimeOffsetFromSecondsSinceEpoch(time); | ||
} | ||
|
||
// Reads the ustar prefix attribute. | ||
|
Uh oh!
There was an error while loading. Please reload this page.