Fix: Avoid throwing if a symbolic link cycle to itself is detected while enumerating...

* Fix: Avoid throwing on FileSystemEntry.Initialize

* Add unit tests that verify cyclic symbolic links

* Fix Windows-specific unit test failures

* Use OperatingSystem.IsWindows()

* CHange 'Assert.Equal(1, ' to 'Assert.Single('

* Rename helper method that creates self referencing symlink

* Spacing

* ConditionalClass for CanCreateSymbolicLinks

* ConditionalClass does not work in browser/ios/android, use ConditionalFact/ConditionalTheory

Co-authored-by: carlossanlop <Carlos Sanchez carlossanlop@users.noreply.github.com>
Co-authored-by: carlossanlop <carlossanlop@users.noreply.github.com>

diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs
index ccf49cd..61ee454 100644 (file)
--- a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs
+++ b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs
@@ -45,7 +45,8 @@ namespace System.IO.Enumeration
             if (isUnknown)
             {
                 isSymlink = entry.IsSymbolicLink;
-                isDirectory = entry._status.IsDirectory(entry.FullPath);
+                // Need to fail silently in case we are enumerating
+                isDirectory = entry._status.IsDirectory(entry.FullPath, continueOnError: true);
             }
             // Same idea as the directory check, just repeated for (and tweaked due to the
             // nature of) symlinks.
@@ -53,7 +54,8 @@ namespace System.IO.Enumeration
             // so we need to reflect that in our isDirectory variable.
             else if (isSymlink)
             {
-                isDirectory = entry._status.IsDirectory(entry.FullPath);
+                // Need to fail silently in case we are enumerating
+                isDirectory = entry._status.IsDirectory(entry.FullPath, continueOnError: true);
             }
 
             entry._status.InitiallyDirectory = isDirectory;

diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseSymbolicLinks.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseSymbolicLinks.cs
new file mode 100644 (file)
index 0000000..caa5ae9
--- /dev/null
+++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseSymbolicLinks.cs
@@ -0,0 +1,18 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Xunit;
+
+namespace System.IO.Tests
+{
+    public abstract class BaseSymbolicLinks : FileSystemTest
+    {
+        protected DirectoryInfo CreateDirectoryContainingSelfReferencingSymbolicLink()
+        {
+            DirectoryInfo testDirectory = Directory.CreateDirectory(GetTestFilePath());
+            string pathToLink = Path.Join(testDirectory.FullName, GetTestFileName());
+            Assert.True(MountHelper.CreateSymbolicLink(pathToLink, pathToLink, isDirectory: true)); // Create a symlink cycle
+            return testDirectory;
+        }
+    }
+}

diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/SymbolicLinks.cs b/src/libraries/System.IO.FileSystem/tests/Directory/SymbolicLinks.cs
new file mode 100644 (file)
index 0000000..970c784
--- /dev/null
+++ b/src/libraries/System.IO.FileSystem/tests/Directory/SymbolicLinks.cs
@@ -0,0 +1,39 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Collections.Generic;
+using System.Linq;
+using Xunit;
+
+namespace System.IO.Tests
+{
+    public class Directory_SymbolicLinks : BaseSymbolicLinks
+    {
+        [ConditionalFact(nameof(CanCreateSymbolicLinks))]
+        public void EnumerateDirectories_LinksWithCycles_ShouldNotThrow()
+        {
+            DirectoryInfo testDirectory = CreateDirectoryContainingSelfReferencingSymbolicLink();
+
+            // Windows differentiates between dir symlinks and file symlinks
+            int expected = OperatingSystem.IsWindows() ? 1 : 0;
+            Assert.Equal(expected, Directory.EnumerateDirectories(testDirectory.FullName).Count());
+        }
+
+        [ConditionalFact(nameof(CanCreateSymbolicLinks))]
+        public void EnumerateFiles_LinksWithCycles_ShouldNotThrow()
+        {
+            DirectoryInfo testDirectory = CreateDirectoryContainingSelfReferencingSymbolicLink();
+
+            // Windows differentiates between dir symlinks and file symlinks
+            int expected = OperatingSystem.IsWindows() ? 0 : 1;
+            Assert.Equal(expected, Directory.EnumerateFiles(testDirectory.FullName).Count());
+        }
+
+        [ConditionalFact(nameof(CanCreateSymbolicLinks))]
+        public void EnumerateFileSystemEntries_LinksWithCycles_ShouldNotThrow()
+        {
+            DirectoryInfo testDirectory = CreateDirectoryContainingSelfReferencingSymbolicLink();
+            Assert.Single(Directory.EnumerateFileSystemEntries(testDirectory.FullName));
+        }
+    }
+}

diff --git a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/SymbolicLinks.cs b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/SymbolicLinks.cs
new file mode 100644 (file)
index 0000000..5dedc0b
--- /dev/null
+++ b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/SymbolicLinks.cs
@@ -0,0 +1,78 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Collections.Generic;
+using System.Linq;
+using Xunit;
+
+namespace System.IO.Tests
+{
+    public class DirectoryInfo_SymbolicLinks : BaseSymbolicLinks
+    {
+        [ConditionalTheory(nameof(CanCreateSymbolicLinks))]
+        [InlineData(false)]
+        [InlineData(true)]
+        public void EnumerateDirectories_LinksWithCycles_ThrowsTooManyLevelsOfSymbolicLinks(bool recurse)
+        {
+            var options  = new EnumerationOptions() { RecurseSubdirectories = recurse };
+            DirectoryInfo testDirectory = CreateDirectoryContainingSelfReferencingSymbolicLink();
+
+            // Windows avoids accessing the cyclic symlink if we do not recurse
+            if (OperatingSystem.IsWindows() && !recurse)
+            {
+                testDirectory.EnumerateDirectories("*", options).Count();
+                testDirectory.GetDirectories("*", options).Count();
+            }
+            else
+            {
+                // Internally transforms the FileSystemEntry to a DirectoryInfo, which performs a disk hit on the cyclic symlink
+                Assert.Throws<IOException>(() => testDirectory.EnumerateDirectories("*", options).Count());
+                Assert.Throws<IOException>(() => testDirectory.GetDirectories("*", options).Count());
+            }
+        }
+
+        [ConditionalTheory(nameof(CanCreateSymbolicLinks))]
+        [InlineData(false)]
+        [InlineData(true)]
+        public void EnumerateFiles_LinksWithCycles_ThrowsTooManyLevelsOfSymbolicLinks(bool recurse)
+        {
+            var options  = new EnumerationOptions() { RecurseSubdirectories = recurse };
+            DirectoryInfo testDirectory = CreateDirectoryContainingSelfReferencingSymbolicLink();
+
+            // Windows avoids accessing the cyclic symlink if we do not recurse
+            if (OperatingSystem.IsWindows() && !recurse)
+            {
+                testDirectory.EnumerateFiles("*", options).Count();
+                testDirectory.GetFiles("*", options).Count();
+            }
+            else
+            {
+                // Internally transforms the FileSystemEntry to a FileInfo, which performs a disk hit on the cyclic symlink
+                Assert.Throws<IOException>(() => testDirectory.EnumerateFiles("*", options).Count());
+                Assert.Throws<IOException>(() => testDirectory.GetFiles("*", options).Count());
+            }
+        }
+
+        [ConditionalTheory(nameof(CanCreateSymbolicLinks))]
+        [InlineData(false)]
+        [InlineData(true)]
+        public void EnumerateFileSystemInfos_LinksWithCycles_ThrowsTooManyLevelsOfSymbolicLinks(bool recurse)
+        {
+            var options  = new EnumerationOptions() { RecurseSubdirectories = recurse };
+            DirectoryInfo testDirectory = CreateDirectoryContainingSelfReferencingSymbolicLink();
+
+            // Windows avoids accessing the cyclic symlink if we do not recurse
+            if (OperatingSystem.IsWindows() && !recurse)
+            {
+                testDirectory.EnumerateFileSystemInfos("*", options).Count();
+                testDirectory.GetFileSystemInfos("*", options).Count();
+            }
+            else
+            {
+                // Internally transforms the FileSystemEntry to a FileSystemInfo, which performs a disk hit on the cyclic symlink
+                Assert.Throws<IOException>(() => testDirectory.EnumerateFileSystemInfos("*", options).Count());
+                Assert.Throws<IOException>(() => testDirectory.GetFileSystemInfos("*", options).Count());
+            }
+        }
+    }
+}

diff --git a/src/libraries/System.IO.FileSystem/tests/Enumeration/SymbolicLinksTests.cs b/src/libraries/System.IO.FileSystem/tests/Enumeration/SymbolicLinksTests.cs
new file mode 100644 (file)
index 0000000..43ea975
--- /dev/null
+++ b/src/libraries/System.IO.FileSystem/tests/Enumeration/SymbolicLinksTests.cs
@@ -0,0 +1,65 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Collections.Generic;
+using System.IO.Enumeration;
+using System.Linq;
+using Xunit;
+
+namespace System.IO.Tests.Enumeration
+{
+    public class Enumeration_SymbolicLinksTests : BaseSymbolicLinks
+    {
+        [ConditionalFact(nameof(CanCreateSymbolicLinks))]
+        public void EnumerateDirectories_LinksWithCycles_ShouldNotThrow()
+        {
+            DirectoryInfo testDirectory = CreateDirectoryContainingSelfReferencingSymbolicLink();
+
+            IEnumerable<string> enumerable = new FileSystemEnumerable<string>(
+                testDirectory.FullName,
+                (ref FileSystemEntry entry) => entry.ToFullPath(),
+                // Skipping attributes would force a disk hit which enters the cyclic symlink
+                new EnumerationOptions(){ AttributesToSkip = 0 })
+            {
+                ShouldIncludePredicate = (ref FileSystemEntry entry) => entry.IsDirectory
+            };
+
+            // Windows differentiates between dir symlinks and file symlinks
+            int expected = OperatingSystem.IsWindows() ? 1 : 0;
+            Assert.Equal(expected, enumerable.Count());
+        }
+
+        [ConditionalFact(nameof(CanCreateSymbolicLinks))]
+        public void EnumerateFiles_LinksWithCycles_ShouldNotThrow()
+        {
+            DirectoryInfo testDirectory = CreateDirectoryContainingSelfReferencingSymbolicLink();
+
+            IEnumerable<string> enumerable = new FileSystemEnumerable<string>(
+                testDirectory.FullName,
+                (ref FileSystemEntry entry) => entry.ToFullPath(),
+                // Skipping attributes would force a disk hit which enters the cyclic symlink
+                new EnumerationOptions(){ AttributesToSkip = 0 })
+            {
+                ShouldIncludePredicate = (ref FileSystemEntry entry) => !entry.IsDirectory
+            };
+
+            // Windows differentiates between dir symlinks and file symlinks
+            int expected = OperatingSystem.IsWindows() ? 0 : 1;
+            Assert.Equal(expected, enumerable.Count());
+        }
+
+        [ConditionalFact(nameof(CanCreateSymbolicLinks))]
+        public void EnumerateFileSystemEntries_LinksWithCycles_ShouldNotThrow()
+        {
+            DirectoryInfo testDirectory = CreateDirectoryContainingSelfReferencingSymbolicLink();
+
+            IEnumerable<string> enumerable = new FileSystemEnumerable<string>(
+                testDirectory.FullName,
+                (ref FileSystemEntry entry) => entry.ToFullPath(),
+                // Skipping attributes would force a disk hit which enters the cyclic symlink
+                new EnumerationOptions(){ AttributesToSkip = 0 });
+
+            Assert.Single(enumerable);
+        }
+    }
+}

diff --git a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj
index 651abf0..dba6172 100644 (file)
--- a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj
+++ b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj
@@ -14,7 +14,10 @@
     <Compile Include="Base\InfoGetSetTimes.cs" />
     <Compile Include="Base\AllGetSetAttributes.cs" />
     <Compile Include="Base\StaticGetSetTimes.cs" />
+    <Compile Include="Base\BaseSymbolicLinks.cs" />
     <Compile Include="Directory\EnumerableTests.cs" />
+    <Compile Include="Directory\SymbolicLinks.cs" />
+    <Compile Include="DirectoryInfo\SymbolicLinks.cs" />
     <Compile Include="FileInfo\GetSetAttributesCommon.cs" />
     <Compile Include="FileInfo\IsReadOnly.cs" />
     <Compile Include="FileInfo\Replace.cs" />
@@ -47,6 +50,7 @@
     <Compile Include="Enumeration\MatchTypesTests.cs" />
     <Compile Include="Enumeration\ExampleTests.cs" />
     <Compile Include="Enumeration\RemovedDirectoryTests.cs" />
+    <Compile Include="Enumeration\SymbolicLinksTests.cs" />
   </ItemGroup>
   <ItemGroup Condition="'$(TargetsUnix)' == 'true'">
     <Compile Include="FileSystemTest.Unix.cs" />