-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Make Get-ChildItem follow symlinks on demand, with checks for link loops #4020
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
Changes from all commits
368f1bd
12819e5
db1e74f
defda05
6a79850
2f7b4ee
532d251
cd1790a
7111a5d
4760c05
671b80e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1539,8 +1539,19 @@ private void GetPathItems( | |
| if (isDirectory) | ||
| { | ||
| DirectoryInfo directory = new DirectoryInfo(path); | ||
| InodeTracker tracker = null; | ||
|
|
||
| if (recurse) | ||
| { | ||
| GetChildDynamicParameters fspDynamicParam = DynamicParameters as GetChildDynamicParameters; | ||
| if (fspDynamicParam != null && fspDynamicParam.FollowSymlink) | ||
| { | ||
| tracker = new InodeTracker(directory.FullName); | ||
| } | ||
| } | ||
|
|
||
| // Enumerate the directory | ||
| Dir(directory, recurse, depth, nameOnly, returnContainers); | ||
| Dir(directory, recurse, depth, nameOnly, returnContainers, tracker); | ||
| } | ||
| else | ||
| { | ||
|
|
@@ -1609,7 +1620,8 @@ private void Dir( | |
| bool recurse, | ||
| uint depth, | ||
| bool nameOnly, | ||
| ReturnContainers returnContainers) | ||
| ReturnContainers returnContainers, | ||
| InodeTracker tracker) // tracker will be non-null only if the user invoked the -FollowSymLinks and -Recurse switch parameters. | ||
| { | ||
| List<IEnumerable<FileSystemInfo>> target = new List<IEnumerable<FileSystemInfo>>(); | ||
|
|
||
|
|
@@ -1788,25 +1800,35 @@ private void Dir( | |
| return; | ||
| } | ||
|
|
||
| // Once the recursion process has begun by being in this function, | ||
| // we do not want to further recurse into directory symbolic links | ||
| // so as to prevent the possibility of an endless symlink loop. | ||
| // This is the behavior of both the Unix 'ls' command and the Windows | ||
| // 'DIR' command. | ||
| if (!InternalSymbolicLinkLinkCodeMethods.IsReparsePoint(recursiveDirectory)) | ||
| bool hidden = false; | ||
| if (!Force) | ||
| { | ||
| bool hidden = false; | ||
| if (!Force) | ||
| hidden = (recursiveDirectory.Attributes & FileAttributes.Hidden) != 0; | ||
| } | ||
|
|
||
| // if "Hidden" is explicitly specified anywhere in the attribute filter, then override | ||
| // default hidden attribute filter. | ||
| if (Force || !hidden || isFilterHiddenSpecified || isSwitchFilterHiddenSpecified) | ||
| { | ||
| // We only want to recurse into symlinks if | ||
| // a) the user has asked to with the -FollowSymLinks switch parameter and | ||
| // b) the directory pointed to by the symlink has not already been visited, | ||
| // preventing symlink loops. | ||
| if (tracker == null) | ||
| { | ||
| hidden = (recursiveDirectory.Attributes & FileAttributes.Hidden) != 0; | ||
| if (InternalSymbolicLinkLinkCodeMethods.IsReparsePoint(recursiveDirectory)) | ||
| { | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| // if "Hidden" is explicitly specified anywhere in the attribute filter, then override | ||
| // default hidden attribute filter. | ||
| if (Force || !hidden || isFilterHiddenSpecified || isSwitchFilterHiddenSpecified) | ||
| else if (!tracker.TryVisitPath(recursiveDirectory.FullName)) | ||
| { | ||
| Dir(recursiveDirectory, recurse, depth - 1, nameOnly, returnContainers); | ||
| WriteWarning(StringUtil.Format(FileSystemProviderStrings.AlreadyListedDirectory, | ||
| recursiveDirectory.FullName)); | ||
| continue; | ||
| } | ||
|
|
||
| Dir(recursiveDirectory, recurse, depth - 1, nameOnly, returnContainers, tracker); | ||
| } | ||
| }//foreach | ||
| }//if | ||
|
|
@@ -7320,6 +7342,52 @@ private struct NetResource | |
| [MarshalAs(UnmanagedType.LPWStr)] | ||
| public string Provider; | ||
| } | ||
|
|
||
| #region InodeTracker | ||
| /// <summary> | ||
| /// Tracks visited files/directories by caching their device IDs and inodes. | ||
| /// </summary> | ||
| private class InodeTracker | ||
| { | ||
| private HashSet<(UInt64, UInt64)> _visitations; | ||
|
|
||
| /// <summary> | ||
| /// Construct a new InodeTracker with an initial path | ||
| /// </summary> | ||
| internal InodeTracker(string path) | ||
| { | ||
| _visitations = new HashSet<(UInt64, UInt64)>(); | ||
|
|
||
| if (InternalSymbolicLinkLinkCodeMethods.GetInodeData(path, out (UInt64, UInt64) inodeData)) | ||
| { | ||
| _visitations.Add(inodeData); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Attempt to mark a path as having been visited. | ||
| /// </summary> | ||
| /// <param name="path"> | ||
| /// Path to the file system item to be visited. | ||
| /// </param> | ||
| /// <returns> | ||
| /// True if the path had not been previously visited and was | ||
| /// successfully marked as visited, false otherwise. | ||
| /// </returns> | ||
| internal bool TryVisitPath(string path) | ||
| { | ||
| bool returnValue = false; | ||
|
|
||
| if (InternalSymbolicLinkLinkCodeMethods.GetInodeData(path, out (UInt64, UInt64) inodeData)) | ||
| { | ||
| returnValue = _visitations.Add(inodeData); | ||
| } | ||
|
|
||
| return returnValue; | ||
| } | ||
| } | ||
|
|
||
| #endregion | ||
| } // class FileSystemProvider | ||
|
|
||
| internal static class SafeInvokeCommand | ||
|
|
@@ -7482,6 +7550,12 @@ internal sealed class GetChildDynamicParameters | |
| [Parameter] | ||
| public FlagsExpression<FileAttributes> Attributes { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the flag to follow symbolic links when recursing. | ||
| /// </summary> | ||
| [Parameter] | ||
| public SwitchParameter FollowSymlink { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the filter directory flag | ||
| /// </summary> | ||
|
|
@@ -8238,6 +8312,45 @@ internal static bool WinIsSameFileSystemItem(string pathOne, string pathTwo) | |
| return false; | ||
| } | ||
|
|
||
| internal static bool GetInodeData(string path, out System.ValueTuple<UInt64, UInt64> inodeData) | ||
| { | ||
| #if UNIX | ||
| bool rv = Platform.NonWindowsGetInodeData(path, out inodeData); | ||
| #else | ||
| bool rv = WinGetInodeData(path, out inodeData); | ||
| #endif | ||
| return rv; | ||
| } | ||
|
|
||
| internal static bool WinGetInodeData(string path, out System.ValueTuple<UInt64, UInt64> inodeData) | ||
| { | ||
| var access = FileAccess.Read; | ||
| var share = FileShare.Read; | ||
| var creation = FileMode.Open; | ||
| var attributes = FileAttributes.BackupSemantics | FileAttributes.PosixSemantics; | ||
|
|
||
| using (var sf = AlternateDataStreamUtilities.NativeMethods.CreateFile(path, access, share, IntPtr.Zero, creation, (int)attributes, IntPtr.Zero)) | ||
| { | ||
| if (!sf.IsInvalid) | ||
| { | ||
| BY_HANDLE_FILE_INFORMATION info; | ||
|
|
||
| if (GetFileInformationByHandle(sf.DangerousGetHandle(), out info)) | ||
|
||
| { | ||
| UInt64 tmp = info.FileIndexHigh; | ||
| tmp = (tmp << 32) | info.FileIndexLow; | ||
|
|
||
| inodeData = (info.VolumeSerialNumber, tmp); | ||
|
|
||
| return true; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| inodeData = (0, 0); | ||
| return false; | ||
| } | ||
|
|
||
| internal static bool IsHardLink(ref IntPtr handle) | ||
| { | ||
| #if UNIX | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| //! @file getinodedata.cpp | ||
| //! @author Jeff Bienstadt <v-jebien@microsoft.com> | ||
| //! @brief Retrieve the device ID and inode number of a file | ||
|
|
||
| #include "getinodedata.h" | ||
|
|
||
| #include <assert.h> | ||
| #include <errno.h> | ||
| #include <locale.h> | ||
| #include <sys/types.h> | ||
| #include <sys/stat.h> | ||
| #include <unistd.h> | ||
| #include <string> | ||
|
|
||
| //! @brief GetInodeData retrieves a file's device and inode information. | ||
|
||
| //! | ||
| //! GetInodeData | ||
| //! | ||
| //! @param[in] fileName | ||
| //! @parblock | ||
| //! A pointer to the buffer that contains the file path | ||
| //! | ||
| //! char* is marshaled as an LPStr, which on Linux is UTF-8. | ||
| //! @endparblock | ||
| //! | ||
| //! @param[out] device | ||
| //! @parblock | ||
| //! Points to a uint64_t value that will contain the file's device ID. | ||
| //! @endparblock | ||
| //! | ||
| //! @param[out] inode | ||
| //! @parblock | ||
| //! Points to a uint64_t value that will contain the file's inode number. | ||
| //! @endparblock | ||
| //! | ||
| //! @retval 0 If the function succeeds, -1 otherwise. | ||
| //! | ||
|
|
||
| int32_t GetInodeData(const char* fileName, uint64_t* device, uint64_t* inode) | ||
| { | ||
| assert(fileName); | ||
| assert(device); | ||
| assert(inode); | ||
| errno = 0; | ||
|
|
||
| struct stat statBuf; | ||
| int ret = stat(fileName, &statBuf); | ||
|
|
||
| if (ret == 0) | ||
| { | ||
| *device = statBuf.st_dev; | ||
| *inode = statBuf.st_ino; | ||
| } | ||
|
|
||
| return ret; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| #pragma once | ||
|
|
||
| #include "pal.h" | ||
|
|
||
| PAL_BEGIN_EXTERNC | ||
|
|
||
| int32_t GetInodeData(const char* fileName, uint64_t* device, uint64_t* inode); | ||
|
|
||
| PAL_END_EXTERNC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that you are following a pattern like
IsSameFileSystemItemorIsHardLink. Ideally, code here should be refactored to consolidate Unix/Windows implementations in one place and eliminate wrapper functions likeIsSameFileSystemItemandIsHardLink. Instead they should beI'm not asking you to do this change for this PR, just want to call out that it should be this way moving forward. I opened #4086 to track the refactoring task.