Skip to content
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

tab completion of arguments (file paths, variable names, etc) fails to escape some special characters #9881

Closed
msftrncs opened this issue Jun 13, 2019 · 23 comments · May be fixed by #10226
Closed
Labels
Issue-Question ideally support can be provided via other mechanisms, but sometimes folks do open an issue to get a Resolution-No Activity Issue has had no activity for 6 months or more WG-Engine-Providers built-in PowerShell providers such as FileSystem, Certificates, Registry, etc.

Comments

@msftrncs
Copy link
Contributor

msftrncs commented Jun 13, 2019

EDIT: 2019-06-25 - Additional conditions and clarifications, corrected expected behavior as requiring the curly quotes to be doubled, added second example demonstrating the tab completion when the first character of the file name is not a quote character, added third example for curly double quotes

I am not sure what is really responsible for tab completion of path names in PowerShell, but here is what I found while testing a similar function in posh-git.

Steps to reproduce

Example 1

echo hello >"`u{2018}hello`u{2019}.txt"

get-content   # press tab until file `.\‘hello’.txt` is shown

Example 2

echo hello >"hello`u{2019}.txt"

get-content   # press tab until file `'.\hello’.txt'` is shown

Example 3

echo hello >"hello`u{201C}.txt"

get-content  "hello" # press tab until file `".\hello“.txt"` is shown

Expected behavior

Example 1

get-content '.\‘‘hello’’.txt'

Example 2

get-content '.\hello’’.txt'

Example 3

get-content ".\hello““.txt"

Actual behavior

Example 1

get-content .\hello.txt

This generates an error saying the file hello.txt cannot be found. Note this particular behavior is regardless of the type of quote used, as long as the quote is a legal filename character and is at the start of the file name and is a special character for PowerShell.

Example 2

get-content '.\hello.txt'

This will cause PSReadLine to want to continue a multiline input because the quoted string is still open.

Example 3

get-content ".\hello.txt"

This will cause PSReadLine to want to continue a multiline input because the quoted string is still open.

Environment data

Name                           Value
----                           -----
PSVersion                      6.2.0
PSEdition                      Core
GitCommitId                    6.2.0
OS                             Microsoft Windows 10.0.18912
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

In dahlbyk/posh-git#683 (comment) @rkeithhill commented that it would be nice if PowerShell has a static method for escaping arguments, and it was that comment that triggered me to test this possibility. I have not tried to test all characters that could be special but found in file names to see if any more are missing escaping.

Another issue with tab completion, server/share names with spaces, #7569, but this only affects the path while it is just '\server\share with a space'...

@msftrncs msftrncs added the Issue-Question ideally support can be provided via other mechanisms, but sometimes folks do open an issue to get a label Jun 13, 2019
@iSazonov
Copy link
Collaborator

/cc @kwkam

@iSazonov iSazonov added the WG-Engine-Providers built-in PowerShell providers such as FileSystem, Certificates, Registry, etc. label Jun 13, 2019
@kwkam
Copy link
Contributor

kwkam commented Jun 22, 2019

@iSazonov It looks like the parser also treats non-ascii quote symbols as quoting characters. Is this and expected behaviour in PowerShell? or should the quoting be limited to the ascii ' and " only?

@vexx32
Copy link
Collaborator

vexx32 commented Jun 22, 2019

It's by design. The behaviour was introduced to minimise annoyances when copying code via some other application that may want to use smart quotes for whatever reason.

@iSazonov
Copy link
Collaborator

If we look the error output:

Get-Content C:\temp\hello.txt
Get-Content : Cannot find path 'C:\temp\hello.txt' because it does not exist.
At line:1 char:1
+ Get-Content C:\temp\hello.txt
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo          : ObjectNotFound: (C:\temp\hello.txt:String) [Get-Content], ItemNotFoundException
+ FullyQualifiedErrorId : PathNotFound,Microsoft.PowerShell.Commands.GetContentCommand

we can see that PowerShell eats the quote chars - Cannot find path 'C:\temp\hello.txt' - and search exactly another name hello.txt.
It look as bug in globbing.

If the chars worked as quotes follow would works but do not:

Get-Content .\hello.txt
Get-Content : Cannot find path 'C:\temp\hello.txt' because it does not exist.
At line:1 char:1
+ Get-Content .\hello.txt
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo          : ObjectNotFound: (C:\temp\hello.txt:String) [Get-Content], ItemNotFoundException
+ FullyQualifiedErrorId : PathNotFound,Microsoft.PowerShell.Commands.GetContentCommand

and again we see that PowerShell search C:\temp\hello.txt.

If we use another Unicode chars this works as expected:

echo hello >"`u{2020}hello`u{2021}.txt"
Get-Content .\†hello‡.txt

hello

@msftrncs
Copy link
Contributor Author

This issue is about the tab completer, not the parser. The parser is working as it should. The tab completer failed to escape the special characters in the path when it was accepted for completion of an argument.

@iSazonov
Copy link
Collaborator

The same for single quotes:

Get-Content .\'hello'.txt
Get-Content : Cannot find path 'C:\temp\hello.txt' because it does not exist.
At line:1 char:1
+ Get-Content .\'hello'.txt

@iSazonov
Copy link
Collaborator

I think the issue is in CompletionRequiresQuotes()

char[] charToCheck = escape ? new[] { '$', '[', ']', '`' } : new[] { '$', '`' };

@kwkam
Copy link
Contributor

kwkam commented Jun 24, 2019

@msftrncs That's interesting, would you please create a file, for example, test.ps1

Write-Host -NoNewline 'Without quote: ['
Write-Host -NoNewline hello.txt
Write-Host ']'

Write-Host -NoNewline 'Single quoted: ['
Write-Host -NoNewline 'hello.txt'
Write-Host ']'

Write-Host -NoNewline 'Double quoted: ['
Write-Host -NoNewline "‘hello’.txt"
Write-Host ']'

and execute it to see if you get the output as below?

Without quote: []
Single quoted: [ hello.txt]
Double quoted: [‘hello’.txt]

@msftrncs
Copy link
Contributor Author

The result is as expected:

Without quote: []
Single quoted: [ hello.txt]
Double quoted: [‘hello’.txt]

To show context (VS Code, using PowerShell/EditorSyntax#156) :
image

image

The fancy quotes (`u{2018}-`u{201B}) will require doubling them to escape them if they are to be contained in a single quoted string. This is the same as a single ' when used in a file name.

@msftrncs
Copy link
Contributor Author

I tested most of the rest of the potentially special characters with tab completion, and the only ones that seem to not be properly handled are `u{2018}-`u{201B} (curly single quotes). The curly double quotes appear to get handled correctly here.

@iSazonov
Copy link
Collaborator

If somebody want to fix this see my code and PR references above.

@msftrncs
Copy link
Contributor Author

@iSazonov, I might think this line(s) could cause problems:

if (quoteInUse == "'")
{
name = name.Replace("'", "''");
}
else

There are approximately 16 of these such constructs in this file.

Also, I misstated the problem earlier when I said that only the single quotes seem affected. After realizing the issue, I can also demonstrate failure of curly double quotes. Thus the following code is also probably short of what it needs to be.

// When double quote is in use, we have to escape the backtip and '$' even when using literal path
// Get-Content -LiteralPath ".\a``g.txt"
completionText = completionText.Replace("`", "``");
completionText = completionText.Replace("$", "`$");
}

To demonstrate, generate a file name with:

echo hello > "hello`u{201c}there.txt"

Then try to tab complete it while encapsulating the starting file name in doublequotes:

gc "hello<TAB>

Result is:

gc ".\hello“there.txt"

@iSazonov
Copy link
Collaborator

@msftrncs I do some unifications in #9992. See IsSingleQuote() and IsDoubleQuote(). We need review all places where we call the methods.

@kwkam
Copy link
Contributor

kwkam commented Jun 25, 2019

@msftrncs If it is expected that the curly single quotes are behaving as quoting character, please change the Expected behaviour to, for example,

get-content '.\‘‘hello’’.txt'

so others will not get confused.

@msftrncs
Copy link
Contributor Author

Interesting, @kwkam, that was my fault that I did not double quote the expected sample, as I was caught up on the fact that it didn't even bother to wrap the whole argument in quotes. However, a further issue is that sometimes it does wrap the argument in quotes and some times it does not, and now I will have to figure out that pattern. I think it was because I used a special quote as the first character of the file name, and I think I have this confirmed now as well, so I will revise the OP to demonstrate both scenario's.

@msftrncs msftrncs changed the title tab completion fails to escape some special characters tab completion of file paths fails to escape some special characters Jul 1, 2019
@msftrncs
Copy link
Contributor Author

https://gist.github.com/msftrncs/bff8c6c5e28ff92a19efb8a5556a4238

I mention this gist here as it demonstrates a PS filter that is able to handle the basic escaping of an argument for use by tab completion.

@msftrncs msftrncs changed the title tab completion of file paths fails to escape some special characters tab completion of arguments (file paths, variable names, etc) fails to escape some special characters Jul 21, 2019
@msftrncs
Copy link
Contributor Author

I've changed the title of this issue as I have determined its more widespread of a problem. It would appear that all argument type completers fail to correctly escape arguments with characters such as curly quotes ([`u{2018}-`u{201E}]), but also, because each completer seems to have its own escaping logic results can be inconsistent between different completions.

Additional points:

  • path completion of unc paths with spaces fails to quote resulting path #7569 is an example where the file path argument completer completely fails to escape/quote paths when providing a list of UNC server share names.
  • I have since found that the variable name argument completer fails to escape/quote variable names that start with a $ or $$ but possess no other characters needing quoted. The only variable in that category that doesn't need quoted is $ itself (but must be prevented from being followed by either ( or {). (Technically, a $ only needs to be escaped (and thus quoted) if its followed by a valid variable character, a { or a ()
  • I believe I have determined, that on 'nix OS, the file path completer (when not LiteralPath) does not escape * and ? characters (briefly referenced in Bug fixes needed to support interchangeable use of "/" (forward slash) and "\" (backslash) in file paths on Unix-like platforms #9244 by @mklement0) (though [ and ] are), probably because those are otherwise invalid file name characters on Windows, but they are escaped by other completers. Notably, this causes a problem when a variable: drive is used with a CMDLET such as Get-Content, because this is completed through the file path argument completer and not the variable name argument completer.

I am continuing to work through understanding the completers in depth.

@msftrncs
Copy link
Contributor Author

I have an intention to post a PR regarding at least parts of this issue, implementing the escapers already found in the CodeGeneration class, and we'll see how far I can go from there.

@iSazonov
Copy link
Collaborator

@msftrncs Before pull the PR please review tests. If you find any gaps in the tests we need to pull new tests before to exclude regressions.

msftrncs added a commit to msftrncs/PowerShell that referenced this issue Jul 24, 2019
msftrncs added a commit to msftrncs/PowerShell that referenced this issue Jul 25, 2019
Enhance all argument completers to use QuoteArgument code generator to
insure consistent quoting and escaping by all argument completion
completers.

Fixes PowerShell#9881
Fixes PowerShell#7569
msftrncs added a commit to msftrncs/PowerShell that referenced this issue Oct 24, 2019
Enhance all argument completers to use QuoteArgument code generator to
insure consistent quoting and escaping by all argument completion
completers.

Fixes PowerShell#9881
Fixes PowerShell#7569
msftrncs added a commit to msftrncs/PowerShell that referenced this issue Dec 2, 2019
Enhance all argument completers to use QuoteArgument code generator to
insure consistent quoting and escaping by all argument completion
completers.

Fixes PowerShell#9881
Fixes PowerShell#7569
Copy link
Contributor

This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.

2 similar comments
Copy link
Contributor

This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.

Copy link
Contributor

This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.

@microsoft-github-policy-service microsoft-github-policy-service bot added Resolution-No Activity Issue has had no activity for 6 months or more labels Nov 16, 2023
Copy link
Contributor

This issue has been marked as "No Activity" as there has been no activity for 6 months. It has been closed for housekeeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Question ideally support can be provided via other mechanisms, but sometimes folks do open an issue to get a Resolution-No Activity Issue has had no activity for 6 months or more WG-Engine-Providers built-in PowerShell providers such as FileSystem, Certificates, Registry, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants