Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
May 24, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,17 @@ internal static FileAttributes Initialize(
if (isUnknown)
{
isSymlink = entry.IsSymbolicLink;
isDirectory = entry._status.IsDirectory(entry.FullPath);
// Need to fail silently in case we are enumerating
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the comment... isn't all of this code about enumerating?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enumeration always goes through this path, yes. I do not know if in the future we will reach FileSystemEntry.Initialize another way.

Next time I touch this code, I can change the comment to // This call needs to fail silently.

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.
// Whether we had the dirent structure or not, we treat a symlink to a directory as a directory,
// 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;
Expand Down
18 changes: 18 additions & 0 deletions src/libraries/System.IO.FileSystem/tests/Base/BaseSymbolicLinks.cs
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
Original file line number Diff line number Diff line change
@@ -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));
}
}
}
Original file line number Diff line number Diff line change
@@ -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();
Comment on lines +23 to +24
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assert how many entries this gets.

}
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());
jozkee marked this conversation as resolved.
Show resolved Hide resolved
}
}

[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());
}
}
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down Expand Up @@ -46,6 +49,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" />
Expand Down