swiftbar / SwiftBar

Powerful macOS menu bar customization tool
https://swiftbar.app
MIT License
2.95k stars 92 forks source link

bash params not work when pass special chars #366

Closed futurist closed 11 months ago

futurist commented 1 year ago

Describe the bug

If plugin contains below line:

echo "--delete | bash=test.sh terminal=true param1=_update param2='$name'"

The terminal will show syntax error if $name is abc(123)

To Reproduce Steps to reproduce the behavior:

  1. put above line in test plugin
  2. click "delete" button
  3. terminal show syntax error

Expected behavior

params should be passed to bash script correctly (in quote?)

Environment:

Plugin Example:

#!/bin/bash

name='abc(123)'
echo test
echo ---
echo test
echo "--delete | bash=test.sh  terminal=true param1=_update param2=id param3=delete param4='$name'"

Additional Context:

when name='abc (123)' (a space in it), the code works, but not when abc(123)

melonamin commented 1 year ago

Added simple escaping (take args in quotes), this will work almost for every case, but would fail if the argument contains a single quote.

dchevell commented 1 year ago

@melonamin FYI single-quoting all args seems to break a few things in my experience - in prior versions, it's valid to specify a param containing spaces by encapsulating it in quotes. But now, it causes the param to get double quoted, and breaks it. The most obvious example is when plugins use another bash subshell to run something more complicated:

Copy hostname | shell=bash param0=-c param1="hostname | pbcopy"

That's a trivial example, but the bitbar/xbar plugin libraries contain real examples just like this, and I've run into this issue with my own plugins as well. I don't have a workaround at the moment - it's either impractical or impossible to break down these things to individual params (e.g. the above wouldn't work, you can't quote an individual pipe character). Unfortunately I don't think quoting all params is a workable solution

melonamin commented 1 year ago

Ok, broke faster than I expected... I'll take another go

dchevell commented 1 year ago

If you could avoid the double quoting (i.e. don't quote an already-quoted string, so that params containing spaces can still be used), it might work. There's varying degrees of sophistication you could apply - from full on shell lexical parsing (a bit like python's shlex module), to just testing if the string is already surrounded by quotes.

Maybe something like:

let quotedStringRegex = #/^(?:'.*'|".*")$/#

return out.map {
    if $0.firstMatch(of: quotedStringRegex) == nil {
        "'\($0)'"
    } else {
       $0
    }
}

(My swift is terrible, but you get the idea)

melonamin commented 11 months ago

I've updated the logic a bit, check it out in the latest public release

kdeldycke commented 11 months ago

Probably related to: https://github.com/swiftbar/SwiftBar/issues/308