Fix New-Item -Path/-Target with wildcard char #9259

kwkam · 2019-03-31T05:28:23Z

PR Summary

Unescape non-literal path when -Name is not set.
Fix failure with symlink/junction target which contains special character.

PR Context

This works around for New-Item treating -Path as literal path while
it can also be globbable. For example, assuming there is "[file]1"
in current directory, and tab completion suggests './`[file`]1'
for the -Path argument, but

New-Item -Path './`[file`]2'

will create a file named "`[file`]2" instead of "[file]2".

Also fixes an issue that New-Item does not handle -Target properly
when it contains escaped wildcard character. For example, there is
a directory "[dir]1" in current directory, and tab completion
suggests './`[dir`]1' for the -Target argument, but

New-Item -Type Junction -Path './`[dir`]2' -Target './`[dir`]1'

will complain that it cannot find the item.

PR Checklist

Unescape non-literal path when -Name is not set. This works around for New-Item treating -Path as literal path while it can also be globbable. For example, assuming there is "[file]1" in current directory, and tab completion suggests './`[file`]1' for the -Path argument, but > New-Item -Path './`[file`]2' will create a file named "`[file`]2" instead of "[file]2".

test/powershell/Modules/Microsoft.PowerShell.Management/New-Item.Tests.ps1

test/powershell/Modules/Microsoft.PowerShell.Management/Remove-Item.Tests.ps1

Co-Authored-By: kwkam <kwkam@users.noreply.github.com>

stale · 2019-05-01T11:47:10Z

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.

SteveL-MSFT · 2019-05-04T20:28:27Z

test/powershell/Modules/Microsoft.PowerShell.Management/New-Item.Tests.ps1

+        $expectedTarget = Join-Path -Path $tmpDirectory -ChildPath $testlinkSrcSpName
+
+        $fileInfo = Get-ChildItem -Path $FullyQualifiedLinkSp
+        $fileInfo.Target | Should -Match ([regex]::Escape($expectedTarget))


Why can't we do an -BeExactly assertion here instead of a -Match?

I guess we can.
Actually this was created using the tests in the same file as reference:

PowerShell/test/powershell/Modules/Microsoft.PowerShell.Management/New-Item.Tests.ps1

Line 217 in 2737e74

$fileInfo.Target | Should -Match ([regex]::Escape($FullyQualifiedFile))

PowerShell/test/powershell/Modules/Microsoft.PowerShell.Management/New-Item.Tests.ps1

Line 242 in 2737e74

$fileInfo.Target | Should -Match ([regex]::Escape($FullyQualifiedFolder))

Perhaps we have to review the New-Item test sometime.

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

adityapatwardhan · 2019-05-10T17:34:29Z

I believe we should not change this behavior. To workaround your problem you could simply use the following:

PS D:\temp\newitem> New-Item -Name `[file`]2


    Directory: D:\temp\newitem

Mode                LastWriteTime         Length Name
----                -------------         ------ ----
-a----         5/10/2019 10:32 AM              0 [file]2

kwkam · 2019-05-11T03:52:24Z

The problem is the inconsistency of -Path behaviour in the *-Item cmdlets.

# to get '[file]1'
Get-Item -Path './`[file`]1'

# to move '[file]1'
Move-Item -Path './`[file`]1' -Destination ./dir

# to copy '[file]1'
Copy-Item -Path './dir/`[file`]1' -Destination ..

# to remove '[file]1'
Remove-Item -Path './dir/`[file`]1'

All above commands behave correctly with tab completion, but when we do

# what if we try to overwrite '[file]1'?
New-Item -Path './`[file`]1' -Value 'This is [file]1'

A new file `[file`]1 is created instead, which is unexpected, and in my opinion, is a flaw in the cmdlet since -Path is supposed to handle wildcard path and escaped path.

Of course, we can use -Name to workaround the issue (because there is no -LiteralPath in New-Item), but I believe -Name is not the proper replacement, as @mklement0 commented in #6232 (comment), -Name is not supposed to be path. Anyway, even if we have the workaround, a bug needs a fix.

ghost · 2019-06-21T12:00:20Z

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 30 days. It will be closed if no further activity occurs within 10 days of this comment.

iSazonov · 2019-06-21T12:19:09Z

@adityapatwardhan Could you please update your review?

adityapatwardhan · 2019-06-24T19:30:02Z

Changing the behavior of Path is a breaking change and hence I would like the committee to review it.

joeyaiello · 2019-07-03T22:22:54Z

@PowerShell/powershell-committee discussed this one.

The Issue at hand is that tab completion uses single quotes and escaped characterss in what amounts to a literal path, and you only need one or the other to work properly. We agreed not to fix it in the New-Item behavior, which would be a significant breaking change, but we should fix it in the tab completion by not adding backticks (because double quotes will break where $ is in the filename).

We'd welcome a PR to fix the tab completion, but given that we're not taking this one, closing it out.

iSazonov · 2019-07-04T12:29:06Z

@kwkam Have you plans to fix the tab completion?

kwkam added 4 commits March 31, 2019 13:09

Add test

38222e8

Use named parameters in New-Item test

97aca06

Cosmetic change on Remove-Item.Tests

80b5673

kwkam requested review from BrucePay and daxian-dbw as code owners March 31, 2019 05:28

iSazonov reviewed Apr 1, 2019

View reviewed changes

Suppress New-Item output in test

e3ee292

Co-Authored-By: kwkam <kwkam@users.noreply.github.com>

stale bot added the Stale label May 1, 2019

SteveL-MSFT assigned adityapatwardhan May 4, 2019

stale bot removed the Stale label May 4, 2019

SteveL-MSFT reviewed May 4, 2019

View reviewed changes

kwkam added 3 commits May 5, 2019 17:00

Use BeExactly instead of Match in test

582f67d

Use Get-Item instead of Get-ChildItem

84a23ed

Fix symlink target

581dd2a

iSazonov reviewed May 7, 2019

View reviewed changes

src/System.Management.Automation/engine/SessionStateContainer.cs Outdated Show resolved Hide resolved

SteveL-MSFT approved these changes May 7, 2019

View reviewed changes

Prepend dot to relative path

bf08aad

iSazonov reviewed May 8, 2019

View reviewed changes

src/System.Management.Automation/engine/SessionStateContainer.cs Outdated Show resolved Hide resolved

Use char in StartsWith

258e2cd

adityapatwardhan added the Review - Waiting on Author label May 10, 2019

kwkam changed the title ~~Fix New-Item -Path with wildcard char~~ Fix New-Item -Path/-Target with wildcard char May 11, 2019

TravisEz13 added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Review - Waiting on Author labels Jun 21, 2019

ghost added the Stale label Jun 21, 2019

ghost removed the Stale label Jun 21, 2019

adityapatwardhan added Review - Committee The PR/Issue needs a review from the PowerShell Committee and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels Jun 24, 2019

adityapatwardhan added the Breaking-Change breaking change that may affect users label Jun 24, 2019

joeyaiello closed this Jul 3, 2019

iSazonov added Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Jul 4, 2019

Fix New-Item -Path/-Target with wildcard char #9259

Fix New-Item -Path/-Target with wildcard char #9259

Uh oh!

Conversation

kwkam commented Mar 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

PR Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stale bot commented May 1, 2019

Uh oh!

SteveL-MSFT May 4, 2019

Choose a reason for hiding this comment

Uh oh!

kwkam May 5, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adityapatwardhan commented May 10, 2019

Uh oh!

kwkam commented May 11, 2019

Uh oh!

ghost commented Jun 21, 2019

Uh oh!

iSazonov commented Jun 21, 2019

Uh oh!

adityapatwardhan commented Jun 24, 2019

Uh oh!

joeyaiello commented Jul 3, 2019

Uh oh!

iSazonov commented Jul 4, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

kwkam commented Mar 31, 2019 •

edited

Loading