-
Notifications
You must be signed in to change notification settings - Fork 27
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
Output escaping is broken #3
Comments
I didn't notice that the quotes should be kept I'll deal with this later |
I'm really not fluent in rust, but in bash and it seems you should use someting like https://docs.rs/shellwords/latest/shellwords/fn.escape.html to output variable contents and connands |
The problem occurs a little further ahead。The value The escape is not a problem:
|
Imo that is exactly the problem. |
Proper escaping needs to be added for all escapes.. It's not just dollar sign, that was only a trivial example. Also, its not just variables that are excuted, all shell commands are because of the dangerous use of eval. Imagine pwd being changed to rm -rf below... Take these examples
|
I'm afraid it's far from fixed right now. my simple testsetup: #!/bin/bash
# @describe A demo cli
# @arg value! a test value
eval "$(argc -e $0 "$@")"
if [[ "$1" == "$argc_value" ]]; then
echo "Okay"
else
echo "ERROR $1 results in $argc_value"
fi Shell quotiong really isn'T simple. pleas reconsider using a proven library for that job |
Okay, I've a new build. Please test it! |
Testing 0.3.1 i'Ve found the following problematic argument values:
or any other mulitbyte character |
At least variable values are not propperly escaped.
e.g. running the
demo.sh
like this:export foo="this is broken" ; ./demo.sh download -t '$foo' bar
should result in
but results in
this is since calling
argc -e demo.sh download -t '$foo' bar
results inThe text was updated successfully, but these errors were encountered: