8000 Adding support for native command globbing on UNIX by BrucePay · Pull Request #3643 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

Adding support for native command globbing on UNIX #3643

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 1, 2017
Merged

Adding support for native command globbing on UNIX #3643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f451da9
Implement UNIX-like globbing for native commands
Apr 27, 2017
e5f68fb
Update tests to use the recommended skip pattern
daxian-dbw May 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 120 additions & 19 deletions src/System.Management.Automation/engine/NativeCommandParameterBinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,39 +192,140 @@ private void appendOneNativeArgument(ExecutionContext context, object obj, char
//
// We need to check quotes that the win32 argument parser checks which is currently
// just the normal double quotes, no other special quotes. Also note that mismatched
// quotes are supported.

bool needQuotes = false, followingBackslash = false;
int quoteCount = 0;
for (int i = 0; i < arg.Length; i++)
{
if (arg[i] == '"' && !followingBackslash)
{
quoteCount += 1;
}
else if (char.IsWhiteSpace(arg[i]) && (quoteCount % 2 == 0))
{
needQuotes = true;
}

followingBackslash = arg[i] == '\\';
}

if (needQuotes)
// quotes are supported
if (NeedQuotes(arg))
{
_arguments.Append('"');
_arguments.Append(arg);
_arguments.Append('"');
}
else
{
#if UNIX
// On UNIX systems, we expand arguments containing wildcard expressions against
// the file system just like bash, etc.
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we settled on calling the C api glob.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have no recollection of any such thing. Why would we do that rather than using PowerShell's intrinsic globbing capability?

Copy link
Contributor

Choose a reason for hiding this comment

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

To be compatible as compatible with bash as reasonably possible. There are things glob does that we don't with character classes.

I don't recall discussing or looking closely at the inverse - if our wildcards support things that glob does not.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

PowerShell globbing supports all of the constructs described in glob(3) but is case-insensitive plus it understands powershell drives which fnmatch(3) is unaware of. Having the globbing behavior change based on the type of the command sounds like a very bad user experience.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure? /bin/echo * should not echo files starting with ., and /bin/echo "*" should echo *, not the files in the current directory.

Maybe less interesting, but glob(7) says glob supports character classes like [:upper:] and more - we definitely don't support those. That said, it doesn't look like bash supports those.

Case-insensitive also concerns me. rm M* shouldn't remove files starting with lowercase m.

I'm not saying that glob(3) is exactly what we should use - just that there may be subtleties that we miss by not using a POSIX api.

As for PowerShell drives - I'm really curious if they'll be used on non-Windows platforms - I'm skeptical - symbolic links have served that purpose perfectly and work outside of PowerShell.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looking at the bash source, a lot of the subtleties around globbing are in bash itself, not the glob routine. For example ~ expansion is not handled by glob. Likewise quote processing is handled in bash itself, not by the glob routine. Given that PowerShell does it's quote removal (and addition) in a very different way than bash, there are going to be differences. (To make /bin/echo "*" work, we need to be able to be able to determine if a string value was parsed as a bareword literal at execution time.) Also note that Bash optionally supports extended globbing (See Bash Extended Globbing for an overview). Anyway, I've opened a new issue to track this investigation #3655

if (System.Management.Automation.WildcardPattern.ContainsWildcardCharacters(arg))
{
// See if the current working directory is a filesystem provider location
// We won't do the expansion if it isn't since native commands can only access the file system.
var cwdinfo = Context.EngineSessionState.CurrentLocation;

// If it's a filesystem location then expand the wildcards
if (string.Equals(cwdinfo.Provider.Name, Microsoft.PowerShell.Commands.FileSystemProvider.ProviderName,
StringComparison.OrdinalIgnoreCase))
{
bool normalizePath = true;
// On UNIX, paths starting with ~ are not normalized
if (arg.Length > 0 && arg[0] == '~')
{
normalizePath = false;
}

// See if there are any matching paths otherwise just add the pattern as the argument
var paths = Context.EngineSessionState.InvokeProvider.ChildItem.Get(arg, false);
if (paths.Count > 0)
{
bool first = true;
foreach (var path in paths)
{
object pbo = path.BaseObject;
if (! first)
{
_arguments.Append(" ");
}
else
{
if (! (pbo is System.IO.FileSystemInfo))
{
// If the object is not a filesystem object, then just append
// the pattern unchanged
_arguments.Append(arg);
break;
}
first = false;
}
var expandedPath = (pbo as System.IO.FileSystemInfo).FullName;
if (normalizePath)
{
expandedPath = Context.SessionState.Path.NormalizeRelativePath(expandedPath, cwdinfo.ProviderPath);
}
// If the path contains spaces, then add quotes around it.
if (NeedQuotes(expandedPath))
{
_arguments.Append("\"");
_arguments.Append(expandedPath);
_arguments.Append("\"");
}
else
{
_arguments.Append(expandedPath);
}
}
}
else
{
_arguments.Append(arg);
}
}
else
{
_arguments.Append(arg);
}
}
else
{
// Even if there are no wildcards, we still need to possibly
// expand ~ into the filesystem provider home directory path
ProviderInfo fileSystemProvider = Context.EngineSessionState.GetSingleProvider(
Microsoft.PowerShell.Commands.FileSystemProvider.ProviderName);
string home = fileSystemProvider.Home;
if (string.Equals(arg, "~"))
{
_arguments.Append(home);
}
else if (arg.StartsWith("~/", StringComparison.OrdinalIgnoreCase))
{
var replacementString = home + arg.Substring(1);
_arguments.Append(replacementString);
}
else
{
_arguments.Append(arg);
}
}
#else
_arguments.Append(arg);
#endif
}
}
}
} while (list != null);
}

/// <summary>
/// Check to see if the string contains spaces and therefore must be quoted.
/// </summary>
/// <param name="stringToCheck">The string to check for spaces</param>
private bool NeedQuotes(string stringToCheck)
{
bool needQuotes = false, followingBackslash = false;
int quoteCount = 0;
for (int i = 0; i < stringToCheck.Length; i++)
{
if (stringToCheck[i] == '"' && !followingBackslash)
{
quoteCount += 1;
}
else if (char.IsWhiteSpace(stringToCheck[i]) && (quoteCount % 2 == 0))
{
needQuotes = true;
}
followingBackslash = stringToCheck[i] == '\\';
}
return needQuotes;
}


/// <summary>
/// The native command to bind to
/// </summary>
Expand Down
59 changes: 59 additions & 0 deletions test/powershell/Language/Scripting/NativeExecution/NativeUnixGlobbing.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@

Describe 'Native UNIX globbing tests' -tags "CI" {

BeforeAll {
if (-not $IsWindows )
{
"" > "$TESTDRIVE/abc.txt"
"" > "$TESTDRIVE/bbb.txt"
"" > "$TESTDRIVE/cbb.txt"
}

$defaultParamValues = $PSDefaultParameterValues.Clone()
$PSDefaultParameterValues["it:skip"] = $IsWindows
}

AfterAll {
$global:PSDefaultParameterValues = $defaultParamValues
}

# Test * expansion
It 'The globbing pattern *.txt should match 3 files' {
(/bin/ls $TESTDRIVE/*.txt).Length | Should Be 3
}
It 'The globbing pattern *b.txt should match 2 files whose basenames end in "b"' {
(/bin/ls $TESTDRIVE/*b.txt).Length | Should Be 2
}
# Test character classes
It 'The globbing pattern should match 2 files whose names start with either "a" or "b"' {
(/bin/ls $TESTDRIVE/[ab]*.txt).Length | Should Be 2
}
It 'Globbing abc.* should return one file name "abc.txt"' {
/bin/ls $TESTDRIVE/abc.* | Should Match "abc.txt"
}
# Test that ? matches any single character
It 'Globbing [cde]b?.* should return one file name "cbb.txt"' {
/bin/ls $TESTDRIVE/[cde]b?.* | Should Match "cbb.txt"
}
It 'Should return the original pattern if there are no matches' {
/bin/echo $TESTDRIVE/*.nosuchfile | Should Match "\*\.nosuchfile$"
}
# Test the behavior in non-filesystem drives
It 'Should not expand patterns on non-filesystem drives' {
/bin/echo env:ps* | Should BeExactly "env:ps*"
}
# Test the behavior for files with spaces in the names
It 'Globbing filenames with spaces should match 2 files' {
"" > "$TESTDRIVE/foo bar.txt"
"" > "$TESTDRIVE/foo baz.txt"
(/bin/ls $TESTDRIVE/foo*.txt).Length | Should Be 2
}
# Test ~ expansion
It 'Tilde should be replaced by the filesystem provider home directory' {
/bin/echo ~ | Should BeExactly ($executioncontext.SessionState.Provider.Get("FileSystem").Home)
}
# Test ~ expansion with a path fragment (e.g. ~/foo)
It '~/foo should be replaced by the <filesystem provider home directory>/foo' {
/bin/echo ~/foo | Should BeExactly "$($executioncontext.SessionState.Provider.Get("FileSystem").Home)/foo"
}
}
0