System.IO.Directory.GetFiles() API usage with recursive symlinks on Unix

[am11]

When the recursive symlink is encountered by GetFiles(path, pattern, SearchOption.AllDirectories) methos, .NET Core throws PathTooLongException (whereas Mono, in this case, throws IndexOutOfRangeException).

• Is it recommended for the consumers to handle the recursion cases in our code by keeping track of the directory graph node visits?

• To achieve this, would it be possible to provide an overload which accepts a predicate to determine when to skip / bail out of entering certain directories or symlinks during the traversal? Something like:

  public static string[] GetFiles(string path, string searchPattern, SearchOption searchOption);
  + public static string[] GetFiles(string path, string searchPattern, SearchOption searchOption,
  +                                 Func<string, bool> predicate);

I was looking for the best approach to handle the exceptions during the FS scan and found a similar issue with GetDirectories API in CLI repo: dotnet/cli#5578 which pointed me to the man page about /proc/[pid]/root symlink being recursive by design (added that info in code comment below).

Code:

public class Program
{
  public static void Main(string[] args)
  {
    var files = System.IO.Directory.GetFiles(args[0], "*.*",
                                             System.IO.SearchOption.AllDirectories);
    System.Console.WriteLine($"Files count: {files.Length}");
  }
}

Steps to repro:

# Setup and invocation:

# Platform:
# Ubuntu Trusty 14.04 (Bash on Unbuntu on Windows 10; tested dist info with `cat /etc/*release`)

sudo sh -c 'echo "deb [arch=amd64] https://apt-mo.trafficmanager.net/repos/dotnet-release/ trusty main" > /etc/apt/sources.list.d/dotnetdev.list'
sudo apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv-keys 417A0893
sudo apt-get update
sudo apt-get install dotnet-dev-1.0.0-rc4-004771

mkdir ~/test && cd $_
dotnet new console

cat <<EOT > Program.cs
public class Program
{
  public static void Main(string[] args)
  {
    var files = System.IO.Directory.GetFiles(args[0], "*.*",
                                             System.IO.SearchOption.AllDirectories);
    System.Console.WriteLine($"Files count: {files.Length}");
  }
}
EOT

sudo dotnet restore && sudo dotnet run -- /proc/self

# Note that /proc/self encounters a symlink at /proc/self/root and proc
# man page (https://linux.die.net/man/5/proc) says:
#
# /proc/[pid]/root
# UNIX and Linux support the idea of a per-process root of the file system, set by the chroot(2)
# system call. This file is a symbolic link that points to the process's root directory, and behaves
# as exe, fd/*, etc. do. 
# In a multithreaded process, the contents of this symbolic link are not available if the main
# thread has already terminated (typically by calling pthread_exit(3)). 

Exception:

Unhandled Exception: System.IO.PathTooLongException: The specified file name or path is too long, or a component of the specified path is too long.
   at System.IO.UnixFileSystem.FileSystemEnumerable`1.OpenDirectory(String fullPath)
   at System.IO.UnixFileSystem.FileSystemEnumerable`1.<Enumerate>d__11.MoveNext()
   at System.Collections.Generic.EnumerableHelpers.ToArray[T](IEnumerable`1 source, Int32& length)
   at System.IO.Directory.InternalGetFileDirectoryNames(String path, String userPathOriginal, String searchPattern, Boolean includeFiles, Boolean includeDirs, SearchOption searchOption)
   at System.IO.Directory.GetFiles(String path, String searchPattern, SearchOption searchOption)
   at Program.Main(String[] args)

[JeremyKuhne]

The same problem happens with Windows as well. Using EnumerateFiles will allow you to process the results as you go, but won't let you skip, just abort.

I don't think we can change the behavior of the existing API as tracking the recursion would have a significant impact on the performance. Adding an updated API is possible- you could turn this into an API request/review if you want.

[ianhays]

I agree with Jeremy on leaving the existing behavior as-is. I think an API to disallow link following would be fine or an API to take a predicate would be better.

[am11]

Thanks for the feedback @JeremyKuhne, @ianhays and sorry for the late reply; I have been doing some research and come to a agreement with Jeremy's comment above that with the existing EnumerateFiles API, the suggested predicate overload wouldn't be necessary. This is just my opinion and perhaps still up to debate if there are still some advantages of having a predicate signature, particularly the code that is written within the predicate function cannot be written in foreach(var file in directoryFiles) { } body for example.

However, to accomplish this use case, some utility methods such as:

// in Path or File/Directory Info types

// returns true if path is a symbolic link
public bool IsSymbolicLink(string path)

// if path is symbolic link, returns path where the symlink is pointing to
// if path is not a symbolic link, returns path as is
// on case insensitive filesystems, return the path with actual casing
public string GetRealPath(string path)

I have added some details with basic reasoning in https://github.com/dotnet/corefx/issues/10120#issuecomment-279795521. I would like to prepare a proposal for these two APIs with the same rationale (mainly their advantages and the fact that those are available in many language frameworks), but I am unsure at this point how to handle the UNC paths on Windows and answers to other questions / concerns which were raised in the aforementioned issue.

Generally, how much do we need to cover from implementation viewpoint in the API proposal? If you think off-hand that these are (from technical point of view) not good additions to CoreFX, please let me know. :)

[JeremyKuhne]

@am11 It isn't so much what we have to handle (though that obviously matters) as how we define the API and document it. Most APIs that deal with IO are going to have limitations and caveats- being aware of the corner cases we can choose terms and focus functionality to minimize confusion and give results that are as predictable as possible.

For the IsSymbolicLink() for example, we'd want to take into account terminology and variants in Windows. There are 3 types of links- how do we reflect that? Should we have IsLinkedFile() or perhaps GetLinkType() with None, Symbolic, Hard, etc.?

For GetRealPath() I struggle with the right term and I'm sort of unsettled on what that really gives back. On Windows should it give back the "canonical" volume, which is Guid based \\?\Volume{572FE591-B2EF-4617-AFEC-8DE83A32A7C5}? The same volume can be given any number of aliases. While this seems niche at first I expect it to become more common. There are only 26 possible DOS path drive letters, I can see wanting to use volumes without "mounting" them or using something other than A-Z:.

I do think both of your suggestions we really need and we should create issues for them. They are on my bucket list I just haven't gotten very far with them as I really want to make sure we nail them. I think to put the proposals up for official review we really need to have a ton of details on all the different inputs, which doesn't necessarily mean prototype code but it could be hard to fully understand the implications without it. If we can get enough active discussion and participation (particularly from folks who are more intimately familiar with Unix than I am) I'm happy to help keep the momentum going in any way I can.

I'll note that I've written a lot of the IO P/Invoke wrappers in my Winterop project specifically to explore these sort of things.

[JeremyKuhne]

We have a customizable enumeration API now which should allow you to skip links by looking at the file attributes. System.IO.FileSystemEnumerable