From 823afd4fd5f1b66bf7fcc4a2216088efaae874e9 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Mon, 20 Apr 2026 07:58:29 +0100 Subject: [PATCH] fixed header format behavior --- docs/TAR_GAP_ANALYSIS.md | 88 +++++-------------- .../Writers/Tar/TarWriter.Async.cs | 4 +- src/SharpCompress/Writers/Tar/TarWriter.cs | 2 +- .../Tar/TarReaderAsyncTests.cs | 30 +++++++ .../SharpCompress.Test/Tar/TarReaderTests.cs | 30 +++++++ .../Tar/TarWriterAsyncTests.cs | 73 +++++++++++++++ .../Tar/TarWriterDirectoryTests.cs | 43 +++++++++ .../SharpCompress.Test/Tar/TarWriterTests.cs | 57 ++++++++++++ 8 files changed, 259 insertions(+), 68 deletions(-) diff --git a/docs/TAR_GAP_ANALYSIS.md b/docs/TAR_GAP_ANALYSIS.md index 533b9e3c..0bc6c684 100644 --- a/docs/TAR_GAP_ANALYSIS.md +++ b/docs/TAR_GAP_ANALYSIS.md @@ -22,6 +22,9 @@ Primary references: - `Tar.XZ` is now documented as read-only (`Writer API = N/A`) in `docs/FORMATS.md`. - Local PAX extended headers (`x`) are now implemented on the read path for selected keys. - Tar tests now include local PAX coverage for reader/archive sync and async paths. +- `TarWriterOptions.HeaderFormat` is now honored in sync and async file and directory write paths. +- Tar tests now cover `USTAR` and `GNU_TAR_LONG_LINK`, including USTAR long-name failure scenarios. +- Symlink coverage now includes `TarWithSymlink.tar.gz` for reader sync and async paths. ## Claimed vs Actual Support @@ -122,34 +125,16 @@ Recommended action: ## Write-Path Gaps -### `HeaderFormat` is not honored consistently +### `HeaderFormat` consistency is resolved -`TarWriterOptions.HeaderFormat` exists and defaults to `GNU_TAR_LONG_LINK`, but the configured value is not consistently applied. +`TarWriterOptions.HeaderFormat` is now applied across: -### Sync directory write path +- sync file writes +- sync directory writes +- async file writes +- async directory writes -`TarWriter.WriteDirectory` creates headers using: - -- `new TarHeader(WriterOptions.ArchiveEncoding)` - -This uses the default tar header format rather than the writer's configured `headerFormat` field. - -Impact: - -- directory entries written through the sync path do not follow `TarWriterOptions.HeaderFormat` - -### Async write path - -`TarWriter.WriteAsync` and `WriteDirectoryAsync` also create headers using the default constructor rather than the configured header format. - -Impact: - -- async writes ignore `TarWriterOptions.HeaderFormat` for both file and directory entries - -Recommended action: - -- pass the configured header format to all `TarHeader` constructions in sync and async write paths -- add tests for both `GNU_TAR_LONG_LINK` and `USTAR` +Regression tests now cover both `USTAR` and `GNU_TAR_LONG_LINK` behavior. ### No public link-writing support @@ -207,49 +192,28 @@ Recommended action: - either align the contracts or document the difference explicitly -### Async and sync write behavior do not align on header format handling +### Header format alignment between sync and async is resolved -This is the most visible sync/async inconsistency in the current Tar writer implementation. - -Recommended action: - -- fix the implementation first -- add matching sync and async tests to keep the behavior aligned +Sync and async Tar writer paths now both honor `TarWriterOptions.HeaderFormat`, and matching tests are present for both paths. ## Test Coverage Gaps -### Symlink coverage exists in test data but not in assertions +### Symlink coverage is now present for reader paths -There is a tar archive containing symlinks: +Symlink behavior is now asserted for sync and async reader paths using: - `tests/TestArchives/Archives/TarWithSymlink.tar.gz` -Current Tar tests do not assert tar symlink behavior against that fixture. +Archive-path symlink assertions currently rely on small tar fixtures rather than this large compressed sample. -Impact: +### Header format coverage is now present -- the code claims practical read support for link targets, but coverage does not verify it - -Recommended action: - -- add reader and archive tests asserting `EntryType`-derived behavior and `LinkTarget` - -### No tests for `HeaderFormat` - -There are currently no tests covering: +Tar tests now cover: - `TarWriterOptions.HeaderFormat = USTAR` - `TarWriterOptions.HeaderFormat = GNU_TAR_LONG_LINK` -- long-name failures in USTAR mode -- long-name success in GNU mode through the async writer path - -Impact: - -- the current header-format regressions were able to exist without test coverage - -Recommended action: - -- add dedicated sync and async writer tests for header format selection +- long-name failure in USTAR mode +- long-name success in GNU mode through sync and async writer paths ### No tests for sparse or global PAX headers @@ -291,7 +255,7 @@ Missing implementation-specific details include: - GNU long-name and long-link support - USTAR prefix handling - oldgnu numeric quirk handling -- missing PAX support +- partial PAX support boundaries (local supported, global pending) - missing sparse support - reader vs archive behavior differences for compressed tar - file-size requirements for writing from non-seekable sources @@ -317,17 +281,11 @@ Recommended action: ### Priority 1 -- Fix `TarWriterOptions.HeaderFormat` handling in sync and async writer paths -- Add tests for header-format behavior -- Add symlink coverage using `TarWithSymlink.tar.gz` - -### Priority 2 - - Implement and document global PAX (`g`) support - Decide and document the support position for sparse files - Decide and document support boundaries for non-modeled PAX keys (`uname`, `gname`, vendor keys) -### Priority 3 +### Priority 2 - Add negative writer tests for unsupported wrapper compressions - Evaluate whether sync and async archive open contracts should match exactly @@ -339,7 +297,7 @@ The SharpCompress Tar implementation is strong on common read scenarios and basi - documentation overstating or under-describing support - incomplete feature coverage for less common tar dialect features -- sync/async and file/directory inconsistencies in writer header-format handling -- test coverage holes around links and advanced tar metadata features +- archive/read behavioral differences that are not always explicit in docs +- test coverage holes around advanced tar metadata features `docs/TAR_SPEC.md` should be treated as the implementation baseline. This document identifies where that baseline is incomplete, inconsistent, or incorrectly reflected elsewhere in the repository. diff --git a/src/SharpCompress/Writers/Tar/TarWriter.Async.cs b/src/SharpCompress/Writers/Tar/TarWriter.Async.cs index c3cc7bff..cefc6241 100644 --- a/src/SharpCompress/Writers/Tar/TarWriter.Async.cs +++ b/src/SharpCompress/Writers/Tar/TarWriter.Async.cs @@ -24,7 +24,7 @@ public partial class TarWriter return; } - var header = new TarHeader(WriterOptions.ArchiveEncoding); + var header = new TarHeader(WriterOptions.ArchiveEncoding, headerFormat); header.LastModifiedTime = modificationTime ?? TarHeader.EPOCH; header.Name = normalizedName; header.Size = 0; @@ -62,7 +62,7 @@ public partial class TarWriter var realSize = size ?? source.Length; - var header = new TarHeader(WriterOptions.ArchiveEncoding); + var header = new TarHeader(WriterOptions.ArchiveEncoding, headerFormat); header.LastModifiedTime = modificationTime ?? TarHeader.EPOCH; header.Name = NormalizeFilename(filename); diff --git a/src/SharpCompress/Writers/Tar/TarWriter.cs b/src/SharpCompress/Writers/Tar/TarWriter.cs index 12775903..c4078b36 100644 --- a/src/SharpCompress/Writers/Tar/TarWriter.cs +++ b/src/SharpCompress/Writers/Tar/TarWriter.cs @@ -97,7 +97,7 @@ public partial class TarWriter : AbstractWriter return; // Skip empty or root directory } - var header = new TarHeader(WriterOptions.ArchiveEncoding); + var header = new TarHeader(WriterOptions.ArchiveEncoding, headerFormat); header.LastModifiedTime = modificationTime ?? TarHeader.EPOCH; header.Name = normalizedName; header.Size = 0; diff --git a/tests/SharpCompress.Test/Tar/TarReaderAsyncTests.cs b/tests/SharpCompress.Test/Tar/TarReaderAsyncTests.cs index e09c4d56..509ab1cd 100644 --- a/tests/SharpCompress.Test/Tar/TarReaderAsyncTests.cs +++ b/tests/SharpCompress.Test/Tar/TarReaderAsyncTests.cs @@ -203,6 +203,36 @@ public class TarReaderAsyncTests : ReaderTests Assert.False(await reader.MoveToNextEntryAsync()); } + [Fact] + public async ValueTask Tar_WithSymlink_Reader_SurfacesLinkTargets_Async() + { + var archivePath = Path.Combine(TEST_ARCHIVES_PATH, "TarWithSymlink.tar.gz"); + + using Stream stream = File.OpenRead(archivePath); + await using var reader = await TarReader.OpenAsyncReader(stream); + + var foundVulkanToolsLink = false; + var foundVulkanSamplesLink = false; + + while (await reader.MoveToNextEntryAsync()) + { + if (reader.Entry.Key == "MoltenVK-1.0.21/Demos/LunarG-VulkanSamples/Vulkan-Tools") + { + foundVulkanToolsLink = true; + Assert.Equal("../../External/Vulkan-Tools", reader.Entry.LinkTarget); + } + + if (reader.Entry.Key == "MoltenVK-1.0.21/Demos/LunarG-VulkanSamples/VulkanSamples") + { + foundVulkanSamplesLink = true; + Assert.Equal("../../External/VulkanSamples", reader.Entry.LinkTarget); + } + } + + Assert.True(foundVulkanToolsLink); + Assert.True(foundVulkanSamplesLink); + } + [Fact] public void Tar_Containing_Rar_Reader_Async() { diff --git a/tests/SharpCompress.Test/Tar/TarReaderTests.cs b/tests/SharpCompress.Test/Tar/TarReaderTests.cs index a54a0bad..872d915e 100644 --- a/tests/SharpCompress.Test/Tar/TarReaderTests.cs +++ b/tests/SharpCompress.Test/Tar/TarReaderTests.cs @@ -223,6 +223,36 @@ public class TarReaderTests : ReaderTests Assert.False(reader.MoveToNextEntry()); } + [Fact] + public void Tar_WithSymlink_Reader_SurfacesLinkTargets() + { + var archivePath = Path.Combine(TEST_ARCHIVES_PATH, "TarWithSymlink.tar.gz"); + + using Stream stream = File.OpenRead(archivePath); + using var reader = TarReader.OpenReader(stream); + + var foundVulkanToolsLink = false; + var foundVulkanSamplesLink = false; + + while (reader.MoveToNextEntry()) + { + if (reader.Entry.Key == "MoltenVK-1.0.21/Demos/LunarG-VulkanSamples/Vulkan-Tools") + { + foundVulkanToolsLink = true; + Assert.Equal("../../External/Vulkan-Tools", reader.Entry.LinkTarget); + } + + if (reader.Entry.Key == "MoltenVK-1.0.21/Demos/LunarG-VulkanSamples/VulkanSamples") + { + foundVulkanSamplesLink = true; + Assert.Equal("../../External/VulkanSamples", reader.Entry.LinkTarget); + } + } + + Assert.True(foundVulkanToolsLink); + Assert.True(foundVulkanSamplesLink); + } + [Fact] public void Tar_BZip2_Skip_Entry_Stream() { diff --git a/tests/SharpCompress.Test/Tar/TarWriterAsyncTests.cs b/tests/SharpCompress.Test/Tar/TarWriterAsyncTests.cs index d9884543..d0f6138e 100644 --- a/tests/SharpCompress.Test/Tar/TarWriterAsyncTests.cs +++ b/tests/SharpCompress.Test/Tar/TarWriterAsyncTests.cs @@ -1,7 +1,10 @@ using System.IO; +using System.Linq; using System.Text; using System.Threading.Tasks; +using SharpCompress.Archives.Tar; using SharpCompress.Common; +using SharpCompress.Common.Tar.Headers; using SharpCompress.Test.Mocks; using SharpCompress.Writers.Tar; using Xunit; @@ -81,4 +84,74 @@ public class TarWriterAsyncTests : WriterTests : paddedContentWithHeader; Assert.Equal(expectedStreamLength, stream.Length); } + + [Fact] + public async ValueTask Tar_Ustar_HeaderFormat_WritesShortPath_Async() + { + using var stream = new MemoryStream(); + var options = new TarWriterOptions(CompressionType.None, true, TarHeaderWriteFormat.USTAR); + using (var writer = new TarWriter(new AsyncOnlyStream(stream), options)) + using (var content = new MemoryStream(Encoding.UTF8.GetBytes("hello"))) + { + await writer.WriteAsync("dir/file.txt", content, null); + } + + stream.Position = 0; + using var archive = TarArchive.OpenArchive(stream); + Assert.Single(archive.Entries); + Assert.Equal("dir/file.txt", archive.Entries.Single().Key); + } + + [Fact] + public async ValueTask Tar_Ustar_HeaderFormat_ThrowsForLongPath_Async() + { + var longName = new string('a', 160) + ".txt"; + + using var stream = new MemoryStream(); + var options = new TarWriterOptions(CompressionType.None, true, TarHeaderWriteFormat.USTAR); + using var writer = new TarWriter(new AsyncOnlyStream(stream), options); + using var content = new MemoryStream(Encoding.UTF8.GetBytes("hello")); + + await Assert.ThrowsAsync(async () => + await writer.WriteAsync(longName, content, null) + ); + } + + [Fact] + public async ValueTask Tar_GnuLongLink_HeaderFormat_WritesLongPath_Async() + { + var longName = new string('a', 160) + ".txt"; + + using var stream = new MemoryStream(); + var options = new TarWriterOptions( + CompressionType.None, + true, + TarHeaderWriteFormat.GNU_TAR_LONG_LINK + ); + + using (var writer = new TarWriter(new AsyncOnlyStream(stream), options)) + using (var content = new MemoryStream(Encoding.UTF8.GetBytes("hello"))) + { + await writer.WriteAsync(longName, content, null); + } + + stream.Position = 0; + using var archive = TarArchive.OpenArchive(stream); + Assert.Single(archive.Entries); + Assert.Equal(longName, archive.Entries.Single().Key); + } + + [Fact] + public async ValueTask Tar_Ustar_HeaderFormat_ThrowsForLongDirectory_Async() + { + var longDirectory = new string('a', 170); + + using var stream = new MemoryStream(); + var options = new TarWriterOptions(CompressionType.None, true, TarHeaderWriteFormat.USTAR); + using var writer = new TarWriter(new AsyncOnlyStream(stream), options); + + await Assert.ThrowsAsync(async () => + await writer.WriteDirectoryAsync(longDirectory, null) + ); + } } diff --git a/tests/SharpCompress.Test/Tar/TarWriterDirectoryTests.cs b/tests/SharpCompress.Test/Tar/TarWriterDirectoryTests.cs index b900f9b5..279fb511 100644 --- a/tests/SharpCompress.Test/Tar/TarWriterDirectoryTests.cs +++ b/tests/SharpCompress.Test/Tar/TarWriterDirectoryTests.cs @@ -3,6 +3,7 @@ using System.IO; using System.Linq; using SharpCompress.Archives.Tar; using SharpCompress.Common; +using SharpCompress.Common.Tar.Headers; using SharpCompress.Writers.Tar; using Xunit; @@ -161,4 +162,46 @@ public class TarWriterDirectoryTests : TestBase Assert.Equal("dir2/", entries[2].Key); Assert.True(entries[2].IsDirectory); } + + [Fact] + public void TarWriter_WriteDirectory_Ustar_ThrowsForLongDirectoryName() + { + using var memoryStream = new MemoryStream(); + using var writer = new TarWriter( + memoryStream, + new TarWriterOptions(CompressionType.None, true, TarHeaderWriteFormat.USTAR) + ); + + var longDirectoryName = new string('a', 170); + Assert.Throws(() => + writer.WriteDirectory(longDirectoryName, DateTime.Now) + ); + } + + [Fact] + public void TarWriter_WriteDirectory_GnuLongLink_WritesLongDirectoryName() + { + var longDirectoryName = new string('a', 170); + + using var memoryStream = new MemoryStream(); + using ( + var writer = new TarWriter( + memoryStream, + new TarWriterOptions( + CompressionType.None, + true, + TarHeaderWriteFormat.GNU_TAR_LONG_LINK + ) + ) + ) + { + writer.WriteDirectory(longDirectoryName, DateTime.Now); + } + + memoryStream.Position = 0; + using var archive = TarArchive.OpenArchive(memoryStream); + var entry = archive.Entries.Single(); + Assert.Equal(longDirectoryName + "/", entry.Key); + Assert.True(entry.IsDirectory); + } } diff --git a/tests/SharpCompress.Test/Tar/TarWriterTests.cs b/tests/SharpCompress.Test/Tar/TarWriterTests.cs index 3f81e0c2..3c073f79 100644 --- a/tests/SharpCompress.Test/Tar/TarWriterTests.cs +++ b/tests/SharpCompress.Test/Tar/TarWriterTests.cs @@ -1,6 +1,9 @@ using System.IO; +using System.Linq; using System.Text; +using SharpCompress.Archives.Tar; using SharpCompress.Common; +using SharpCompress.Common.Tar.Headers; using SharpCompress.Writers.Tar; using Xunit; @@ -75,4 +78,58 @@ public class TarWriterTests : WriterTests : paddedContentWithHeader; Assert.Equal(expectedStreamLength, stream.Length); } + + [Fact] + public void Tar_Ustar_HeaderFormat_WritesShortPath() + { + using var stream = new MemoryStream(); + var options = new TarWriterOptions(CompressionType.None, true, TarHeaderWriteFormat.USTAR); + using (var writer = new TarWriter(stream, options)) + using (var content = new MemoryStream(Encoding.UTF8.GetBytes("hello"))) + { + writer.Write("dir/file.txt", content, null); + } + + stream.Position = 0; + using var archive = TarArchive.OpenArchive(stream); + Assert.Single(archive.Entries); + Assert.Equal("dir/file.txt", archive.Entries.Single().Key); + } + + [Fact] + public void Tar_Ustar_HeaderFormat_ThrowsForLongPath() + { + var longName = new string('a', 160) + ".txt"; + + using var stream = new MemoryStream(); + var options = new TarWriterOptions(CompressionType.None, true, TarHeaderWriteFormat.USTAR); + using var writer = new TarWriter(stream, options); + using var content = new MemoryStream(Encoding.UTF8.GetBytes("hello")); + + Assert.Throws(() => writer.Write(longName, content, null)); + } + + [Fact] + public void Tar_GnuLongLink_HeaderFormat_WritesLongPath() + { + var longName = new string('a', 160) + ".txt"; + + using var stream = new MemoryStream(); + var options = new TarWriterOptions( + CompressionType.None, + true, + TarHeaderWriteFormat.GNU_TAR_LONG_LINK + ); + + using (var writer = new TarWriter(stream, options)) + using (var content = new MemoryStream(Encoding.UTF8.GetBytes("hello"))) + { + writer.Write(longName, content, null); + } + + stream.Position = 0; + using var archive = TarArchive.OpenArchive(stream); + Assert.Single(archive.Entries); + Assert.Equal(longName, archive.Entries.Single().Key); + } }