sshguard fails to monitor logs unless listed on command line

[Steve Wardle]

OS is macOS Caalina 10.15.5 SSHGUARD version 2.4.1 Either invocation method produces the same result.

If FILES or LOGREADER is specified in the configuration file rather than on the command line the variable $tailcmd is set to "/usr/local/libexec/sshg-logtail ".

The problem seems to be the evaluation of [ -n "$@" ] in the sshguard script. This returns true even though "$@" is empty. Reverting to [ ! -z "$@" ] fixes the problem.

I also tried this test which gives the same result on both macOS and Centos 8:

if [ ! -z "$@" ]; then echo "true"; fi
if [ -n "$@" ]; then echo "true"; fi
true

‌

[Kevin Zheng]

My apologies for taking so long to get around to this issue.

Huh, I thought -n was just the opposite of -z? Do you happen to be using the same shell on Catalina and CentOS?

[Steve Wardle]

On all my systems I use the defaults for sh.

On macOS Catalina (10.15) sh invokes bash version 3.2

On Scientific Linux 6 sh invokes bash version 4.1.

On Scientific Linux 7 sh invokes bash version 4.2.

On Centos 8 sh invokes bash version 4.4.

All behave the same way.

Steve

‌

[Kevin Zheng]

I’ve pushed 9740689 replacing all ‘-n’ with ‘! -z’. I don’t really understand what’s going on yet, but until then this breakage should be reverted. Does this fix the problems you reported?

[Steve Wardle]

I fixed it by replacing ‘-n’ with ‘! -z’ only in the test for $@ which that seems to behave differently from the other tests using ‘-n’ which work as expected.

The new version does work as expected, thanks.

Steve

[Kevin Zheng]

Thanks for checking, and thanks for the clarification. I really wonder why $@ is special…

[Lennart Lövstrand]

Hi, I realize that this bug already has been resolved, but I’d like to add a few comments to explain why it works as it does and perhaps convince you that there may be a better way to solve the problem. The reason why [ -n "$@" ] succeeds (i.e. returns true) when there are no arguments is because "$@" doesn’t expand into the empty string – it expands into “nothing.” That is, [ -n "$@" ] turns into [ -n ], not [ -n "" ]. Since [ -n ] is not a valid test expression, the result (AFAIK) is undefined. In this case it turned out to be true, but it might as well have been false in another the implementation.

What I presume you indented to do is [ -n "$*" ]. That does indeed turn into [ -n "" ] when there are no arguments and thus evaluates to false. However, since “$*” may do a lot of unnecessary string concatenations and also doesn’t account for the case when someone explicitly supplies an empty string as an argument, an arguably better way to test for the actual absence of arguments is to do [ $# -eq 0 ], i.e. count the arguments and see if the result is zero. You could also do [ $# = 0 ] if you prefer, although that technically is comparing two strings (“0”) instead of numeric values.

I’d suggest re-fixing this bug as [ ! -z "$@" ] expands into [ ! -z ], which also is an invalid test expression, but just so happens to return false. Let me know if you’d like me to provide you with a diff or submit a pull request, I’m preparing one for another bug anyway and could make one for this case too.

[BTW: My apologies if I’m being overly explanatory. You probably know this already, but just didn’t think of it. I know, it’s really easy to overlook.]