Fix for issue where iterating over directories ending in a dot would unexpectedly...
authorCarlos Sanchez Lopez <1175054+carlossanlop@users.noreply.github.com>
Fri, 21 Jun 2019 02:58:33 +0000 (19:58 -0700)
committerGitHub <noreply@github.com>
Fri, 21 Jun 2019 02:58:33 +0000 (19:58 -0700)
Issue
In Windows, when iterating over the files of a directory whose name ends in a dot, the iteration fails by throwing an unexpected exception, even when using the proper prefix "\\?\" for the full directory path.

Fix
Add a new boolean argument at the end of the enumeration function signatures that will indicate if the passed path is normalized or not. If it is normalized (which only happens when the path is created by us, not by the user), then we can create the handle using that path. If it's not normalized, we attempt to get its full path before creating the handle, which will throw if a full path can't be obtained.

Added a new unit test to verify the fixed behavior and updated some others whose behavior changed with this fix.

Commit migrated from https://github.com/dotnet/corefx/commit/8e95261ef710e0df980a6d46923eb3c0e8b0bc83

14 files changed:
src/libraries/System.IO.FileSystem/src/System/IO/Directory.cs
src/libraries/System.IO.FileSystem/src/System/IO/DirectoryInfo.cs
src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEnumerable.cs
src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEnumerableFactory.cs
src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEnumerator.Unix.cs
src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEnumerator.Windows.cs
src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEnumerator.cs
src/libraries/System.IO.FileSystem/src/System/IO/EnumerationOptions.cs
src/libraries/System.IO.FileSystem/tests/Directory/EnumerableAPIs.cs
src/libraries/System.IO.FileSystem/tests/Directory/EnumerableTests.cs
src/libraries/System.IO.FileSystem/tests/Directory/GetFileSystemEntries_str.cs
src/libraries/System.IO.FileSystem/tests/Directory/GetFileSystemEntries_str_str.cs
src/libraries/System.IO.FileSystem/tests/DirectoryInfo/EnumerableAPIs.cs
src/libraries/System.IO.IsolatedStorage/tests/System/IO/IsolatedStorage/GetFileNamesTests.cs

index 0460a23..c25f026 100644 (file)
@@ -177,7 +177,7 @@ namespace System.IO
             if (searchPattern == null)
                 throw new ArgumentNullException(nameof(searchPattern));
 
-            FileSystemEnumerableFactory.NormalizeInputs(ref path, ref searchPattern, options);
+            FileSystemEnumerableFactory.NormalizeInputs(ref path, ref searchPattern, options.MatchType);
 
             switch (searchTarget)
             {
index a71556d..a296577 100644 (file)
@@ -20,6 +20,8 @@ namespace System.IO
 {
     public sealed partial class DirectoryInfo : FileSystemInfo
     {
+        private bool _isNormalized;
+
         public DirectoryInfo(string path)
         {
             Init(originalPath: path,
@@ -45,6 +47,8 @@ namespace System.IO
                     Path.GetFileName(Path.TrimEndingDirectorySeparator(fullPath.AsSpan()))).ToString();
 
             FullPath = fullPath;
+
+            _isNormalized = isNormalized;
         }
 
         public DirectoryInfo Parent
@@ -165,7 +169,7 @@ namespace System.IO
         public IEnumerable<FileSystemInfo> EnumerateFileSystemInfos(string searchPattern, EnumerationOptions enumerationOptions)
             => InternalEnumerateInfos(FullPath, searchPattern, SearchTarget.Both, enumerationOptions);
 
-        internal static IEnumerable<FileSystemInfo> InternalEnumerateInfos(
+        private IEnumerable<FileSystemInfo> InternalEnumerateInfos(
             string path,
             string searchPattern,
             SearchTarget searchTarget,
@@ -175,16 +179,16 @@ namespace System.IO
             if (searchPattern == null)
                 throw new ArgumentNullException(nameof(searchPattern));
 
-            FileSystemEnumerableFactory.NormalizeInputs(ref path, ref searchPattern, options);
+            _isNormalized &= FileSystemEnumerableFactory.NormalizeInputs(ref path, ref searchPattern, options.MatchType);
 
             switch (searchTarget)
             {
                 case SearchTarget.Directories:
-                    return FileSystemEnumerableFactory.DirectoryInfos(path, searchPattern, options);
+                    return FileSystemEnumerableFactory.DirectoryInfos(path, searchPattern, options, _isNormalized);
                 case SearchTarget.Files:
-                    return FileSystemEnumerableFactory.FileInfos(path, searchPattern, options);
+                    return FileSystemEnumerableFactory.FileInfos(path, searchPattern, options, _isNormalized);
                 case SearchTarget.Both:
-                    return FileSystemEnumerableFactory.FileSystemInfos(path, searchPattern, options);
+                    return FileSystemEnumerableFactory.FileSystemInfos(path, searchPattern, options, _isNormalized);
                 default:
                     throw new ArgumentException(SR.ArgumentOutOfRange_Enum, nameof(searchTarget));
             }
index 35d81e1..f3601a0 100644 (file)
@@ -24,6 +24,11 @@ namespace System.IO.Enumeration
         private readonly string _directory;
 
         public FileSystemEnumerable(string directory, FindTransform transform, EnumerationOptions options = null)
+            : this(directory, transform, options, isNormalized: false)
+        {
+        }
+
+        internal FileSystemEnumerable(string directory, FindTransform transform, EnumerationOptions options, bool isNormalized)
         {
             _directory = directory ?? throw new ArgumentNullException(nameof(directory));
             _transform = transform ?? throw new ArgumentNullException(nameof(transform));
@@ -31,7 +36,7 @@ namespace System.IO.Enumeration
 
             // We need to create the enumerator up front to ensure that we throw I/O exceptions for
             // the root directory on creation of the enumerable.
-            _enumerator = new DelegateEnumerator(this);
+            _enumerator = new DelegateEnumerator(this, isNormalized);
         }
 
         public FindPredicate ShouldIncludePredicate { get; set; }
@@ -39,7 +44,7 @@ namespace System.IO.Enumeration
 
         public IEnumerator<TResult> GetEnumerator()
         {
-            return Interlocked.Exchange(ref _enumerator, null) ?? new DelegateEnumerator(this);
+            return Interlocked.Exchange(ref _enumerator, null) ?? new DelegateEnumerator(this, isNormalized: false);
         }
 
         IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
@@ -58,8 +63,8 @@ namespace System.IO.Enumeration
         {
             private readonly FileSystemEnumerable<TResult> _enumerable;
 
-            public DelegateEnumerator(FileSystemEnumerable<TResult> enumerable)
-                : base(enumerable._directory, enumerable._options)
+            public DelegateEnumerator(FileSystemEnumerable<TResult> enumerable, bool isNormalized)
+                : base(enumerable._directory, isNormalized, enumerable._options)
             {
                 _enumerable = enumerable;
             }
index 4615ca1..5c1fba0 100644 (file)
@@ -20,11 +20,32 @@ namespace System.IO.Enumeration
         // which is not true in Windows and as such we'll escape any that occur on the input string.
         private readonly static char[] s_unixEscapeChars = { '\\', '"', '<', '>' };
 
-        internal static void NormalizeInputs(ref string directory, ref string expression, EnumerationOptions options)
+        /// <summary>
+        /// Validates the directory and expression strings to check that they have no invalid characters, any special DOS wildcard characters in Win32 in the expression get replaced with their proper escaped representation, and if the expression string begins with a directory name, the directory name is moved and appended at the end of the directory string.
+        /// </summary>
+        /// <param name="directory">A reference to a directory string that we will be checking for normalization.</param>
+        /// <param name="expression">A reference to a expression string that we will be checking for normalization.</param>
+        /// <param name="matchType">The kind of matching we want to check in the expression. If the value is Win32, we will replace special DOS wild characters to their safely escaped representation. This replacement does not affect the normalization status of the expression.</param>
+        /// <returns><cref langword="false" /> if the directory reference string get modified inside this function due to the expression beginning with a directory name. <cref langword="true" /> if the directory reference string was not modified.</returns>
+        /// <exception cref="ArgumentException">
+        /// The expression is a rooted path.
+        /// -or-
+        /// The directory or the expression reference strings contain a null character.
+        /// </exception>
+        /// <exception cref="ArgumentOutOfRangeException">
+        /// The match type is out of the range of the valid MatchType enum values.
+        /// </exception>
+        internal static bool NormalizeInputs(ref string directory, ref string expression, MatchType matchType)
         {
             if (Path.IsPathRooted(expression))
                 throw new ArgumentException(SR.Arg_Path2IsRooted, nameof(expression));
 
+            if (expression.Contains('\0'))
+                throw new ArgumentException(SR.Argument_InvalidPathChars, expression);
+
+            if (directory.Contains('\0'))
+                throw new ArgumentException(SR.Argument_InvalidPathChars, directory);
+
             // We always allowed breaking the passed ref directory and filter to be separated
             // any way the user wanted. Looking for "C:\foo\*.cs" could be passed as "C:\" and
             // "foo\*.cs" or "C:\foo" and "*.cs", for example. As such we need to combine and
@@ -34,17 +55,26 @@ namespace System.IO.Enumeration
 
             ReadOnlySpan<char> directoryName = Path.GetDirectoryName(expression.AsSpan());
 
+            bool isDirectoryModified = true;
+
             if (directoryName.Length != 0)
             {
                 // Need to fix up the input paths
                 directory = Path.Join(directory.AsSpan(), directoryName);
                 expression = expression.Substring(directoryName.Length + 1);
+
+                isDirectoryModified = false;
             }
 
-            switch (options.MatchType)
+            switch (matchType)
             {
                 case MatchType.Win32:
-                    if (string.IsNullOrEmpty(expression) || expression == "." || expression == "*.*")
+                    if (expression == "*")
+                    {
+                        // Most common case
+                        break;
+                    }
+                    else if (string.IsNullOrEmpty(expression) || expression == "." || expression == "*.*")
                     {
                         // Historically we always treated "." as "*"
                         expression = "*";
@@ -69,8 +99,10 @@ namespace System.IO.Enumeration
                 case MatchType.Simple:
                     break;
                 default:
-                    throw new ArgumentOutOfRangeException(nameof(options));
+                    throw new ArgumentOutOfRangeException(nameof(matchType));
             }
+
+            return isDirectoryModified;
         }
 
         private static bool MatchesPattern(string expression, ReadOnlySpan<char> name, EnumerationOptions options)
@@ -134,12 +166,14 @@ namespace System.IO.Enumeration
         internal static IEnumerable<FileInfo> FileInfos(
             string directory,
             string expression,
-            EnumerationOptions options)
+            EnumerationOptions options,
+            bool isNormalized)
         {
              return new FileSystemEnumerable<FileInfo>(
                 directory,
                 (ref FileSystemEntry entry) => (FileInfo)entry.ToFileSystemInfo(),
-                options)
+                options,
+                isNormalized)
              {
                  ShouldIncludePredicate = (ref FileSystemEntry entry) =>
                      !entry.IsDirectory && MatchesPattern(expression, entry.FileName, options)
@@ -149,12 +183,14 @@ namespace System.IO.Enumeration
         internal static IEnumerable<DirectoryInfo> DirectoryInfos(
             string directory,
             string expression,
-            EnumerationOptions options)
+            EnumerationOptions options,
+            bool isNormalized)
         {
             return new FileSystemEnumerable<DirectoryInfo>(
                directory,
                (ref FileSystemEntry entry) => (DirectoryInfo)entry.ToFileSystemInfo(),
-               options)
+               options,
+               isNormalized)
             {
                 ShouldIncludePredicate = (ref FileSystemEntry entry) =>
                     entry.IsDirectory && MatchesPattern(expression, entry.FileName, options)
@@ -164,12 +200,14 @@ namespace System.IO.Enumeration
         internal static IEnumerable<FileSystemInfo> FileSystemInfos(
             string directory,
             string expression,
-            EnumerationOptions options)
+            EnumerationOptions options,
+            bool isNormalized)
         {
             return new FileSystemEnumerable<FileSystemInfo>(
                directory,
                (ref FileSystemEntry entry) => entry.ToFileSystemInfo(),
-               options)
+               options,
+               isNormalized)
             {
                 ShouldIncludePredicate = (ref FileSystemEntry entry) =>
                     MatchesPattern(expression, entry.FileName, options)
index b8b115f..46b2952 100644 (file)
@@ -33,17 +33,8 @@ namespace System.IO.Enumeration
         // Used to get the raw entry data
         private byte[] _entryBuffer;
 
-        /// <summary>
-        /// Encapsulates a find operation.
-        /// </summary>
-        /// <param name="directory">The directory to search in.</param>
-        /// <param name="options">Enumeration options to use.</param>
-        public FileSystemEnumerator(string directory, EnumerationOptions options = null)
+        private void Init()
         {
-            _originalRootDirectory = directory ?? throw new ArgumentNullException(nameof(directory));
-            _rootDirectory = Path.TrimEndingDirectorySeparator(Path.GetFullPath(directory));
-            _options = options ?? EnumerationOptions.Default;
-
             // We need to initialize the directory handle up front to ensure
             // we immediately throw IO exceptions for missing directory/etc.
             _directoryHandle = CreateDirectoryHandle(_rootDirectory);
index b996a98..4a6c3ac 100644 (file)
@@ -41,17 +41,8 @@ namespace System.IO.Enumeration
         private bool _lastEntryFound;
         private Queue<(IntPtr Handle, string Path)> _pending;
 
-        /// <summary>
-        /// Encapsulates a find operation.
-        /// </summary>
-        /// <param name="directory">The directory to search in.</param>
-        /// <param name="options">Enumeration options to use.</param>
-        public FileSystemEnumerator(string directory, EnumerationOptions options = null)
+        private void Init()
         {
-            _originalRootDirectory = directory ?? throw new ArgumentNullException(nameof(directory));
-            _rootDirectory = Path.TrimEndingDirectorySeparator(Path.GetFullPath(directory));
-            _options = options ?? EnumerationOptions.Default;
-
             // We'll only suppress the media insertion prompt on the topmost directory as that is the
             // most likely scenario and we don't want to take the perf hit for large enumerations.
             // (We weren't consistent with how we handled this historically.)
index a00fdd1..8082351 100644 (file)
@@ -16,6 +16,33 @@ namespace System.IO.Enumeration
     public unsafe abstract partial class FileSystemEnumerator<TResult> : CriticalFinalizerObject, IEnumerator<TResult>
     {
         /// <summary>
+        /// Encapsulates a find operation.
+        /// </summary>
+        /// <param name="directory">The directory to search in.</param>
+        /// <param name="options">Enumeration options to use.</param>
+        public FileSystemEnumerator(string directory, EnumerationOptions options = null)
+            : this(directory, isNormalized: false, options)
+        {
+        }
+
+        /// <summary>
+        /// Encapsulates a find operation.
+        /// </summary>
+        /// <param name="directory">The directory to search in.</param>
+        /// <param name="isNormalized">Whether the directory path is already normalized or not.</param>
+        /// <param name="options">Enumeration options to use.</param>
+        internal FileSystemEnumerator(string directory, bool isNormalized, EnumerationOptions options = null)
+        {
+            _originalRootDirectory = directory ?? throw new ArgumentNullException(nameof(directory));
+
+            string path = isNormalized ? directory : Path.GetFullPath(directory);
+            _rootDirectory = Path.TrimEndingDirectorySeparator(path);
+            _options = options ?? EnumerationOptions.Default;
+
+            Init();
+        }
+
+        /// <summary>
         /// Return true if the given file system entry should be included in the results.
         /// </summary>
         protected virtual bool ShouldIncludeEntry(ref FileSystemEntry entry) => true;
index a6b904f..b6dccac 100644 (file)
@@ -89,7 +89,6 @@ namespace System.IO
         /// </remarks>
         public MatchType MatchType { get; set; }
 
-
         /// <summary>
         /// For APIs that allow specifying a match expression this will allow you to specify case matching behavior.
         /// </summary>
index 72ccb77..a75f318 100644 (file)
@@ -10,7 +10,7 @@ namespace System.IO.Tests
 {
     #region EnumerateFiles
 
-    public class Directory_EnumFiles_Str : Directory_GetFiles_str
+    public class Directory_EnumFiles_str : Directory_GetFiles_str
     {
         public override string[] GetEntries(string path)
         {
index 5d5081e..f7404bf 100644 (file)
@@ -42,6 +42,77 @@ namespace System.IO.Tests
             FSAssert.EqualWhenOrdered(new string[] { subDirectory1.FullName, subDirectory2.FullName }, Directory.EnumerateDirectories(rootDirectory.FullName, string.Empty, SearchOption.AllDirectories));
         }
 
+         [Fact]
+         [PlatformSpecific(TestPlatforms.Windows)]
+         public void EnumerateDirectories_TrailingDot()
+         {
+             string prefix = @"\\?\";
+             string tempPath = GetTestFilePath();
+             string fileName = "Test.txt";
+
+             string[] dirPaths = {
+                 Path.Join(prefix, tempPath, "Test"),
+                 Path.Join(prefix, tempPath, "TestDot."),
+                 Path.Join(prefix, tempPath, "TestDotDot..")
+             };
+
+             // Create directories and their files using "\\?\C:\" paths
+             foreach (string dirPath in dirPaths)
+             {
+                 if (Directory.Exists(dirPath))
+                 {
+                     Directory.Delete(dirPath, recursive: true);
+                 }
+
+                 Directory.CreateDirectory(dirPath);
+                 
+                 // Directory.Exists should work with directories containing trailing dots and prefixed with \\?\
+                 Assert.True(Directory.Exists(dirPath));
+
+                 string filePath = Path.Join(dirPath, fileName);
+                 using FileStream fs = File.Create(filePath);
+
+                 // File.Exists should work with directories containing trailing dots and prefixed with \\?\
+                 Assert.True(File.Exists(filePath));
+             }
+             
+             try
+             {
+                 // Enumerate directories and their files using "C:\" paths
+                 DirectoryInfo sourceInfo = new DirectoryInfo(tempPath);
+                 foreach (DirectoryInfo dirInfo in sourceInfo.EnumerateDirectories("*", SearchOption.AllDirectories))
+                 {
+                     // DirectoryInfo.Exists should work with or without \\?\ for folders with trailing dots
+                     Assert.True(dirInfo.Exists);
+
+                     if (dirInfo.FullName.EndsWith("."))
+                     {
+                         // Directory.Exists is not expected to work with directories containing trailing dots and not prefixed with \\?\
+                         Assert.False(Directory.Exists(dirInfo.FullName));
+                     }
+
+                     foreach (FileInfo fileInfo in dirInfo.EnumerateFiles("*.*", SearchOption.TopDirectoryOnly))
+                     {
+                         // FileInfo.Exists should work with or without \\?\ for folders with trailing dots
+                         Assert.True(fileInfo.Exists);
+
+                         if (fileInfo.Directory.FullName.EndsWith("."))
+                         {
+                             // File.Exists is not expected to work with directories containing trailing dots and not prefixed with \\?\
+                             Assert.False(File.Exists(fileInfo.FullName));
+                         }
+                     }
+                 }
+             }
+             finally
+             {
+                 foreach (string dirPath in dirPaths)
+                 {
+                     Directory.Delete(dirPath, recursive: true);
+                 }
+             }
+         }
+
         class ThreadSafeRepro
         {
             volatile IEnumerator<string> _enumerator;
index 5e2c17c..b9089fa 100644 (file)
@@ -2,7 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System.Diagnostics;
 using Microsoft.DotNet.RemoteExecutor;
 using Xunit;
 
index 21ae5bb..567b338 100644 (file)
@@ -630,10 +630,8 @@ namespace System.IO.Tests
         private static char[] NewWildcards = new char[] { '<', '>', '\"' };
 
         [Fact]
-        [PlatformSpecific(TestPlatforms.Windows)]
-        public void WindowsSearchPatternInvalid_Core()
+        public void SearchPatternInvalid_Core()
         {
-            GetEntries(TestDirectory, "\0");
             GetEntries(TestDirectory, "|");
 
             Assert.All(Path.GetInvalidFileNameChars().Except(OldWildcards).Except(NewWildcards), invalidChar =>
@@ -641,39 +639,45 @@ namespace System.IO.Tests
                 switch (invalidChar)
                 {
                     case '\\':
+                        if (PlatformDetection.IsWindows)
+                        {
+                            Assert.Throws<DirectoryNotFoundException>(() => GetEntries(TestDirectory, string.Format("te{0}st", invalidChar.ToString())));
+                        }
+                        else
+                        {
+                            GetEntries(TestDirectory, string.Format("te{0}st", invalidChar.ToString()));
+                        }
+                        break;
+
                     case '/':
-                        Assert.Throws<DirectoryNotFoundException>(() => GetEntries(Directory.GetCurrentDirectory(), string.Format("te{0}st", invalidChar.ToString())));
+                        Assert.Throws<DirectoryNotFoundException>(() => GetEntries(TestDirectory, string.Format("te{0}st", invalidChar.ToString())));
                         break;
+
+                    case '\0':
+                        Assert.Throws<ArgumentException>(() => GetEntries(TestDirectory, "\0"));
+                        break;
+
                     default:
-                        GetEntries(Directory.GetCurrentDirectory(), string.Format("te{0}st", invalidChar.ToString()));
+                        GetEntries(TestDirectory, string.Format("te{0}st", invalidChar.ToString()));
                         break;
                 }
             });
         }
 
-        [Fact]
-        [PlatformSpecific(TestPlatforms.Windows)]  // Windows-invalid search patterns throw
+        [Fact] // .NET Core doesn't throw on wildcards
         public void WindowsSearchPatternInvalid_Wildcards_netcoreapp()
         {
             Assert.All(OldWildcards, invalidChar =>
             {
-                GetEntries(Directory.GetCurrentDirectory(), string.Format("te{0}st", invalidChar.ToString()));
+                GetEntries(TestDirectory, string.Format("te{0}st", invalidChar.ToString()));
             });
             Assert.All(NewWildcards, invalidChar =>
             {
-                GetEntries(Directory.GetCurrentDirectory(), string.Format("te{0}st", invalidChar.ToString()));
+                GetEntries(TestDirectory, string.Format("te{0}st", invalidChar.ToString()));
             });
         }
 
         [Fact]
-        [PlatformSpecific(TestPlatforms.AnyUnix)]  // Unix-invalid search patterns throws no exception 
-        public void UnixSearchPatternInvalid()
-        {
-            GetEntries(TestDirectory, "\0");
-            GetEntries(TestDirectory, string.Format("te{0}st", "\0".ToString()));
-        }
-
-        [Fact]
         [PlatformSpecific(TestPlatforms.Windows)]  // ? in search pattern returns results
         public virtual void WindowsSearchPatternQuestionMarks()
         {
index dd1875a..a9d4398 100644 (file)
@@ -9,7 +9,7 @@ namespace System.IO.Tests
 {
     #region EnumerateFiles
 
-    public class DirectoryInfo_EnumerateFiles_Str : Directory_GetFiles_str
+    public class DirectoryInfo_EnumerateFiles_str : Directory_GetFiles_str
     {
         public override string[] GetEntries(string path)
         {
index 6c70308..ec71e86 100644 (file)
@@ -51,13 +51,11 @@ namespace System.IO.IsolatedStorage
         }
 
         [Fact]
-        [ActiveIssue(25428, TestPlatforms.AnyUnix)]
         public void GetFileNames_RaisesInvalidPath_Core()
         {
-            // We are no longer as aggressive with filters for enumerating files
             using (IsolatedStorageFile isf = IsolatedStorageFile.GetUserStoreForAssembly())
             {
-                isf.GetFileNames("\0bad");
+                Assert.Throws<ArgumentException>(() => isf.GetFileNames("\0bad"));
             }
         }