diff --git a/src/Containers/Microsoft.NET.Build.Containers/DigestUtils.cs b/src/Containers/Microsoft.NET.Build.Containers/DigestUtils.cs
index 56fdce68dc40..7d1690084140 100644
--- a/src/Containers/Microsoft.NET.Build.Containers/DigestUtils.cs
+++ b/src/Containers/Microsoft.NET.Build.Containers/DigestUtils.cs
@@ -113,15 +113,6 @@ internal static string ComputeSha256(string content)
return Convert.ToHexStringLower(hash);
}
- ///
- /// Validates hash value against the expected hash, failing with a
- /// consistent error message if they don't match.
- ///
- internal static void ValidateHashValueAsync(ReadOnlySpan actualHash, ReadOnlySpan expectedHash)
- {
- InvalidDigestException.ThrowIfMismatched(expectedHash, actualHash);
- }
-
///
/// Validates a digest string against the OCI grammar and registered
/// algorithms, returning the parsed algorithm and encoded portions. Throws
diff --git a/src/Containers/Microsoft.NET.Build.Containers/Layer.cs b/src/Containers/Microsoft.NET.Build.Containers/Layer.cs
index 1cb57628f390..59ee4cab6b00 100644
--- a/src/Containers/Microsoft.NET.Build.Containers/Layer.cs
+++ b/src/Containers/Microsoft.NET.Build.Containers/Layer.cs
@@ -89,139 +89,146 @@ public static Layer FromDirectory(string directory, string containerPath, bool i
}
string tempTarballPath = ContentStore.GetTempFile();
- using (FileStream fs = File.Create(tempTarballPath))
+ try
{
- using (HashDigestGZipStream gz = new(fs, leaveOpen: true))
+ using (FileStream fs = File.Create(tempTarballPath))
{
- using (TarWriter writer = new(gz, TarEntryFormat.Pax, leaveOpen: true))
+ using (HashDigestGZipStream gz = new(fs, leaveOpen: true))
{
- // Windows layers need a Files folder
- if (isWindowsLayer)
+ using (TarWriter writer = new(gz, TarEntryFormat.Pax, leaveOpen: true))
{
- var entry = new PaxTarEntry(TarEntryType.Directory, "Files", entryAttributes);
- writer.WriteEntry(entry);
- }
-
- // Write an entry for the application directory.
- WriteTarEntryForFile(writer, new DirectoryInfo(directory), containerPath, entryAttributes, isWindowsLayer ? null : userId);
-
- // Write entries for the application directory contents.
- var fileList = new FileSystemEnumerable<(FileSystemInfo file, string containerPath)>(
- directory: directory,
- transform: (ref FileSystemEntry entry) =>
- {
- FileSystemInfo fsi = entry.ToFileSystemInfo();
- string relativePath = Path.GetRelativePath(directory, fsi.FullName);
- if (OperatingSystem.IsWindows())
+ // Windows layers need a Files folder
+ if (isWindowsLayer)
+ {
+ var entry = new PaxTarEntry(TarEntryType.Directory, "Files", entryAttributes);
+ writer.WriteEntry(entry);
+ }
+
+ // Write an entry for the application directory.
+ WriteTarEntryForFile(writer, new DirectoryInfo(directory), containerPath, entryAttributes, isWindowsLayer ? null : userId);
+
+ // Write entries for the application directory contents.
+ var fileList = new FileSystemEnumerable<(FileSystemInfo file, string containerPath)>(
+ directory: directory,
+ transform: (ref FileSystemEntry entry) =>
{
- // Use only '/' directory separators.
- relativePath = relativePath.Replace('\\', '/');
- }
- return (fsi, $"{containerPath}/{relativePath}");
- },
- options: new EnumerationOptions()
- {
- AttributesToSkip = FileAttributes.System, // Include hidden files
- RecurseSubdirectories = true
- });
- foreach (var item in fileList)
- {
- WriteTarEntryForFile(writer, item.file, item.containerPath, entryAttributes, isWindowsLayer ? null : userId);
- }
-
- // Windows layers need a Hives folder, we do not need to create any Registry Hive deltas inside
- if (isWindowsLayer)
- {
- var entry = new PaxTarEntry(TarEntryType.Directory, "Hives", entryAttributes);
- writer.WriteEntry(entry);
- }
-
- } // Dispose of the TarWriter before getting the hash so the final data get written to the tar stream
-
- int bytesWritten = gz.GetCurrentUncompressedHash(uncompressedHash);
- Debug.Assert(bytesWritten == uncompressedHash.Length);
- }
+ FileSystemInfo fsi = entry.ToFileSystemInfo();
+ string relativePath = Path.GetRelativePath(directory, fsi.FullName);
+ if (OperatingSystem.IsWindows())
+ {
+ // Use only '/' directory separators.
+ relativePath = relativePath.Replace('\\', '/');
+ }
+ return (fsi, $"{containerPath}/{relativePath}");
+ },
+ options: new EnumerationOptions()
+ {
+ AttributesToSkip = FileAttributes.System, // Include hidden files
+ RecurseSubdirectories = true
+ });
+ foreach (var item in fileList)
+ {
+ WriteTarEntryForFile(writer, item.file, item.containerPath, entryAttributes, isWindowsLayer ? null : userId);
+ }
+
+ // Windows layers need a Hives folder, we do not need to create any Registry Hive deltas inside
+ if (isWindowsLayer)
+ {
+ var entry = new PaxTarEntry(TarEntryType.Directory, "Hives", entryAttributes);
+ writer.WriteEntry(entry);
+ }
+
+ } // Dispose of the TarWriter before getting the hash so the final data get written to the tar stream
+
+ int bytesWritten = gz.GetCurrentUncompressedHash(uncompressedHash);
+ Debug.Assert(bytesWritten == uncompressedHash.Length);
+ }
- fileSize = fs.Length;
+ fileSize = fs.Length;
- fs.Position = 0;
+ fs.Position = 0;
- int bW = SHA256.HashData(fs, hash);
- Debug.Assert(bW == hash.Length);
+ int bW = SHA256.HashData(fs, hash);
+ Debug.Assert(bW == hash.Length);
- // Writes a tar entry corresponding to the file system item.
- static void WriteTarEntryForFile(TarWriter writer, FileSystemInfo file, string containerPath, IEnumerable> entryAttributes, int? userId)
- {
- UnixFileMode mode = DetermineFileMode(file);
- PaxTarEntry entry;
-
- if (file is FileInfo)
+ // Writes a tar entry corresponding to the file system item.
+ static void WriteTarEntryForFile(TarWriter writer, FileSystemInfo file, string containerPath, IEnumerable> entryAttributes, int? userId)
{
- var fileStream = File.OpenRead(file.FullName);
- entry = new(TarEntryType.RegularFile, containerPath, entryAttributes)
+ UnixFileMode mode = DetermineFileMode(file);
+ PaxTarEntry entry;
+
+ if (file is FileInfo)
{
- DataStream = fileStream,
- };
- }
- else
- {
- entry = new(TarEntryType.Directory, containerPath, entryAttributes);
- }
+ var fileStream = File.OpenRead(file.FullName);
+ entry = new(TarEntryType.RegularFile, containerPath, entryAttributes)
+ {
+ DataStream = fileStream,
+ };
+ }
+ else
+ {
+ entry = new(TarEntryType.Directory, containerPath, entryAttributes);
+ }
- entry.Mode = mode;
- if (userId is int uid)
- {
- entry.Uid = uid;
- }
+ entry.Mode = mode;
+ if (userId is int uid)
+ {
+ entry.Uid = uid;
+ }
- writer.WriteEntry(entry);
+ writer.WriteEntry(entry);
- if (entry.DataStream is not null)
- {
- // no longer relying on the `using` of the FileStream, so need to do it manually
- entry.DataStream.Dispose();
- }
+ if (entry.DataStream is not null)
+ {
+ // no longer relying on the `using` of the FileStream, so need to do it manually
+ entry.DataStream.Dispose();
+ }
- static UnixFileMode DetermineFileMode(FileSystemInfo file)
- {
- const UnixFileMode nonExecuteMode = UnixFileMode.UserRead | UnixFileMode.UserWrite |
- UnixFileMode.GroupRead |
- UnixFileMode.OtherRead;
- const UnixFileMode executeMode = nonExecuteMode | UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute;
-
- // On Unix, we can determine the x-bit based on the filesystem permission.
- // On Windows, we use executable permissions for all entries.
- return (OperatingSystem.IsWindows() || ((file.UnixFileMode | UnixFileMode.UserExecute) != 0)) ? executeMode : nonExecuteMode;
+ static UnixFileMode DetermineFileMode(FileSystemInfo file)
+ {
+ const UnixFileMode nonExecuteMode = UnixFileMode.UserRead | UnixFileMode.UserWrite |
+ UnixFileMode.GroupRead |
+ UnixFileMode.OtherRead;
+ const UnixFileMode executeMode = nonExecuteMode | UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute;
+
+ // On Unix, we can determine the x-bit based on the filesystem permission.
+ // On Windows, we use executable permissions for all entries.
+ return (OperatingSystem.IsWindows() || ((file.UnixFileMode | UnixFileMode.UserExecute) != 0)) ? executeMode : nonExecuteMode;
+ }
}
}
- }
- string contentHash = Convert.ToHexStringLower(hash);
- string uncompressedContentHash = Convert.ToHexStringLower(uncompressedHash);
+ string contentHash = Convert.ToHexStringLower(hash);
+ string uncompressedContentHash = Convert.ToHexStringLower(uncompressedHash);
- string layerMediaType = manifestMediaType switch
- {
- // TODO: configurable? gzip always?
- SchemaTypes.DockerManifestV2 => SchemaTypes.DockerLayerGzip,
- SchemaTypes.OciManifestV1 => SchemaTypes.OciLayerGzipV1,
- _ => throw new ArgumentException(Resource.FormatString(nameof(Strings.UnrecognizedMediaType), manifestMediaType))
- };
+ string layerMediaType = manifestMediaType switch
+ {
+ // TODO: configurable? gzip always?
+ SchemaTypes.DockerManifestV2 => SchemaTypes.DockerLayerGzip,
+ SchemaTypes.OciManifestV1 => SchemaTypes.OciLayerGzipV1,
+ _ => throw new ArgumentException(Resource.FormatString(nameof(Strings.UnrecognizedMediaType), manifestMediaType))
+ };
- Descriptor descriptor = new()
- {
- MediaType = layerMediaType,
- Size = fileSize,
- Digest = $"sha256:{contentHash}",
- UncompressedDigest = $"sha256:{uncompressedContentHash}",
- };
+ Descriptor descriptor = new()
+ {
+ MediaType = layerMediaType,
+ Size = fileSize,
+ Digest = $"sha256:{contentHash}",
+ UncompressedDigest = $"sha256:{uncompressedContentHash}",
+ };
- string storedContent = ContentStore.PathForDescriptor(descriptor);
+ string storedContent = ContentStore.PathForDescriptor(descriptor);
- Directory.CreateDirectory(ContentStore.ContentRoot);
+ Directory.CreateDirectory(ContentStore.ContentRoot);
- File.Move(tempTarballPath, storedContent, overwrite: true);
+ File.Move(tempTarballPath, storedContent, overwrite: true);
- return new(storedContent, descriptor);
+ return new(storedContent, descriptor);
+ }
+ finally
+ {
+ File.Delete(tempTarballPath);
+ }
}
internal virtual Stream OpenBackingFile() => File.OpenRead(BackingFile);
diff --git a/src/Containers/Microsoft.NET.Build.Containers/Registry/Registry.cs b/src/Containers/Microsoft.NET.Build.Containers/Registry/Registry.cs
index 1b359e4d7886..3375371b1987 100644
--- a/src/Containers/Microsoft.NET.Build.Containers/Registry/Registry.cs
+++ b/src/Containers/Microsoft.NET.Build.Containers/Registry/Registry.cs
@@ -414,7 +414,7 @@ public async Task DownloadBlobAsync(string repository, Descriptor descri
try
{
- var fileStream = File.OpenRead(localPath);
+ using var fileStream = File.OpenRead(localPath);
var actualHash = SHA256.HashData(fileStream);
var expectedHash = DigestUtils.GetEncodedValue(descriptor.Digest);
@@ -434,48 +434,55 @@ public async Task DownloadBlobAsync(string repository, Descriptor descri
{
// Incorrect digest
_logger.LogTrace(
- "Digest validation failed for cached blob {1} ({2}), redownloading from registry.",
+ "Digest validation failed for cached blob {Path} ({Error}), redownloading from registry.",
localPath, exception.Message);
}
string tempTarballPath = ContentStore.GetTempFile();
- int retryCount = 0;
- while (retryCount < MaxDownloadRetries)
+ try
{
- try
+ int retryCount = 0;
+ while (retryCount < MaxDownloadRetries)
{
- // No local copy, so download one
- using Stream responseStream = await _registryAPI.Blob.GetStreamAsync(repository, descriptor.Digest, cancellationToken).ConfigureAwait(false);
-
- using (FileStream fs = File.Create(tempTarballPath))
+ try
{
- await responseStream
- .CopyToAndVerifyAsync(fs, descriptor.Digest, cancellationToken)
- .ConfigureAwait(false);
+ // No local copy, so download one
+ using Stream responseStream = await _registryAPI.Blob.GetStreamAsync(repository, descriptor.Digest, cancellationToken).ConfigureAwait(false);
+
+ using (FileStream fs = File.Create(tempTarballPath))
+ {
+ await responseStream
+ .CopyToAndVerifyAsync(fs, descriptor.Digest, cancellationToken)
+ .ConfigureAwait(false);
+ }
+
+ // Break the loop if successful
+ break;
}
-
- // Break the loop if successful
- break;
- }
- catch (Exception ex)
- {
- retryCount++;
- if (retryCount >= MaxDownloadRetries)
+ catch (Exception ex)
{
- throw new UnableToDownloadFromRepositoryException(repository);
- }
+ retryCount++;
+ if (retryCount >= MaxDownloadRetries)
+ {
+ throw new UnableToDownloadFromRepositoryException(repository);
+ }
- _logger.LogTrace("Download attempt {0}/{1} for repository '{2}' failed. Error: {3}", retryCount, MaxDownloadRetries, repository, ex.ToString());
+ _logger.LogTrace("Download attempt {0}/{1} for repository '{2}' failed. Error: {3}", retryCount, MaxDownloadRetries, repository, ex.ToString());
- // Wait before retrying
- await Task.Delay(_retryDelayProvider(), cancellationToken).ConfigureAwait(false);
+ // Wait before retrying
+ await Task.Delay(_retryDelayProvider(), cancellationToken).ConfigureAwait(false);
+ }
}
- }
- File.Move(tempTarballPath, localPath, overwrite: true);
+ File.Move(tempTarballPath, localPath, overwrite: true);
- return localPath;
+ return localPath;
+ }
+ finally
+ {
+ File.Delete(tempTarballPath);
+ }
}
internal async Task PushLayerAsync(Layer layer, string repository, CancellationToken cancellationToken)
diff --git a/test/Microsoft.NET.Build.Containers.UnitTests/LayerTests.cs b/test/Microsoft.NET.Build.Containers.UnitTests/LayerTests.cs
new file mode 100644
index 000000000000..aac304ed732e
--- /dev/null
+++ b/test/Microsoft.NET.Build.Containers.UnitTests/LayerTests.cs
@@ -0,0 +1,33 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.NET.Build.Containers.UnitTests;
+
+[TestClass]
+public class LayerTests
+{
+ [TestMethod]
+ [DoNotParallelize]
+ public void FromDirectory_DeletesTempFileWhenCreationFails()
+ {
+ DirectoryInfo artifactRoot = Directory.CreateTempSubdirectory();
+ DirectoryInfo layerContent = Directory.CreateTempSubdirectory();
+ string priorArtifactRoot = ContentStore.ArtifactRoot;
+
+ try
+ {
+ ContentStore.ArtifactRoot = artifactRoot.FullName;
+
+ Assert.ThrowsExactly(() =>
+ Layer.FromDirectory(layerContent.FullName, "/app", false, "unsupported"));
+
+ Assert.IsEmpty(Directory.EnumerateFiles(ContentStore.TempPath));
+ }
+ finally
+ {
+ ContentStore.ArtifactRoot = priorArtifactRoot;
+ artifactRoot.Delete(recursive: true);
+ layerContent.Delete(recursive: true);
+ }
+ }
+}
diff --git a/test/Microsoft.NET.Build.Containers.UnitTests/RegistryTests.cs b/test/Microsoft.NET.Build.Containers.UnitTests/RegistryTests.cs
index ed6535a9b64b..5288dfaee974 100644
--- a/test/Microsoft.NET.Build.Containers.UnitTests/RegistryTests.cs
+++ b/test/Microsoft.NET.Build.Containers.UnitTests/RegistryTests.cs
@@ -588,6 +588,79 @@ public async Task DownloadBlobAsync_RetriesOnFailure()
}
}
+ [TestMethod]
+ [DoNotParallelize]
+ public async Task DownloadBlobAsync_RedownloadsCachedBlobWithInvalidDigest()
+ {
+ var logger = _loggerFactory.CreateLogger(nameof(DownloadBlobAsync_RedownloadsCachedBlobWithInvalidDigest));
+ string repoName = "testRepo";
+ byte[] expectedContent = [1, 2, 3];
+ var descriptor = new Descriptor(SchemaTypes.OciLayerGzipV1, "sha256:039058c6f2c0cb492c533b0a4d14ef77cc0f78abccced5287d84a1a2011cfb81", expectedContent.Length);
+ var mockRegistryAPI = new Mock(MockBehavior.Strict);
+ mockRegistryAPI
+ .Setup(api => api.Blob.GetStreamAsync(repoName, descriptor.Digest, CancellationToken.None))
+ .ReturnsAsync(new MemoryStream(expectedContent));
+
+ DirectoryInfo artifactRoot = Directory.CreateTempSubdirectory();
+ string priorArtifactRoot = ContentStore.ArtifactRoot;
+
+ try
+ {
+ ContentStore.ArtifactRoot = artifactRoot.FullName;
+ string localPath = ContentStore.PathForDescriptor(descriptor);
+ await File.WriteAllBytesAsync(localPath, [4, 5, 6], TestContext.CancellationToken);
+
+ string result = await new Registry(repoName, logger, mockRegistryAPI.Object)
+ .DownloadBlobAsync(repoName, descriptor, CancellationToken.None);
+
+ Assert.AreEqual(localPath, result);
+ Assert.AreSequenceEqual(expectedContent, await File.ReadAllBytesAsync(result, TestContext.CancellationToken));
+ mockRegistryAPI.Verify(
+ api => api.Blob.GetStreamAsync(repoName, descriptor.Digest, CancellationToken.None),
+ Times.Once());
+ }
+ finally
+ {
+ ContentStore.ArtifactRoot = priorArtifactRoot;
+ artifactRoot.Delete(recursive: true);
+ }
+ }
+
+ [TestMethod]
+ [DoNotParallelize]
+ public async Task DownloadBlobAsync_RejectsDownloadedBlobWithInvalidDigest()
+ {
+ var logger = _loggerFactory.CreateLogger(nameof(DownloadBlobAsync_RejectsDownloadedBlobWithInvalidDigest));
+ string repoName = "testRepo";
+ var descriptor = new Descriptor(SchemaTypes.OciLayerGzipV1, "sha256:039058c6f2c0cb492c533b0a4d14ef77cc0f78abccced5287d84a1a2011cfb81", 3);
+ var mockRegistryAPI = new Mock(MockBehavior.Strict);
+ mockRegistryAPI
+ .Setup(api => api.Blob.GetStreamAsync(repoName, descriptor.Digest, CancellationToken.None))
+ .ReturnsAsync(() => new MemoryStream([4, 5, 6]));
+
+ DirectoryInfo artifactRoot = Directory.CreateTempSubdirectory();
+ string priorArtifactRoot = ContentStore.ArtifactRoot;
+
+ try
+ {
+ ContentStore.ArtifactRoot = artifactRoot.FullName;
+ Registry registry = new(repoName, logger, mockRegistryAPI.Object, null, () => TimeSpan.Zero);
+
+ await Assert.ThrowsExactlyAsync(
+ () => registry.DownloadBlobAsync(repoName, descriptor, CancellationToken.None));
+
+ mockRegistryAPI.Verify(
+ api => api.Blob.GetStreamAsync(repoName, descriptor.Digest, CancellationToken.None),
+ Times.Exactly(5));
+ Assert.IsEmpty(Directory.EnumerateFiles(ContentStore.TempPath));
+ }
+ finally
+ {
+ ContentStore.ArtifactRoot = priorArtifactRoot;
+ artifactRoot.Delete(recursive: true);
+ }
+ }
+
[TestMethod]
public async Task DownloadBlobAsync_ThrowsAfterMaxRetries()
{