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

Set-Content should not allow writing to multiple files using wildcards #6729

Closed
mklement0 opened this issue Apr 25, 2018 · 10 comments
Closed
Labels
Breaking-Change breaking change that may affect users Committee-Reviewed PS-Committee has reviewed this and made a decision Issue-Bug Issue has been identified as a bug in the product Resolution-By Design The reported behavior is by design. WG-Cmdlets-Core cmdlets in the Microsoft.PowerShell.Core module

Comments

@mklement0
Copy link
Contributor

mklement0 commented Apr 25, 2018

Follow-up from #6714

Currently, Set-Content writes to whatever existing files match a wildcard pattern passed to its (implied) -Path parameter.

For instance, the following will overwrite all existing *.txt files in the current directory:

'hi' | Set-Content *.txt # !! Overwrites ALL *.txt files

This behavior is risky and should be changed to match the more sensible behavior of other output-to-file cmdlets such as Out-File (and therefore the > / >> operators), which generate an error if the wildcard expression resolves to more than one file.

It is worth examining all output-to-file cmdlets on this occasion.

Technically, this is a breaking change, but presumably one that falls into Bucket 3: Unlikely Grey Area.

Environment data

PowerShell Core v6.0.2 on macOS 10.13.4
PowerShell Core v6.0.2 on Ubuntu 16.04.4 LTS
PowerShell Core v6.0.2 on Microsoft Windows 10 Pro (64-bit; Version 1709, OS Build: 16299.371)
Windows PowerShell v5.1.16299.251 on Microsoft Windows 10 Pro (64-bit; Version 1709, OS Build: 16299.371)
@BrucePay
Copy link
Collaborator

Assuming they knew about it, I easily can see people using this to (re)initialize a set of file so I don't think it's really a bucket 3 change. Consider that

'hi' | Set-Content *.txt

is certainly much pithier than

foreach ($f in dir *.txt) { "hi" | set-content $f } 

@mklement0
Copy link
Contributor Author

mklement0 commented Apr 25, 2018

Yes, as with any breaking change that takes away functionality there is a risk that people have relied on it.

My guess is that, given that the behavior is (a) surprising to begin with and (b) is not present in Out-File / >, it's unlikely, but it's just a guess.

As for the pithiness; not quite as concise as the original, but a reasonable substitute, in my opinion, given the benefits of taking away the risky behavior:

'hi' | Set-Content (Get-Item *.txt)

Note that, due to #6057, (Get-Item *.txt).PSPath should currently be used for full robustness.

@BrucePay BrucePay added Issue-Bug Issue has been identified as a bug in the product WG-Cmdlets-Core cmdlets in the Microsoft.PowerShell.Core module labels Apr 26, 2018
@BrucePay
Copy link
Collaborator

Based on

'hi' | Set-Content (Get-Item *.txt)

your issue is not what I thought it was. It appears that you still want Set-Content to be able to write identical content to multiple files. Is this the case?

@mklement0
Copy link
Contributor Author

It appears that you still want Set-Content to be able to write identical content to multiple files. Is this the case?

Yes, if you pass an array of literal file paths to -Path / -LiteralPath, which to me unambiguously signals the intent.

No change need there, because these parameters are already typed [string[]].

By contrast, the wildcard-based way of targeting multiple file is the fraught behavior, which I think should be eliminated.

I used the 'hi' | Set-Content (Get-Item *.txt) example to demonstrate that someone who truly wants to target multiple files will still be able to do so, even once the wildcard-based method is taken away.

Now you could ask what should happen if an array is passed to -Path (possibly implicitly) and at least one element of that array is a wildcard expression?

For simplicity, my vote is for disallowing any wildcard expression that matches more than 1 file, whether stand-alone or as part of an array.

To summarize, the desired behavior is:

  • You can pass one or more literal paths to -LiteralPath.

  • You can pass one or more literal paths or wildcard patterns - in any combination - to -Path, as long as no single wildcard pattern matches more than 1 file.

@vexx32
Copy link
Collaborator

vexx32 commented Apr 26, 2018

This seems like the sort of thing that should be the default, but able to be worked around. You've provided one workaround, but consider perhaps that we could also allow the -Force parameter to bypass this restriction. In other words:

# This doesn't work:
"blank" | Set-Content -Path "*.txt"

# But this does
"blank" | Set-Content -Path "*.txt" -Force

@mklement0
Copy link
Contributor Author

mklement0 commented Apr 26, 2018

@vexx32:

Having the ability be opt-in is definitely preferable, but note that we'd be adding meaning to the existing use of -Force ("Forces the cmdlet to set the contents of a file, even if the file is read-only.")

Also, you could argue that we'd have to implement the same thing for all output-to-file cmdlets (Out-File, Export-Csv, ...)

On the other hand, as I've just realized, Set-Content already is special in that at least at first glance it appears to be the only one that supports an array of output filenames.

@iSazonov
Copy link
Collaborator

iSazonov commented Dec 1, 2021

I think we could remove the dangerous behavior. In File System Provider V2 at least. And follow to one principle for all cmdlet - target path parameter should always be LiteralPath. (We already did this for web cmdlets and Out-File is under question too #9475.)

@iSazonov iSazonov added the Review - Committee The PR/Issue needs a review from the PowerShell Committee label Dec 1, 2021
@SteveL-MSFT
Copy link
Member

@PowerShell/powershell-committee looked at this and we compared it to behavior in zsh and bash. It seems that zsh does support redirection to multiple files matching the wildcard while bash complains that the destination is ambiguous. Also given that there seems to be legitimate cases where you want to initialize multiple files to the same content, then we think this is not a bucket 3 breaking change. Also, if -WhatIf is used with Set-Content it does inform you of what was matched and would be written. As noted, Out-File restricts to matching a single file so we have a bit of inconsistency between Out-File and Set-Content, but those two cmdlets don't have to be identical otherwise they'd be a single cmdlet. We believe this is By Design

@SteveL-MSFT SteveL-MSFT added Committee-Reviewed PS-Committee has reviewed this and made a decision Resolution-By Design The reported behavior is by design. and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Dec 1, 2021
@iSazonov

This comment has been minimized.

@ghost
Copy link

ghost commented Dec 3, 2021

This issue has been marked as by-design and has not had any activity for 1 day. It has been closed for housekeeping purposes.

@ghost ghost closed this as completed Dec 3, 2021
RossPatterson added a commit to RossPatterson/grammars-v4 that referenced this issue Sep 24, 2022
…rror.

Error message: Out-File: Cannot perform operation because the wildcard path D:\a\grammars-v4\grammars-v4\molecule\examples\(NH4)2[Pt(SCN)6].txt.tree.out did not resolve to a file.

Crazily, this is intentional, despite the fact that Out-File (and it's synonym, ">") can only generate a single file.  See ttps://github.com/PowerShell/PowerShell/issues/6729#issuecomment-984165303.
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking-Change breaking change that may affect users Committee-Reviewed PS-Committee has reviewed this and made a decision Issue-Bug Issue has been identified as a bug in the product Resolution-By Design The reported behavior is by design. WG-Cmdlets-Core cmdlets in the Microsoft.PowerShell.Core module
Projects
None yet
Development

No branches or pull requests

5 participants