FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Prevent Get-ChildItem from recursing into symlinks (#1875). by jeffbi · Pull Request #3780 · PowerShell/PowerShell · GitHub

/ PowerShell Public

Prevent Get-ChildItem from recursing into symlinks (#1875). - #3780

Merged
Dongbo Wang (daxian-dbw) merged 3 commits into
PowerShell:masterfrom
jeffbi:child-item-1875
May 15, 2017
ConversationCommits (3)ChecksFiles changed
Merged

Prevent Get-ChildItem from recursing into symlinks (#1875).#3780
Dongbo Wang (daxian-dbw) merged 3 commits into
PowerShell:masterfrom
jeffbi:child-item-1875

Conversation

jeffbi commented May 13, 2017 •
edited
Loading

Copy link
Copy Markdown

Brings the Get-ChildItem more in line with the Unix ls -r and the Windows DIR /S native commands. Like these commands the cmdlet will display symbolic links to directories found during recursion but will not recurse into them.

Like the Unix ls command---and unlike the Windows DIR /S command--- the cmdlet will recurse into symlinks given on the command line.

This also will fix the underlying problem behind #3761.

Prevent Get-ChildItem from recursing into symlinks (#1875).
8dbbab5 
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.

Ilya (iSazonov) commented May 13, 2017

Copy link
Copy Markdown
Collaborator

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)?

jeffbi commented May 13, 2017

Copy link
Copy Markdown
Author

Ilya (@iSazonov) Updated the description.

Ilya (iSazonov) reviewed May 13, 2017
View reviewed changes
Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs Outdated
{
Dir(recursiveDirectory, recurse, depth - 1, nameOnly, returnContainers);
bool hidden = false;
if (!Force) hidden = (recursiveDirectory.Attributes & FileAttributes.Hidden) != 0;

Ilya (iSazonov) May 13, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Please use pattern:

if ( ... )
{
    ...
}

jeffbi May 13, 2017

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ilya (@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?

Ilya (iSazonov) May 14, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 May 14, 2017

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Got it, thanks.

Fixed

Ilya (iSazonov) commented May 15, 2017

Copy link
Copy Markdown
Collaborator

jeffbi Thanks for the good fix!

LGTM.

jeffbi commented May 15, 2017

Copy link
Copy Markdown
Author

Ilya (@iSazonov) Thanks for the review!

Dongbo Wang (daxian-dbw) reviewed May 15, 2017
View reviewed changes
Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs
// 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))

Dongbo Wang (daxian-dbw) May 15, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

jeffbi May 15, 2017

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Comments added.

Comment thread test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 Outdated
$ci[1].Name | Should MatchExactly $filenamePattern
$ci[2].Name | Should MatchExactly $filenamePattern
}
It "Get-ChildItem does not recurse into symbolic links" {

Dongbo Wang (daxian-dbw) May 15, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

jeffbi May 15, 2017

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Title changed.

Dongbo Wang (daxian-dbw) commented May 15, 2017

Copy link
Copy Markdown
Member

Great fix jeffbi! This will fix #3761, right?

jeffbi commented May 15, 2017

Copy link
Copy Markdown
Author

Dongbo Wang (@daxian-dbw) Yes, this prevents the situation that drives #3761.

Dongbo Wang (daxian-dbw) commented May 15, 2017

Copy link
Copy Markdown
Member

Somehow the AppVeyor CI status is not reported back to Github, but the AppVeyor CI build was successful:
https://ci.appveyor.com/project/PowerShell/powershell/build/6.0.0-beta.1-3181. Given that, I will merge this PR.

Dongbo Wang (daxian-dbw) merged commit ee1a897 into PowerShell:master May 15, 2017
jeffbi deleted the child-item-1875 branch May 15, 2017 21:45
Steve Lee (SteveL-MSFT) added the Breaking-Change breaking change that may affect users label May 31, 2017
Michael Klement (mklement0) mentioned this pull request Jun 6, 2017
Closed

Michael Klement (mklement0) commented Jun 6, 2017

Copy link
Copy Markdown
Contributor

I think we need the ability to opt in with respect to symlink recursion - please see #3951

Michael Klement (mklement0) mentioned this pull request Jun 6, 2017
Closed
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
Prevent Get-ChildItem from recursing into symlinks (PowerShell#3780)
b9e2c78 
Brings the Get-ChildItem more in line with the Unix ls -r and the Windows DIR /S native commands. Like these commands, the cmdlet will display symbolic links to directories found during recursion but will not recurse into them.
Like the Unix ls command---and unlike the Windows DIR /S command--- the cmdlet will recurse into symlinks given on the command line.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Ilya (iSazonov) iSazonov left review comments
Dongbo Wang (daxian-dbw) daxian-dbw approved these changes

Assignees

Dongbo Wang (daxian-dbw)

Labels

Breaking-Change breaking change that may affect users

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL