Mantis - Quercus
Viewing Issue Advanced Details
2356 major always 01-22-08 15:20 01-23-08 22:27
rvt  
nam  
normal  
closed  
fixed  
none    
none 3.1.5  
0002356: strcmp function fails with comparing comparing empty strings with numbers
strcmp fails when comparing numbers and empty strings.

Example:

strcmp('0', '') returns 0 while it should return 1

ey Guys,
(all based on latest SVN version of 19 jan 2008)

I created the following test:

resin Results:
strcmp('a', 'a') : 0
strcmp('b', 'a') : 1
strcmp('a', 'b') : -1
strcmp('', '0') : 0
strcmp(' ', '0') : 0
strcmp('', 'a') : -1
strcmp('', '') : 0
strcmp('0', '') : 0
strcmp('00', '') : 0
strcmp('10', '') : 1
strcmp('1', '1') : 0
strcmp('1', '2') : -1
strcmp('2', '1') : 1
strcmp('-1', '-2') : 0
strcmp('-2', '-1') : -1
strcmp('2', '2') : 1

mod_php results:
strcmp('a', 'a') : 0
strcmp('b', 'a') : 1
strcmp('a', 'b') : -1
strcmp('', '0') : -1
strcmp(' ', '0') : -16
strcmp('', 'a') : -1
strcmp('', '') : 0
strcmp('0', '') : 1
strcmp('00', '') : 2
strcmp('10', '') : 2
strcmp('1', '1') : 0
strcmp('1', '2') : -1
strcmp('2', '1') : 1
strcmp('-1', '-2') : 0
strcmp('-2', '-1') : -1
strcmp('2', '2') : 1


As you can see all evaluations between empty strings and numbers are wrong,
looking at the java code (see below the mail) it's clear that there is an test done
to see if both values can evaluate to a number (if (isNumberConvertible() || rValue.isNumberConvertible()) { )

However apparently an empty string or string with spaces is seen as a number 0 (nul).

To solve this issue properly would you feel that

1) isNumberConvertible() should return false on empty strings

2) isNumberConvertible() should return true on empty strings, but the cmp functions needs to get changed.

This is what I did to solve my problem for now, when I comment out the complete evaluation for numbers and do a true string compare,
this would be the output of resin (see code after:):

strcmp('a', 'a') : 0
strcmp('b', 'a') : 1
strcmp('a', 'b') : -1
strcmp('', '0') : -1
strcmp(' ', '0') : -1
strcmp('', 'a') : -1
strcmp('', '') : 0
strcmp('0', '') : 1
strcmp('00', '') : 1
strcmp('10', '') : 1
strcmp('1', '1') : 0
strcmp('1', '2') : -1
strcmp('2', '1') : 1
strcmp('-1', '-2') : 0
strcmp('-2', '-1') : -1
strcmp('2', '2') : 1


Now somebody did decide to check for numbers and do a comparison from there,
may be it was speed reasons? or any other behafure I mist have missed?

Let me know how you feel about this so I can propose a proper patch and an entry in bug trac later on,


kind regards,
Ries van Twisk





before:
  public int cmp(Value rValue)
  {
    if (isNumberConvertible() || rValue.isNumberConvertible()) {
      double l = toDouble();
      double r = rValue.toDouble();
                                                                                                                                                                                                                                                         
      if (l == r)
        return 0;
      else if (l < r)
        return -1;
      else
        return 1;
    }
    else {
      int result = toString().compareTo(rValue.toString());
                                                                                                                                                                                                                                                         
      if (result == 0)
        return 0;
      else if (result > 0)
        return 1;
      else
        return -1;
    }
  }



after (always compare true strings):
  public int cmp(Value rValue)
  {
                                                                                                                                                                                                
      int result = toString().compareTo(rValue.toString());
                                                                                                                                                                                                                                                         
      if (result == 0)
        return 0;
      else if (result > 0)
        return 1;
      else
        return -1;
  }


Notes
(0002674)
nam   
01-23-08 22:27   
php/110h