Some filesystem enumeration cases may throw if a symbolic link cycle is detected (link to itself)

[carlossanlop]

Problem

This recent PR introduced fixes for the ReadOnly and Hidden flags. While investigating the implementation of the recently approved symbolic link APIs, @jozkee and I discovered a potential regression for which we did not have any unit tests yet.

Repro steps

• In Unix, create a folder, then add a symbolic link with a cycle (for example, make it point to itself with ln -s mylink mylink).
• Enumerate the files with FileSystemEnumerable (making sure the transform predicate returns a string with the path), or call one of the Directory.Enumerate* or Directory.Get* methods that return a list of strings with the paths.

Expected

The enumeration invocation is performed to its completion without throwing.

Actual

The enumeration invocation throws due to encountering a symbolic link with a cycle to itself.

Root cause

Before the PR, the method FileSystemEntry.Initialize, which is a called by FileSystemEnumerator.MoveNext, would silently skip the retrieval of the directory information via stat or lstat:

runtime/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs

Line 51 in 2600044

&& Interop.Sys.Stat(entry.FullPath, out Interop.Sys.FileStatus targetStatus) >= 0)

runtime/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs

Line 63 in 2600044

&& (Interop.Sys.LStat(entry.FullPath, out Interop.Sys.FileStatus linkTargetStatus) >= 0))

Now, we are throwing because we are retrieving the directory information by calling the FileStatysm.IsDirectory method, which internally throws unless otherwise specified:

runtime/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs

Line 48 in 6a81cfa

isDirectory = entry._status.IsDirectory(entry.FullPath);

runtime/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs

Line 56 in 6a81cfa

isDirectory = entry._status.IsDirectory(entry.FullPath);

Fix

Pass the continueOnError: true argument to the IsDirectory calls.

Add unit tests to verify enumeration of directories containing cyclic symbolic links is performed without problems.

Notes

• There are some APIs that enumerate, then explicitly convert the FileSystemEntry instances to FileSystemInfos. Those cases should throw (this was the expected behavior before that PR), because that conversion forces a stat/lstat to retrieve the file information, and that will fail due to the cycle.

• We may decide to change the throwing behavior the future. There's a request to detect symlink cycles and continue creating FileSystemInfo instances even with the detected error: Allow opting out of following directory links (reparse points / symlinks) during recursive file-system enumerations #52666

Tagging subscribers to this area: @carlossanlop
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Problem

This recent PR introduced fixes for the ReadOnly and Hidden flags. While investigating the implementation of the recently approved symbolic link APIs, @jozkee and I discovered a potential regression for which we did not have any unit tests yet.

Repro steps

• In Unix, create a folder, then add a symbolic link with a cycle (for example, make it point to itself with ln -s mylink mylink).
• Enumerate the files with FileSystemEnumerable (making sure the transform predicate returns a string with the path), or call one of the Directory.Enumerate* or Directory.Get* methods that return a list of strings with the paths.

Expected

The enumeration invocation is performed to its completion without throwing.

Actual

The enumeration invocation throws due to encountering a symbolic link with a cycle.

Root cause

Before the PR, the method FileSystemEntry.Initialize, which is a called by FileSystemEnumerator.MoveNext, would silently skip the retrieval of the directory information via stat or lstat:

runtime/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs

Line 51 in 2600044

&& Interop.Sys.Stat(entry.FullPath, out Interop.Sys.FileStatus targetStatus) >= 0)

runtime/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs

Line 63 in 2600044

&& (Interop.Sys.LStat(entry.FullPath, out Interop.Sys.FileStatus linkTargetStatus) >= 0))

Now, we are throwing because we are retrieving the directory information by calling the FileStatysm.IsDirectory method, which internally throws unless otherwise specified:

runtime/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs

Line 48 in 6a81cfa

isDirectory = entry._status.IsDirectory(entry.FullPath);

runtime/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs

Line 56 in 6a81cfa

isDirectory = entry._status.IsDirectory(entry.FullPath);

Fix

Pass the continueOnError: true argument to the IsDirectory calls.

Add unit tests to verify enumeration of directories containing cyclic symbolic links is performed without problems.

Notes

• There are some APIs that enumerate, then explicitly convert the FileSystemEntry instances to FileSystemInfos. Those cases should throw (this was the expected behavior before that PR), because that conversion forces a stat/lstat to retrieve the file information, and that will fail due to the cycle.

• We may decide to change the throwing behavior the future, if there's a request to detect symlink cycles and continue creating FileSystemInfo instances even with the detected error.

Author:	carlossanlop
Assignees:	carlossanlop
Labels:	area-System.IO
Milestone:	6.0.0