ast icon indicating copy to clipboard operation
ast copied to clipboard

Should `test -t` with no arg be supported?

Open krader1961 opened this issue 6 years ago • 3 comments

I was mildly surprised to find this block of code was not covered by a unit test since we had explicit test -t tests:

https://github.com/att/ast/blob/1a5e6ef0f12ae4afabaf4763833812e7364ca772/src/cmd/ksh93/bltins/test.c#L302-L315

After looking at the code I realized the code in the if (cp) block would only be executed by doing something like test -t 1 -a -t 1. The problem is that omitting the file descriptor as implied by the source code as being acceptable produces inconsistent results. Basically, this undocumented "feature" only works correctly if -t is the final, or only, term in a test expression and is missing its argument.

Note too that bundling the value (e.g., test -t1) is also silently, and incorrectly, handled.

krader1961 avatar Jul 09 '19 05:07 krader1961

edited my comment because I misread the original question.

I would say no, definitely not. ksh93 (as I just discovered thanks to your question) is the only shell where test -t is taken as test -t 1, but this is incompatible with POSIX, where test -t is just a case of test arbitrary_string which tests if the arbitrary string is empty – an equivalent of test -n arbitrary_string. test -t is therefore equivalent to test -n '-t', test -t1 is equivalent to test -n '-t1', etc.

This is just one of the many well-known POSIX test/[ pitfalls that all shells aiming for POSIX compatibility must keep supporting. The test command is inherently broken, but can never be fixed. This is why David Korn invented the[[ command as a reserved word to eliminate inherent ambiguities (available only in bash, *ksh, zsh, and in yash as of very recently). It's also why in my portable shell library, modernish, I've gone a completely different route for POSIX shells.

Anyway, I'm afraid POSIX breakage must be restored here because the alternative is actually worse; now you cannot do [ "$var" ] for arbitrary values of var! If $var happens to be -t you get an unintended test for stdout on a terminal.

By the way, note that ksh93 does not handle test -t1 the way you think it does: this it handles as testing the emptiness of the string -t1.

$ ksh -c 'test -t' >/dev/null; echo $?
1
$ ksh -c 'test -t1' >/dev/null; echo $?
0

McDutchie avatar Jul 12 '19 01:07 McDutchie

I'll need to look at the code again with the instrumentation I put in place but I'm pretty sure test -t1 is handled by the logic that handles test -t $fd. Which means it is definitely broken but perhaps not precisely in the way I stated. Note that this is because of another anti-pattern that occurs too often in this code. In this case it's this statement: if (c2_eq(arg, '-', 't')). Elsewhere you'll see it spelled out explicitly like this without the aid of a macro: if (arg[0] == '-' && arg[1] == 't'). And in many of the cases I've looked at the code does not correctly handle the case where the string is shorter or longer than expected. The authors were unwisely trying to shave a few microseconds off the run time by avoiding strcmp().

krader1961 avatar Jul 12 '19 01:07 krader1961

Relabeled as a compatibility bug since, as @McDutchie points out, doing test -t is supposed to be treated like test -n -t according to POSIX. Similarly for test -t1. No way to know how long this has been broken but it goes back at least 12 years to ksh93u+.

krader1961 avatar Jul 12 '19 01:07 krader1961