#3135 Issue closed: Hardcoded 'eval $command' is problematic and against "final power to the user"¶
Labels: discuss / RFC, fixed / solved / done
jsmeix opened issue at 2024-01-22 11:02:¶
In default.conf we describe several cases
where the user can specify commands to be run by ReaR
and we run those user specified commands usually via
eval $command
to make it easier for the user to specify e.g.
command='echo current date: $( date )'
and get $... evaluated not when it is specified
but when it is executed.
But in general eval arbitrary $... is problematic, cf.
https://github.com/rear/rear/pull/3089#discussion_r1412293833
and the subsequent comments therein.
And with the hardcoded eval the user can not specify
when he does not want to get $... evaluated like
command='echo to show the date use "echo current date: $( date )"'
because
# command='echo to show the date use "echo current date: $( date )"'
# $command
to show the date use "echo current date: $( date )"
# eval $command
to show the date use echo current date: Mon 22 Jan 2024 11:53:31 AM CET
By the way: Note the mising double quote characters with eval ...
So our hardcoded 'eval $command' is problematic
and it is against final power to the user.
Therefore I am thinking about to remove our hardcoded 'eval'
and explain in default.conf that the user must manually
specify 'eval' when he needs it.
So instead of
command='echo current date: $( date )'
the user would have to specify
command='eval echo current date: $( date )'
and ReaR could simply and safely call plain $command
for example like
# command='eval echo current date: $( date )'
# $command ; sleep 1 ; $command
current date: Mon 22 Jan 2024 11:59:39 AM CET
current date: Mon 22 Jan 2024 11:59:40 AM CET
pcahyna commented at 2024-01-26 18:21:¶
This would be a big compatibility break, so it should belong to ReaR 3.0, not 2.8.
I am not even convinced that this change would be an improvement. Yes,
in
https://github.com/rear/rear/pull/3089#discussion_r1412293833
I noted that eval of an arbitrary variable expansion is problematic.
But I said that in the context of something like
eval echo "Launching \'$RECOVERY_COMMANDS_LABEL\' automatically",
where the user does not expect that the $RECOVERY_COMMANDS_LABEL
string is being passed to eval (one expects it to be some arbitrary
string for display). As I clarified in
https://github.com/rear/rear/pull/3089#discussion_r1412295915
, I believe eval "$command" to be fine. $command is documented to be
executed this way, so it is not an arbitrary string.
Your argument about "with the hardcoded eval the user can not specify
when he does not want to get $... evaluated" is interesting, but I don't
consider it convincing. Let's say I want the literal string
to show the date use "echo current date: $( date )" displayed. In
plain shell, I would use
echo 'to show the date use "echo current date: $( date )"'
without the single quotes it would not have worked either, so to assign
it to command one needs to quote it properly at one more level:
command="echo 'to show the date use \"echo current date: \$( date )\"'"
which then works as expected:
$ eval "$command"
to show the date use "echo current date: $( date )"
unwieldy? yes. Against "final power to the user"? I would not say so.
The bash quoting rules are well documented and it is quite easy to
understand what one needs to put into $command: it must get assigned the
same string as one would type at the command line. The complexity arises
from the amount of quoting that is sometimes needed to achieve the
result. The $'...' quoting mechanism can help a bit here - one is
forced to quote only the ' character and nothing else:
$ command=$'echo \'to show the date use "echo current date: $( date )"\''
$ eval "$command"
to show the date use "echo current date: $( date )"
which looks a bit better.
Your suggestion to call the command as merely $command sounds simpler,
but this simplicity is IMO superficial and hides lots of surprising
complexity. Let's say that in your example I want to put some extra
spaces for readability:
# note the doubled spaces
$ command='echo to show the date use "echo current date: $( date )"'
result:
$ $command
to show the date use "echo current date: $( date )"
where are my spaces? Maybe you would expect the extra space before "
to disappear, but what about the one before $(, which is inside a
quoted string? What has happened? Let's examine a simpler example:
# note the triple spaces
$ command="echo 'a b c'"
$ $command
'a b c'
where are my triple spaces and what is the ' character doing there?
Let's examine the result of the $command expansion:
$ for i in $command ; do $i ; done
bash: 'a: command not found
bash: b: command not found
bash: c': command not found
What happens is that the shell performs whitespace splitting on the
value of $command, resulting in for words:
echo 'a b c' and then executes this sequence (with echo as the
command and 'a b c' as its three arguments).
I must say that when you let the shell to split the command string into
words and then execute the sequence of words, it is so difficult for me
how to make it do what I need that I much prefer eval with the
"quoting hell" needed to prepare its input (which is in most cases even
quite unnecessary).
Worse, how do you execute multiple commands?
$ command="echo a b c ; echo d e f"
$ $command
a b c ; echo d e f
not only are the extra spaces eaten - the ; is not intepreted and the
second echo is not executed at all. Yes, you could use eval:
$ command="eval echo a b c ; echo d e f"
$ $command
a b c
d e f
-- almost right, but I still don't know how to get the extra spaces there, some word splitting is still going on:
$ command="eval echo 'a b c' ; echo d e f"
$ $command
a b c
d e f
not quite right either, neither is
$ command="eval 'echo a b c ; echo d e f'"
:
$ $command
bash: echo a b c ; echo d e f: command not found
I admit I am lost here.
pcahyna commented at 2024-01-26 19:49:¶
The underlying problem is that with eval $command it gets processed
twice: when command gets first assigned in the config file (
command=... ) and then when eval $command evaluates it. Both steps
have fairly easy rules. With merely $command you get another
processing step after $command is expanded but before it gets executed:
word splitting on whitespace (and probably other stuff, like filename
expansion), which complicates the process, despite the execution of
$command being simpler than the execution of eval $command.
jsmeix commented at 2024-02-07 14:15:¶
@pcahyna
thank you so much for your detailed explanations.
In particular your reasoning
I believe eval "$command" to be fine.
$command is documented to be executed this way,
so it is not an arbitrary string.
...
The bash quoting rules are well documented and it is quite easy
to understand what one needs to put into $command:
it must get assigned the same string
as one would type at the command line.
The complexity arises from the amount of quoting
that is sometimes needed to achieve the result.
convinced me that the current eval "$command" is fine
and now with this issue here it is even perfectly well
documented WHY the current eval "$command" is fine.
[Export of Github issue for rear/rear.]