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

double quotes in string literal arguments get removed when calling native commands #3049

Closed
vors opened this issue Jan 25, 2017 · 16 comments
Closed
Labels
Issue-Question ideally support can be provided via other mechanisms, but sometimes folks do open an issue to get a OS-Linux OS-macOS Resolution-Duplicate The issue is a duplicate. WG-Engine core PowerShell engine, interpreter, and runtime

Comments

@vors
Copy link
Collaborator

vors commented Jan 25, 2017

Steps to reproduce

On macOS run

osascript -e 'display notification "FOO" with title "TITLE"'

Expected behavior

Show notification, just like in bash

Actual behavior

PS /> osascript -e 'display notification "FOO" with title "TITLE"'                                                             
21:24: execution error: The variable FOO is not defined. (-2753)
PS /> bash                                                                                                                     
bash-3.2$ osascript -e 'display notification "FOO" with title "TITLE"'
bash-3.2$

Environment data

> $PSVersionTable
Name                           Value                                                                                                                   
----                           -----                                                                                                                   
PSVersion                      6.0.0-alpha                                                                                                             
PSEdition                      Core                                                                                                                    
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}                                                                                                 
BuildVersion                   3.0.0.0                                                                                                                 
GitCommitId                    v6.0.0-alpha.14                                                                                                         
CLRVersion                                                                                                                                             
WSManStackVersion              3.0                                                                                                                     
PSRemotingProtocolVersion      2.3                                                                                                                     
SerializationVersion           1.1.0.1  
@vors vors added Issue-Question ideally support can be provided via other mechanisms, but sometimes folks do open an issue to get a OS-macOS labels Jan 25, 2017
@thezim
Copy link
Contributor

thezim commented Jan 27, 2017

The following change does work.

osascript -e 'display notification \"FOO\" with title \"TITLE\"'

Per the man page.

-e statement
       Enter one line of a script.  If -e is given, osascript will not look for a filename in the argument list.
       Multiple -e options may be given to build up a multi-line script.  Because most scripts use characters that
       are special to many shell programs (for example, AppleScript uses single and double quote marks, ``('',
       ``)'', and ``*''), the statement will have to be correctly quoted and escaped to get it past the shell
       intact.

@vors
Copy link
Collaborator Author

vors commented Jan 27, 2017

@thezim good to know, thank you!
I still think there is an issue, because it's super unintitive for me, why I need to escape these quotes, when the whole parameter is already wrapped in the single-quoted string.

Also it related to the pillar of "making PS more compatible with existing shells to allow more commands work thru copy-paste".

@thezim
Copy link
Contributor

thezim commented Jan 27, 2017

@vor I agree. I would prefer your example to work too.

@thezim
Copy link
Contributor

thezim commented Jan 28, 2017

Invoke-AppleScript would be a nice cmdlet for macOS. Following would lead me to believe is could be done.

http://jonathanpeppers.com/Blog/xamarin-ios-under-the-hood-calling-objective-c-from-csharp
https://docs.microsoft.com/en-us/dotnet/articles/standard/native-interop

@thezim
Copy link
Contributor

thezim commented Jan 28, 2017

@vors This works too.

'display notification "FOO" with title "TITLE"' | osascript

@lzybkr lzybkr changed the title apple script doesn't play nicely with powershell double quotes in string literal arguments get removed when calling native commands Jan 28, 2017
@lzybkr
Copy link
Member

lzybkr commented Jan 28, 2017

Repro (using test code in our repo):

PS> cd test/tools/EchoArgs
PS> dotnet run 'a "b" c'
Arg 0 is <a b c>
PS> bash
$ dotnet run 'a "b" c'
Arg 0 is <a "b" c>

@lzybkr
Copy link
Member

lzybkr commented Jan 28, 2017

Note that the behavior is consistent with Windows, but maybe it shouldn't be.

@DemiMarie
Copy link

Why is it this way on Windows? If I recall correctly, this behavior was a major annoyance.

@TSlivede
Copy link

TSlivede commented Feb 8, 2017

Correct me if I'm wrong, but isn't this a duplicate of #1995 ?

@lzybkr I think it absolutely must be consistent with Windows, but double escaping should not be necessary on any platform.

@HemantMahawar HemantMahawar added the WG-Engine core PowerShell engine, interpreter, and runtime label Feb 15, 2017
@BrucePay
Copy link
Collaborator

@vors Note that in the workaround on macOS, the double quotes are being escaped with ''. PowerShell doesn't treat \ as the escape character so clearly osascript is doing it's own escape processing.
@lzybkr the value of argv[0] is exactly what I'd expect it to be. Since windows CreateProcess passes arguments as a simple string, process startup code needs to break that string up into individual arguments. It does this by processing the quotes that are passed, doing quote removal for string arguments.

@lzybkr
Copy link
Member

lzybkr commented Feb 17, 2017

@BrucePay - The annoyance is that calling a process differs from calling a command:

#437 PS> function myechoargs { for($i = 0; $i -lt $args.Length; $i++) { "arg ${i}: <$($args[$i])>"}}
#438 PS> myechoargs 'a "b" c'
arg 0: <a "b" c>
#439 PS> echoargs.exe 'a "b" c'
arg 0: <a b c>

Add to this - the behavior of bash differs from windows - it is more like calling a script/function than an executable.

I don't see a perfect solution. As I see it, the incompatible goals are:

  1. be more bash like
  2. be less surprising
  3. write portable PowerShell scripts
  4. not break existing PowerShell scripts

People do find the behavior on Windows confusing.
People do find many command lines for cmd/bash and should just work in PowerShell - this is one example where it doesn't.

PowerShell Core could chose to break compatibility with Windows PowerShell to deliver on goals 1, 2, and 3.
We could make no changes, and we've delivered on 3 and 4.

@TSlivede
Copy link

@BrucePay Although Windows does only send a single Commandline to the new Process, almost every CLI-App splits this Commandline according to these rules. Therefore Powershell should build the Commandline in a way, that matches those rules instead of just sometimes adding quotes around arguments.
Almost every other modern Commandshell on windows does this:

People also expect this from Powershell. See:

@lzybkr I think 4. is not really a Problem, as it never really worked on Windows and even changed from time to time. For example in Powershell on Windows 10 (PSVersion: 5.1.14393.693; PSEdition: Desktop; BuildVersion: 10.0.14393.693) it's completely impossible to send a"b c to the argv[1] of the next Process via a normal call:

  • PS: echoargs 'a\"b c' -> Commandline: \path\to\echoargs.exe a\"b c
  • PS: echoargs '"a\"b c"' -> Commandline: \path\to\echoargs.exe ""a\"b c""

Necessary Commandline for argv[1]=a"b c: \path\to\echoargs.exe "a\"b c"
The only option to accomplish this is via --%, but that gets really botched, if one needs to pass content from calculations or variables.

@lzybkr
Copy link
Member

lzybkr commented Feb 17, 2017

@TSlivede - I feel comfortable saying somebody depends on the current behavior.

That said, compatibility can't really be guaranteed (different runtimes amongst other big differences), so it's really a best effort.

Personally I'm in favor of the breaking change - far too many people avoid the easy way of running an external command because of issues like this.

@TSlivede
Copy link

TSlivede commented May 14, 2017

@vors asked in #1995 if anybody wants to start an RFC process.
I wrote a draft, here it is.

@SteveL-MSFT
Copy link
Member

Dupe of #1995

@jeff-hykin
Copy link

jeff-hykin commented May 9, 2021

For anyone looking for an essentially bulletproof workaround to the original issue, here's one:

Example Usage

execute osascript -e 'display notification "FOO" with title "TITLE"'

# better/safer usage
execute 'osascript' '-e' 'display notification "FOO" with title "TITLE"'

Code

# example: execute 'echo' 'hello "world"' 'I am free "finally don''t need to worry about escaping"' 
# fixes: https://github.com/PowerShell/PowerShell/issues/3049
function execute {
    # 
    # argument processing
    # 
    $all_args = $PsBoundParameters.Values + $args
    
    # 
    # escape all of them
    # 
    $escaped_arg_string = "&"
    $partly_escaped_arg = "&"
    $is_first_arg = $true
    ForEach ($item in $args) {
        # single quotes escape everything... everything except 
        $escaped_item = $item
        $escaped_item = ($escaped_item -replace "'", "''")
        $escaped_twice_item = ($escaped_item -replace '"', '""')
        
        # the command name/path is handled different
        if ($is_first_arg) {
            $is_first_arg = $false
            # powershell doens't escape the double quotes of the command name
            # so we dont double escape them this one time
            $escaped_twice_item = $escaped_item
        }
        $escaped_arg_string = $escaped_arg_string + " '" + $escaped_twice_item + "'"
        # post-7.1
        $partly_escaped_arg = $partly_escaped_arg + " '" + $escaped_item + "'"
    }
    
    # all this because sometimes its an array, sometimes its not
    $results = (Get-Command -CommandType "All" -Name $args[0] -ErrorAction SilentlyContinue).Source
    if ($results -is [array]) {
        $results = $results[0]
    }
    $main_item_source = $results
    
    # all this because sometimes its an array, sometimes its not
    $results = (Get-Command -CommandType "Application" -Name $args[0] -ErrorAction SilentlyContinue).Source
    if ($results -is [array]) {
        $results = $results[0]
    }
    $main_application_source = $results
    
    # if on older version, note: https://github.com/PowerShell/PowerShell/issues/13089
    # and if it is calling an Application (not a script/function)
    if (($PSVersionTable.PSVersion) -lt ([version]"7.2.0") -and ($main_item_source -eq $main_application_source)) {
        # use the fix
        return (Invoke-Expression $escaped_arg_string)
    # if on newer version or calling an Application
    } else {
        return (Invoke-Expression $partly_escaped_arg)
    }
}

This should always escape the arguments correctly, even when taking into account the planned 7.2 change here.

Comparison

#
# system echo (linux)
#

# broken version
& '/bin/echo' 'hello "world"' 'I am free "finally don''t need to worry about escaping"'       
# >>> hello world I am free finally don't need to worry about escaping

# fixed version
execute '/bin/echo' 'hello "world"' 'I am free "finally don''t need to worry about escaping"' 
# >>> hello "world" I am free "finally don't need to worry about escaping"


#
# powershell's echo
#

# default behavior
& 'echo' 'hello "world"' 'I am free "finally don''t need to worry about escaping"' 
# >>> hello "world"
# >>> I am free "finally don't need to worry about escaping"

# backwards compatible behavior (doesn't double-escape)
execute 'echo' 'hello "world"' 'I am free "finally don''t need to worry about escaping"' 
# >>> hello "world"
# >>> I am free "finally don't need to worry about escaping"

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 OS-Linux OS-macOS Resolution-Duplicate The issue is a duplicate. WG-Engine core PowerShell engine, interpreter, and runtime
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants