Skip to content

Commit 887461a

Browse files
committed
fix: Restore tar archive write performance regressed by padding trim
1 parent d6cf08d commit 887461a

4 files changed

Lines changed: 76 additions & 17 deletions

File tree

src/Testcontainers/Containers/TarOutputMemoryStream.cs

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ namespace DotNet.Testcontainers.Containers
55
using System.Text;
66
using System.Threading;
77
using System.Threading.Tasks;
8-
using DotNet.Testcontainers;
98
using DotNet.Testcontainers.Configurations;
109
using ICSharpCode.SharpZipLib.Tar;
1110
using Microsoft.Extensions.Logging;
@@ -17,8 +16,12 @@ public sealed class TarOutputMemoryStream : TarOutputStream
1716
{
1817
private readonly string _targetDirectoryPath;
1918

19+
private readonly MemoryStream _memoryStream;
20+
2021
private readonly ILogger _logger;
2122

23+
private long _archiveLength;
24+
2225
private long _contentLength;
2326

2427
/// <summary>
@@ -37,8 +40,14 @@ public TarOutputMemoryStream(string targetDirectoryPath, ILogger logger)
3740
/// </summary>
3841
/// <param name="logger">The logger.</param>
3942
public TarOutputMemoryStream(ILogger logger)
40-
: base(new MemoryStream(), TarArchiveDefaults.TarBlockFactor, Encoding.Default)
43+
: this(new MemoryStream(), logger)
44+
{
45+
}
46+
47+
private TarOutputMemoryStream(MemoryStream memoryStream, ILogger logger)
48+
: base(memoryStream, Encoding.Default)
4149
{
50+
_memoryStream = memoryStream;
4251
_logger = logger;
4352
IsStreamOwner = false;
4453
}
@@ -90,6 +99,7 @@ await WriteAsync(fileContent, ct)
9099
await CloseEntryAsync(ct)
91100
.ConfigureAwait(false);
92101

102+
_ = Interlocked.Add(ref _archiveLength, GetEntryArchiveLength(tarEntry.Size));
93103
_ = Interlocked.Add(ref _contentLength, tarEntry.Size);
94104
}
95105

@@ -163,8 +173,46 @@ await stream.CopyToAsync(this, 81920, ct)
163173
await CloseEntryAsync(ct)
164174
.ConfigureAwait(false);
165175

176+
_ = Interlocked.Add(ref _archiveLength, GetEntryArchiveLength(tarEntry.Size));
166177
_ = Interlocked.Add(ref _contentLength, tarEntry.Size);
167178
}
168179
}
180+
181+
/// <inheritdoc />
182+
protected override void Dispose(bool disposing)
183+
{
184+
base.Dispose(disposing);
185+
186+
if (!disposing || _archiveLength == 0)
187+
{
188+
return;
189+
}
190+
191+
// SharpZipLib pads the final record with zeros beyond the two end-of-archive
192+
// blocks (the record size is block factor × 512 B). Some archive readers, such
193+
// as Podman's, stop reading at the end-of-archive blocks and close the pipe
194+
// before the surplus padding has been flushed, causing the sender to fail with
195+
// a broken pipe (HTTP 500). Trim the archive to its logical length to drop the
196+
// padding while keeping the two end-of-archive blocks intact. See:
197+
// https://github.com/testcontainers/testcontainers-dotnet/issues/1683.
198+
var length = _archiveLength + 2 * TarBuffer.BlockSize;
199+
200+
if (_memoryStream.Length > length)
201+
{
202+
_memoryStream.SetLength(length);
203+
}
204+
}
205+
206+
/// <summary>
207+
/// Gets the number of bytes a tar entry occupies in the archive. A single header
208+
/// block plus its content padded to the next block boundary.
209+
/// </summary>
210+
/// <param name="size">The tar entry's content length.</param>
211+
/// <returns>The block-aligned length of the tar entry.</returns>
212+
private static long GetEntryArchiveLength(long size)
213+
{
214+
var blocks = (size + TarBuffer.BlockSize - 1) / TarBuffer.BlockSize;
215+
return TarBuffer.BlockSize + blocks * TarBuffer.BlockSize;
216+
}
169217
}
170218
}

src/Testcontainers/Images/DockerfileArchive.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ public async Task<string> Tar(CancellationToken ct = default)
192192

193193
using (var tarOutputFileStream = new FileStream(dockerfileArchiveFilePath, FileMode.Create, FileAccess.Write))
194194
{
195-
using (var tarOutputStream = new TarOutputStream(tarOutputFileStream, TarArchiveDefaults.TarBlockFactor, Encoding.Default))
195+
using (var tarOutputStream = new TarOutputStream(tarOutputFileStream, Encoding.Default))
196196
{
197197
tarOutputStream.IsStreamOwner = false;
198198

src/Testcontainers/TarArchiveDefaults.cs

Lines changed: 0 additions & 14 deletions
This file was deleted.

tests/Testcontainers.Platform.Linux.Tests/TarOutputMemoryStreamTest.cs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,31 @@ public void TestFileExistsInTarFile()
5959
Assert.Contains(actual, file => file.EndsWith(_testFile.Name));
6060
}
6161

62+
[Fact]
63+
[Trait(nameof(DockerCli.DockerPlatform), nameof(DockerCli.DockerPlatform.Linux))]
64+
public void TarFileDoesNotContainPaddingBeyondEofBlocks()
65+
{
66+
// Given
67+
_tarOutputMemoryStream.Close();
68+
_tarOutputMemoryStream.Seek(0, SeekOrigin.Begin);
69+
70+
using var buffer = new MemoryStream();
71+
_tarOutputMemoryStream.CopyTo(buffer);
72+
73+
var archive = buffer.ToArray();
74+
75+
// When
76+
var length = archive.Length;
77+
78+
// Then
79+
// The archive holds a single 1-byte file: one header block, one data block and
80+
// the two end-of-archive blocks. SharpZipLib's record padding (block factor 20)
81+
// would otherwise inflate it to a 10240-byte record boundary.
82+
Assert.Equal(4 * TarBuffer.BlockSize, length);
83+
Assert.Equal(0, length % TarBuffer.BlockSize);
84+
Assert.All(archive.Skip(length - (2 * TarBuffer.BlockSize)), b => Assert.Equal(0, b));
85+
}
86+
6287
[UsedImplicitly]
6388
public sealed class FromResourceMapping : TarOutputMemoryStreamTest, IResourceMapping, IClassFixture<FromResourceMapping.HttpFixture>, IAsyncLifetime
6489
{

0 commit comments

Comments
 (0)