Skip to content
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

Do not read position when tar archive stream is unseekable #70178

Merged
merged 6 commits into from
Jun 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,6 @@ private void WriteFinalRecords()
byte[] emptyRecord = new byte[TarHelpers.RecordSize];
_archiveStream.Write(emptyRecord);
_archiveStream.Write(emptyRecord);
_archiveStream.SetLength(_archiveStream.Position);
}

// Partial method for reading an entry from disk and writing it into the archive stream.
Expand Down
80 changes: 80 additions & 0 deletions src/libraries/System.Formats.Tar/tests/CompressedTar.Tests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.IO.Compression;
using System.IO;
using Xunit;

namespace System.Formats.Tar.Tests
{
public class CompressedTar_Tests : TarTestsBase
{
[Fact]
public void TarGz_TarWriter_TarReader()
Copy link
Member

Choose a reason for hiding this comment

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

are these tests related to the fix?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. This test passes a GZipStream (an unseekable stream) to the TarWriter. TarWriter will finish adding the entries, then will add the two empty records, and then it would've called that incorrect SetLength call (which I just removed completely since it's not needed).

{
using TempDirectory root = new TempDirectory();

string archivePath = Path.Join(root.Path, "compressed.tar.gz");

string fileName = "file.txt";
string filePath = Path.Join(root.Path, fileName);
File.Create(filePath).Dispose();

// Create tar.gz archive
using (FileStream streamToCompress = File.Create(archivePath))
{
using GZipStream compressorStream = new GZipStream(streamToCompress, CompressionMode.Compress);
using TarWriter writer = new TarWriter(compressorStream);
writer.WriteEntry(fileName: filePath, entryName: fileName);
}
FileInfo fileInfo = new FileInfo(archivePath);
Assert.True(fileInfo.Exists);
Assert.True(fileInfo.Length > 0);

// Verify tar.gz archive contents
using (FileStream streamToDecompress = File.OpenRead(archivePath))
{
using GZipStream decompressorStream = new GZipStream(streamToDecompress, CompressionMode.Decompress);
using TarReader reader = new TarReader(decompressorStream);
TarEntry entry = reader.GetNextEntry();
Assert.Equal(TarFormat.Pax, reader.Format);
Assert.Equal(fileName, entry.Name);
Assert.Null(reader.GetNextEntry());
}
}

[Fact]
public void TarGz_TarFile_CreateFromDir_ExtractToDir()
{
using TempDirectory root = new TempDirectory();

string archivePath = Path.Join(root.Path, "compressed.tar.gz");

string sourceDirectory = Path.Join(root.Path, "source");
Directory.CreateDirectory(sourceDirectory);

string destinationDirectory = Path.Join(root.Path, "destination");
Directory.CreateDirectory(destinationDirectory);

string fileName = "file.txt";
string filePath = Path.Join(sourceDirectory, fileName);
File.Create(filePath).Dispose();

using (FileStream streamToCompress = File.Create(archivePath))
{
using GZipStream compressorStream = new GZipStream(streamToCompress, CompressionMode.Compress);
TarFile.CreateFromDirectory(sourceDirectory, compressorStream, includeBaseDirectory: false);
}
FileInfo fileInfo = new FileInfo(archivePath);
Assert.True(fileInfo.Exists);
Assert.True(fileInfo.Length > 0);

using (FileStream streamToDecompress = File.OpenRead(archivePath))
{
using GZipStream decompressorStream = new GZipStream(streamToDecompress, CompressionMode.Decompress);
TarFile.ExtractToDirectory(decompressorStream, destinationDirectory, overwriteFiles: true);
Assert.True(File.Exists(filePath));
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
</PropertyGroup>
<ItemGroup>
<Compile Include="$(CommonTestPath)System\IO\TempDirectory.cs" Link="Common\System\IO\TempDirectory.cs" />
<Compile Include="CompressedTar.Tests.cs" />
<Compile Include="TarEntry\TarEntryV7.Tests.cs" />
<Compile Include="TarEntry\TarEntryUstar.Tests.cs" />
<Compile Include="TarEntry\TarEntryPax.Tests.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,29 @@ public void Constructor_NoEntryInsertion_WritesNothing()
Assert.Equal(0, archiveStream.Length);
}

[Fact]
public void Write_To_UnseekableStream()
{
using MemoryStream inner = new MemoryStream();
using WrappedStream wrapped = new WrappedStream(inner, canRead: true, canWrite: true, canSeek: false);

using (TarWriter writer = new TarWriter(wrapped, leaveOpen: true))
{
PaxTarEntry paxEntry = new PaxTarEntry(TarEntryType.RegularFile, "file.txt");
writer.WriteEntry(paxEntry);
} // The final records should get written, and the length should not be set because position cannot be read

inner.Seek(0, SeekOrigin.Begin); // Rewind the base stream (wrapped cannot be rewound)

using (TarReader reader = new TarReader(wrapped))
{
TarEntry entry = reader.GetNextEntry();
Assert.Equal(TarFormat.Pax, reader.Format);
Assert.Equal(TarEntryType.RegularFile, entry.EntryType);
Assert.Null(reader.GetNextEntry());
}
}

[Fact]
Copy link
Member Author

Choose a reason for hiding this comment

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

We have a test that verifies the behavior of the TarEntry.DataStream when reading an unseekable stream:

public void GetNextEntry_CopyDataFalse_UnseekableArchive_Exceptions()

But that test wasn't creating an archive using TarWriter against an unseekable stream, hence why this bug wasn't caught before. That's why I'm also adding this extra test to write to a generic unseekable stream.

public void VerifyChecksumV7()
{
Expand Down