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
Start-Process doesn't pass arguments with embedded whitespace and double quotes correctly #5576
Comments
|
I think it just executes try this code Start-Process -Wait -NoNewWindow pwsh.exe -ArgumentList '-noprofile', '-file', '"./t 1.ps1"'
Start-Process -Wait -NoNewWindow pwsh.exe -ArgumentList '-noprofile', '-file', '"', './t 1.ps1', '"' |
|
The code where it deconstructs |
|
I see the feature is documented:
I don't understand how we can fix this for all platforms and for an indefinite number of applications. If there is no standard to which we should follow I would rather expect a common way of passing arguments so that they reach the goal application in exactly the user specified form. |
|
Well, I guess we can only improve it (see my PR 5703) or maybe the better solution is to add an |
|
@bergmeister: Thanks for taking this on in your PR, but there are additional edge cases to consider, such as tokens ending in Ultimately, the single string passed to Longer-term, once https://github.com/dotnet/corefx/issues/23592 is implemented in CoreFx, PowerShell will simply be able to pass the array through. The linked issue also points to a utility method in the CoreFx code where the kind of array-to-command-line transformation that would be needed here is used internally. And, yes, this will break things, inevitably and in multiple scenarios, but I think if PowerShell wants to be taken seriously as a multi-platform shell, there's no way around that. While we may consider adding a new |
|
@mklement0 As far as I understand the shortest way to get this is to implement https://github.com/dotnet/corefx/issues/23592, isn't it? |
|
@iSazonov: Yes, with said proposal implemented, PowerShell could just use the new |
|
Ok. Shall I close the PR then or do we want to consider it as an intermediate improvement? |
|
I think it is not critical and not a secure hole so we can wait and it is better to direct our efforts to other areas or contribute directly in CoreFX now. |
|
On a related note, it seems to truncate at the (Windows PS 7.0.0-preview3) read the reply. ho boy... this is bad |
|
@Artoria2e5: The problem isn't That is, your |
|
This is very annoying when wrapping Linux command line tools. Could this be considered for PowerShell 7? It seems like the fix would be simple (just use |
Having used I'd second the recommendation to switch to ArgumentList if we can. |
|
I have a PR to fix this, just need to add tests. In the second example: Start-Process -Wait -NoNewWindow pwsh -ArgumentList '-noprofile', '-command', '"Hi!"'This becomes: Start-Process -Wait -NoNewWindow pwsh -ArgumentList '-noprofile', '-command', '"""Hi!"""' |
|
Glad to see that this is getting tackled.
As a shell command / Windows command line, What On Windows: On Unix, the array of verbatim tokens needs to be:
The collection-based |
|
While I definitely welcome a fix for this, note that fixing I like the idea of renaming (aliasing) to As for naming the new parameter
One more thing we could do to avoid confusion, to complement the renaming to
public class StringArrayToScalarTransformationAttribute : ArgumentTransformationAttribute {
public override object Transform(EngineIntrinsics engineIntrinsics, object inputData) {
return inputData switch {
Array a => string.Join(' ', (object[]) inputData),
object o when o is IEnumerable && !(o is IDictionary) => throw new ArgumentException("Input type not supported."),
_ => inputData
};
}
} |
|
I think it is better to avoid unneeded complicity - best is the enemy of good.
With IntelliSense and tab-completion it is minor.
Yes, PowerShell users do not see and don't think about underlying .NET API's. For developers we add docs and comments too. If we blog post and enhance PSSA to recommend the new parameter I believe user adaptation will be easy. |
|
/cc @SteveL-MSFT @daxian-dbw for PowerShell Committee review. |
What unneeded complexity? If you're referring to re-typing
We want go give power users an official short alias, irrespective of tab-completion, just like we do with
Docs, blog posts, and comments are only part of the puzzle: the parameter names themselves shouldn't be counterintuitive: We can't fix the
|
|
With tab completion |
|
If your concern is that
Note that the Here's a quick proof of concept: Add-Type @'
using System;
using System.Management.Automation;
[Cmdlet("Start", "Process", DefaultParameterSetName = "ArgString")]
public class StartProcessCommand : PSCmdlet {
[Parameter(Position = 0)]
public string FilePath { get; set; }
[Parameter(ParameterSetName = "ArgArray", Position = 1)]
[Alias("Arga")]
public string[] ArgumentArray { get; set; }
[Parameter(ParameterSetName = "ArgString", Position = 1)]
[Alias("Args", "ArgumentList")]
// string-array-to-string-scalar transformation attribute would go here.
public string ArgumentString { get; set; }
protected override void ProcessRecord() {
WriteObject(ParameterSetName);
}
}
'@ -PassThru | % Assembly | Import-ModuleTyping That is, we would correctly prioritize the new parameter. Sadly, the positional use of the arguments parameter must continue to default to |
|
@PowerShell/powershell-committee need to review a new proposed parameter to accept an array of args passed as an array of args |
|
@PowerShell/powershell-committee reviewed this, we would prefer to have a switch rather than a new parameter that changes the behavior to |
|
New behavior is a preferred behavior so the new switch looks like an extra parameter in scripts. |
|
The path through a new parameter rather than a switch seems clearer to me:
|
|
I also think that a new switch is the wrong way to go. Note that the feared name confusion could be minimized based on the above proposal: rename With the renamed parameter, the syntax diagram would then only show |
|
The new parameter thing sounda good to me. Adding it to my PR to reduce the scope of that experimental switch. |
|
In my scripts I use The only issue I have is that |
To get direct results, do not use Your command is an example of a workaround, which shouldn't be necessary: Leaving the extra layer of Start-Process regsvr32.exe -ArgumentList '"C:\Some Path With Spaces\file.ocx"', '/s'This embedded double-quoting shouldn't be necessary, because Start-Process regsvr32.exe -ArgumentList 'C:\Some Path With Spaces\file.ocx', '/s'That is, all you should have to focus on is to satisfy PowerShell's own syntax requirements. If we changed the behavior of |
|
There are ways:
Anyway I am for a break. If this break where already 3 years in past, probably nobody would complain now. Thanks for you tip, but I need to elevate the process. Would be good if return value would work if |
Note: Start-Process requires that each item in the -ArgumentList must be double-quoted if there are spaces, or else they be treated as individual arguments. See: PowerShell/PowerShell#5576 Hence it does becomes a bit clumsy to build `-ArgumentList`. And it means that if each item's value has spaces and also has a double quote, the double quote must be escaped to be wrapped by outer double-quotes.
- on Server 2016, $pwdList needs to be cast to `[char []]`, or else it'll join as integers - remove double quotes and single quotes as Start-Process cannot handle double-quotes see PowerShell/PowerShell#5576
|
The underlying issue here has been improved somewhat in Powershell 7.2 by the experimental Unfortunately it does not affect It's a real shame that output: Given that, it seems an |
|
confirmed I tried 20 variations of this. it simply starts power shell and then closes instantly the ps script which runs just fine using the powershell ide thing. but it's useless if I can't call it from a program to create a new powershell with UI and then start the script and the jar is found on betacraft.uk but is reproducible with any java application with a UI |
|
also calling it straight from java with powershell.exe /c needing to specify which exe to process the command the rest works in the powershell UI but not from java. if you were able to get the same characters from cmd you could also reproduce. powershell is broken for the start-process and maybe in general ProcessBuilder pb = new ProcessBuilder(new String[]
{
"powershell",
"/c",
"start-process",
"powershell",
"-ArgumentList",
"'-File',",
"'\"C:/Users/jredfox/Desktop/spacing test.ps1\"',",
"'-ExecutionPolicy',",
"'Bypass'",
"-NoNewWindow"
}).inheritIO();
Process p = pb.start();
while(p.isAlive());
for(String s : pb.command())
System.out.print(s + " "); |
|
@jredfox The backstory here is that Windows itself has absolutely insane command-line handling. See this classic 2011 blog, which still describes the state of the art for the Win32 API: https://docs.microsoft.com/en-us/archive/blogs/twistylittlepassagesallalike/everyone-quotes-command-line-arguments-the-wrong-way This makes it very hard for Powershell to do the right thing in a general purpose way, so they just punted on the problem entirely, like the underlying .NET process creation calls did. The pwsh 7.2 experimental feature I linked to above improves this for "bare" command invocation and for the "&" operator, but has no effect on |

Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.

Updated later to include the problem with embedded double quotes.
Steps to reproduce
Embedded whitespace:
Embedded double quotes:
Expected behavior
In both cases:
That is, script file
./t 1.ps1should execute, and double-quoted string literal"Hi!"should print.Actual behavior
Embedded whitespace:
Invocation fails, because the
./t 1.ps1is passed as two arguments:The only way to make this currently work is to embed (potentially escaped) double quotes:
'"./t 1.ps1"'or"`"./t 1.ps1`""; e.g.:Update: Overall, the best workaround is to pass a single string containing all arguments to
-ArgumentListand use embedded quoting and escaping as necessary:Embedded double quotes:
The embedded double quotes are unexpectedly removed.
The only way to make this currently work is to
\-escape the embedded double quotes:Update: Again, the best workaround is to use a single string:
Environment data
The text was updated successfully, but these errors were encountered: