Merge pull request #5548 from BDisp/issue/filedialog-unreadable-entry

Fixes #5544. FileDialog/OpenDialog: one unreadable entry makes the entire directory render empty (`FileDialogState.GetChildren` swallows the exception)

Author: tig

Terminal.Gui/Views/FileDialogs/FileDialogState.cs

@@ -34,45 +34,81 @@ protected FileDialogState (IDirectoryInfo dir, FileDialog parent, bool skipIniti
     public FileSystemInfoStats? Selected { get; set; }
 
     protected IEnumerable<FileSystemInfoStats> GetChildren (IDirectoryInfo dir)
+    {
+        List<FileSystemInfoStats> children = [];
+
+        AddReadableChildren (children, dir);
+
+        // if only allowing specific file types
+        if (Parent.AllowedTypes.Count > 0 && Parent.OpenMode == OpenMode.File)
+        {
+            children = children.Where (c => c.IsDir || (c.FileSystemInfo is IFileInfo f && Parent.IsCompatibleWithAllowedExtensions (f))).ToList ();
+        }
+
+        // if there's a UI filter in place too
+        if (Parent.CurrentFilter is { })
+        {
+            children = children.Where (MatchesApiFilter).ToList ();
+        }
+
+        AddParentNavigation (children, dir);
+
+        return children;
+    }
+
+    private void AddReadableChildren (List<FileSystemInfoStats> children, IDirectoryInfo dir)
     {
         try
         {
-            List<FileSystemInfoStats> children;
+            IEnumerable<IFileSystemInfo> entries;
 
             // if directories only
             if (Parent.OpenMode == OpenMode.Directory)
             {
-                children = dir.GetDirectories ().Select (e => new FileSystemInfoStats (e, Parent.Style.Culture)).ToList ();
+                entries = dir.GetDirectories ();
             }
             else
             {
-                children = dir.GetFileSystemInfos ().Select (e => new FileSystemInfoStats (e, Parent.Style.Culture)).ToList ();
+                entries = dir.GetFileSystemInfos ();
             }
 
-            // if only allowing specific file types
-            if (Parent.AllowedTypes.Count > 0 && Parent.OpenMode == OpenMode.File)
+            foreach (IFileSystemInfo entry in entries)
             {
-                children = children.Where (c => c.IsDir || (c.FileSystemInfo is IFileInfo f && Parent.IsCompatibleWithAllowedExtensions (f))).ToList ();
+                AddReadableChild (children, entry);
             }
+        }
+        catch (Exception)
+        {
+            // Access permission exceptions, missing directories, etc.
+        }
+    }
 
-            // if there's a UI filter in place too
-            if (Parent.CurrentFilter is { })
-            {
-                children = children.Where (MatchesApiFilter).ToList ();
-            }
+    private void AddReadableChild (List<FileSystemInfoStats> children, IFileSystemInfo entry)
+    {
+        try
+        {
+            children.Add (new FileSystemInfoStats (entry, Parent.Style.Culture));
+        }
+        catch (Exception)
+        {
+            // A single unreadable entry should not hide the rest of the directory.
+        }
+    }
 
-            // allow navigating up as '..'
-            if (dir.Parent is { })
-            {
-                children.Add (new FileSystemInfoStats (dir.Parent, Parent.Style.Culture) { IsParent = true });
-            }
+    private void AddParentNavigation (List<FileSystemInfoStats> children, IDirectoryInfo dir)
+    {
+        if (dir.Parent is not { } parent)
+        {
+            return;
+        }
 
-            return children;
+        try
+        {
+            children.Add (new FileSystemInfoStats (parent, Parent.Style.Culture) { IsParent = true });
         }
         catch (Exception)
         {
-            // Access permissions Exceptions, Dir not exists etc
-            return [];
+            // If even the parent cannot be stat'ed/read metadata, keep the readable children.
         }
     }
 

Tests/UnitTestsParallelizable/Views/FileDialogResultTests.cs

@@ -1,7 +1,9 @@
 // Copilot
 
 using System.Reflection;
+using System.IO.Abstractions;
 using System.IO.Abstractions.TestingHelpers;
+using Moq;
 
 namespace UnitTests.Views;
 
@@ -162,7 +164,7 @@ public void OpenDialog_UsesInnerTableSeparatorsWithoutOuterBorders ()
         FieldInfo? tableViewField = typeof (FileDialog).GetField ("_tableView", BindingFlags.Instance | BindingFlags.NonPublic);
         Assert.NotNull (tableViewField);
 
-        TableView tableView = Assert.IsType<TableView> (tableViewField!.GetValue (od));
+        TableView tableView = Assert.IsType<TableView> (tableViewField.GetValue (od));
 
         Assert.True (tableView.Style.ShowVerticalCellLines);
         Assert.True (tableView.Style.ShowVerticalHeaderLines);
@@ -181,7 +183,7 @@ public void FileDialog_PathField_End_MovesInsertionPointToEnd ()
         FieldInfo? tbPathField = typeof (FileDialog).GetField ("_tbPath", BindingFlags.Instance | BindingFlags.NonPublic);
         Assert.NotNull (tbPathField);
 
-        TextField tbPath = Assert.IsType<TextField> (tbPathField!.GetValue (fd));
+        TextField tbPath = Assert.IsType<TextField> (tbPathField.GetValue (fd));
         tbPath.Text = "/testdir/example.txt";
 
         tbPath.NewKeyDownEvent (Key.Home);
@@ -208,7 +210,7 @@ public void FileDialog_PathField_BadChars_AreSuppressed (char badChar)
         FieldInfo? tbPathField = typeof (FileDialog).GetField ("_tbPath", BindingFlags.Instance | BindingFlags.NonPublic);
         Assert.NotNull (tbPathField);
 
-        TextField tbPath = Assert.IsType<TextField> (tbPathField!.GetValue (fd));
+        TextField tbPath = Assert.IsType<TextField> (tbPathField.GetValue (fd));
         tbPath.Text = "/testdir/";
         tbPath.MoveEnd ();
 
@@ -251,6 +253,52 @@ public void FileDialog_MixedMode_PathSetBeforeEndInit_RespectsAllowedTypes ()
         Assert.Equal (["allowed.Designer.cs", "subdir"], visibleEntries);
     }
 
+    [Fact]
+    public void FileDialog_MixedMode_SkipsUnreadableEntry_AndKeepsReadableEntries ()
+    {
+        IFileSystem fileSystem = CreateFileSystemWithDirectory (out IDirectoryInfo directory);
+        IFileInfo goodFile = CreateFile ("/testdir/good.txt", "good.txt");
+        IDirectoryInfo goodDirectory = CreateDirectory ("/testdir/good-dir", "good-dir", directory);
+        IFileInfo badFile = CreateUnreadableFile ("/testdir/bad.txt", "bad.txt");
+
+        Mock.Get (directory)
+            .Setup (d => d.GetFileSystemInfos ())
+            .Returns ([goodFile, badFile, goodDirectory]);
+
+        using FileDialog fd = new TestableFileDialog (fileSystem);
+        fd.OpenMode = OpenMode.Mixed;
+
+        fd.Path = "/testdir";
+
+        Assert.NotNull (fd.State);
+        Assert.Contains (fd.State!.Children, c => c.Name == "good.txt");
+        Assert.Contains (fd.State.Children, c => c.Name == "good-dir");
+        Assert.Contains (fd.State.Children, c => c.IsParent && c.Name == "..");
+        Assert.DoesNotContain (fd.State.Children, c => c.Name == "bad.txt");
+    }
+
+    [Fact]
+    public void FileDialog_DirectoryMode_SkipsUnreadableDirectory_AndKeepsParentNavigation ()
+    {
+        IFileSystem fileSystem = CreateFileSystemWithDirectory (out IDirectoryInfo directory);
+        IDirectoryInfo goodDirectory = CreateDirectory ("/testdir/good-dir", "good-dir", directory);
+        IDirectoryInfo badDirectory = CreateUnreadableDirectory ("/testdir/bad-dir", "bad-dir", directory);
+
+        Mock.Get (directory)
+            .Setup (d => d.GetDirectories ())
+            .Returns ([goodDirectory, badDirectory]);
+
+        using FileDialog fd = new TestableFileDialog (fileSystem);
+        fd.OpenMode = OpenMode.Directory;
+
+        fd.Path = "/testdir";
+
+        Assert.NotNull (fd.State);
+        Assert.Contains (fd.State!.Children, c => c.Name == "good-dir");
+        Assert.Contains (fd.State.Children, c => c.IsParent && c.Name == "..");
+        Assert.DoesNotContain (fd.State.Children, c => c.Name == "bad-dir");
+    }
+
     [Fact]
     public void FileDialog_Accepting_Directory_From_Table_Keeps_Path_On_Opened_Directory ()
     {
@@ -310,10 +358,76 @@ private static int FindRowByName (TableView tableView, string name)
         return -1;
     }
 
+    private static IFileSystem CreateFileSystemWithDirectory (out IDirectoryInfo directory)
+    {
+        Mock<IFileSystem> fileSystem = new ();
+        Mock<IDirectoryInfoFactory> directoryInfoFactory = new ();
+        Mock<IDirectoryInfo> parent = CreateDirectoryMock ("/", string.Empty, null);
+        Mock<IDirectoryInfo> dir = CreateDirectoryMock ("/testdir", "testdir", parent.Object);
+
+        directoryInfoFactory.Setup (f => f.New ("/testdir")).Returns (dir.Object);
+        fileSystem.SetupGet (f => f.DirectoryInfo).Returns (directoryInfoFactory.Object);
+
+        directory = dir.Object;
+
+        return fileSystem.Object;
+    }
+
+    private static IFileInfo CreateFile (string fullName, string name)
+    {
+        Mock<IFileInfo> file = new ();
+        file.SetupGet (f => f.Exists).Returns (true);
+        file.SetupGet (f => f.FullName).Returns (fullName);
+        file.SetupGet (f => f.Name).Returns (name);
+        file.SetupGet (f => f.Extension).Returns (System.IO.Path.GetExtension (name));
+        file.SetupGet (f => f.Length).Returns (12);
+        file.SetupGet (f => f.LastWriteTime).Returns (new DateTime (2026, 1, 1));
+
+        return file.Object;
+    }
+
+    private static IFileInfo CreateUnreadableFile (string fullName, string name)
+    {
+        Mock<IFileInfo> file = Mock.Get (CreateFile (fullName, name));
+        file.SetupGet (f => f.LastWriteTime).Throws (new IOException ("Transport endpoint is not connected"));
+
+        return file.Object;
+    }
+
+    private static IDirectoryInfo CreateDirectory (string fullName, string name, IDirectoryInfo? parent)
+    {
+        return CreateDirectoryMock (fullName, name, parent).Object;
+    }
+
+    private static IDirectoryInfo CreateUnreadableDirectory (string fullName, string name, IDirectoryInfo? parent)
+    {
+        Mock<IDirectoryInfo> directory = CreateDirectoryMock (fullName, name, parent);
+        directory.SetupGet (d => d.LastWriteTime).Throws (new UnauthorizedAccessException ());
+
+        return directory.Object;
+    }
+
+    private static Mock<IDirectoryInfo> CreateDirectoryMock (string fullName, string name, IDirectoryInfo? parent)
+    {
+        Mock<IDirectoryInfo> directory = new ();
+        directory.SetupGet (d => d.Exists).Returns (true);
+        directory.SetupGet (d => d.FullName).Returns (fullName);
+        directory.SetupGet (d => d.Name).Returns (name);
+        directory.SetupGet (d => d.Extension).Returns (string.Empty);
+        directory.SetupGet (d => d.Parent).Returns (parent);
+        directory.SetupGet (d => d.LastWriteTime).Returns (new DateTime (2026, 1, 1));
+        directory.Setup (d => d.GetFileSystemInfos ()).Returns ([]);
+        directory.Setup (d => d.GetDirectories ()).Returns ([]);
+
+        return directory;
+    }
+
     /// <summary>Testable subclass that exposes the internal file-system constructor.</summary>
     private sealed class TestableFileDialog : FileDialog
     {
         public TestableFileDialog (MockFileSystem fs) : base (fs) { }
+
+        public TestableFileDialog (IFileSystem fileSystem) : base (fileSystem) { }
     }
 
     /// <summary>Testable subclass for OpenDialog.</summary>