diff --git a/generator/.DevConfigs/922d5cff-5eaf-43f8-82bc-6b8f6ffac4da.json b/generator/.DevConfigs/922d5cff-5eaf-43f8-82bc-6b8f6ffac4da.json new file mode 100644 index 000000000000..9cc2e1f711ed --- /dev/null +++ b/generator/.DevConfigs/922d5cff-5eaf-43f8-82bc-6b8f6ffac4da.json @@ -0,0 +1,11 @@ +{ + "services": [ + { + "serviceName": "S3", + "type": "minor", + "changeLogMessages": [ + "BREAKING CHANGE: A security fix has been applied to the Transfer Utility Download Directory command which now checks if the directory is case-sensitive before proceeding with the download. This prevents potential overwriting of files in case-insensitive file systems. If you would like to revert to the previous behavior, you can set the 'SkipDirectoryCaseSensitivityCheck' property on the 'TransferUtilityConfig' to true." + ] + } + ] +} \ No newline at end of file diff --git a/sdk/src/Services/S3/Custom/Transfer/Internal/_bcl+netstandard/DownloadDirectoryCommand.cs b/sdk/src/Services/S3/Custom/Transfer/Internal/_bcl+netstandard/DownloadDirectoryCommand.cs index 85cb94ac4662..8a193d569f05 100644 --- a/sdk/src/Services/S3/Custom/Transfer/Internal/_bcl+netstandard/DownloadDirectoryCommand.cs +++ b/sdk/src/Services/S3/Custom/Transfer/Internal/_bcl+netstandard/DownloadDirectoryCommand.cs @@ -66,10 +66,13 @@ public override async Task ExecuteAsyn // Step 3: Filter to actual files (exclude directory markers) var objectsToDownload = FilterObjectsToDownload(s3Objects); - + // Step 4: Setup resources and execute downloads using (var resources = CreateDownloadResources(cancellationToken)) { + await CheckIfKeysDifferByCase(objectsToDownload, prefixLength, resources.InternalCancellationTokenSource) + .ConfigureAwait(false); + await ExecuteParallelDownloadsAsync( objectsToDownload, prefixLength, @@ -93,6 +96,63 @@ await ExecuteParallelDownloadsAsync( } } + /// + /// Checks if any S3 object keys differ only by case. + /// On case-insensitive file systems, this can lead to conflicts where objects can be overwritten. + /// If such conflicts are found, the configured failure policy is applied. + /// + private async Task CheckIfKeysDifferByCase(List s3Objects, int prefixLength, CancellationTokenSource cancellationTokenSource) + { + if (_config.SkipDirectoryCaseSensitivityCheck) + { + _logger.DebugFormat("DownloadDirectoryCommand.CheckIfKeysDifferByCase: Skipping case sensitivity check as per configuration."); + return; + } + var isCaseSensitive = FileSystemHelper.IsDirectoryCaseSensitive(this._request.LocalDirectory); + if (isCaseSensitive) + { + _logger.DebugFormat("DownloadDirectoryCommand.CheckIfKeysDifferByCase: Target directory is case-sensitive, skipping check."); + return; + } + + var hashSet = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var s3Object in s3Objects) + { + // Create failure callback + Action onFailure = (ex) => + { + var downloadRequest = ConstructTransferUtilityDownloadRequest(s3Object, prefixLength); + this._request.OnRaiseObjectDownloadFailedEvent( + new ObjectDownloadFailedEventArgs( + this._request, + downloadRequest, + ex)); + }; + + await _failurePolicy.ExecuteAsync( + () => + { + string currentKey = s3Object.Key; + + // Check the key against the hash set for case-insensitive duplicates + if (!hashSet.Add(currentKey)) + { + // A case-insensitive match was found. + // Handle the conflict according to the configured failure policy. + throw new AmazonS3Exception( + "The S3 directory contains multiple objects whose keys differ only by case. " + + "This can lead to conflicts when downloading to a case-insensitive file system. " + + $"The key {currentKey} conflicts with another object that differs only by case."); + } + + return Task.CompletedTask; + }, + onFailure, + cancellationTokenSource + ).ConfigureAwait(false); + } + } + /// /// Encapsulates disposable resources used during directory download. /// diff --git a/sdk/src/Services/S3/Custom/Transfer/TransferUtilityConfig.cs b/sdk/src/Services/S3/Custom/Transfer/TransferUtilityConfig.cs index c652bd4a5a36..ddf85a44d3b0 100644 --- a/sdk/src/Services/S3/Custom/Transfer/TransferUtilityConfig.cs +++ b/sdk/src/Services/S3/Custom/Transfer/TransferUtilityConfig.cs @@ -49,6 +49,7 @@ public partial class TransferUtilityConfig public TransferUtilityConfig() { this.ConcurrentServiceRequests = 10; + this.SkipDirectoryCaseSensitivityCheck = false; } /// @@ -81,5 +82,12 @@ public int ConcurrentServiceRequests this._concurrentServiceRequests = value; } } + + /// + /// When true, disables directory case-sensitivity checks that are used to detect + /// potential filename collisions (for example, S3 keys that differ only by case + /// on a case-insensitive filesystem). The default is false. + /// + public bool SkipDirectoryCaseSensitivityCheck { get; set; } } } diff --git a/sdk/src/Services/S3/Custom/Util/FileSystemHelper.cs b/sdk/src/Services/S3/Custom/Util/FileSystemHelper.cs new file mode 100644 index 000000000000..3cad6dd24990 --- /dev/null +++ b/sdk/src/Services/S3/Custom/Util/FileSystemHelper.cs @@ -0,0 +1,89 @@ +using System; +using System.IO; + +namespace Amazon.S3.Util +{ + internal static class FileSystemHelper + { + /// + /// Determines if the specified directory path is case-sensitive. + /// + /// The full path to the directory to check. + /// + /// Case-sensitivity can vary by directory or volume depending on the platform and configuration: + /// + /// + /// + /// On Windows, NTFS is case-preserving but usually case-insensitive; however, case-sensitivity + /// can be enabled per-directory (for example via WSL tooling), so two directories on the same + /// volume may behave differently. + /// + /// + /// + /// + /// On Unix-like systems and macOS, different mount points or volumes can have different + /// case-sensitivity semantics. + /// + /// + /// + /// Because of this, the check is performed against the actual target directory instead of a + /// global temporary location. This ensures we detect collisions (such as S3 keys that differ + /// only by case) according to the behavior of the specific filesystem where we will create files. + /// + /// + /// True if the file system at the path is case-sensitive; False if case-insensitive. + /// + /// If the directoryPath is null or empty. + /// If the directory does not exist. + public static bool IsDirectoryCaseSensitive(string directoryPath) + { + if (string.IsNullOrEmpty(directoryPath)) + { + throw new ArgumentNullException(nameof(directoryPath)); + } + + if (!Directory.Exists(directoryPath)) + { + throw new DirectoryNotFoundException($"The directory '{directoryPath}' does not exist."); + } + +#pragma warning disable CA1308 // Normalize strings to uppercase + string tempFileLower = Path.Combine(directoryPath, Guid.NewGuid().ToString().ToLowerInvariant() + ".tmp"); +#pragma warning restore CA1308 // Normalize strings to uppercase + string tempFileUpper = Path.Combine(directoryPath, Path.GetFileName(tempFileLower).ToUpperInvariant()); + bool isCaseSensitive = true; // Assume sensitive by default + +#pragma warning disable CA1031 // Do not catch general exception types + try + { + // Create the file with a lowercase name + using (File.Create(tempFileLower)) { } + + // Check if the uppercase version of the file exists. + // If it does, the file system is case-insensitive. + if (File.Exists(tempFileUpper)) + { + isCaseSensitive = false; + } + } + catch (Exception) + { + // Fallback assumption is that the file system is case-sensitive + isCaseSensitive = true; + } + finally + { + try + { + // Ensure temporary files are cleaned up in all cases + if (File.Exists(tempFileLower)) File.Delete(tempFileLower); + if (File.Exists(tempFileUpper)) File.Delete(tempFileUpper); + } + catch (Exception) { } + } +#pragma warning restore CA1031 // Do not catch general exception types + + return isCaseSensitive; + } + } +} diff --git a/sdk/test/Services/S3/UnitTests/Custom/DownloadDirectoryCommandTests.cs b/sdk/test/Services/S3/UnitTests/Custom/DownloadDirectoryCommandTests.cs index 900acf89d93b..5ed130dc24ce 100644 --- a/sdk/test/Services/S3/UnitTests/Custom/DownloadDirectoryCommandTests.cs +++ b/sdk/test/Services/S3/UnitTests/Custom/DownloadDirectoryCommandTests.cs @@ -2,6 +2,7 @@ using Amazon.S3.Model; using Amazon.S3.Transfer; using Amazon.S3.Transfer.Internal; +using Amazon.S3.Util; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; using System; @@ -1009,6 +1010,139 @@ public async Task ExecuteAsync_SequentialMode_DownloadsOneAtATime() #endregion + #region Case-Insensitive Key Conflict Tests + + [TestMethod] + public async Task ExecuteAsync_CaseInsensitiveKeyConflict_AbortOnFailure_ThrowsAndNoFilesDownloaded() + { + var request = CreateDownloadDirectoryRequest(); + var isCaseSensitive = FileSystemHelper.IsDirectoryCaseSensitive(request.LocalDirectory); + if (isCaseSensitive) + { + return; + } + request.FailurePolicy = FailurePolicy.AbortOnFailure; + + var listResponse = new ListObjectsResponse + { + S3Objects = new List + { + new S3Object { Key = "prefix/File.txt", Size = 10 }, + new S3Object { Key = "prefix/file.txt", Size = 20 } + } + }; + + _mockS3Client.Setup(c => c.ListObjectsAsync( + It.IsAny(), + It.IsAny())) + .ReturnsAsync(listResponse); + + var command = new DownloadDirectoryCommand(_mockS3Client.Object, request, _config, useMultipartDownload: false); + + try + { + await command.ExecuteAsync(CancellationToken.None); + Assert.Fail("Expected an exception due to case-insensitive key conflict."); + } + catch (Exception ex) + { + StringAssert.Contains(ex.Message, "conflict", "Exception message should indicate a key path conflict."); + } + } + + [TestMethod] + public async Task ExecuteAsync_CaseInsensitiveKeyConflict_ContinueOnFailure_ReportsFailureInResponse() + { + var request = CreateDownloadDirectoryRequest(); + var isCaseSensitive = FileSystemHelper.IsDirectoryCaseSensitive(request.LocalDirectory); + if (isCaseSensitive) + { + return; + } + request.FailurePolicy = FailurePolicy.ContinueOnFailure; + + var listResponse = new ListObjectsResponse + { + S3Objects = new List + { + new S3Object { Key = "prefix/File.txt", Size = 10 }, + new S3Object { Key = "prefix/file.txt", Size = 20 } + } + }; + + _mockS3Client.Setup(c => c.ListObjectsAsync( + It.IsAny(), + It.IsAny())) + .ReturnsAsync(listResponse); + + var command = new DownloadDirectoryCommand(_mockS3Client.Object, request, _config, useMultipartDownload: false); + + var response = await command.ExecuteAsync(CancellationToken.None); + + Assert.IsNotNull(response, "Response should not be null."); + Assert.AreEqual(0, response.ObjectsDownloaded, "No files should be downloaded when all conflicted."); + Assert.IsTrue(response.ObjectsFailed >= 1, "At least one failure should be recorded."); + Assert.IsNotNull(response.Errors, "Errors collection should be populated."); + Assert.IsTrue(response.Errors.Count >= 1, "Errors collection should contain at least one error."); + Assert.IsTrue(response.Result == DirectoryResult.Failure || + response.Result == DirectoryResult.PartialSuccess, + "Result should indicate failure or partial success due to conflicts."); + } + + [TestMethod] + public async Task ExecuteAsync_CaseInsensitiveKeyConflict_SkipCaseCheck_AllowsDownloads() + { + var request = CreateDownloadDirectoryRequest(); + var isCaseSensitive = FileSystemHelper.IsDirectoryCaseSensitive(request.LocalDirectory); + if (isCaseSensitive) + { + // This test validates behavior only on case-insensitive filesystems. + // On case-sensitive systems, there is no conflict to detect. + return; + } + + request.FailurePolicy = FailurePolicy.AbortOnFailure; + + var listResponse = new ListObjectsResponse + { + S3Objects = new List + { + new S3Object { Key = "prefix/File.txt", Size = 10 }, + new S3Object { Key = "prefix/file.txt", Size = 20 } + } + }; + + _mockS3Client.Setup(c => c.ListObjectsAsync( + It.IsAny(), + It.IsAny())) + .ReturnsAsync(listResponse); + + SetupGetObjectForFile("prefix/File.txt", 10, setupForMultipart: false); + SetupGetObjectForFile("prefix/file.txt", 20, setupForMultipart: false); + + // Enable skipping of directory case-sensitivity checks so that the + // conflict detection logic is bypassed. + var config = new TransferUtilityConfig + { + ConcurrentServiceRequests = 4, + SkipDirectoryCaseSensitivityCheck = true + }; + + var command = new DownloadDirectoryCommand(_mockS3Client.Object, request, config, useMultipartDownload: false); + + // Act: with the skip flag enabled, the operation should not throw a + // conflict exception and should attempt to download objects. + var response = await command.ExecuteAsync(CancellationToken.None); + + Assert.IsNotNull(response, "Response should not be null when skipping case check."); + // Behavior of how many files succeed may vary by implementation + // (last one wins on overwrite), but we at least assert that no + // conflict exception prevented completion. + Assert.IsTrue(response.ObjectsDownloaded >= 1, "At least one object should have been downloaded when skipping case checks."); + } + + #endregion + #region Helper Methods private TransferUtilityDownloadDirectoryRequest CreateDownloadDirectoryRequest(