Why must I not quote a string variable when running it as a command?

Question

My Ubuntu/Debian-based Linux update POSIX shell script seems to require that I not double quote the string variable with the stored command, which is being executed. As I don't understand this issue, I'd like to ask why that is? And if the code is correct as it is?

Warning: SC2086, wiki, "Double quote to prevent globbing and word splitting."

The script follows, the problematic part is highlighted:

#!/bin/sh

# exit script when it tries to use undeclared variables
set -u

# color definitions
readonly red=$(tput bold)$(tput setaf 1)
readonly green=$(tput bold)$(tput setaf 2)
readonly yellow=$(tput bold)$(tput setaf 3)
readonly white=$(tput bold)$(tput setaf 7)
readonly color_reset=$(tput sgr0)
# to create blocks of texts, I separate them with this line
readonly block_separator='----------------------------------------'

step_number=0
execute_jobs ()
{
    while [ ${#} -gt 1 ]
    do
        job_description=${1}
        job_command=${2}

        step_number=$(( step_number + 1 ))

        printf '%s\n' "Step #${step_number}: ${green}${job_description}${color_reset}"

        printf '%s\n' "Command: ${yellow}${job_command}${color_reset}"

        printf '%s\n' "${white}${block_separator}${color_reset}"

        # RUN THE ACTUAL COMMAND
        # ShellCheck warns me I should double quote the parameter
        # If I did, it would become a string (I think) and I'd get 'command not found' (proven)
        # As I don't understand the issue, I left it as I wrote it, without quotes
        ### shellcheck disable=SC2086

if sudo ${job_command} # <-- HERE

        then
            printf '\n'
        else
            printf '%s\n\n' "${red}An error occurred.${color_reset}"
            exit 1
        fi

        shift 2
    done
}

execute_jobs \
    'configure packages'                         'dpkg --configure --pending' \
    'fix broken dependencies'                    'apt-get --assume-yes --fix-broken install' \
    'update cache'                               'apt-get update' \
    'upgrade packages'                           'apt-get --assume-yes upgrade' \
    'upgrade packages with possible removals'    'apt-get --assume-yes dist-upgrade' \
    'remove unused packages'                     'apt-get --assume-yes --purge autoremove' \
    'clean up old packages'                      'apt-get autoclean'

Your script is pretty fragile. Read [I'm trying to put a command in a variable, but the complex cases always fail!](https://mywiki.wooledge.org/BashFAQ/050) -- at least turn off globbing: `set -f` — glenn jackman, May 10 '19 at 16:54

score 6 · Accepted Answer · answered May 10 '19 at 09:12

6

You can’t quote the variable here:

if sudo ${job_command}

because you do want word splitting. If you quote, the command becomes (for your first step)

if sudo "dpkg --configure --pending"

and sudo looks for the command dpkg --configure --pending, rather than the command dpkg with the arguments --configure and --pending, as indicated by its error message:

sudo: dpkg --configure --pending: command not found

(try it with extra spaces to make it more explicit).

With the quotes omitted, the shell splits the arguments, and everything works as expected.

answered May 10 '19 at 09:12

Stephen Kitt

411,918
54
1,065
1,164

Note that leaving `${job_command}` unquoted involves `$IFS` splitting and globbing. So if relying on that brain-damaged split+glob operator to do the splitting, in the general case, you'd likely want to make sure `$IFS` contains the separator you want and disable globbing. – Stéphane Chazelas Nov 10 '22 at 08:26
I suppect the OP intended `$job_command` to contain shell code and then the command would be `sudo sh -c "$job_command"` – Stéphane Chazelas Nov 10 '22 at 08:34
@Stéphane all the intended contents are given in the question, and the `sh -c` approach would indeed be better (or Kusalananda’s split arguments approach). – Stephen Kitt Nov 10 '22 at 08:40

Kusalananda · Answer 2 · 2022-11-10T08:28:56.857

The way you have written your code now, you are relying on the shell to correctly split the string that holds your command into a command name with additional options and other arguments. The shell will only do this if you leave your variable expansion unquoted.

However, the shell will not do it correctly if your command string contains some arguments that should be split on spaces and others that shouldn't be or if it contains substrings that look like filename globbing patterns that shouldn't be expanded.

An alternative implementation of your script that allows you to properly quote all expansions:

#!/bin/sh

step=0
execute_job () {
    step=$(( step + 1 ))
    printf 'Step #%s: "%s"\n' "$step" "$1"
    shift
    printf 'Command: %s\n' "$*"

    if sudo -- "$@"; then
        echo Done
    else
        echo 'Some error occured'
        exit 1
    fi
}

execute_job 'configure packages'      dpkg --configure --pending
execute_job 'fix broken dependencies' apt-get --assume-yes --fix-broken install
# etc.

Here, I let execute_job care about a single job instead of all jobs. I treat the first argument to the function as the job description string. Then I take any further arguments as the command to execute. The command that should be executed is passed as several arguments, not a single string.

When printing out the command string, I use "$*", which is one single quoted string made up from concatenating the positional parameters with the first character from $IFS as the delimiter (a space by default).

When executing the command, I use "$@", which is the list of positional parameters individually quoted to avoid word splitting and filename globbing.

This would allow you to use the function to do things like

execute_job 'printf test' printf '%s\n' 'line 1' 'line 2'

... to print line 1 and line 2 on two separate lines (or any other command that has arguments with spaces, tabs, newlines, or globbing characters).

Why must I not quote a string variable when running it as a command?

2 Answers2