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

shell quoting broken #155

Closed
ezrizhu opened this issue Mar 31, 2024 · 4 comments
Closed

shell quoting broken #155

ezrizhu opened this issue Mar 31, 2024 · 4 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@ezrizhu
Copy link
Collaborator

ezrizhu commented Mar 31, 2024

try bash -c "echo a"
actually executes
bash -c echo a

@ezrizhu ezrizhu added bug Something isn't working help wanted Extra attention is needed labels Mar 31, 2024
@mgree
Copy link
Contributor

mgree commented Apr 25, 2024

The solution is to appropriately quote when we generate $script_to_execute. A basic version of this would not be too hard, but subtleties of escaping abound.

Would be easily resolved in a rewrite not in the shell.

@mgree
Copy link
Contributor

mgree commented Apr 25, 2024

for arg in "$@"
do
    case $arg in
        (*[$(printf " \n\t()<>&;|\`?*\$\"'")\\]*)
          quoted=$(echo "$arg" | sed -e "s/'/'\\\\''/")
          printf "'%s' " "$quoted";;
        (*)   printf "%s " "$arg";;
    esac
done
printf "\n"

is a workable first cut.

mgree added a commit that referenced this issue Apr 25, 2024
@mgree
Copy link
Contributor

mgree commented Apr 26, 2024

Our tests expect that try "echo hi>foo" will preserve the redirect and that try "echo hi; echo bye" will preserve the pair of commands.

I don't see how we can support both this existing behavior and appropriate quoting of things like bash -c. An advantage of the current behavior is that you can run try "bash -c \"echo hi\"" and get the right behavior... whereas it's not clear how to preserve the shell syntax with this modified behavior.

Unless you have a better idea, I'll delete the branch and close the issue.

@ezrizhu
Copy link
Collaborator Author

ezrizhu commented Apr 26, 2024

I don't have a better idea, feel free to delete the branch. I'll close the issue once the docs has been updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants