#781366 bash: flaws in Debian's default for (VI|EMACS|FC|POSIX_FC)_EDIT_COMMAND

Package:
bash
Source:
bash
Description:
GNU Bourne Again SHell
Submitter:
Christoph Anton Mitterer
Date:
2015-03-29 03:21:12 UTC
Severity:
normal
#781366#5
Date:
2015-03-28 06:37:26 UTC
From:
To:
Hi.

Currently Debian sets something like the following for VI_EDIT_COMMAND,
EMACS_EDIT_COMMAND, FC_EDIT_COMMAND and POSIX_FC_EDIT_COMMAND:
While this is nice it has several flaws:
- PATH may be set to a bogus value not containing vi and thus that would
  fail.


Why do you use "command" here?
If you simply want /usr/bin/editor, than you could just give that.
If you want to allow either the shell to make "editor" a builtin command
or the user to redefine "editor" as function or alias than this has the
problems:
- if it was an alias like "alias editor='vim -C'" it ould return the
  alias line, and subsequently things would break
- if it's another "editor" in a user supplied PATH, that may contain
  spaces or other characters that need quoting


I'd suggest the following, which however doesn't solve the alias issue:
"${FCEDIT:-${EDITOR:-"$(command -v editor || PATH=/usr/bin printf /usr/bin/vi)"}}"

Explanations:
- The reason for the "$(…)" is obviously to solve the potential problem
  from above.
- Using the full pathname of vi, to solve the problem of a bogus PATH.
  And I assume here that vi is never going to be a built-in.
- Strictly speaking, command and printf/echo are not defined by POSIX to
  be built-ins.
  For bash (and this is the only one that this patch affects) they are
  however (and for "command" a non-built-in version seems to not even
  exist).
  But,... the user might have done something awkward like:
  - disabled the built-in printf/echo, e.g. via enabled -n or some other
    trick
  - plus set a PATH which doesn't contain the binary version of
    printf/echo, which both are part of coreutils (thus
    essential+required) and busybox and thus guaranteed to be there).
  You say this wouldn't happen? Well I know a friend called murphy, and
  since this doesn't cost anything, why not making it right.
- I personally rather use printf than echo,... it's not slower and
  strictly speaking, POSIX has deprecated echo.
  If you can't live with that, change the "PATH=/usr/bin" to "PATH=/bin".


Now how to solve the alias issue... not really sure about that,
one could either use sed and remove any starting "^alias " but that's
quite hacky and requires another process.
Maybe:
"${FCEDIT:-${EDITOR:-"$( (unalias -a ; command -v editor || PATH=/usr/bin printf /usr/bin/vi) )"}}"
- that would remove any aliases in the subshell environment between the
  inner (…).
  Don't forget the spaces between the braces,... $((…)) would lead to
  trying arithmetic exapnsion ;-)

Cheers,
Chris.

#781366#10
Date:
2015-03-29 03:19:10 UTC
From:
To:
I just noted that I forgot another level of quoting that would be needed
for those rare cases of users having awkward characters in their path.

Also I've added { …; } around the first version (the 2nd already has "(…
)", yes the different forms are on purpose) and a "2> /dev/null to not
clutter up messages in case any errors occur)

Thus my first suggestion would need to read:
"${FCEDIT:-"${EDITOR:-"$( { command -v editor || PATH=/usr/bin printf /usr/bin/vi; } 2> /dev/null )"}"}"

and the one that works around the aliases issue would need to read:
"${FCEDIT:-"${EDITOR:-"$( (unalias -a ; command -v editor || PATH=/usr/bin printf /usr/bin/vi) 2> /dev/null )"}"}"




Cheers,
Chris.