Prevent Get-ChildItem from recursing into symlinks (#1875).#3780
Prevent Get-ChildItem from recursing into symlinks (#1875).#3780daxian-dbw merged 3 commits intoPowerShell:masterPowerShell/PowerShell:masterfrom jeffbi:child-item-1875Copy head branch name to clipboard
Conversation
Brings the cmdlet more in line with the Unix "ls -r" and the Windows "DIR /S" commands. Like the Unix "ls" command, the cmdlet will recurse into symlinks given on the command line, but not into symlinks found during recursion.
|
@jeffbi Could you please expand "Brings the cmdlet more in line with the Unix ls -r and the Windows DIR /S commands" for future docs (describe a behavior of the native commands)? |
|
@iSazonov Updated the description. |
| { | ||
| Dir(recursiveDirectory, recurse, depth - 1, nameOnly, returnContainers); | ||
| bool hidden = false; | ||
| if (!Force) hidden = (recursiveDirectory.Attributes & FileAttributes.Hidden) != 0; |
There was a problem hiding this comment.
Please use pattern:
if ( ... )
{
...
}
There was a problem hiding this comment.
@iSazonov This all-on-one-line pattern, which I don't like either, is used in a couple of places in the code. Shall I change them all?
There was a problem hiding this comment.
General rule - do not make changes that do not belong to the main remedy in order not to complicate the review.
But maintainers may request/allow to correct bad patterns.
|
@jeffbi Thanks for the good fix! LGTM. |
|
@iSazonov Thanks for the review! |
| // if "Hidden" is explicitly specified anywhere in the attribute filter, then override | ||
| // default hidden attribute filter. | ||
| if (Force || !hidden || isFilterHiddenSpecified || isSwitchFilterHiddenSpecified) | ||
| if (!InternalSymbolicLinkLinkCodeMethods.IsReparsePoint(recursiveDirectory)) |
There was a problem hiding this comment.
@jeffbi Could you please summarize the description and add it as comments right before this statement? I hope the comment can explain why we are checking IsReparsePoint on recursiveDirectory and what behavior of Get-ChildItem we are trying to get by having this check. I'm sure the comments will be very helpful to other people when looking at this code.
| $ci[1].Name | Should MatchExactly $filenamePattern | ||
| $ci[2].Name | Should MatchExactly $filenamePattern | ||
| } | ||
| It "Get-ChildItem does not recurse into symbolic links" { |
There was a problem hiding this comment.
This test case title is a little confusing because Get-ChildItem $alphaLink -Recurse works recursively as expected :)
Maybe the following is better?
Get-ChildItem does not recurse into symbolic links unless it's explicitly specified on command line
|
@daxian-dbw Yes, this prevents the situation that drives #3761. |
|
Somehow the AppVeyor CI status is not reported back to Github, but the AppVeyor CI build was successful: |
|
I think we need the ability to opt in with respect to symlink recursion - please see #3951 |
Brings the Get-ChildItem more in line with the Unix
ls -rand the WindowsDIR /Snative commands. Like these commands the cmdlet will display symbolic links to directories found during recursion but will not recurse into them.Like the Unix
lscommand---and unlike the WindowsDIR /Scommand--- the cmdlet will recurse into symlinks given on the command line.This also will fix the underlying problem behind #3761.