3 bugs - Directory.GetFiles - Directory.GetDirectories & Directory.Move

[YetAnotherPhil]

I tried your Pri.LongPath package yesterday & found the following issues:

1. This fails to find matching files in sub-directories: Directory.GetFiles(FilePath,FileNamePattern,System.IO.SearchOption.AllDirectories)
2. This fails to find matching directories in sub-directories: Directory.GetDirectories(FilePath,FolderNamePattern,System.IO.SearchOption.AllDirectories)
3. This oftens throws a 'System.UnauthorizedAccessException: Access to the path denied' expection: Directory.Move(oldFolder, newFolder)

[thomaslevesque]

I can't reproduce 1 and 2, these cases work fine for me.

As for 3, this is probably because you have something open in the source directory. You would get the same error with System.IO.Directory.Move.

[YetAnotherPhil]

Thanks for the super fast reply. Please can you try out these examples to reproduce the first 2 errors: Create : C:\Test\a folder 1\somefile.txt Try: Directory.GetFiles("C:\Test","some_.txt",System.IO.SearchOption.AllDirectories) Try: Directory.GetDirectories("C:\Test","_folder*",System.IO.SearchOption.AllDirectories)

The System.IO.Directory.Move works fine for me i.e. never encounters this exception. Your implementation however, fails often (~40%). There is definitely a bug here.

[YetAnotherPhil]

These 2 examples fail (i.e. no files or directories found) for me...

Create: C:\Test\a folder 1\b folder 2\somefile.txt

var files = Directory.GetFiles("C:\Test","some*.txt",System.IO.SearchOption.AllDirectories);

var dirs = Directory.GetDirectories("C:\Test","b folder*",System.IO.SearchOption.AllDirectories);

[thomaslevesque]

OK, I was just testing with "." as the filter... now I see the problem.

(I'm not the maintainer of this project, btw)

[peteraritchie]

Yes, thanks for the follow up @thomaslevesque. Your day must start earlier than mine :) I gather you've reproduced it? Thanks for the report @YetAnotherPhil, I'll look into it.

[YetAnotherPhil]

Thanks guys, much appreciated

[YetAnotherPhil]

Here's a modified version of your EnumerateFileSystemIteratorRecursive (not thoroughly tested by any means!)...

    private static IEnumerable<string> EnumerateFileSystemIteratorRecursive(string normalizedPath, string normalizedSearchPattern, bool includeDirectories, bool includeFiles)
    {
        // NOTE: Any exceptions thrown from this method are thrown on a call to IEnumerator<string>.MoveNext()
        var pending = new Queue<string>();
        pending.Enqueue(normalizedPath);
        while (pending.Count > 0)
        {
            normalizedPath = pending.Dequeue();
            string path = Path.RemoveLongPathPrefix(normalizedPath);

            foreach(var dir in Directory.GetDirectories(path))
                {
                pending.Enqueue(Path.NormalizeLongPath(dir));
                }

            NativeMethods.WIN32_FIND_DATA findData;
            using (SafeFindHandle handle = BeginFind(Path.Combine(normalizedPath, normalizedSearchPattern), out findData))
            {
                if (handle == null)
                    continue;

                do
                {
                    string currentFileName = findData.cFileName;

                    var fullPath = Path.Combine(path, currentFileName);
                    if (IsDirectory(findData.dwFileAttributes))
                    {
                        var fullNormalizedPath = Path.Combine(normalizedPath, currentFileName);
                        System.Diagnostics.Debug.Assert(Directory.Exists(fullPath));
                        System.Diagnostics.Debug.Assert(Directory.Exists(Path.RemoveLongPathPrefix(fullNormalizedPath)));
                        if (!IsCurrentOrParentDirectory(currentFileName))
                        {
                            //pending.Enqueue(fullNormalizedPath);
                            if (includeDirectories)
                            {
                                yield return fullPath;
                            }
                        }
                    }
                    else
                    {
                        if (includeFiles)
                        {
                            yield return fullPath;
                        }
                    }
                } while (NativeMethods.FindNextFile(handle, out findData));

                int errorCode = Marshal.GetLastWin32Error();
                if (errorCode != NativeMethods.ERROR_NO_MORE_FILES)
                    throw Common.GetExceptionFromWin32Error(errorCode);
            }
        }
    }

[peteraritchie]

Thanks. I need to work through some Visual Studio issues that I've encountered with this project; but I hope to thow this on my backlog soon.

[YetAnotherPhil]

The Directory.Move bug... The wrong (different) exception was thrown when a file\folder is locked. Below is a modified version of your GetExceptionFromWin32Error to correct the issue:

    internal static Exception GetExceptionFromWin32Error(int errorCode, string parameterName)
    {
        string message = GetMessageFromErrorCode(errorCode);

        switch (errorCode)
        {
            case NativeMethods.ERROR_FILE_NOT_FOUND:
                return new System.IO.FileNotFoundException(message);

            case NativeMethods.ERROR_PATH_NOT_FOUND:
                return new System.IO.DirectoryNotFoundException(message);

            case NativeMethods.ERROR_ACCESS_DENIED:
                //return new UnauthorizedAccessException(message);
                return new System.IO.IOException(message);

            case NativeMethods.ERROR_FILENAME_EXCED_RANGE:
                return new System.IO.PathTooLongException(message);

            case NativeMethods.ERROR_INVALID_DRIVE:
                return new System.IO.DriveNotFoundException(message);

            case NativeMethods.ERROR_OPERATION_ABORTED:
                return new OperationCanceledException(message);

            case NativeMethods.ERROR_INVALID_NAME:
                return new ArgumentException(message, parameterName);

            default:
                return new System.IO.IOException(message, NativeMethods.MakeHRFromErrorCode(errorCode));

        }
    }

[thomaslevesque]

This change doesn't make sense, you're just hiding some information. If the system returned "access denied", it's better to throw an UnauthorizedException; that's what the BCL is doing as well. If you just throw an IOException, there is no way to detect that it was actually an access denied error.

[YetAnotherPhil]

Try this...

Create c:\test\somefile.txt

Open Explorer.exe in c:\test to lock the folder

Now run

try { Pri.LongPath.Directory.Move("c:\test", "c:\test2"); } catch(Exception e) { var err = e.ToString(); }

try { System.IO.Directory.Move("c:\test", "c:\test2"); } catch(Exception e) { var err = e.ToString(); }

You should see System.IO.Directory.Move raises a System.IO.IOException. The Access Denied message isn't lost, it's just the type of exception you're throwing differs from System.IO.Directory.Move.

[peteraritchie]

This is some hours code to expand the unit tests. I'll verify the results and evaluate how to change the code to fix this and not break something else. Thanks

[peteraritchie]

I've committed a change that should deal with the first issue. Similar to what you posted @YetAnotherPhil, but a little more efficient.

[peteraritchie]

I just added some unit tests to verify case two is also fixed.

[peteraritchie]

Commit 8b6011ee69d0c46e9e3265f2df0c879e458c3f44 fixes the UnauthorizedAccessException and includes tests to verify.