Merge pull request #2247 from stephentoub/enumerate_symlink

Fix FileSystem enumeration with broken symlinks

Author: stephentoub

src/System.IO.FileSystem/src/System/IO/UnixFileSystem.cs

@@ -318,7 +318,7 @@ public override bool DirectoryExists(string fullPath)
             return DirectoryExists(fullPath, out errno);
         }
 
-        private bool DirectoryExists(string fullPath, out int errno)
+        private static bool DirectoryExists(string fullPath, out int errno)
         {
             return FileExists(fullPath, Interop.libcoreclr.FileTypes.S_IFDIR, out errno);
         }
@@ -329,7 +329,7 @@ public override bool FileExists(string fullPath)
             return FileExists(fullPath, Interop.libcoreclr.FileTypes.S_IFREG, out errno);
         }
 
-        private bool FileExists(string fullPath, int fileType, out int errno)
+        private static bool FileExists(string fullPath, int fileType, out int errno)
         {
             Interop.libcoreclr.fileinfo fileinfo;
             while (true)
@@ -475,48 +475,53 @@ private IEnumerator<T> Enumerate(Interop.libc.SafeDirHandle dirHandle)
                             string name = Interop.libc.GetDirEntName(curEntry);
 
                             // Get from the dir entry whether the entry is a file or directory.
-                            // If we're not sure from the dir entry itself, stat to the entry.
-                            bool isDir = false, isFile = false;
+                            // We classify everything as a file unless we know it to be a directory,
+                            // e.g. a FIFO will be classified as a file.
+                            bool isDir;
                             switch (Interop.libc.GetDirEntType(curEntry))
                             {
                                 case Interop.libc.DType.DT_DIR:
+                                    // We know it's a directory.
                                     isDir = true;
                                     break;
-                                case Interop.libc.DType.DT_REG:
-                                    isFile = true;
-                                    break;
                                 case Interop.libc.DType.DT_LNK:
                                 case Interop.libc.DType.DT_UNKNOWN:
-                                    string fullPath = Path.Combine(dirPath.FullPath, name);
-                                    Interop.libcoreclr.fileinfo fileinfo;
-                                    while (Interop.CheckIo(Interop.libcoreclr.GetFileInformationFromPath(fullPath, out fileinfo), fullPath)) ;
-                                    isDir = (fileinfo.mode & Interop.libcoreclr.FileTypes.S_IFMT) == Interop.libcoreclr.FileTypes.S_IFDIR;
-                                    isFile = (fileinfo.mode & Interop.libcoreclr.FileTypes.S_IFMT) == Interop.libcoreclr.FileTypes.S_IFREG;
+                                    // It's a symlink or unknown: stat to it to see if we can resolve it to a directory.
+                                    // If we can't (e.g.symlink to a file, broken symlink, etc.), we'll just treat it as a file.
+                                    int errnoIgnored;
+                                    isDir = DirectoryExists(Path.Combine(dirPath.FullPath, name), out errnoIgnored);
+                                    break;
+                                default:
+                                    // Otherwise, treat it as a file.  This includes regular files,
+                                    // FIFOs, etc.
+                                    isDir = false;
                                     break;
                             }
-                            bool matchesSearchPattern =
-                                (isFile || isDir) &&
-                                Interop.libc.fnmatch(_searchPattern, name, Interop.libc.FnmatchFlags.None) == 0;
 
                             // Yield the result if the user has asked for it.  In the case of directories,
                             // always explore it by pushing it onto the stack, regardless of whether
                             // we're returning directories.
-                            if (isDir && !ShouldIgnoreDirectory(name))
+                            if (isDir)
                             {
-                                if (_includeDirectories && matchesSearchPattern)
-                                {
-                                    yield return _translateResult(Path.Combine(dirPath.UserPath, name), /*isDirectory*/true);
-                                }
-                                if (_searchOption == SearchOption.AllDirectories)
+                                if (!ShouldIgnoreDirectory(name))
                                 {
-                                    if (toExplore == null)
+                                    if (_includeDirectories &&
+                                        Interop.libc.fnmatch(_searchPattern, name, Interop.libc.FnmatchFlags.None) == 0)
+                                    {
+                                        yield return _translateResult(Path.Combine(dirPath.UserPath, name), /*isDirectory*/true);
+                                    }
+                                    if (_searchOption == SearchOption.AllDirectories)
                                     {
-                                        toExplore = new Stack<PathPair>();
+                                        if (toExplore == null)
+                                        {
+                                            toExplore = new Stack<PathPair>();
+                                        }
+                                        toExplore.Push(new PathPair(Path.Combine(dirPath.UserPath, name), Path.Combine(dirPath.FullPath, name)));
                                     }
-                                    toExplore.Push(new PathPair(Path.Combine(dirPath.UserPath, name), Path.Combine(dirPath.FullPath, name)));
                                 }
                             }
-                            else if (isFile && _includeFiles && matchesSearchPattern)
+                            else if (_includeFiles &&
+                                     Interop.libc.fnmatch(_searchPattern, name, Interop.libc.FnmatchFlags.None) == 0)
                             {
                                 yield return _translateResult(Path.Combine(dirPath.UserPath, name), /*isDirectory*/false);
                             }

src/System.IO.FileSystem/tests/Directory/GetDirectories_str.cs

@@ -3,11 +3,9 @@
 
 using System;
 using System.IO;
-using System.Collections;
-using System.Globalization;
-using System.Text;
-using System.Threading;
+using System.Linq;
 using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
 using Xunit;
 
 public class Directory_GetDirectories_str
@@ -347,6 +345,33 @@ public static void printerr(String err, [CallerMemberName] string memberName = "
     {
         Console.WriteLine("ERROR: ({0}, {1}, {2}) {3}", memberName, filePath, lineNumber, err);
     }
+
+    [Fact]
+    [PlatformSpecific(PlatformID.AnyUnix)]
+    public void EnumerateWithSymLinkToDirectory()
+    {
+        using (var containingFolder = new TemporaryDirectory())
+        {
+            // Test a symlink to a directory that does and then doesn't exist
+            using (var targetDir = new TemporaryDirectory())
+            {
+                // Create a symlink to a folder that exists
+                string linkPath = Path.Combine(containingFolder.Path, Path.GetRandomFileName());
+                Assert.Equal(0, symlink(targetDir.Path, linkPath));
+                Assert.True(Directory.Exists(linkPath));
+                Assert.Equal(1, Directory.GetDirectories(containingFolder.Path).Count());
+            }
+
+            // The target file is gone and the symlink still exists; since it can't be resolved,
+            // it's treated as a file rather than as a directory.
+            Assert.Equal(0, Directory.GetDirectories(containingFolder.Path).Count());
+            Assert.Equal(1, Directory.GetFiles(containingFolder.Path).Count());
+        }
+    }
+
+    [DllImport("libc", SetLastError = true)]
+    private static extern int symlink(string path1, string path2);
+
 }
 
 

src/System.IO.FileSystem/tests/Directory/GetFiles_str.cs

@@ -4,6 +4,8 @@
 using System;
 using System.IO;
 using System.IO.FileSystem.Tests;
+using System.Linq;
+using System.Runtime.InteropServices;
 using Xunit;
 
 public class Directory_GetFiles_str : FileSystemTest
@@ -156,5 +158,34 @@ public void UnixFilePathWithSpaces()
         }
     }
 
+    [Fact]
+    [PlatformSpecific(PlatformID.AnyUnix)]
+    public void EnumerateWithSymLinkToFile()
+    {
+        using (var containingFolder = new TemporaryDirectory())
+        {
+            string linkPath;
+
+            // Test a symlink to a file that does and then doesn't exist
+            using (var targetFile = new TemporaryFile())
+            {
+                linkPath = Path.Combine(containingFolder.Path, Path.GetRandomFileName());
+                Assert.Equal(0, symlink(targetFile.Path, linkPath));
+                Assert.True(File.Exists(linkPath));
+                Assert.Equal(1, GetFiles(containingFolder.Path).Count());
+            }
+
+            // The symlink still exists even though the target file is gone.
+            Assert.Equal(1, GetFiles(containingFolder.Path).Count());
+
+            // The symlink is gone
+            File.Delete(linkPath);
+            Assert.Equal(0, GetFiles(containingFolder.Path).Count());
+        }
+    }
+
+    [DllImport("libc", SetLastError = true)]
+    private static extern int symlink(string path1, string path2);
+    
     #endregion
 }