always quote shell arguments to test -z/-n

In shell code like:

test -z $foo
test -n $foo

that does not quote its arguments, it's easy to think that
it is actually looking at the contents of $foo in each case.
But if $foo is empty, then "test" does not see any argument
at all! The results are quite subtle.

POSIX specifies that test's behavior depends on the number
of arguments it sees, and if $foo is empty, it sees only
one. The behavior in this case is:

1 argument: Exit true (0) if $1 is not null; otherwise,
exit false.

So in the "-z $foo" case, if $foo is empty, then we check
that "-z" is non-null, and it returns success. Which happens
to match what we expected.  But for "-n $foo", if $foo is
empty, we'll see that "-n" is non-null and still return
success. That's the opposite of what we intended!

Furthermore, if $foo contains whitespace, we'll end up with
more than 2 arguments. The results in this case are
generally unspecified (unless the first part of $foo happens
to be a valid binary operator, in which case the results are
specified but certainly not what we intended).

And on top of this, even though "test -z $foo" _should_ work
for the empty case, some older shells (reportedly ksh88)
complain about the missing argument.

So let's make sure we consistently quote our variable
arguments to "test". After this patch, the results of:

git grep 'test -[zn] [^"]'

are empty.

Reported-by: Armin Kunaschik <megabreit@googlemail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 9ea30756f13349c96784f0091da4c3c667166485..470413b9f9b603b8f9fe8718607bf66f6c794cdf 100644 (file)
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -866,12 +866,12 @@ add_exec_commands () {
 # $3: the input filename
 check_commit_sha () {
        badsha=0
-       if test -z $1
+       if test -z "$1"
        then
                badsha=1
        else
                sha1_verif="$(git rev-parse --verify --quiet $1^{commit})"
-               if test -z $sha1_verif
+               if test -z "$sha1_verif"
                then
                        badsha=1
                fi

diff --git a/git-stash.sh b/git-stash.sh
index c7c65e25f50e7b558952a5180081ad499f2a90fc..c7509e8da47cf296143e3fa53a7f431f7672d78e 100755 (executable)
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -185,7 +185,7 @@ store_stash () {
 
        git update-ref --create-reflog -m "$stash_msg" $ref_stash $w_commit
        ret=$?
-       test $ret != 0 && test -z $quiet &&
+       test $ret != 0 && test -z "$quiet" &&
        die "$(eval_gettext "Cannot update \$ref_stash with \$w_commit")"
        return $ret
 }
@@ -277,7 +277,7 @@ save_stash () {
                        git clean --force --quiet -d $CLEAN_X_OPTION
                fi
 
-               if test "$keep_index" = "t" && test -n $i_tree
+               if test "$keep_index" = "t" && test -n "$i_tree"
                then
                        git read-tree --reset -u $i_tree
                fi

diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index ea5ace99a14f7f31b63dfd9cf25123725a525519..9473c2779ef0df109f6943a949583676ea5ceabf 100755 (executable)
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh
@@ -82,7 +82,7 @@ test_expect_success 'am -3 --abort removes otherfile-4' '
        test 4 = "$(cat otherfile-4)" &&
        git am --abort &&
        test_cmp_rev initial HEAD &&
-       test -z $(git ls-files -u) &&
+       test -z "$(git ls-files -u)" &&
        test_path_is_missing otherfile-4
 '