[release/7.0] Change some exception types thrown in Tar APIs (#74893)

* Throw ArgumentException on unsupported tar entry type

* Adjust tests

* Also change exception type for internal TarEntry conversion constructor called by all other public conversion constructors. Add missing test gap.

* LinkName setter null check.

* Internal constructors SeekableSubReadStream and SubReadStream unseekable exception should be ArgumentException.

* DataStream setter for regular file should throw ArgumentException if passed an unreadable stream.

* TarFile CreateFromDirectory unwritable destination change exception to ArgumentException.

* Change to ArgumentException when ExtractToDirectory is an unreadable stream. Add missing exception docs.

* Add some missing exception docs for TarEntry.

* Change TarReader constructor exception if unreadable stream. Close test gap.

* Change TarWriter exception for unwritable stream to ArgumentException, adjust docs and tests.

* Add missing documentation for exceptions in constructors.

* Change wording of conversion constructors comment when passing a Pax GEA entry.

* Apply suggestions by Jozkee

* Add exception to LinkName if the entry type is hard/symlink and the user tries to set an empty string. Add tests.

* Convert all FormatException to InvalidDataException

* Address more suggestions

Co-authored-by: carlossanlop <[email protected]>

Author: github-actions[bot]

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

@@ -20,14 +20,16 @@ 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>
-        /// <exception cref="ArgumentException"><paramref name="entryName"/> is null or empty.</exception>
-        /// <exception cref="InvalidOperationException">The entry type is not supported for creating an entry.</exception>
         /// <remarks>When creating an instance using the <see cref="GnuTarEntry(TarEntryType, string)"/> constructor, only the following entry types are supported:
         /// <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>
         /// </list>
         /// </remarks>
+        /// <exception cref="ArgumentNullException"><paramref name="entryName"/> is <see langword="null"/>.</exception>
+        /// <exception cref="ArgumentException"><para><paramref name="entryName"/> is empty.</para>
+        /// <para>-or-</para>
+        /// <para><paramref name="entryType"/> is not supported in the specified format.</para></exception>
         public GnuTarEntry(TarEntryType entryType, string entryName)
             : base(entryType, entryName, TarEntryFormat.Gnu, isGea: false)
         {
@@ -38,6 +40,9 @@ public GnuTarEntry(TarEntryType entryType, string entryName)
         /// <summary>
         /// Initializes a new <see cref="GnuTarEntry"/> instance by converting the specified <paramref name="other"/> entry into the GNU format.
         /// </summary>
+        /// <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>
         public GnuTarEntry(TarEntry other)
             : base(other, TarEntryFormat.Gnu)
         {

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

@@ -25,8 +25,6 @@ internal PaxTarEntry(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>
-        /// <exception cref="ArgumentException"><paramref name="entryName"/> is null or empty.</exception>
-        /// <exception cref="InvalidOperationException">The entry type is not supported for creating an entry.</exception>
         /// <remarks><para>When creating an instance using the <see cref="PaxTarEntry(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>
@@ -47,6 +45,10 @@ internal PaxTarEntry(TarHeader header, TarReader readerOfOrigin)
         /// <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="entryName"/> is <see langword="null"/>.</exception>
+        /// <exception cref="ArgumentException"><para><paramref name="entryName"/> is empty.</para>
+        /// <para>-or-</para>
+        /// <para><paramref name="entryType"/> is not supported in the specified format.</para></exception>
         public PaxTarEntry(TarEntryType entryType, string entryName)
             : base(entryType, entryName, TarEntryFormat.Pax, isGea: false)
         {
@@ -62,9 +64,6 @@ public PaxTarEntry(TarEntryType entryType, string entryName)
         /// <param name="entryType">The type of the entry.</param>
         /// <param name="entryName">A string with the path and file name of this entry.</param>
         /// <param name="extendedAttributes">An enumeration of string key-value pairs that represents the metadata to include in the Extended Attributes entry that precedes the current entry.</param>
-        /// <exception cref="ArgumentNullException"><paramref name="extendedAttributes"/> is <see langword="null"/>.</exception>
-        /// <exception cref="ArgumentException"><paramref name="entryName"/> is null or empty.</exception>
-        /// <exception cref="InvalidOperationException">The entry type is not supported for creating an entry.</exception>
         /// <remarks>When creating an instance using the <see cref="PaxTarEntry(TarEntryType, string)"/> constructor, only the following entry types are supported:
         /// <list type="bullet">
         /// <item>In all platforms: <see cref="TarEntryType.Directory"/>, <see cref="TarEntryType.HardLink"/>, <see cref="TarEntryType.SymbolicLink"/>, <see cref="TarEntryType.RegularFile"/>.</item>
@@ -85,6 +84,10 @@ public PaxTarEntry(TarEntryType entryType, string entryName)
         /// <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>
+        /// <exception cref="ArgumentException"><para><paramref name="entryName"/> is empty.</para>
+        /// <para>-or-</para>
+        /// <para><paramref name="entryType"/> is not supported in the specified format.</para></exception>
         public PaxTarEntry(TarEntryType entryType, string entryName, IEnumerable<KeyValuePair<string, string>> extendedAttributes)
             : base(entryType, entryName, TarEntryFormat.Pax, isGea: false)
         {
@@ -100,6 +103,9 @@ 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>
+        /// <para>-or-</para>
+        /// <para>The entry type of <paramref name="other"/> is not supported for conversion to the PAX format.</para></exception>
         public PaxTarEntry(TarEntry other)
             : base(other, TarEntryFormat.Pax)
         {

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

@@ -18,7 +18,7 @@ public SeekableSubReadStream(Stream superStream, long startPosition, long maxLen
         {
             if (!superStream.CanSeek)
             {
-                throw new InvalidOperationException(SR.IO_NotSupported_UnseekableStream);
+                throw new ArgumentException(SR.IO_NotSupported_UnseekableStream, nameof(superStream));
             }
         }
 

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

@@ -25,7 +25,7 @@ public SubReadStream(Stream superStream, long startPosition, long maxLength)
         {
             if (!superStream.CanRead)
             {
-                throw new InvalidOperationException(SR.IO_NotSupported_UnreadableStream);
+                throw new ArgumentException(SR.IO_NotSupported_UnreadableStream, nameof(superStream));
             }
             _startInSuperStream = startPosition;
             _positionInSuperStream = startPosition;

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

@@ -51,12 +51,12 @@ internal TarEntry(TarEntry other, TarEntryFormat format)
         {
             if (other is PaxGlobalExtendedAttributesTarEntry)
             {
-                throw new InvalidOperationException(SR.TarCannotConvertPaxGlobalExtendedAttributesEntry);
+                throw new ArgumentException(SR.TarCannotConvertPaxGlobalExtendedAttributesEntry, nameof(other));
             }
 
             TarEntryType compatibleEntryType = TarHelpers.GetCorrectTypeFlagForFormat(format, other.EntryType);
 
-            TarHelpers.ThrowIfEntryTypeNotSupported(compatibleEntryType, format);
+            TarHelpers.ThrowIfEntryTypeNotSupported(compatibleEntryType, format, nameof(other));
 
             _readerOfOrigin = other._readerOfOrigin;
 
@@ -92,6 +92,7 @@ public int Gid
         /// A timestamps that represents the last time the contents of the file represented by this entry were modified.
         /// </summary>
         /// <remarks>In Unix platforms, this timestamp is commonly known as <c>mtime</c>.</remarks>
+        /// <exception cref="ArgumentOutOfRangeException">The specified value is larger than <see cref="DateTimeOffset.UnixEpoch"/>.</exception>
         public DateTimeOffset ModificationTime
         {
             get => _header._mTime;
@@ -114,7 +115,9 @@ public DateTimeOffset ModificationTime
         /// <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.
         /// </summary>
-        /// <exception cref="InvalidOperationException">Cannot set the link name if the entry type is not <see cref="TarEntryType.HardLink"/> or <see cref="TarEntryType.SymbolicLink"/>.</exception>
+        /// <exception cref="InvalidOperationException">The entry type is not <see cref="TarEntryType.HardLink"/> or <see cref="TarEntryType.SymbolicLink"/>.</exception>
+        /// <exception cref="ArgumentNullException">The specified value is <see langword="null"/>.</exception>
+        /// <exception cref="ArgumentException">The specified value is empty.</exception>
         public string LinkName
         {
             get => _header._linkName ?? string.Empty;
@@ -124,6 +127,7 @@ public string LinkName
                 {
                     throw new InvalidOperationException(SR.TarEntryHardLinkOrSymLinkExpected);
                 }
+                ArgumentException.ThrowIfNullOrEmpty(value);
                 _header._linkName = value;
             }
         }
@@ -177,7 +181,8 @@ public int Uid
         /// <para>Elevation is required to extract a <see cref="TarEntryType.BlockDevice"/> or <see cref="TarEntryType.CharacterDevice"/> to disk.</para>
         /// <para>Symbolic links can be recreated using <see cref="File.CreateSymbolicLink(string, string)"/>, <see cref="Directory.CreateSymbolicLink(string, string)"/> or <see cref="FileSystemInfo.CreateAsSymbolicLink(string)"/>.</para>
         /// <para>Hard links can only be extracted when using <see cref="TarFile.ExtractToDirectory(Stream, string, bool)"/> or <see cref="TarFile.ExtractToDirectory(string, string, bool)"/>.</para></remarks>
-        /// <exception cref="ArgumentException"><paramref name="destinationFileName"/> is <see langword="null"/> or empty.</exception>
+        /// <exception cref="ArgumentNullException"><paramref name="destinationFileName"/> is <see langword="null"/>.</exception>
+        /// <exception cref="ArgumentException"><paramref name="destinationFileName"/> is empty.</exception>
         /// <exception cref="IOException"><para>The parent directory of <paramref name="destinationFileName"/> does not exist.</para>
         /// <para>-or-</para>
         /// <para><paramref name="overwrite"/> is <see langword="false"/> and a file already exists in <paramref name="destinationFileName"/>.</para>
@@ -206,7 +211,8 @@ public void ExtractToFile(string destinationFileName, bool overwrite)
         /// <returns>A task that represents the asynchronous extraction operation.</returns>
         /// <remarks><para>Files of type <see cref="TarEntryType.BlockDevice"/>, <see cref="TarEntryType.CharacterDevice"/> or <see cref="TarEntryType.Fifo"/> can only be extracted in Unix platforms.</para>
         /// <para>Elevation is required to extract a <see cref="TarEntryType.BlockDevice"/> or <see cref="TarEntryType.CharacterDevice"/> to disk.</para></remarks>
-        /// <exception cref="ArgumentException"><paramref name="destinationFileName"/> is <see langword="null"/> or empty.</exception>
+        /// <exception cref="ArgumentNullException"><paramref name="destinationFileName"/> is <see langword="null"/>.</exception>
+        /// <exception cref="ArgumentException"><paramref name="destinationFileName"/> is empty.</exception>
         /// <exception cref="IOException"><para>The parent directory of <paramref name="destinationFileName"/> does not exist.</para>
         /// <para>-or-</para>
         /// <para><paramref name="overwrite"/> is <see langword="false"/> and a file already exists in <paramref name="destinationFileName"/>.</para>
@@ -237,9 +243,8 @@ public Task ExtractToFileAsync(string destinationFileName, bool overwrite, Cance
         /// <para>Sets a new stream that represents the data section, if it makes sense for the <see cref="EntryType"/> to contain data; if a stream already existed, the old stream gets disposed before substituting it with the new stream. Setting a <see langword="null"/> stream is allowed.</para></value>
         /// <remarks>If you write data to this data stream, make sure to rewind it to the desired start position before writing this entry into an archive using <see cref="TarWriter.WriteEntry(TarEntry)"/>.</remarks>
         /// <exception cref="InvalidOperationException">Setting a data section is not supported because the <see cref="EntryType"/> is not <see cref="TarEntryType.RegularFile"/> (or <see cref="TarEntryType.V7RegularFile"/> for an archive of <see cref="TarEntryFormat.V7"/> format).</exception>
-        /// <exception cref="IOException"><para>Cannot set an unreadable stream.</para>
-        /// <para>-or-</para>
-        /// <para>An I/O problem occurred.</para></exception>
+        /// <exception cref="ArgumentException">Cannot set an unreadable stream.</exception>
+        /// <exception cref="IOException">An I/O problem occurred.</exception>
         public Stream? DataStream
         {
             get => _header._dataStream;
@@ -252,7 +257,7 @@ public Stream? DataStream
 
                 if (value != null && !value.CanRead)
                 {
-                    throw new IOException(SR.IO_NotSupported_UnreadableStream);
+                    throw new ArgumentException(SR.IO_NotSupported_UnreadableStream, nameof(value));
                 }
 
                 if (_readerOfOrigin != null)
@@ -339,7 +344,7 @@ internal Task ExtractRelativeToDirectoryAsync(string destinationDirectoryPath, b
             {
                 if (string.IsNullOrEmpty(LinkName))
                 {
-                    throw new FormatException(SR.TarEntryHardLinkOrSymlinkLinkNameEmpty);
+                    throw new InvalidDataException(SR.TarEntryHardLinkOrSymlinkLinkNameEmpty);
                 }
 
                 linkTargetPath = GetSanitizedFullPath(destinationDirectoryPath, LinkName);
@@ -511,7 +516,7 @@ private void VerifyPathsForEntryType(string filePath, string? linkTargetPath, bo
                 }
                 else
                 {
-                    throw new FormatException(SR.TarEntryHardLinkOrSymlinkLinkNameEmpty);
+                    throw new InvalidDataException(SR.TarEntryHardLinkOrSymlinkLinkNameEmpty);
                 }
             }
         }