1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-03-09 15:50:02 +00:00

test/[/[[: Fix incorrect 0-skipping, float precision (re: c734568b)

The arguments to the binary numerical comparison operators (-eq,
-gt, etc.) in the [[ and test/[ commands are treated as arithmetic
expressions, even if $((...)) is not used. But there is some
seriously incorrect behaviour:

Reproducers (all should output 0/true):

    $ [[ 0x0A -eq 10 ]]; echo $?
    1
    $ [[ 1+0x0A -eq 11 ]]; echo $?
    0
    $ (set --posix; [[ 010 -eq 8 ]]); echo $?
    1
    $ (set --posix; [[ +010 -eq 8 ]]); echo $?
    0
    $ [[ 0xA -eq 10 ]]; echo $?
    1
    $ xA=10; [[ 0xA -eq 10 ]]; echo $?
    0
    $ xA=WTF; [[ 0xA -eq 10 ]]; echo $?
    ksh: WTF: parameter not set

(POSIX mode enables the recognition of the initial 0 as a prefix
indicating an octal number in arithmetic expressions.)

The cause is the two 'while' loops in this section in test_binop()
in src/cmd/ksh93/bltins/test.c:

502:	if(op&TEST_ARITH)
503:	{
504:		while(*left=='0')
505:			left++;
506:		while(*right=='0')
507:			right++;
508:		lnum = sh_arith(left);
509:		rnum = sh_arith(right);
510:	}

So initial zeros are unconditionally skipped. Ostensibly this is to
disable the recognition of the initial zero as an octal prefix as
well as 0x as a hexadecimal prefix. This would be okay for
enforcing a decimal-only limitation for simple numbers, but to do
this for arithmetic expressions is flagrantly incorrect, to say the
least. (insert standard rant about AT&T quality standards here)

The fix for '[[' is simply to delete the two 'while' loops. But
that creates a problem for the deprecated-but-standard 'test'/'['
command, which also uses the test_binop() function. According to
POSIX, test/[ only parses simple decimal numbers, so octal, etc. is
not a thing. But, after that fix, 'test 08 -eq 10' in POSIX mode
yields true, which is unlike every other shell. (Note that this is
unlike [[ 08 -eq 10 ]], which yields true on bash because '[['
parses operands as arithmetic expressions.)

For test/[ in non-POSIX mode, we don't need to change anything. For
POSIX mode, we should only parse literal decimal numbers for these
operators in test/[, disallowing unexpanded arithmetic expressions.
This makes ksh's POSIX-mode test/[ act like every other shell and
like external .../bin/test commands shipped by OSs.

src/cmd/ksh93/bltins/text.c: test_binop():
- Correct a type mismatch. The left and right hand numbers should
  be Sfdouble_t, the maximum double length on the current system,
  and the type that sh_arith() returns. Instead, long double
  (typeset -lF) values were reduced to double (typeset -F) before
  comparing!
- For test/[ in POSIX, only accept simple decimal numbers: use
  strtold() instead of sh_arith(). Do skip initial zeros here as
  this disables the recognition of the hexadecimal prefix. Throw an
  error on an invalid decimal number. Floating point constants are
  still parsed, but that's fine as this does not cause any
  incompatibility with POSIX.

- For code legibility, move handling of TEST_EQ, etc. to within the
  if(op&TEST_ARITH) block. This also allows lnum and rnum to be
  local to that block.
This commit is contained in:
Martijn Dekker 2022-07-26 19:04:33 +02:00
parent 82e6e60019
commit 592ce7415a
7 changed files with 89 additions and 25 deletions

9
NEWS
View file

@ -3,6 +3,15 @@ For full details, see the git log at: https://github.com/ksh93/ksh/tree/1.0
Any uppercase BUG_* names are modernish shell bug IDs.
2022-07-26:
- Fixed incorrect handling of initial zeros in test/[ and [[ arithmetic
comparison operators. For example, [[ 0x0A -eq 10 ]] yielded false but
[[ 1+0x0A -eq 11 ]] yielded true.
- Fixed comparing long floating point (typeset -lF) values in test/[ and [[;
the values were reduced to standard floating point before comparing.
2022-07-25:
- Release candidate: ksh 93u+m/1.0.0-rc.1.