Should variables be quoted when executed?

Question

The general rule in shell scripting is that variables should always be quoted unless there is a compelling reason not to. For more details than you probably want to know, have a look at this great Q&A: Security implications of forgetting to quote a variable in bash/POSIX shells.

Consider, however, a function like the following:

run_this(){
    $@
}

Should $@ be quoted there or not? I played with it for a bit and couldn't find any case where the lack of quotes caused a problem. On the other hand, using the quotes makes it break when passing a command containing spaces as a quoted variable:

#!/usr/bin/sh
set -x
run_this(){
    $@
}
run_that(){
    "$@"
}
comm="ls -l"
run_this "$comm"
run_that "$comm"

Running the script above returns:

$ a.sh
+ comm='ls -l'
+ run_this 'ls -l'
+ ls -l
total 8
-rw-r--r-- 1 terdon users  0 Dec 22 12:58 da
-rw-r--r-- 1 terdon users 45 Dec 22 13:33 file
-rw-r--r-- 1 terdon users 43 Dec 22 12:38 file~
+ run_that 'ls -l'
+ 'ls -l'
/home/terdon/scripts/a.sh: line 7: ls -l: command not found

I can get around that if I use run_that $comm instead of run_that "$comm", but since the run_this (unquoted) function works with both, it seems like the safer bet.

So, in the specific case of using $@ in a function whose job is to execute $@ as a command, should $@ be quoted? Please explain why it should/shouldn't be quoted and give an example of data that can break it.

Answer 1

The problem lies in how the command is passed to the function:

$ run_this ls -l Untitled\ Document.pdf 
ls: cannot access Untitled: No such file or directory
ls: cannot access Document.pdf: No such file or directory
$ run_that ls -l Untitled\ Document.pdf 
-rw------- 1 muru muru 33879 Dec 20 11:09 Untitled Document.pdf

"$@" as in run_that should be used in the general case where your function is prefixed to a normally written command (as above).

Trying to use the unquoted $@ as in run_this makes it impossible to pass a filename with whitespace. None of these attempts will work:

$ run_this 'ls -l Untitled\ Document.pdf'
ls: cannot access Untitled\: No such file or directory
ls: cannot access Document.pdf: No such file or directory
$ run_this 'ls -l "Untitled\ Document.pdf"'
ls: cannot access "Untitled\: No such file or directory
ls: cannot access Document.pdf": No such file or directory
$ run_this 'ls -l Untitled Document.pdf'
ls: cannot access Untitled: No such file or directory
ls: cannot access Document.pdf: No such file or directory
$ run_this 'ls -l' 'Untitled Document.pdf'
ls: cannot access Untitled: No such file or directory
ls: cannot access Document.pdf: No such file or directory

The reason it doesn't work is that the unquoted expansion goes through word splitting, which splits on any whitespace and doesn't provide means for interpreting quotes or such (for that, you'd need to use eval).

See also e.g.

• BashFAQ 050 (or "I'm trying to put a command in a variable, but the complex cases always fail!")
• How can we run a command stored in a variable?

Answer 2

It's either:

interpret_this_shell_code() {
  eval "$1"
}

Or:

interpret_the_shell_code_resulting_from_the_concatenation_of_those_strings_with_spaces() {
  eval "$@"
}

or:

execute_this_simple_command_with_these_arguments() {
  "$@"
}

But:

execute_the_simple_command_with_the_arguments_resulting_from_split+glob_applied_to_these_strings() {
  $@
}

Does not make much sense.

If you want to execute the ls -l command (not the ls command with ls and -l as arguments), you'd do:

interpret_this_shell_code '"ls -l"'
execute_this_simple_command_with_these_arguments 'ls -l'

But if (more likely), it's the ls command with ls and -l as arguments, you'd run:

interpret_this_shell_code 'ls -l'
execute_this_simple_command_with_these_arguments ls -l

Now, if it's more than a simple command you want to execute, if you want to do variable assignments, redirections, pipes..., only interpret_this_shell_code will do:

interpret_this_shell_code 'ls -l 2> /dev/null'

though of course you can always do:

execute_this_simple_command_with_these_arguments eval '
  ls -l 2> /dev/null'

Answer 3

Looking at it from the bash/ksh/zsh perspective, $* and $@ are a special case of general array expansion. Array expansions aren't like normal variable expansions:

$ a=("a b c" "d e" f)
$ printf ' -> %s\n' "${a[*]}"
 -> a b c d e f
$ printf ' -> %s\n' "${a[@]}"
-> a b c
-> d e
-> f
$ printf ' -> %s\n' ${a[*]}
 -> a
 -> b
 -> c
 -> d
 -> e
 -> f
$ printf ' -> %s\n' ${a[@]}
 -> a
 -> b
 -> c
 -> d
 -> e
 -> f

With the $*/${a[*]} expansions you get the array joined with the first value of IFS—which is space by default—into one giant string. If you don't quote it, it gets split like a normal string would.

With the $@/${a[@]} expansions, the behavior depends on whether the $@/${a[@]} expansion is quoted or not:

1. if it is quoted ("$@" or "${a[@]}"), you get the equivalent of "$1" "$2" "$3" #... or "${a[1]}" "${a[2]}" "${a[3]}" # ...
2. if it isn't quoted ($@ or ${a[@]}) you get the equivalent of $1 $2 $3 #... or ${a[1]} ${a[2]} ${a[3]} # ...

For wrapping commands, you most definitely want the quoted @ expansions (1.).

---

More good info on bash (and bash-like) arrays: https://lukeshu.com/blog/bash-arrays.html

Answer 4

Since when you don't double quote $@, you left all the globbing issues in the link you gave to your function.

How could you run a command named *? You can not do it with run_this:

$ ls
1 2
$ run_this '*'
dash: 2: 1: not found
$ run_that '*'
dash: 3: *: not found

And you see, even when error occurred, run_that gave you a more meaningful message.

The only way to expand $@ to individual words is double quotes it. If you want to run it as a command, you should pass the command and it parameters as separated words. That the thing you did on the caller side, not inside your function.

$ cmd=ls
$ param1=-l
$ run_that "$cmd" "$param1"
total 0
-rw-r--r-- 1 cuonglm cuonglm 0 Dec 23 17:33 1
-rw-r--r-- 1 cuonglm cuonglm 0 Dec 23 17:33 2

is a better choice. Or if your shell support arrays:

$ cmd=(ls -l)
$ run_that "${cmd[@]}"
total 0
-rw-r--r-- 1 cuonglm cuonglm 0 Dec 23 17:33 1
-rw-r--r-- 1 cuonglm cuonglm 0 Dec 23 17:33 2

Even when the shell does not support array at all, you can still play with it by using "$@".

Answer 5

Executing variables in bash is a failure-prone technique. It's simply impossible to write a run_this function which correctly handles all the edge cases, like:

• pipelines (e.g. ls | grep filename)
• input/output redirections (e.g. ls > /dev/null)
• shell statements like if while etc.

If all you want to do is avoid code repetition, you're better off using functions. For example, instead of:

run_this(){
    "$@"
}
command="ls -l"
...
run_this "$command"

You should write

command() {
    ls -l
}
...
command

If the commands are only available at run time, you should use eval, which is specifically designed to handle all the quirks which will make run_this fail:

command="ls -l | grep filename > /dev/null"
...
eval "$command"

Note that eval is known for security issues, but if you pass variables from untrusted sources to run_this, you will face arbitrary code execution just as well.

Answer 6

The choice is yours. If you do not quote $@ any of its values undergo additional expansion and interpretation. If you do quote it all of the arguments passed the function are reproduced in its expansion verbatim. You'll never be able to reliably handle shell syntax tokens like &>| and etc either way without parsing out the arguments yourself anyway - and so you're left with the more reasonable choices of handing your function one of either:

1. Exactly the words used in the execution of a single simple command with "$@".

...or...

2. A further expanded and interpreted version of your arguments which are only then applied together as a simple command with $@.

Neither way is wrong if it is intentional and if the effects of what you choose are well understood. Both ways have advantages one over the other, though the advantages of the second are seldom likely to be particularly useful. Still...

(run_this(){ $@; }; IFS=@ run_this 'ls@-dl@/tmp')

---

drwxrwxrwt 22 root root 660 Dec 28 19:58 /tmp

...it isn't useless, just rarely likely to be of much use. And in a bash shell, because bash doesn't by default stick a variable definition to its environment even when said definition is prepended to the command-line of a special builtin or to a function, the global value for $IFS is unaffected, and its declaration is local only to the run_this() call.

Similarly:

(run_this(){ $@; }; set -f; run_this ls -l \*)

---

ls: cannot access *: No such file or directory

...the globbing is also configurable. Quotes serve a purpose - they're not for nothing. Without them shell expansion undergo extra interpretation - configurable interpretation. It used to be - with some very old shells - that $IFS was globally applied to all input, and not just expansions. In fact, said shells behaved very like run_this() does in that they broke all input words on the value of $IFS. And so, if what you're looking for is that very old shell behavior, then you should use run_this().

I'm not looking for it, and I'm fairly hard pressed at the moment to come up with a useful example for it. I generally prefer for the commands my shell runs to be those which I type at it. And so, given the choice, I would almost always run_that(). Except that...

(run_that(){ "$@"; }; IFS=l run_that 'ls' '-ld' '/tmp')

---

drwxrwxrwt 22 root root 660 Dec 28 19:58 /tmp

Just about anything can be quoted. Commands will run quoted. It works because by the time the command is actually run, all input words have already undergone quote-removal - which is the last stage of the shell's input interpretation process. So the difference between 'ls' and ls can only matter while the shell is interpreting - and that's why quoting ls ensures that any alias named ls is not substituted for my quoted ls command word. Other than that, the only things which quotes affect are the delimiting of words (which is how and why variable/input-whitespace quoting works), and the interpretation of metacharacters and reserved words.

So:

'for' f in ...
 do   :
 done

---

bash: for: command not found
bash:  do: unexpected token 'do'
bash:  do: unexpected token 'done'

You'll never be able to do that with either of run_this() or run_that().

But function names, or $PATH'd commands, or builtins will execute just fine quoted or unquoted, and that's exactly how run_this() and run_that() work in the first place. You won't be able to do anything useful with $<>|&(){} any of those. Short of eval, is.

(run_that(){ "$@"; }; run_that eval printf '"%s\n"' '"$@"')

---

eval
printf
"%s\n"
"$@"

But without it, you're constrained to the limits of a simple command by virtue of the quotes you use (even when you don't because the $@ acts like a quote at the beginning of the process when the command is parsed for metacharacters). The same constraint is true of command-line assignments and redirections, which are limited to the function's command-line. But that's not a big deal:

(run_that(){ "$@";}; echo hey | run_that cat)

---

hey

I could have as easily < redirected input or > output there as I did open the pipe.

Anyway, in a round-about way, there is no right or wrong way here - each way has its uses. It's just that you should write it as you intend to use it, and you should know what you mean to do. Omitting quotes can have a purpose - otherwise there wouldn't be quotes at all - but if you omit them for reasons not relevant to your purpose, you're just writing bad code. Do what you mean; I try to anyway.