-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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
Comments
The following change does work. osascript -e 'display notification \"FOO\" with title \"TITLE\"' Per the man page.
|
@thezim good to know, thank you! Also it related to the pillar of "making PS more compatible with existing shells to allow more commands work thru copy-paste". |
@vor I agree. I would prefer your example to work too. |
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 |
@vors This works too. 'display notification "FOO" with title "TITLE"' | osascript |
Repro (using test code in our repo):
|
Note that the behavior is consistent with Windows, but maybe it shouldn't be. |
Why is it this way on Windows? If I recall correctly, this behavior was a major annoyance. |
@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. |
@BrucePay - The annoyance is that calling a process differs from calling a command:
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:
People do find the behavior on Windows confusing. PowerShell Core could chose to break compatibility with Windows PowerShell to deliver on goals 1, 2, and 3. |
@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.
People also expect this from Powershell. See:
@lzybkr I think
Necessary Commandline for |
@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. |
Dupe of #1995 |
For anyone looking for an essentially bulletproof workaround to the original issue, here's one: Example Usageexecute 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" |
Steps to reproduce
On macOS run
Expected behavior
Show notification, just like in bash
Actual behavior
Environment data
The text was updated successfully, but these errors were encountered: