From 0d12dfd06f69611ba47f797cb114139a52b3a59b Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Fri, 25 Sep 2015 21:47:30 -0400 Subject: [PATCH] Improve shell quoting hygiene Most of these problems were found by ShellCheck (http://www.shellcheck.net). Signed-off-by: Anders Kaseorg --- bin/write-rabbitmq-consumers-state-file | 4 ++-- .../files/munin-plugins/humbug_send_receive | 2 +- .../check_email_deliverer_backlog | 4 ++-- .../check_email_deliverer_process | 6 ++--- .../files/nagios_plugins/check_worker_memory | 2 +- .../zulip_internal/files/postgresql/env-wal-e | 4 ++-- scripts/setup/configure-rabbitmq | 4 ++-- scripts/setup/initialize-database | 2 +- scripts/setup/install | 2 +- scripts/upgrade-zulip | 2 +- tools/build-deb | 16 ++++++------- tools/build-release-tarball | 24 +++++++++---------- tools/clean-branches | 10 ++++---- tools/deploy-branch | 16 ++++++------- tools/deployment-lock-ctl | 14 +++++------ .../iframe-bot/last-messages-wrapper.sh | 2 +- tools/deprecated/include-supported.example | 2 +- tools/deprecated/install-server | 18 +++++++------- tools/do-destroy-rebuild-test-database | 2 +- tools/generate-fixtures | 2 +- tools/postgres-init-db | 16 ++++++------- tools/postgres-init-test-db | 2 +- tools/test-js-with-node | 6 ++--- 23 files changed, 81 insertions(+), 81 deletions(-) diff --git a/bin/write-rabbitmq-consumers-state-file b/bin/write-rabbitmq-consumers-state-file index 61a72657ee1df..a84c60be5ede7 100755 --- a/bin/write-rabbitmq-consumers-state-file +++ b/bin/write-rabbitmq-consumers-state-file @@ -11,5 +11,5 @@ ZULIP_DIR=/home/zulip/deployments/current STATE_DIR=/var/lib/nagios_state STATE_FILE=$STATE_DIR/check-rabbitmq-consumers-$queue -$ZULIP_DIR/bots/check-rabbitmq-consumers --queue=$queue &> ${STATE_FILE}-tmp; -mv ${STATE_FILE}-tmp $STATE_FILE +"$ZULIP_DIR/bots/check-rabbitmq-consumers" "--queue=$queue" &> "${STATE_FILE}-tmp"; +mv "${STATE_FILE}-tmp" "$STATE_FILE" diff --git a/puppet/zulip_internal/files/munin-plugins/humbug_send_receive b/puppet/zulip_internal/files/munin-plugins/humbug_send_receive index ab2ebe898e3ca..0dd5b6573a4fb 100755 --- a/puppet/zulip_internal/files/munin-plugins/humbug_send_receive +++ b/puppet/zulip_internal/files/munin-plugins/humbug_send_receive @@ -4,4 +4,4 @@ if [ "$(hostname)" = "staging.zulip.net" ]; then else site="https://api.zulip.com" fi -/home/zulip/deployments/current/bots/check_send_receive.py --munin $1 --site="$site" +/home/zulip/deployments/current/bots/check_send_receive.py --munin "$1" --site="$site" diff --git a/puppet/zulip_internal/files/nagios_plugins/check_email_deliverer_backlog b/puppet/zulip_internal/files/nagios_plugins/check_email_deliverer_backlog index 7558cddf20f46..9c65fb2afafa9 100755 --- a/puppet/zulip_internal/files/nagios_plugins/check_email_deliverer_backlog +++ b/puppet/zulip_internal/files/nagios_plugins/check_email_deliverer_backlog @@ -9,11 +9,11 @@ cd /home/zulip/deployments/current BACKLOG=$(./manage.py print_email_delivery_backlog) -if [ $BACKLOG -gt 0 -a $BACKLOG -lt 10 ] +if [ "$BACKLOG" -gt 0 ] && [ "$BACKLOG" -lt 10 ] then echo "backlog of $BACKLOG" exit 1 -elif [ $BACKLOG -ge 10 ] +elif [ "$BACKLOG" -ge 10 ] then echo "backlog of $BACKLOG" exit 2 diff --git a/puppet/zulip_internal/files/nagios_plugins/check_email_deliverer_process b/puppet/zulip_internal/files/nagios_plugins/check_email_deliverer_process index b2fd6ede137ec..518a3fb0033bf 100755 --- a/puppet/zulip_internal/files/nagios_plugins/check_email_deliverer_process +++ b/puppet/zulip_internal/files/nagios_plugins/check_email_deliverer_process @@ -13,14 +13,14 @@ if [ "$STATUS" == "RUNNING" ] then echo "Running" exit 0 -elif [ $(echo "$STATUS" | egrep '(STOPPED)|(STARTING)|(BACKOFF)|(STOPPING)|(EXITED)|(FATAL)|(UNKNOWN)$') ] +elif [ "$(echo "$STATUS" | egrep '(STOPPED)|(STARTING)|(BACKOFF)|(STOPPING)|(EXITED)|(FATAL)|(UNKNOWN)$')" ] then # not "RUNNING", but a recognized supervisor status - echo $STATUS + echo "$STATUS" exit 1 else # we don't recognize the second column in this SUPERVISOR_STATUS. # This may be indicative of a supervisor configuration problem - echo $SUPERVISOR_STATUS + echo "$SUPERVISOR_STATUS" exit 3 fi diff --git a/puppet/zulip_internal/files/nagios_plugins/check_worker_memory b/puppet/zulip_internal/files/nagios_plugins/check_worker_memory index ac2f704572c41..a0f602613a113 100755 --- a/puppet/zulip_internal/files/nagios_plugins/check_worker_memory +++ b/puppet/zulip_internal/files/nagios_plugins/check_worker_memory @@ -1,7 +1,7 @@ #!/bin/bash # Checks for any Zulip queue workers that are leaking memory and thus have a high vsize datafile=$(mktemp) -ps -o vsize,size,pid,user,command --sort -vsize $(pgrep -f '^python /home/zulip/deployments/current/manage.py process_queue') > "$datafile" +ps -o vsize,size,pid,user,command --sort -vsize "$(pgrep -f '^python /home/zulip/deployments/current/manage.py process_queue')" > "$datafile" cat "$datafile" top_worker=$(cat "$datafile" | head -n2 | tail -n1) top_worker_memory_usage=$(echo "$top_worker" | cut -f1 -d" ") diff --git a/puppet/zulip_internal/files/postgresql/env-wal-e b/puppet/zulip_internal/files/postgresql/env-wal-e index d9b2061d8d228..32dfe7471de0f 100755 --- a/puppet/zulip_internal/files/postgresql/env-wal-e +++ b/puppet/zulip_internal/files/postgresql/env-wal-e @@ -5,9 +5,9 @@ fi export AWS_ACCESS_KEY_ID=xxxxxxxxxxxxxxxxxxxx export AWS_SECRET_ACCESS_KEY=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx -s3_backup_bucket=$(crudini --get $ZULIP_CONF database s3_backup_bucket 2>&1) +s3_backup_bucket=$(crudini --get "$ZULIP_CONF" database s3_backup_bucket 2>&1) if [ $? -ne 0 ]; then - echo "Could not determine which s3 bucket to use:" $s3_backup_bucket + echo "Could not determine which s3 bucket to use:" "$s3_backup_bucket" exit 1 fi export WALE_S3_PREFIX=s3://$s3_backup_bucket diff --git a/scripts/setup/configure-rabbitmq b/scripts/setup/configure-rabbitmq index 23ef13ed5c7a3..5400a1ae92ee6 100755 --- a/scripts/setup/configure-rabbitmq +++ b/scripts/setup/configure-rabbitmq @@ -3,9 +3,9 @@ # Delete the "guest" default user and replace it with a Zulip user # with a real password -RMQPW=$($(dirname $0)/../../bin/get-django-setting RABBITMQ_PASSWORD) +RMQPW=$("$(dirname "$0")/../../bin/get-django-setting" RABBITMQ_PASSWORD) sudo rabbitmqctl delete_user zulip || true sudo rabbitmqctl delete_user guest || true -sudo rabbitmqctl add_user zulip $RMQPW +sudo rabbitmqctl add_user zulip "$RMQPW" sudo rabbitmqctl set_user_tags zulip administrator sudo rabbitmqctl set_permissions -p / zulip '.*' '.*' '.*' diff --git a/scripts/setup/initialize-database b/scripts/setup/initialize-database index d7721fecf8696..ff16e60292256 100755 --- a/scripts/setup/initialize-database +++ b/scripts/setup/initialize-database @@ -1,7 +1,7 @@ #!/bin/bash -xe # Change to root directory of the checkout that we're running from -cd $(dirname $0)/../.. +cd "$(dirname "$0")/../.." python manage.py checkconfig diff --git a/scripts/setup/install b/scripts/setup/install index 63e2ce3960aa6..5d87e93518fbe 100755 --- a/scripts/setup/install +++ b/scripts/setup/install @@ -1,3 +1,3 @@ #!/bin/bash mkdir -p /var/log/zulip -$(dirname $(dirname $0))/lib/install "$@" 2>&1 | tee -a /var/log/zulip/install.log +"$(dirname "$(dirname "$0")")/lib/install" "$@" 2>&1 | tee -a /var/log/zulip/install.log diff --git a/scripts/upgrade-zulip b/scripts/upgrade-zulip index 4cb21db76fe0c..42c0267f24cf3 100755 --- a/scripts/upgrade-zulip +++ b/scripts/upgrade-zulip @@ -1,2 +1,2 @@ #!/bin/bash -$(dirname $0)/lib/upgrade-zulip "$@" 2>&1 | tee -a /var/log/zulip/upgrade.log +"$(dirname "$0")/lib/upgrade-zulip" "$@" 2>&1 | tee -a /var/log/zulip/upgrade.log diff --git a/tools/build-deb b/tools/build-deb index 2e94e62ca15af..36e8c6c193e21 100755 --- a/tools/build-deb +++ b/tools/build-deb @@ -12,7 +12,7 @@ dist=$1 file=$2 if [ -z "$dist" ] || [ -z "$file" ]; then - echo "$(echo $0 | rev | cut -d "/" -f 1-1 | rev) -- build Debian packages on a Zulip buildslave" + echo "$(echo "$0" | rev | cut -d "/" -f 1-1 | rev) -- build Debian packages on a Zulip buildslave" echo echo "USAGE: $0 dist path/to/package.dsc" exit 1 @@ -21,10 +21,10 @@ fi set -xe ret=0 -path=$(ssh -q -l $BUILDD_USERNAME $BUILDD_HOST -- mktemp -d $BUILDD_BASE_PATH/$USER.`date -u +%F.%R`.XXXXXXX)/ +path=$(ssh -q -l "$BUILDD_USERNAME" "$BUILDD_HOST" -- "mktemp -d '$BUILDD_BASE_PATH/$USER.`date -u +%F.%R`.XXXXXXX'")/ -dcmd rsync -vz --copy-links $file $BUILDD_USERNAME@$BUILDD_HOST:$path/ -file=$(basename $file) +dcmd rsync -vz --copy-links "$file" "$BUILDD_USERNAME@$BUILDD_HOST:$path/" +file=$(basename "$file") # -A specifies to build arch-all packages (non-arch dependent) in addition to # binary packages @@ -38,9 +38,9 @@ file=$(basename $file) # # We always build for amd64. There is no 32-bit. -ssh -t -l $BUILDD_USERNAME $BUILDD_HOST -- "cd $path && sbuild -A -s --force-orig-source --dist=$dist --arch=amd64 $file" +ssh -t -l "$BUILDD_USERNAME" "$BUILDD_HOST" -- "cd '$path' && sbuild -A -s --force-orig-source '--dist=$dist' --arch=amd64 '$file'" -rsync -Lvz --copy-links $BUILDD_USERNAME@$BUILDD_HOST:$path/* . -ssh -l $BUILDD_USERNAME $BUILDD_HOST rm -r $path +rsync -Lvz --copy-links "$BUILDD_USERNAME@$BUILDD_HOST:$path/*" . +ssh -l "$BUILDD_USERNAME" "$BUILDD_HOST" rm -r "$path" -exit $ret +exit "$ret" diff --git a/tools/build-release-tarball b/tools/build-release-tarball index 31cc6551cdccc..5d4195203311f 100755 --- a/tools/build-release-tarball +++ b/tools/build-release-tarball @@ -11,8 +11,8 @@ prefix="zulip-server-$version" if [ "$(uname)" = "Darwin" ]; then TMPDIR=/tmp/voyager-build - rm -Rf $TMPDIR - mkdir -p $TMPDIR + rm -Rf "$TMPDIR" + mkdir -p "$TMPDIR" else TMPDIR=$(mktemp -d) fi @@ -27,10 +27,10 @@ TMP_CHECKOUT=$TMPDIR/$prefix/ TARBALL=$TMPDIR/$prefix.tar # .gitattributes lists the files that are not exported -git archive -o $TARBALL --prefix=$prefix/ HEAD +git archive -o "$TARBALL" "--prefix=$prefix/" HEAD -if tar -tf $TARBALL | grep -q -e zilencer -e zproject/local_settings.py -e puppet/zulip_internal; then +if tar -tf "$TARBALL" | grep -q -e zilencer -e zproject/local_settings.py -e puppet/zulip_internal; then echo "Excluded files remain in tarball!"; echo "Versions of git 1.8.1.1 - 1.8.1.6 have broken .gitattributes syntax"; exit 1; @@ -39,8 +39,8 @@ else fi # Check out a temporary full copy of the index to generate static files -git checkout-index -f -a --prefix $TMP_CHECKOUT -cd $TMP_CHECKOUT +git checkout-index -f -a --prefix "$TMP_CHECKOUT" +cd "$TMP_CHECKOUT" # Use default settings so there is no chance of leaking secrets cp zproject/local_settings_template.py zproject/local_settings.py @@ -70,8 +70,8 @@ echo; echo "\033[33mRunning update-prod-static; check ${TMP_CHECKOUT}update-prod set -x ./tools/update-prod-static -echo $GITID > build_id -echo $version > version +echo "$GITID" > build_id +echo "$version" > version mv update-prod-static.log .. rm -f zproject/dev-secrets.conf @@ -79,11 +79,11 @@ rm -f zproject/dev-secrets.conf # We don't need duplicate copies of emoji with hashed paths, and they would break bugdown find prod-static/serve/third/gemoji/images/emoji/ -regex '.*\.[0-9a-f]+\.png' -delete -cd $TMPDIR +cd "$TMPDIR" -tar --append -f $TARBALL $prefix/prod-static $prefix/build_id $prefix/version +tar --append -f "$TARBALL" "$prefix/prod-static" "$prefix/build_id" "$prefix/version" -rm -rf $prefix +rm -rf "$prefix" -gzip $TARBALL +gzip "$TARBALL" echo "Generated $TARBALL.gz" diff --git a/tools/clean-branches b/tools/clean-branches index f82fe249d5c94..a480f4ca10809 100755 --- a/tools/clean-branches +++ b/tools/clean-branches @@ -4,7 +4,7 @@ # and also any branches in origin which are ancestors of # origin/master and are named like $USER-*. -push_args='' +push_args=() function is_merged { ! git rev-list -n 1 origin/master.."$1" | grep -q . @@ -31,7 +31,7 @@ function clean_ref { echo -n "Deleting remote branch $remote_name" echo " (was $(git rev-parse --short "$ref"))" # NB: this won't handle spaces in ref names - push_args="$push_args :$remote_name" + push_args=("${push_args[@]}" ":$remote_name") fi ;; esac @@ -44,8 +44,8 @@ fi git fetch --prune origin -eval $(git for-each-ref --shell --format='clean_ref %(refname);') +eval "$(git for-each-ref --shell --format='clean_ref %(refname);')" -if [ -n "$push_args" ]; then - git push origin $push_args +if [ "${#push_args}" -ne 0 ]; then + git push origin "${push_args[@]}" fi diff --git a/tools/deploy-branch b/tools/deploy-branch index 5f2bc93557b1d..4755f2ce0cfb0 100755 --- a/tools/deploy-branch +++ b/tools/deploy-branch @@ -2,7 +2,7 @@ function error_out { echo -en '\e[0;31m' - echo $1 + echo "$1" echo -en '\e[0m' exit 1 } @@ -12,16 +12,16 @@ status=$(git status --porcelain | grep -v '^??') old_ref=$(git rev-list --max-count=1 HEAD) branch=$1 -branch_ref=$(git rev-list --max-count=1 $branch) +branch_ref=$(git rev-list --max-count=1 "$branch") [ $? -ne 0 ] && error_out "Unknown branch: $branch" if [ "$old_ref" == "$branch_ref" ]; then new_ref=master else - ref_name=$(git describe --all --exact $old_ref) + ref_name=$(git describe --all --exact "$old_ref") if [ $? -eq 0 ]; then - new_ref=$(echo $ref_name | perl -pe 's{^(heads|remotes)/}{}') + new_ref=$(echo "$ref_name" | perl -pe 's{^(heads|remotes)/}{}') else new_ref=$old_ref fi @@ -31,13 +31,13 @@ fi git fetch -p -git rebase origin/master $branch +git rebase origin/master "$branch" [ $? -ne 0 ] && error_out "Rebase onto origin/master failed" git push . HEAD:master git push origin master [ $? -ne 0 ] && error_out "Push of master to origin/master failed" -git checkout $new_ref -git branch -D $branch -git push origin :$branch +git checkout "$new_ref" +git branch -D "$branch" +git push origin ":$branch" diff --git a/tools/deployment-lock-ctl b/tools/deployment-lock-ctl index 00ecbe6df6465..1aba340b47377 100755 --- a/tools/deployment-lock-ctl +++ b/tools/deployment-lock-ctl @@ -15,23 +15,23 @@ exit 1 } -if [[ $# < 1 || $# >2 ]]; then +if [[ $# -lt 1 || $# -gt 2 ]]; then usage fi function get_status { - if (ssh zulip@$1.zulip.net '[ -d "'"$lockdir"'" ]'); then + if (ssh "zulip@$1.zulip.net" '[ -d "'"$lockdir"'" ]'); then printf "%-10s %b" "$1" "is currently \e[31mlocked\e[0m\n" else printf "%-10s %b" "$1" "is currently \e[32munlocked\e[0m\n" fi } -if [[ $1 == "lock" ]]; then +if [[ "$1" == "lock" ]]; then verb="mkdir" -elif [[ $1 == "unlock" ]]; then +elif [[ "$1" == "unlock" ]]; then verb="rmdir" -elif [[ $# == 1 && $1 == "status" ]]; then +elif [[ $# == 1 && "$1" == "status" ]]; then get_status "staging" get_status "prod0" exit @@ -40,11 +40,11 @@ else fi -if [[ $2 == "staging" ]]; then +if [[ "$2" == "staging" ]]; then ssh zulip@staging.zulip.net "$verb $lockdir" get_status "staging" exit -elif [[ $2 == "prod" ]]; then +elif [[ "$2" == "prod" ]]; then ssh zulip@prod0.zulip.net "$verb $lockdir" get_status "prod0" exit diff --git a/tools/deprecated/iframe-bot/last-messages-wrapper.sh b/tools/deprecated/iframe-bot/last-messages-wrapper.sh index 3ce8e57cdd66d..d03aa988a0df9 100755 --- a/tools/deprecated/iframe-bot/last-messages-wrapper.sh +++ b/tools/deprecated/iframe-bot/last-messages-wrapper.sh @@ -11,7 +11,7 @@ COUNT=50 mkdir -p output while true; do -if python show-last-messages --api-key=$API_KEY --user=$BOT_EMAIL --streams="$STREAMS" --count=$COUNT; then +if python show-last-messages --api-key="$API_KEY" --user="$BOT_EMAIL" --streams="$STREAMS" --count="$COUNT"; then echo "[`date`] Success"; mv output-candidate.html output/zulip.html touch output/zulip.html diff --git a/tools/deprecated/include-supported.example b/tools/deprecated/include-supported.example index 48fc51a9c79d5..ae2d6e5d70c40 100644 --- a/tools/deprecated/include-supported.example +++ b/tools/deprecated/include-supported.example @@ -21,5 +21,5 @@ if [ $# = 2 ]; then fi for dist in $DISTS; do - reprepro --ignore=wrongdistribution include$type $dist "$1" + reprepro --ignore=wrongdistribution "include$TYPE" "$dist" "$1" done diff --git a/tools/deprecated/install-server b/tools/deprecated/install-server index 51b547b64646b..404b47112f563 100755 --- a/tools/deprecated/install-server +++ b/tools/deprecated/install-server @@ -8,7 +8,7 @@ if [ -z "$hostname" ]; then echo "USAGE: $0 server type hostname [branch]" exit 1 fi -if ! $(echo "$hostname" | grep -q zulip); then +if ! echo "$hostname" | grep -q zulip; then echo "USAGE: $0 server type hostname [branch]" echo "Hostname must have zulip in it." exit 1 @@ -35,11 +35,11 @@ fi # Force RSA keys. We do this because the ECDSA key is not printed on syslog, # and our puppet configuration does not use ECDSA. If we don't do this, # we'll get key errors after puppet apply. -SSH_OPTS="-o HostKeyAlgorithms=ssh-rsa" +SSH_OPTS=(-o HostKeyAlgorithms=ssh-rsa) set +e -ssh $SSH_OPTS "$server" -t -i "$amazon_key_file" -ladmin -o "ControlMaster=no" < /etc/hostname sed -i 's/localhost$/localhost $hostname/' /etc/hosts @@ -69,9 +69,9 @@ EOF # Give new server git access # TODO: Don't give servers push access to our git! -scp $SSH_OPTS -i "$amazon_key_file" "$server_private_key_file" root@"$server":/root/.ssh/id_rsa +scp "${SSH_OPTS[@]}" -i "$amazon_key_file" "$server_private_key_file" root@"$server":/root/.ssh/id_rsa -ssh $SSH_OPTS "$server" -t -i "$amazon_key_file" -lroot < /root/.ssh/known_hosts <> ~/.pgpass +if ! grep -q "$PGPASS_ESCAPED_PREFIX" ~/.pgpass; then + echo "$PGPASS_PREFIX$PASSWORD" >> ~/.pgpass else sed -i "s/$PGPASS_ESCAPED_PREFIX.*\$/$PGPASS_PREFIX$PASSWORD/" ~/.pgpass fi chmod go-rw ~/.pgpass -psql -h localhost postgres $USERNAME <