exec expansion splits args based on space after variable expansion

[KyleSanderson]

Discussed on discord - just a quick summary here because I'm likely going to forget.

brr Exec currently does not support spaces within quotes, and passes them as individual args. Because of the ordering of the macro expansion

autobrr/internal/action/exec.go

Line 26 in 5a45851

// parse and replace values in argument string before continuing

into the arg string split

autobrr/internal/action/exec.go

Line 33 in 5a45851

// we need to split on space into a string slice, so we can spread the args into exec

it is impossible to pass arguments and macros containing a space to programs. Invoking a script is a current workaround, because $SHELL can repair the damage by bringing the arg string together again by wrapping in quotes. Changing this ordering can help, but the implementation will still be incorrect as expected spaces (even if escaped) will be split.

STR: "{{ .TorrentName }}" > Sally Goes to the Mall S04E29 > curl -XPOST

There's a couple ways to tackle this, an out-of-tree one being https://github.com/mattn/go-shellwords which could add the ability to have environment variables as well on execution.

[ghost]

Technically speaking, this issue hasn't been fixed, rather just circumvented in a specific use-case.

To fix this properly, I'd steer away from the native .Split method, and instead use a custom splitting function which will take care of an escape character and handle it that way. Here's my implementation in Python:

def split_args(s: str) -> list[str]:
    results = []

    buffer = ""
    in_quotes = False
    for i, ch in enumerate(s):
        if ch == "\"":
            if i != 0 and s[i - 1] == "\\":
                if in_quotes:
                    buffer += ch
                continue
            in_quotes = not in_quotes
        elif ch == " " and not in_quotes:
            if not buffer:
                continue
            results.append(buffer)
            buffer = ""
        else:
            buffer += ch

    if buffer:
        results.append(buffer)

    return results


if __name__ == "__main__":
    test_cases: dict[str, list[str]] = {
        "Sally Goes to the Mall S04E29": [
            "Sally",
            "Goes",
            "to",
            "the",
            "Mall",
            "S04E29"
        ],
        "Hi there, how's it going?": [
            "Hi",
            "there,",
            "how's",
            "it",
            "going?"
        ],
        "       Hi there, how's it going?": [
            "Hi",
            "there,",
            "how's",
            "it",
            "going?"
        ],
        "Hi there, how's it going?           ": [
            "Hi",
            "there,",
            "how's",
            "it",
            "going?"
        ],
        "Hi there,               how's  it going?": [
            "Hi",
            "there,",
            "how's",
            "it",
            "going?"
        ],
        "Let's test \"if quoting works\"": [
            "Let's",
            "test",
            "if quoting works"
        ],
        "\"Sally Goes to the Mall S04E29\"": [
            "Sally Goes to the Mall S04E29"
        ],
        "Ignore double quotes \"\" :)": [
            "Ignore",
            "double",
            "quotes",
            ":)"
        ],
        "Let's test \"if escaping \\\" while in a quote \\ \ works\"": [
            "Let's",
            "test",
            "if escaping \\\" while in a quote \\ \ works"
        ]
    }
    for k, v in test_cases.items():
        print(f"Testing: {k}")

        r = split_args(k)
        if r != v:
            print("\tTest case failed!!")
            print(f"\tGot: {r}")
            print(f"\tInstead of: {v}")
        else:
            print("\tOK")

Then when you replace the template, you wrap the new value around in quotes and escape inner quotes by replacing " with " and finally pass it on to the custom splitting function. That way arguments the forwarded properly to exec.Command.

On the other hand, this should be a moderately rare use-case, so even if we don't handle it properly, it shouldn't really be a problem, as it can ultimately be circumvented in other ways. Still, might be good to hear other people's opinions on this.

[ghost]

Another way to go at this issue is to have each argument go into a separate input field (UI-wise), and on the backend side, each input field is separately parsed ("processed") and it's passed directly as an array to exec.Command.

However, I don't know if this breaks something? This may be a bit more work frontend-wise, but leaves the burden off the backend.