[JEXL-366] Fail to evaluate string and number comparison

• Type: Improvement
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 3.2.1
• Fix Version/s: 3.3
• Assignee: Henri Biestro
• Reporter: Hussachai Puripunpinyo
• Created: 26/Apr/22 05:55
• Updated: 27/Mar/23 10:59
• Resolved: 06/May/22 11:29

Description

The comparison logic doesn't cover the case when one operand is a string and another operand is a numerable type (int, short, long,..).

The expected result for '1.0' == 1 should be true but it fails because the string comparison check is after the numerable type check. JEXL tries to parse '1.0' using toLong function and it fails with this error message `For input string: "1.0"`

Moving a string comparison up before other number type checks will not cover some corner cases such as
'1.00' == 1.0 // String comparison will yield false but it obviously doesn't make sense.

The proposed change is to add the following code to handle the corner case when one operand is string and another operand is numerable. To cover this corner case, we can apply toBigDecimal to both lhs and rhs since it should cover any arbitrary number in a string form, and it handles other number types well.

if (isNumberable(left) || isNumberable(right)) {
    if (left instanceof String || right instanceof String) {
        final BigDecimal l = toBigDecimal(left);
        final BigDecimal r = toBigDecimal(right);
        return l.compareTo(r);
    } else {
        // this code block remains the same
    }
    return 0;
} 

JEXL syntax is very similar to ECMA script except a few small set that are not the same. So, I think following the ECMA spec for this comparison check makes sense.

The following code is JavaScript and it can be used in the JEXL test to make sure that the behavior of comparison are the same.
Note that '1.0' == 1 yields true

function assert(condition, source) {
    if (!condition) {
        throw `Assertion failed for ${source}`;
    }
}
  // Basic compare
let exprs = [
  "1 == 1", true,
  "1 != 1", false,
  "1 != 2", true,
  "1 > 2", false,
  "1 >= 2", false,
  "1 < 2", true,
  "1 <= 2", true,
  // Int <-> Float Coercion
  "1.0 == 1", true,
  "1 == 1.0", true,
  "1.1 != 1", true,
  "1.1 < 2", true,
  // numbers and strings
  "'1' == 1", true,
  "'' == 0", true, // empty string is coerced to zero (ECMA compliance)
  "1.0 >= '1'", true,
  "1.0 > '1'", false
];for (e = 0; e < exprs.length; e += 2) {
  let stext = exprs[e];
  let expected = exprs[e + 1];
  assert(eval(stext) == expected, stext);
  
} 

 

Issue Links

• links to: GitHub Pull Request #92