Skip to content

Prevent exception when attempting to write PaxTarEntry obtained from TarReader #75237

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.

Sign up for GitHub

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

Closed
carlossanlop wants to merge 8 commits into from
Closed

Prevent exception when attempting to write PaxTarEntry obtained from TarReader #75237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3bbdbd0
Prevent exception when collecting extended attributes before writing …
carlossanlop Sep 8, 2022
a2e9d66
Add tests
carlossanlop Sep 8, 2022
b53f9c6
Proper method name
carlossanlop Sep 8, 2022
6c25b36
Allow dictionary with reserved keys only when reading from an archive…
carlossanlop Sep 20, 2022
69981f9
Tests, sync and async
carlossanlop Sep 20, 2022
42cfe4f
Address minor feedback suggestions.
carlossanlop Sep 20, 2022
a27de47
Merge conflict fix
carlossanlop Sep 20, 2022
2fdaeb3
Escape test chars
carlossanlop Sep 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/libraries/System.Formats.Tar/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,9 @@
<data name="TarPosixFormatExpected" xml:space="preserve">
<value>A POSIX format was expected (Ustar or PAX), but could not be reliably determined for entry '{0}'.</value>
</data>
<data name="TarReservedExtendedAttribute" xml:space="preserve">
<value>The extended attributes dictionary cannot contain the reserved key '{0}'.</value>
</data>
<data name="TarSizeFieldNegative" xml:space="preserve">
<value>The size field is negative in a tar entry.</value>
</data>
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/GnuTarEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ 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 = _header.MTime; // mtime was set in base constructor
_header._cTime = _header.MTime;
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion ...ibraries/System.Formats.Tar/src/System/Formats/Tar/PaxGlobalExtendedAttributesTarEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public PaxGlobalExtendedAttributesTarEntry(IEnumerable<KeyValuePair<string, stri
: base(TarEntryType.GlobalExtendedAttributes, TarHeader.GlobalHeadFormatPrefix, TarEntryFormat.Pax, isGea: true)
{
ArgumentNullException.ThrowIfNull(globalExtendedAttributes);
_header.InitializeExtendedAttributesWithExisting(globalExtendedAttributes);
_header.InitializeExtendedAttributesWithExisting(globalExtendedAttributes, allowReservedKeys: false);
}

/// <summary>
Expand Down
12 changes: 6 additions & 6 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/PaxTarEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public PaxTarEntry(TarEntryType entryType, string entryName)
{
_header._prefix = string.Empty;

Debug.Assert(_header._mTime != default);
Debug.Assert(_header.MTime != default);
AddNewAccessAndChangeTimestampsIfNotExist(useMTime: true);
}

Expand Down Expand Up @@ -94,9 +94,9 @@ public PaxTarEntry(TarEntryType entryType, string entryName, IEnumerable<KeyValu
ArgumentNullException.ThrowIfNull(extendedAttributes);

_header._prefix = string.Empty;
_header.InitializeExtendedAttributesWithExisting(extendedAttributes);
_header.InitializeExtendedAttributesWithExisting(extendedAttributes, allowReservedKeys: false);

Debug.Assert(_header._mTime != default);
Debug.Assert(_header.MTime != default);
AddNewAccessAndChangeTimestampsIfNotExist(useMTime: true);
}

Expand All @@ -116,7 +116,7 @@ public PaxTarEntry(TarEntry other)

if (other is PaxTarEntry paxOther)
{
_header.InitializeExtendedAttributesWithExisting(paxOther.ExtendedAttributes);
_header.InitializeExtendedAttributesWithExisting(paxOther.ExtendedAttributes, allowReservedKeys: true);
}
else
{
Expand Down Expand Up @@ -158,13 +158,13 @@ public PaxTarEntry(TarEntry other)
// or 'DateTimeOffset.UtcNow', depending on the value of 'useMTime'.
private void AddNewAccessAndChangeTimestampsIfNotExist(bool useMTime)
{
Debug.Assert(!useMTime || (useMTime && _header._mTime != default));
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);
string secondsFromEpochString = TarHelpers.GetTimestampStringFromDateTimeOffset(useMTime ? _header.MTime : DateTimeOffset.UtcNow);

if (!containsATime)
{
Expand Down
24 changes: 12 additions & 12 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/PosixTarEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ internal PosixTarEntry(TarHeader header, TarReader readerOfOrigin, TarEntryForma
internal PosixTarEntry(TarEntryType entryType, string entryName, TarEntryFormat format, bool isGea)
: base(entryType, entryName, format, isGea)
{
_header._uName = string.Empty;
_header._gName = string.Empty;
_header.UName = string.Empty;
_header.GName = string.Empty;
_header._devMajor = 0;
_header._devMinor = 0;
}
Expand All @@ -34,15 +34,15 @@ internal PosixTarEntry(TarEntry other, TarEntryFormat format)
{
if (other is PosixTarEntry)
{
Debug.Assert(other._header._uName != null);
Debug.Assert(other._header._gName != null);
_header._uName = other._header._uName;
_header._gName = other._header._gName;
Debug.Assert(other._header.UName != null);
Debug.Assert(other._header.GName != null);
_header.UName = other._header.UName;
_header.GName = other._header.GName;
_header._devMajor = other._header._devMajor;
_header._devMinor = other._header._devMinor;
}
_header._uName ??= string.Empty;
_header._gName ??= string.Empty;
_header.UName ??= string.Empty;
_header.GName ??= string.Empty;
}

/// <summary>
Expand Down Expand Up @@ -99,11 +99,11 @@ public int DeviceMinor
/// <remarks><see cref="GroupName"/> is only used in Unix platforms.</remarks>
public string GroupName
{
get => _header._gName ?? string.Empty;
get => _header.GName ?? string.Empty;
set
{
ArgumentNullException.ThrowIfNull(value);
_header._gName = value;
_header.GName = value;
}
}

Expand All @@ -114,11 +114,11 @@ public string GroupName
/// <exception cref="ArgumentNullException">Cannot set a null user name.</exception>
public string UserName
{
get => _header._uName ?? string.Empty;
get => _header.UName ?? string.Empty;
set
{
ArgumentNullException.ThrowIfNull(value);
_header._uName = value;
_header.UName = value;
}
}
}
Expand Down
16 changes: 8 additions & 8 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public abstract partial class TarEntry
internal TarHeader _header;

// Used to access the data section of this entry in an unseekable file
private TarReader? _readerOfOrigin;
internal TarReader? _readerOfOrigin;
Copy link
Member

@jozkee jozkee Sep 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
internal TarReader? _readerOfOrigin;
private TarReader? _readerOfOrigin;


// Constructor called when reading a TarEntry from a TarReader.
internal TarEntry(TarHeader header, TarReader readerOfOrigin, TarEntryFormat format)
Expand Down Expand Up @@ -95,22 +95,22 @@ public int Gid
/// <exception cref="ArgumentOutOfRangeException">The specified value is larger than <see cref="DateTimeOffset.UnixEpoch"/>.</exception>
public DateTimeOffset ModificationTime
{
get => _header._mTime;
get => _header.MTime;
set
{
if (value < DateTimeOffset.UnixEpoch)
{
throw new ArgumentOutOfRangeException(nameof(value));
}
_header._mTime = value;
_header.MTime = value;
}
}

/// <summary>
/// When the <see cref="EntryType"/> indicates an entry that can contain data, this property returns the length in bytes of such data.
/// </summary>
/// <remarks>The entry type that commonly contains data is <see cref="TarEntryType.RegularFile"/> (or <see cref="TarEntryType.V7RegularFile"/> in the <see cref="TarEntryFormat.V7"/> format). Other uncommon entry types that can also contain data are: <see cref="TarEntryType.ContiguousFile"/>, <see cref="TarEntryType.DirectoryList"/>, <see cref="TarEntryType.MultiVolume"/> and <see cref="TarEntryType.SparseFile"/>.</remarks>
public long Length => _header._dataStream != null ? _header._dataStream.Length : _header._size;
public long Length => _header._dataStream != null ? _header._dataStream.Length : _header.Size;

/// <summary>
/// When the <see cref="EntryType"/> indicates a <see cref="TarEntryType.SymbolicLink"/> or a <see cref="TarEntryType.HardLink"/>, this property returns the link target path of such link.
Expand All @@ -120,15 +120,15 @@ public DateTimeOffset ModificationTime
/// <exception cref="ArgumentException">The specified value is empty.</exception>
public string LinkName
{
get => _header._linkName ?? string.Empty;
get => _header.LinkName ?? string.Empty;
set
{
if (_header._typeFlag is not TarEntryType.HardLink and not TarEntryType.SymbolicLink)
{
throw new InvalidOperationException(SR.TarEntryHardLinkOrSymLinkExpected);
}
ArgumentException.ThrowIfNullOrEmpty(value);
_header._linkName = value;
_header.LinkName = value;
}
}

Expand All @@ -154,11 +154,11 @@ public UnixFileMode Mode
/// </summary>
public string Name
{
get => _header._name;
get => _header.Name;
set
{
ArgumentException.ThrowIfNullOrEmpty(value);
_header._name = value;
_header.Name = value;
}
}

Expand Down
Loading