Fix tab completion for filename contains literal wildcard character #7407

kwkam · 2018-07-30T13:10:53Z

PR Summary

*System.Management.Automation/engine/CommandCompletion/CompletionCompleters:
Use WildcardPattern::Escape to escape completion text of filename.

*System.Management.Automation/engine/regex:
WildcardPattern::Escape should also escape "`" since
WildcardPattern::Unescape would unescape it,
and the matcher parse it as escape character.

*System.Management.Automation/namespaces/LocationGlobber:
Do not pass regex escaped string to WildcardPattern::Get. This fails
tab completion when doing "./path/to/`[f"<TAB>.

PR Checklist

*System.Management.Automation/engine/CommandCompletion/CompletionCompleters: Use WildcardPattern::Escape to escape completion text of filename. *System.Management.Automation/engine/regex: WildcardPattern::Escape should also escape "`" since WildcardPattern::Unescape would unescape it, and the matcher parse it as escape character. *System.Management.Automation/namespaces/LocationGlobber: Do not pass regex escaped string to WildcardPattern::Get. This fails tab completion when doing "./path/to/`[f"<TAB>.

iSazonov · 2018-07-31T04:34:28Z

@kwkam Please remove "[Feature]" from title in your PRs. We need this prefix only in commit header to force run feature tests.

stale · 2018-08-30T11:58:23Z

This PR has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed if no further activity occurs within 10 days.
Thank you for your contributions.
Community members are welcome to grab these works.

iSazonov · 2018-08-30T13:43:46Z

test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1

@@ -739,6 +739,9 @@ dir -Recurse `
                @{ inputStr = "Get-Process >'.\My ``[Path``]\'"; expected = "'.${separator}My ``[Path``]${separator}test.ps1'" }
                @{ inputStr = "Get-Process >${tempDir}\My"; expected = "'${tempDir}${separator}My ``[Path``]'" }
                @{ inputStr = "Get-Process > '${tempDir}\My ``[Path``]\'"; expected = "'${tempDir}${separator}My ``[Path``]${separator}test.ps1'" }
+                @{ inputStr = "cd 'My ``["; expected = "'.${separator}My ``[Path``]'" }


We should replace the alias with cmdlet and above too.

I'd expect that users will type the path without escape - it doesn't work.

I'd expect that it will work for literal path too - it doesn't work. It could be out of the PR but I'd prefer to remove the fix from the PR and get a full fix in another PR for all scenarios. We should do full review how we process all special chars in Path and in LiteralPath. I do not see the point in partial fixes because they can break something without having a full set of tests.

Updated test.

I disagree. User should type escape for -Path (which support globbing), just like we do in bash (eg. touch \[test\]). But if you typed escape and auto completion still did not work, that is because by default, powershell treats "unquoted string without space" like "double-quoted string", that "double-quoted string" will consume escape character. For file name [test], either Get-Content -Path ```[ or Get-Content -Path '`[ will work.

Would you please let me know what did not work for -LiteraPath?
I tried Get-Item -LiteralPath [(TAB) for a file named [test] and it did auto complete.

Consider interactive UX. User do dir and then type directory or file name as one see on screen - without escapes. In the scenario both the user and PowerShell know right name. And more, we can have both names - with escape char too.

Get-Item '.\My `[

The problem is, -Path support wildcard. When a path can be an expression, the special characters should always be escaped otherwise it will become too complicated. For example, in a directory there are A.txt, B.txt and [AB].txt, what should Get-Item -Path [AB].txt return?

I tested both -Path and -LiteralPath and both will complete the file name. eg.
New-Item -Name 'My [test]'
Get-Item '.\My `[ will be completed to Get-Item '.\My `[test`]', and
Get-Item -LiteralPath '.\My [ will be completed to Get-Item -LiteralPath '.\My [test]'
Did you use the build here? https://ci.appveyor.com/project/PowerShell/powershell/build/v6.1.0-rc.10960/job/3i76g2ar1wqlvw7x/artifacts

test/powershell/Language/Parser/LanguageAndParser.TestFollowup.Tests.ps1

iSazonov · 2018-08-30T14:02:29Z

src/System.Management.Automation/engine/regex.cs

                //
-                if (IsWildcardChar(ch) && !charsNotToEscape.Contains(ch))
+                if ((IsWildcardChar(ch) || ch == escapeChar) &&
+                    !charsNotToEscape.Contains(ch))


Why we always should escape the escape char? What scenario don't work without fix?

When we accidentally created a file named `[test`]2 due to the incorrect handling of wildcard character, we cannot remove it using the -Path parameter with the tab completed name.

When I escaped string A and get B, and try to unescape B, I would expected that A will be returned, but it is not when A contains escape character.

The wildcard pattern parser always treat escape character as escape character.

My understanding is that we should test if a file/directory/path exists before trying escape. If exist then return the path. Then continue with escaping wildcard chars. Then continue with expansion wildcards if -Path.
Also what if there is some special symbols in the path? Seems we should process them in cycle.

Okay, I know what is wrong here. This part is to fix the WildcardPattern::Escape function itself, which is not necessary related to the file system.
When WildcardPattern::Get parse a pattern string, it always treats the escape character as escape character of an escape character. That means, `` will be parsed to `.
And so is the WildcardPattern::Unescape method, [WildcardPattern]::Unescape('``') will return ` instead of ``.
But, [WildcardPattern]::Escape('`') does not escape the escape character, which conflicts with the others.

@kwkam I see your point.
We have the public API

public static string Escape(string pattern)

So the change is a breaking change and it is not desirable to change this behavior until it is established that this is a significant bug.

@iSazonov Yes, we should be careful with changes in public API, but I doubt if this is not a significant bug. For example, there is a list of text:

`[HIGH] patch 1` `[MED] update 1` `[LOW] patch 1` `[MED] patch 2` `[HIGH] hotfix 3` `[LOW] patch 2` `[LOW] hotfix 3`

Now, I want to search for `[LOW] patch 1` and `[LOW] patch 2`, and I am using WildcardPattern:

$prefix = '`[LOW] patch ' $pattern = '[1-2]' $suffix = '`' $sarg = [WildcardPattern]::Escape($prefix) + $pattern + [WildcardPattern]::Escape($suffix) $opts = [Management.Automation.WildcardOptions]::Compiled $matcher = [WildcardPattern]::Get($sarg, $opts) $ListOfText | ? {$matcher.IsMatch($_)}

No result will be return because of the inconsistency between the Escape method and the WildcardPattern parser. (And this is also the cause of the issue in auto-completed filename which contains escape character)

Thanks for good example!

We need PowerShell Committee conclusion.
/cc @SteveL-MSFT

iSazonov · 2018-08-30T14:10:59Z

@kwkam Sorry for delay - it is very sensetive aria and we should very careful.

kwkam · 2018-08-31T21:40:41Z

@iSazonov Yes, that's why this PR was placed behind the cmdlet ones. Please let me know if something is not correct/not working.

SteveL-MSFT · 2018-09-05T22:49:19Z

cc @BrucePay to follow-up

stale · 2018-10-24T15:09:25Z

This PR has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed if no further activity occurs within 10 days.
Thank you for your contributions.
Community members are welcome to grab these works.

iSazonov · 2018-10-25T09:54:45Z

@SteveL-MSFT We have too many frozen PRs. How we can resolve the problem?

SteveL-MSFT · 2018-10-26T17:34:44Z

@BrucePay will follow-up on this specific PR

stale · 2018-11-25T18:31:59Z

This PR has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed if no further activity occurs within 10 days.
Thank you for your contributions.
Community members are welcome to grab these works.

test/powershell/engine/Api/WildcardPattern.Tests.ps1

Co-Authored-By: Ilya <darpa@yandex.ru>

TravisEz13 · 2019-06-24T18:36:16Z

src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs

@@ -4383,6 +4383,12 @@ internal static IEnumerable<CompletionResult> CompleteFilename(CompletionContext
                        if (CompletionRequiresQuotes(completionText, !useLiteralPath))
                        {
                            var quoteInUse = quote == string.Empty ? "'" : quote;
+


should all changes be behind the experimental feature?

This was a condition of the committee

Except filename completion will now also escape * and ? in non-literal path, the behaviour should be identical unless the experimental feature (the breaking part) is enabled.

msftrncs · 2019-07-28T07:46:29Z

I tested a variation of this PR, while testing the PR I am preparing to fix all argument completion patterns along with variable and member completions, #10226.

Previously, I could not use Get-Content to read the content of a file named `[hello`].txt, the file would NOT be read, but there would also be no error.

I did not modify the WildCardPattern.Escape() method, but instead added .Replace("`", "``") to the passed value of every single instance in the PowerShell code base of the Escape() method.

Now Get-Content reads the content of that file.

Even if changes to WildCardPattern.Escape() are considered too breaking, at least manually escape the backticks, in each call.

Unfortunately this doesn't fix the issue with redirection, which accepts a wildcard path, but then sends it to out-file as a literal.

kwkam · 2019-12-07T12:34:57Z

Unfortunately this doesn't fix the issue with redirection, which accepts a wildcard path, but then sends it to out-file as a literal.

@msftrncs It was in another PR #9258

ghost · 2020-05-27T02:04:14Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Mainainer, Please provide feedback and/or mark it as Waiting on Author

iSazonov · 2020-05-27T17:35:45Z

@kwkam Please resolve merge conflicts and address comments.

src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs

ghost · 2020-06-13T02:00:20Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Mainainer, Please provide feedback and/or mark it as Waiting on Author

TravisEz13 · 2020-10-27T17:53:02Z

test/powershell/engine/Api/WildcardPattern.Tests.ps1

@@ -0,0 +1,68 @@
+# Copyright (c) Microsoft Corporation. All rights reserved.


Suggested change

# Copyright (c) Microsoft Corporation. All rights reserved.

# Copyright (c) Microsoft Corporation.

TravisEz13 · 2020-10-27T17:54:27Z

@daxian-dbw Can you review this?

ghost · 2020-11-11T20:00:06Z

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 10 days of this comment.

floh96 · 2021-06-10T10:03:58Z

@iSazonov could this pr be reopened?
If I'm seeing this correctly, it was closed because of a copyright header change....

iSazonov · 2021-06-10T12:35:59Z

@floh96 It makes no sense to reopen. The work could be continue if anybody grabbed the work and MSFT team agreed to review. Now you could open new issue to discuss and vote.

kwkam requested review from anmenaga, BrucePay and daxian-dbw as code owners July 30, 2018 13:10

This was referenced Jul 30, 2018

[Feature] Fix path handling with wildcard char #7396

Closed

Fix New-Item -Path with wildcard char #7404

Closed

kwkam changed the title ~~[Feature] Fix file completion with literal wildcard char~~ Fix file completion with literal wildcard char Jul 31, 2018

stale bot added the Stale label Aug 30, 2018

iSazonov reviewed Aug 30, 2018

View reviewed changes

stale bot removed the Stale label Aug 30, 2018

kwkam added 2 commits September 1, 2018 04:51

Do not use alias in TabCompletion test

b8ce02f

Also test the [WildcardPattern]::Escape function

27cd737

iSazonov added Review - Committee The PR/Issue needs a review from the PowerShell Committee Breaking-Change breaking change that may affect users labels Sep 4, 2018

BrucePay self-assigned this Sep 24, 2018

kwkam mentioned this pull request Oct 12, 2018

Backtick escaping inconsistent #7999

Open

rjmholt mentioned this pull request Oct 12, 2018

Fix PowerShell path escaping PowerShell/PowerShellEditorServices#765

Merged

stale bot added the Stale label Oct 24, 2018

stale bot removed the Stale label Oct 25, 2018

stale bot added the Stale label Nov 25, 2018

kwkam changed the title ~~Fix file completion with literal wildcard char~~ Fix tab completion for filename contains literal wildcard character Dec 4, 2018

ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jun 22, 2019

iSazonov reviewed Jun 24, 2019

View reviewed changes

test/powershell/engine/Api/WildcardPattern.Tests.ps1 Show resolved Hide resolved

Insert blank line after header

811b574

Co-Authored-By: Ilya <darpa@yandex.ru>

TravisEz13 reviewed Jun 24, 2019

View reviewed changes

msftrncs mentioned this pull request Jul 28, 2019

WIP: Correct completions for quoting and escaping #10226

Open

13 tasks

kwkam mentioned this pull request Mar 21, 2020

"Path is not parsed correctly if '[' is present in the path" #12170

Closed

14 tasks

ghost added the Review - Needed The PR is being reviewed label May 27, 2020

TravisEz13 added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Review - Needed The PR is being reviewed labels May 27, 2020

Merge branch 'master' into wc-tabcomp

a09ac29

ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jun 4, 2020

PoshChan reviewed Jun 4, 2020

View reviewed changes

src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs Show resolved Hide resolved

TravisEz13 closed this Jun 5, 2020

TravisEz13 reopened this Jun 5, 2020

ghost added the Review - Needed The PR is being reviewed label Jun 13, 2020

TravisEz13 requested changes Oct 27, 2020

View reviewed changes

ghost added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Review - Needed The PR is being reviewed labels Oct 27, 2020

ghost added the Stale label Nov 11, 2020

ghost closed this Nov 22, 2020

ghost removed the Stale label Jun 10, 2021

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix tab completion for filename contains literal wildcard character #7407

Fix tab completion for filename contains literal wildcard character #7407

kwkam commented Jul 30, 2018

iSazonov commented Jul 31, 2018 •

edited

stale bot commented Aug 30, 2018

iSazonov Aug 30, 2018

kwkam Aug 31, 2018

iSazonov Sep 3, 2018

kwkam Sep 3, 2018

iSazonov Aug 30, 2018

kwkam Aug 31, 2018

iSazonov Sep 3, 2018

kwkam Sep 3, 2018

iSazonov Sep 4, 2018

kwkam Sep 4, 2018

iSazonov Sep 4, 2018

iSazonov commented Aug 30, 2018

kwkam commented Aug 31, 2018

SteveL-MSFT commented Sep 5, 2018

stale bot commented Oct 24, 2018

iSazonov commented Oct 25, 2018

SteveL-MSFT commented Oct 26, 2018

stale bot commented Nov 25, 2018

TravisEz13 Jun 24, 2019 •

edited

TravisEz13 May 27, 2020

kwkam Jun 4, 2020

msftrncs commented Jul 28, 2019 •

edited

kwkam commented Dec 7, 2019

ghost commented May 27, 2020

iSazonov commented May 27, 2020

ghost commented Jun 13, 2020

TravisEz13 Oct 27, 2020

TravisEz13 commented Oct 27, 2020

ghost commented Nov 11, 2020

floh96 commented Jun 10, 2021

iSazonov commented Jun 10, 2021

		@@ -0,0 +1,68 @@
		# Copyright (c) Microsoft Corporation. All rights reserved.

	# Copyright (c) Microsoft Corporation. All rights reserved.
	# Copyright (c) Microsoft Corporation.

Navigation Menu

Fix tab completion for filename contains literal wildcard character #7407

Fix tab completion for filename contains literal wildcard character #7407

Conversation

kwkam commented Jul 30, 2018

PR Summary

PR Checklist

iSazonov commented Jul 31, 2018 • edited

stale bot commented Aug 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iSazonov commented Aug 30, 2018

kwkam commented Aug 31, 2018

SteveL-MSFT commented Sep 5, 2018

stale bot commented Oct 24, 2018

iSazonov commented Oct 25, 2018

SteveL-MSFT commented Oct 26, 2018

stale bot commented Nov 25, 2018

TravisEz13 Jun 24, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msftrncs commented Jul 28, 2019 • edited

kwkam commented Dec 7, 2019

ghost commented May 27, 2020

iSazonov commented May 27, 2020

ghost commented Jun 13, 2020

Choose a reason for hiding this comment

TravisEz13 commented Oct 27, 2020

ghost commented Nov 11, 2020

floh96 commented Jun 10, 2021

iSazonov commented Jun 10, 2021

iSazonov commented Jul 31, 2018 •

edited

TravisEz13 Jun 24, 2019 •

edited

msftrncs commented Jul 28, 2019 •

edited