mjhjmtu

I have a question on how I'm adding arguments when executing a shell script.

I have a simple script which helps me block ip ranges:

~/block_ip.sh:

if [ ! $3 ]
then
 echo "usage ~/block_ip.sh (DROP/ACCEPT) '0.0.0.0' 'amsterdam'"
 exit 1
fi

echo "adding $3"
sudo iptables -I INPUT -s $2 -j $1 -m comment --comment "$3"

If I execute this without arguments the output is as expected:

~$ ./block_ip.sh
usage ~/block_ip.sh (DROP/ACCEPT) '0.0.0.0' 'amsterdam'

However the spaces seem to be the cause of the unexpected output of "binary operator expected":

~$ ./block_ip.sh DROP '1.0.0.0/8' 'south brisbane qld'
./block_ip.sh: line 1: [: brisbane: binary operator expected
adding south brisbane au

But then it adds it despite the unexpected output:

Chain INPUT (policy ACCEPT)
num target prot opt source destination
1 DROP all -- 1.0.0.0/8 anywhere /* south brisbane au */

If this is a quoting issue, how do I form the arguments (without using backslashes to escape the spaces)? Of course, I expect I may need a change to the script, that is an acceptable solution, too.

Yep, that's a quoting issue: [ ! $3 ] expands to [ ! south brisbane qld ] (four arguments between [ and ]). And when it sees four arguments, with the first one a !, [ expects to see something like [ ! arg1 op arg2 ] where op is a binary operator. (this, again is one of the things different between [ .. ] and [[ .. ]]; see this Q and also this Q)

brisbane isn't a valid operator, so it complains and returns 2, which is falsy so the statements inside the if are not executed. To tell the difference between an error and a regular failing test, you'd need to explicitly test the return value against 2.

On the other hand, if $3 is empty, then the test becomes [ ! ], a one-argument test that checks if the only argument is nonempty (it is, it's the one-character string !). In that case, it works as intended, though perhaps not for the reason you'd expect.

You want [ ! "$3" ] or [ -z "$3" ] to keep the string as one argument for [.

Of course you could also invert the sense of the test, and do the actual work inside the if, so that an error in the test would avoid running the main commands. But that would make the code structure a bit more unclear.