X Tutup
The Wayback Machine - https://web.archive.org/web/20220611192057/https://github.com/PowerShell/PowerShell/issues/5576
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

Start-Process doesn't pass arguments with embedded whitespace and double quotes correctly #5576

Open
mklement0 opened this issue Nov 29, 2017 · 44 comments
Labels
Committee-Reviewed Issue-Discussion WG-Cmdlets-Management
Projects
Milestone

Comments

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Nov 29, 2017

Updated later to include the problem with embedded double quotes.

Steps to reproduce

Embedded whitespace:

'"Hi!"' > './t 1.ps1'; Start-Process -Wait -NoNewWindow pwsh -ArgumentList '-noprofile', '-file', './t 1.ps1'

Embedded double quotes:

Start-Process -Wait -NoNewWindow pwsh -ArgumentList '-noprofile', '-command', '"Hi!"'

Expected behavior

In both cases:

Hi!

That is, script file ./t 1.ps1 should execute, and double-quoted string literal "Hi!" should print.

Actual behavior

Embedded whitespace:

Invocation fails, because the ./t 1.ps1 is passed as two arguments:

The argument './t' is not recognized as the name of a script file. Check the spelling of the name, or if a path was included, verify that the path is
correct and try again.

The only way to make this currently work is to embed (potentially escaped) double quotes: '"./t 1.ps1"' or "`"./t 1.ps1`""; e.g.:

Start-Process -Wait -NoNewWindow pwsh -ArgumentList '-noprofile', '-file', "`"./t 1.ps1`""

Update: Overall, the best workaround is to pass a single string containing all arguments to -ArgumentList and use embedded quoting and escaping as necessary:

Start-Process -Wait -NoNewWindow pwsh -ArgumentList '-noprofile -file "./t 1.ps1"'

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:

Start-Process -Wait -NoNewWindow pwsh -ArgumentList '-noprofile', '-command', '\"Hi!\"'

Update: Again, the best workaround is to use a single string:

Start-Process -Wait -NoNewWindow pwsh -ArgumentList '-noprofile -command \"Hi!\"'

Environment data

PowerShell Core v6.0.0-rc (v6.0.0-rc) on Microsoft Windows 7 Enterprise  (64-bit; v6.1.7601)
Windows PowerShell v5.1.14409.1012 on Microsoft Windows 7 Enterprise  (64-bit; v6.1.7601)
@ZSkycat
Copy link

@ZSkycat ZSkycat commented Nov 29, 2017

I think it just executes {filename} {ArgumentList.join(' ')}

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', '"'

@iSazonov iSazonov added WG-Cmdlets-Management Issue-Discussion labels Dec 1, 2017
@SteveL-MSFT SteveL-MSFT added the Up-for-Grabs label Dec 12, 2017
@bergmeister
Copy link
Contributor

@bergmeister bergmeister commented Dec 15, 2017

The code where it deconstructs -Argumentlist to the Argument string for ProcessStartInfo.Arguments here and it simply joins the arguments by separating it with a space character. Therefore it sees ./t and 1.ps1 as two separate arguments. As pointed out by @ZSkycat one has to correctly quote the file.

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Dec 16, 2017

I see the feature is documented:

Arguments are parsed and interpreted by the target application, so must align with the expectations of that application. For.NET applications as demonstrated in the Examples below, spaces are interpreted as a separator between multiple arguments. A single argument that includes spaces must be surrounded by quotation marks, but those quotation marks are not carried through to the target application. In include quotation marks in the final parsed argument, triple-escape each mark.

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.

@bergmeister
Copy link
Contributor

@bergmeister bergmeister commented Dec 16, 2017

Well, I guess we can only improve it (see my PR 5703) or maybe the better solution is to add an -Arguments parameter that is just a string to reduce complexity and leave the -ArgumentList parameter there only for legacy reasons.

@mklement0
Copy link
Contributor Author

@mklement0 mklement0 commented Dec 17, 2017

@bergmeister: Thanks for taking this on in your PR, but there are additional edge cases to consider, such as tokens ending in \ - see #1995 (comment)

Ultimately, the single string passed to ProcessStartInfo.Arguments should be constructed the same way that a direct call to an external program is handled, namely based on inverse of the MSVC++ command-line parsing rules, as discussed in @TSlivede's RFC proposal.

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 -Arguments parameter for someone who wants to pass a single-string, pre-escaped command line (directly assignable to ProcessStartInfo.Arguments - which, notably, still needs to be split back into an array before creating a process on Unix), it is -ArgumentList that must be fixed - which breaks things.

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Dec 18, 2017

@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?

@mklement0
Copy link
Contributor Author

@mklement0 mklement0 commented Dec 18, 2017

@iSazonov: Yes, with said proposal implemented, PowerShell could just use the new ProcessStartInfo.ArgumentList property directly and, on Windows, let CoreFX translate that into a single command line string with appropriate quoting.

@bergmeister
Copy link
Contributor

@bergmeister bergmeister commented Dec 18, 2017

Ok. Shall I close the PR then or do we want to consider it as an intermediate improvement?

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Dec 18, 2017

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.

@Artoria2e5
Copy link

@Artoria2e5 Artoria2e5 commented Sep 18, 2019

On a related note, it seems to truncate at the > character too:

PS> Start-Process -Wait -NoNewWindow node -ArgumentList '-e', 'process.argv.slice(1).forEach((x) => console.log(x))'
[eval]:1
process.argv.slice(1).forEach((x)
                                ^
SyntaxError: missing ) after argument list

(Windows PS 7.0.0-preview3)


read the reply. ho boy... this is bad

@mklement0
Copy link
Contributor Author

@mklement0 mklement0 commented Sep 18, 2019

@Artoria2e5: The problem isn't >, the problem is that process.argv.slice(1).forEach((x) => console.log(x)) isn't passed through as a single argument, it is broken into 3 arguments by whitespace.

That is, your node command is effectively executed as follows, which explains the symptom:

node -e 'process.argv.slice(1).forEach((x)' '=>' 'console.log(x))'

@felixfbecker
Copy link
Contributor

@felixfbecker felixfbecker commented Nov 8, 2019

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 ProcessStartInfo.ArgumentList). Using System.Diagnostics.Process in PowerShell directly works as expected, but is incredibly cumbersome.

@SteveL-MSFT SteveL-MSFT removed Up-for-Grabs Waiting - DotNetCore labels Nov 8, 2019
@SteveL-MSFT SteveL-MSFT self-assigned this Nov 8, 2019
@rjmholt
Copy link
Collaborator

@rjmholt rjmholt commented Nov 8, 2019

It seems like the fix would be simple (just use ProcessStartInfo.ArgumentList

Having used ProcessStartInfo.Arguments recently (had to support netstandard2.0), it's a very hard API to use.

I'd second the recommendation to switch to ArgumentList if we can.

@SteveL-MSFT
Copy link
Member

@SteveL-MSFT SteveL-MSFT commented Nov 8, 2019

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: pwsh -noprofile -command "Hi!" so it should only print out Hi! without the quotes. The quotes are needed to tell PowerShell this is a string and not a command. After this fix, you still need to have nested double quotes if you want quotes:

Start-Process -Wait -NoNewWindow pwsh -ArgumentList '-noprofile', '-command', '"""Hi!"""'

@mklement0
Copy link
Contributor Author

@mklement0 mklement0 commented Nov 8, 2019

Glad to see that this is getting tackled.

This becomes: pwsh -noprofile -command "Hi!" so it should only print out Hi! without the quotes.

As a shell command / Windows command line, pwsh -noprofile -command "Hi!", fails, because the the enclosing " are removed during argument parsing, causing PowerShell to attempt to execute Hi!.

What Start-Process -Wait -NoNewWindow pwsh -ArgumentList '-noprofile', '-command', '"Hi!"' should translate to is:

On Windows:

pwsh -noprofile -command "\"Hi!\""

On Unix, the array of verbatim tokens needs to be:

pwsh, -noprofile, -command, "Hi!"

The collection-based ProcessStartInfo.ArgumentList - not the single-string ProcessStartInfo.Arguments - should do all that for us.

@mklement0
Copy link
Contributor Author

@mklement0 mklement0 commented Jul 30, 2020

While I definitely welcome a fix for this, note that fixing Start-Process is no substitute for fixing #1995, given that Start-Process serves a different purpose than direct invocation, and notably isn't integrated with PowerShell's output streams -
it is #1995 that needs urgent attention; you can work around the Start-Process bug at hand by supplying a single string containing all arguments, with embedded "-quoting.

I like the idea of renaming (aliasing) to ArgumentString.

As for naming the new parameter -Arguments, I see two problems:

  • We also need to think about a short parameter alias that parallels -Args - what would that be for -Arguments?

  • While PowerShell in general needn't mirror the underlying .NET APIs, I find it problematic that the semantics in this case would be the exact opposite of the underlying .NET API's.

-IndividualArguments / -iArgs solves / mitigates these problems. [update: see the tab-completion-friendlier alternative below]


One more thing we could do to avoid confusion, to complement the renaming to -ArgumentString:

  • Change the -ArgumentList parameter type to string, so that the syntax diagram and the documentation can suggest that only a single string with all arguments should be passed.

  • So as not to break backward compatibility, decorate the re-typed parameter with a transformation attribute that stringifies an array argument by joining its elements with spaces - which is effectively what happens behind the scenes at the moment; something along the following lines (in the real implementation, existing internal helper methods and error messages would need to be used):

    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
        };
      }
    }

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Jul 30, 2020

I think it is better to avoid unneeded complicity - best is the enemy of good.

We also need to think about a short parameter alias that parallels -Args - what would that be for -Arguments?

With IntelliSense and tab-completion it is minor.

While PowerShell in general needn't mirror the underlying .NET APIs, I find it problematic that the semantics in this case would be the exact opposite of the underlying .NET API's

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.

@iSazonov iSazonov added the Review - Committee label Jul 30, 2020
@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Jul 30, 2020

/cc @SteveL-MSFT @daxian-dbw for PowerShell Committee review.

@mklement0
Copy link
Contributor Author

@mklement0 mklement0 commented Jul 30, 2020

@iSazonov

better to avoid unneeded complicity

What unneeded complexity? If you're referring to re-typing -ArgumentList to string, there is minor implementation complexity and no added complexity to end users - on the contrary, it makes things clearer for them.

With IntelliSense and tab-completion it is minor.

We want go give power users an official short alias, irrespective of tab-completion, just like we do with -Args for -ArgumentList.

add docs and comments too.

Docs, blog posts, and comments are only part of the puzzle: the parameter names themselves shouldn't be counterintuitive:

We can't fix the -ArgumentList / -Args name anymore (except to de-emphasize -ArgumentList in favor of
-ArgumentString), but we can be more descriptive in the new parameter's name:

-Arguments is ambiguous, -IndividualArguments is not [update: see the tab-completion-friendlier alternative below]

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Jul 30, 2020

With tab completion Arguments is more discoverable and more easy for adoption. Everything else does not make sense until the user reads the parameter description.

@mklement0
Copy link
Contributor Author

@mklement0 mklement0 commented Jul 30, 2020

If your concern is that -IndividualArguments doesn't start with the word "Argument" - a good point - here's another option:

-ArgumentArray / -Arga

Note that the s in the existing -Args alias can then be interpreted as referring to the "String" part of the renamed
-ArgumentString (we could capitalize the final letter for clarity without breaking anyone).

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-Module

Typing Start-Process foo -a<tab> then cycles through the parameters in the following order:
-ArgumentArray, -Arga, -ArgumentString, -Args, and, finally, the de-emphasized original parameter name,
-ArgumentList.

That is, we would correctly prioritize the new parameter.

Sadly, the positional use of the arguments parameter must continue to default to -ArgumentList (-ArgumentString), so as not to break existing code.

@SteveL-MSFT
Copy link
Member

@SteveL-MSFT SteveL-MSFT commented Jul 30, 2020

@PowerShell/powershell-committee need to review a new proposed parameter to accept an array of args passed as an array of args

@TravisEz13 TravisEz13 removed this from the 7.1-Consider milestone Aug 20, 2020
@TravisEz13 TravisEz13 added this to the Future milestone Aug 20, 2020
@SteveL-MSFT
Copy link
Member

@SteveL-MSFT SteveL-MSFT commented Sep 16, 2020

@PowerShell/powershell-committee reviewed this, we would prefer to have a switch rather than a new parameter that changes the behavior to -ArgumentList rather than cause confusion to users between -ArgumentList vs -ArgumentArray. However, we couldn't come up with a good switch name to describe the behavior to the user without being overly verbose so open to any suggestions.

@SteveL-MSFT SteveL-MSFT added Committee-Reviewed and removed Review - Committee labels Sep 16, 2020
@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Sep 17, 2020

New behavior is a preferred behavior so the new switch looks like an extra parameter in scripts.
How are we going to get rid of it after the transition period? Maybe revert the switch logic? For backward compatibility users could add the switch.
Although I would just prefer the new parameter ArgumentArray. I don't think that it will confuse users if we explicitly say in helps that user should use ArgumentArray instead of ArgumentList.

@rjmholt
Copy link
Collaborator

@rjmholt rjmholt commented Sep 17, 2020

The path through a new parameter rather than a switch seems clearer to me:

  • Add a new parameter ArgumentArray (or Arguments, or whatever) with a separate parameter set
  • Positional binding continues to be the old parameter
  • In a later PowerShell version we make the breaking change to make the positional argument refer to the new parameter

@mklement0
Copy link
Contributor Author

@mklement0 mklement0 commented Sep 17, 2020

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 -ArgumentList to
-ArgumentString and make -ArgumentList an alias for it, for programmatic backward compatibility.

With the renamed parameter, the syntax diagram would then only show -ArgumentString, which can be documented as such.

@Artoria2e5
Copy link

@Artoria2e5 Artoria2e5 commented Sep 19, 2020

The new parameter thing sounda good to me. Adding it to my PR to reduce the scope of that experimental switch.

@trivalik
Copy link

@trivalik trivalik commented Mar 30, 2021

In my scripts I use
powershell Start regsvr32.exe -ArgumentList '\"C:\Some Path With Spaces\file.ocx\"','/s'
to preserve quotes. Not sure what here the problem.

The only issue I have is that powershell Start not return values.

@mklement0
Copy link
Contributor Author

@mklement0 mklement0 commented Mar 30, 2021

The only issue I have is that powershell Start not return values.

To get direct results, do not use Start-Process - use direct invocation instead:
regsvr32.exe -ArgumentList 'C:\Some Path With Spaces\file.ocx' /s or, in the context of a CLI call:
powershell -c "regsvr32.exe 'C:\Some Path With Spaces\file.ocx' /s"- see MicrosoftDocs/PowerShell-Docs#6239

Your command is an example of a workaround, which shouldn't be necessary: Leaving the extra layer of " escaping required due to the use of PowerShell's CLI out of the picture, your command amounts to:

Start-Process regsvr32.exe -ArgumentList '"C:\Some Path With Spaces\file.ocx"', '/s'

This embedded double-quoting shouldn't be necessary, because Start-Process should perform all necessary quoting automatically, behind the scenes, when it assembles the single-string argument list for the target process (of necessity, on Windows). That is, you should be able to simply call:

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 -ArgumentList this way, existing workarounds such as yours would break.
Hence the need for a new parameter (and not fixing the current, broken behavior doesn't seem like a satisfactory solution to me).

@trivalik
Copy link

@trivalik trivalik commented Mar 31, 2021

There are ways:

  • use a environment variable to opt out the new behavior
  • add workaround by checking first or in whole string that 2 characters are " then use old behavior

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 -Wait is used.

leojonathanoh added a commit to theohbrothers/Convert-FolderToGitRepo that referenced this issue Aug 15, 2021
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.
lewang added a commit to CircleCI-Public/runner-installation-files that referenced this issue Feb 1, 2022
- 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
@ringerc
Copy link

@ringerc ringerc commented May 16, 2022

The underlying issue here has been improved somewhat in Powershell 7.2 by the experimental PSNativeCommandArgumentPassing feature, which makes the & command preserve quoting. See https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_automatic_variables?view=powershell-7.2#psnativecommandargumentpassing

Unfortunately it does not affect Start-Process, which retains its -ArgumentList string-splatting.

It's a real shame that Start-Process was not updated along with & to respect $PSNativeCommandArgumentPassing. Example:

#!pwsh
#Requires -Version 7.0.0
Set-StrictMode -version 3
$ErrorActionPreference = "Stop"
$VerbosePreference = "Continue"

# Make sure sane command passing is available, force shell restart if not
if (-Not (Get-ExperimentalFeature -Name PSNativeCommandArgumentPassing).Enabled) {
        Write-Error "run `"Enable-ExperimentalFeature -Name PSNativeCommandArgumentPassing`" and retry"
        exit 1
}

# Make "&" preserve quoting (but not unfortunately StartProcess -ArgumentList)
$PSNativeCommandArgumentPassing = "Standard"

# Works but we have to enable an experimental feature first, and it doesn't
# seem to be possible to enable it within the same shell that uses it...
Write-Output "& with PSNativeCommandArgumentPassing=Standard"
&"printf" "%s\n" "arg1" "arg2 with spaces" "arg3 `"with quotes`" in it" "arg4 `${metachars:?shellexec} embedded"

# Still doesn't work. Sigh.
Write-Output "`nStart-Process -ArgumentList"
Start-Process -FilePath "printf" -ArgumentList @("%s\n", "arg1", "arg2 with spaces", "arg3 `"with quotes`" in it", "arg4 `${metachars:?shellexec} embedded") -Wait -Passthru

output:

& with PSNativeCommandArgumentPassing=Standard
arg1
arg2 with spaces
arg3 "with quotes" in it
arg4 ${metachars:?shellexec} embedded

Start-Process -ArgumentList
arg1
arg2
with
spaces
arg3
with quotes
in
it
arg4
${metachars:?shellexec}
embedded

Given that, it seems an -ArgumentVector or similar is still needed.

@jredfox
Copy link

@jredfox jredfox commented May 20, 2022

confirmed I tried 20 variations of this. it simply starts power shell and then closes instantly

start-process powershell -ArgumentList "`"C:\Users\jredfox\Desktop\script s.ps1`""

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

Start-Process java -ArgumentList '-jar', 'C:\Users\jredfox\Desktop\Apps\launcher-1.09_15.jar' -NoNewWindow

and the jar is found on betacraft.uk but is reproducible with any java application with a UI

@jredfox
Copy link

@jredfox jredfox commented May 20, 2022

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 + " ");

@ringerc
Copy link

@ringerc ringerc commented May 23, 2022

@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 Start-Process, so you cannot benefit from it in places where you want the control offered by Start-Process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Committee-Reviewed Issue-Discussion WG-Cmdlets-Management
Projects
Shell
  
To do
X Tutup