add case sensitivity check to download directory command

philasmar · philasmar · commit 960680b2d839 · 2025-12-18T11:09:43.000-05:00
diff --git a/generator/.DevConfigs/922d5cff-5eaf-43f8-82bc-6b8f6ffac4da.json 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."
+      ]
+    }
+  ]
+}
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
@@ -66,10 +66,13 @@ public override async Task<TransferUtilityDownloadDirectoryResponse> 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(
             }
         }
 
+        /// <summary>
+        /// 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.
+        /// </summary>
+        private async Task CheckIfKeysDifferByCase(List<S3Object> 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<string>(StringComparer.OrdinalIgnoreCase);
+            foreach (var s3Object in s3Objects)
+            {
+                // Create failure callback
+                Action<Exception> 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);
+            }
+        }
+
         /// <summary>
         /// Encapsulates disposable resources used during directory download.
         /// </summary>
diff --git 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;
         }
 
         /// <summary>
@@ -81,5 +82,12 @@ public int ConcurrentServiceRequests
                 this._concurrentServiceRequests = value;
             }
         }
+
+        /// <summary>
+        /// 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.
+        /// </summary>
+        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
@@ -0,0 +1,89 @@
+﻿using System;
+using System.IO;
+
+namespace Amazon.S3.Util
+{
+    internal static class FileSystemHelper
+    {
+        /// <summary>
+        /// Determines if the specified directory path is case-sensitive.
+        /// </summary>
+        /// <param name="directoryPath">The full path to the directory to check.</param>
+        /// <remarks>
+        /// Case-sensitivity can vary by directory or volume depending on the platform and configuration:
+        /// <list type="bullet">
+        /// <item>
+        /// <description>
+        /// 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.
+        /// </description>
+        /// </item>
+        /// <item>
+        /// <description>
+        /// On Unix-like systems and macOS, different mount points or volumes can have different
+        /// case-sensitivity semantics.
+        /// </description>
+        /// </item>
+        /// </list>
+        /// 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.
+        /// </remarks>
+        /// <returns>
+        /// True if the file system at the path is case-sensitive; False if case-insensitive.
+        /// </returns>
+        /// <exception cref="ArgumentNullException">If the directoryPath is null or empty.</exception>
+        /// <exception cref="DirectoryNotFoundException">If the directory does not exist.</exception>
+        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
@@ -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<S3Object>
+                {
+                    new S3Object { Key = "prefix/File.txt", Size = 10 },
+                    new S3Object { Key = "prefix/file.txt", Size = 20 }
+                }
+            };
+
+            _mockS3Client.Setup(c => c.ListObjectsAsync(
+                It.IsAny<ListObjectsRequest>(),
+                It.IsAny<CancellationToken>()))
+                .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<S3Object>
+                {
+                    new S3Object { Key = "prefix/File.txt", Size = 10 },
+                    new S3Object { Key = "prefix/file.txt", Size = 20 }
+                }
+            };
+
+            _mockS3Client.Setup(c => c.ListObjectsAsync(
+                It.IsAny<ListObjectsRequest>(),
+                It.IsAny<CancellationToken>()))
+                .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<S3Object>
+                {
+                    new S3Object { Key = "prefix/File.txt", Size = 10 },
+                    new S3Object { Key = "prefix/file.txt", Size = 20 }
+                }
+            };
+
+            _mockS3Client.Setup(c => c.ListObjectsAsync(
+                It.IsAny<ListObjectsRequest>(),
+                It.IsAny<CancellationToken>()))
+                .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(

Original file line number	Diff line number	Diff line change
`@@ -49,6 +49,7 @@ public partial class TransferUtilityConfig`
`49`	`49`	`public TransferUtilityConfig()`
`50`	`50`	`{`
`51`	`51`	`this.ConcurrentServiceRequests = 10;`
	`52`	`+ this.SkipDirectoryCaseSensitivityCheck = false;`
`52`	`53`	`}`
`53`	`54`
`54`	`55`	`/// <summary>`
`@@ -81,5 +82,12 @@ public int ConcurrentServiceRequests`
`81`	`82`	`this._concurrentServiceRequests = value;`
`82`	`83`	`}`
`83`	`84`	`}`
	`85`	`+`
	`86`	`+ /// <summary>`
	`87`	`+ /// When true, disables directory case-sensitivity checks that are used to detect`
	`88`	`+ /// potential filename collisions (for example, S3 keys that differ only by case`
	`89`	`+ /// on a case-insensitive filesystem). The default is false.`
	`90`	`+ /// </summary>`
	`91`	`+ public bool SkipDirectoryCaseSensitivityCheck { get; set; }`
`84`	`92`	`}`
`85`	`93`	`}`