plugin-argu icon indicating copy to clipboard operation
plugin-argu copied to clipboard

Use printf instead of echo to parse short option

Open ryotako opened this issue 7 years ago • 6 comments

Hi, thank you for the good plugin! This is the nicest option parser for fish-shell.

I fixed short option parsing. In the current version, options -e, -n, -s, -E are not parsed because they are options of echo command.

argu e -- -e # output is ''

ryotako avatar Jul 16 '17 14:07 ryotako

I think maybe you mean printf "%s\n" $argv[1], as echo -E intentionally suppresses backslash escapes, but printf doesn't if you use user input as the format string.

pjeby avatar Aug 20 '17 06:08 pjeby

Thanks! I assumed that options contained only alphabets, numbers and -. As you pointed out, backslash escapes cannot be managed correctly in my change.

I just want to use short options -e, -n, -s and -E. How about this?

 # Match a short or long single option.
      case -e -n -s -E
          printf "$argv[1]\n"    
      case '--*' '-?'
          echo -E "$argv[1]"
      ...

ryotako avatar Aug 25 '17 12:08 ryotako

Why not just printf "%s\n" $argv[1]? It works for any value, always. Really, you should never pass a string beginning with a variable as the first non-option argument to either echo or printf. (And there's no reason to ever put variables in the first argument to printf unless you are doing something way more complicated than this library does.)

As for assuming that options only contain the right characters, bear in mind that user input can contain typos or malicious values intended to crash the program or make it misbehave.

So, most echo -Es in the function should be replaced with printf "%s\n" whatever, The only time it's safe to use echo is when the first non-option argument begins with a constant value that doesn't start with '-'. So for example, this is safe:

echo "Unknown option `$argv[1]'." >&2

because it begins with a constant that does not start with -. But anything that doesn't start that way should be using printf without any variables in the first argument. That is, never use printf "$something" because what you should be doing is printf "%s" $something. The constant parts go in the first argument, variable parts in the other arguments.

pjeby avatar Aug 25 '17 16:08 pjeby

Sorry for my late reply and poor English skill...

I did not understand well the behavior of printf. Now I think your suggestion is a perfect solution.

I had fixed it!

ryotako avatar Aug 28 '17 23:08 ryotako

No problem. Glad I could help.

pjeby avatar Aug 29 '17 01:08 pjeby

@sagebind would you mind reviewing? This one bit me too. :pray:

zephraph avatar Dec 02 '19 03:12 zephraph