Disallow TarWriter from writing link entries without LinkName set (do…

…tnet#74892) * Disallow TarWriter from writing link entries without LinkName set * Tests * Docs * Apply suggestions from code review Co-authored-by: David Cantú <[email protected]> * Apply suggestions from code review Co-authored-by: carlossanlop <[email protected]> Co-authored-by: David Cantú <[email protected]>
zhangximing · Sep 2, 2022 · 4cefab0 · 4cefab0
1 parent b609e53
commit 4cefab0
Show file tree

Hide file tree

Showing 10 changed files with 106 additions and 0 deletions.
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
@@ -391,6 +391,9 @@ private int WritePosixName(Span<byte> buffer)
         // Writes all the common fields shared by all formats into the specified spans.
         private int WriteCommonFields(Span<byte> buffer, long actualLength, TarEntryType actualEntryType)
         {
+            // Don't write an empty LinkName if the entry is a hardlink or symlink
+            Debug.Assert(!string.IsNullOrEmpty(_linkName) ^ (_typeFlag is not TarEntryType.SymbolicLink and not TarEntryType.HardLink));
+
             int checksum = 0;
 
             if (_mode > 0)

diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarWriter.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarWriter.cs
@@ -213,13 +213,15 @@ private async Task ReadFileFromDiskAndWriteToArchiveStreamAsEntryAsync(string fu
         /// </item>
         /// </list>
         /// </remarks>
+        /// <exception cref="ArgumentException">The entry type is <see cref="TarEntryType.HardLink"/> or <see cref="TarEntryType.SymbolicLink"/> and the <see cref="TarEntry.LinkName"/> is <see langword="null"/> or empty.</exception>
         /// <exception cref="ObjectDisposedException">The archive stream is disposed.</exception>
         /// <exception cref="ArgumentNullException"><paramref name="entry"/> is <see langword="null"/>.</exception>
         /// <exception cref="IOException">An I/O problem occurred.</exception>
         public void WriteEntry(TarEntry entry)
         {
             ObjectDisposedException.ThrowIf(_isDisposed, this);
             ArgumentNullException.ThrowIfNull(entry);
+            ValidateEntryLinkName(entry._header._typeFlag, entry._header._linkName);
             WriteEntryInternal(entry);
         }
 
@@ -254,6 +256,7 @@ public void WriteEntry(TarEntry entry)
         /// </item>
         /// </list>
         /// </remarks>
+        /// <exception cref="ArgumentException">The entry type is <see cref="TarEntryType.HardLink"/> or <see cref="TarEntryType.SymbolicLink"/> and the <see cref="TarEntry.LinkName"/> is <see langword="null"/> or empty.</exception>
         /// <exception cref="ObjectDisposedException">The archive stream is disposed.</exception>
         /// <exception cref="ArgumentNullException"><paramref name="entry"/> is <see langword="null"/>.</exception>
         /// <exception cref="IOException">An I/O problem occurred.</exception>
@@ -266,6 +269,7 @@ public Task WriteEntryAsync(TarEntry entry, CancellationToken cancellationToken
 
             ObjectDisposedException.ThrowIf(_isDisposed, this);
             ArgumentNullException.ThrowIfNull(entry);
+            ValidateEntryLinkName(entry._header._typeFlag, entry._header._linkName);
             return WriteEntryAsyncInternal(entry, cancellationToken);
         }
 
@@ -369,5 +373,16 @@ private async ValueTask WriteFinalRecordsAsync()
 
             return (fullPath, actualEntryName);
         }
+
+        private static void ValidateEntryLinkName(TarEntryType entryType, string? linkName)
+        {
+            if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
+            {
+                if (string.IsNullOrEmpty(linkName))
+                {
+                    throw new ArgumentException(SR.TarEntryHardLinkOrSymlinkLinkNameEmpty, "entry");
+                }
+            }
+        }
     }
 }
diff --git a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Entry.Gnu.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Entry.Gnu.Tests.cs
@@ -167,6 +167,10 @@ public void Write_Long_Name(TarEntryType entryType)
             using (TarWriter writer = new TarWriter(archiveStream, TarEntryFormat.Gnu, leaveOpen: true))
             {
                 GnuTarEntry entry = new GnuTarEntry(entryType, longName);
+                if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
+                {
+                    entry.LinkName = "linktarget";
+                }
                 writer.WriteEntry(entry);
             }
 
@@ -231,5 +235,15 @@ public void Write_LongName_And_LongLinkName(TarEntryType entryType)
                 Assert.Equal(longLinkName, entry.LinkName);
             }
         }
+
+        [Theory]
+        [InlineData(TarEntryType.HardLink)]
+        [InlineData(TarEntryType.SymbolicLink)]
+        public void Write_LinkEntry_EmptyLinkName_Throws(TarEntryType entryType)
+        {
+            using MemoryStream archiveStream = new MemoryStream();
+            using TarWriter writer = new TarWriter(archiveStream, leaveOpen: false);
+            Assert.Throws<ArgumentException>("entry", () => writer.WriteEntry(new GnuTarEntry(entryType, "link")));
+        }
     }
 }
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
@@ -485,5 +485,15 @@ public void WriteTimestampsBeyondOctalLimitInPax()
                 Assert.Equal(overLimitTimestamp, actualCTime);
             }
         }
+
+        [Theory]
+        [InlineData(TarEntryType.HardLink)]
+        [InlineData(TarEntryType.SymbolicLink)]
+        public void Write_LinkEntry_EmptyLinkName_Throws(TarEntryType entryType)
+        {
+            using MemoryStream archiveStream = new MemoryStream();
+            using TarWriter writer = new TarWriter(archiveStream, leaveOpen: false);
+            Assert.Throws<ArgumentException>("entry", () => writer.WriteEntry(new PaxTarEntry(entryType, "link")));
+        }
     }
 }
diff --git a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Entry.Ustar.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Entry.Ustar.Tests.cs
@@ -152,5 +152,15 @@ public void WriteFifo()
                 VerifyFifo(fifo);
             }
         }
+
+        [Theory]
+        [InlineData(TarEntryType.HardLink)]
+        [InlineData(TarEntryType.SymbolicLink)]
+        public void Write_LinkEntry_EmptyLinkName_Throws(TarEntryType entryType)
+        {
+            using MemoryStream archiveStream = new MemoryStream();
+            using TarWriter writer = new TarWriter(archiveStream, leaveOpen: false);
+            Assert.Throws<ArgumentException>("entry", () => writer.WriteEntry(new UstarTarEntry(entryType, "link")));
+        }
     }
 }
diff --git a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Entry.V7.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Entry.V7.Tests.cs
@@ -92,5 +92,15 @@ public void WriteDirectory()
                 VerifyDirectory(directory);
             }
         }
+
+        [Theory]
+        [InlineData(TarEntryType.HardLink)]
+        [InlineData(TarEntryType.SymbolicLink)]
+        public void Write_LinkEntry_EmptyLinkName_Throws(TarEntryType entryType)
+        {
+            using MemoryStream archiveStream = new MemoryStream();
+            using TarWriter writer = new TarWriter(archiveStream, leaveOpen: false);
+            Assert.Throws<ArgumentException>("entry", () => writer.WriteEntry(new V7TarEntry(entryType, "link")));
+        }
     }
 }
diff --git a/...libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Entry.Gnu.Tests.cs b/...libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Entry.Gnu.Tests.cs
@@ -183,6 +183,10 @@ public async Task Write_Long_Name_Async(TarEntryType entryType)
                 await using (TarWriter writer = new TarWriter(archiveStream, TarEntryFormat.Gnu, leaveOpen: true))
                 {
                     GnuTarEntry entry = new GnuTarEntry(entryType, longName);
+                    if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
+                    {
+                        entry.LinkName = "linktarget";
+                    }
                     await writer.WriteEntryAsync(entry);
                 }
 
@@ -252,5 +256,15 @@ public async Task Write_LongName_And_LongLinkName_Async(TarEntryType entryType)
                 }
             }
         }
+
+        [Theory]
+        [InlineData(TarEntryType.HardLink)]
+        [InlineData(TarEntryType.SymbolicLink)]
+        public async Task Write_LinkEntry_EmptyLinkName_Throws_Async(TarEntryType entryType)
+        {
+            await using MemoryStream archiveStream = new MemoryStream();
+            await using TarWriter writer = new TarWriter(archiveStream, leaveOpen: false);
+            await Assert.ThrowsAsync<ArgumentException>("entry", () => writer.WriteEntryAsync(new GnuTarEntry(entryType, "link")));
+        }
     }
 }
diff --git a/...libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Entry.Pax.Tests.cs b/...libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Entry.Pax.Tests.cs
@@ -505,5 +505,15 @@ public async Task WriteTimestampsBeyondOctalLimitInPax_Async()
                 Assert.Equal(overLimitTimestamp, actualCTime);
             }
         }
+
+        [Theory]
+        [InlineData(TarEntryType.HardLink)]
+        [InlineData(TarEntryType.SymbolicLink)]
+        public async Task Write_LinkEntry_EmptyLinkName_Throws_Async(TarEntryType entryType)
+        {
+            await using MemoryStream archiveStream = new MemoryStream();
+            await using TarWriter writer = new TarWriter(archiveStream, leaveOpen: false);
+            await Assert.ThrowsAsync<ArgumentException>("entry", () => writer.WriteEntryAsync(new PaxTarEntry(entryType, "link")));
+        }
     }
 }
diff --git a/...braries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Entry.Ustar.Tests.cs b/...braries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Entry.Ustar.Tests.cs
@@ -153,5 +153,15 @@ public async Task WriteFifo_Async()
                 VerifyFifo(fifo);
             }
         }
+
+        [Theory]
+        [InlineData(TarEntryType.HardLink)]
+        [InlineData(TarEntryType.SymbolicLink)]
+        public async Task Write_LinkEntry_EmptyLinkName_Throws_Async(TarEntryType entryType)
+        {
+            await using MemoryStream archiveStream = new MemoryStream();
+            await using TarWriter writer = new TarWriter(archiveStream, leaveOpen: false);
+            await Assert.ThrowsAsync<ArgumentException>("entry", () => writer.WriteEntryAsync(new UstarTarEntry(entryType, "link")));
+        }
     }
 }
diff --git a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Entry.V7.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Entry.V7.Tests.cs
@@ -93,5 +93,15 @@ public async Task WriteDirectory_Async()
                 VerifyDirectory(directory);
             }
         }
+
+        [Theory]
+        [InlineData(TarEntryType.HardLink)]
+        [InlineData(TarEntryType.SymbolicLink)]
+        public async Task Write_LinkEntry_EmptyLinkName_Throws_Async(TarEntryType entryType)
+        {
+            await using MemoryStream archiveStream = new MemoryStream();
+            await using TarWriter writer = new TarWriter(archiveStream, leaveOpen: false);
+            await Assert.ThrowsAsync<ArgumentException>("entry", () => writer.WriteEntryAsync(new V7TarEntry(entryType, "link")));
+        }
     }
 }