[ https://issues.apache.org/jira/browse/JEXL-366?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17532645#comment-17532645 ]
Hussachai Puripunpinyo commented on JEXL-366: --------------------------------------------- [~henrib] Can you shed some light on why you want to avoid using BigDecimal? Your change has a flaw when it fallbacks to String comparison. The following method truncate the decimal point. {code:java} private long comparableLong(Object arg) throws NumberFormatException { if (arg instanceof String) { String s = (String) arg; return s.isEmpty()? 0 :(long) Double.parseDouble((String) arg); } else { return toLong(arg); } } {code} There is a bug when one operand is a string with decimal and another one is a numerable. For example "1.01" == 1 This will return true for your change when it should not. > Fail to evaluate string and number comparison > --------------------------------------------- > > Key: JEXL-366 > URL: https://issues.apache.org/jira/browse/JEXL-366 > Project: Commons JEXL > Issue Type: Improvement > Affects Versions: 3.2.1 > Reporter: Hussachai Puripunpinyo > Assignee: Henri Biestro > Priority: Major > Fix For: 3.3 > > > 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. > {code:java} > 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; > } {code} > 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 > {code:java} > 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); > > } {code} > -- This message was sent by Atlassian Jira (v8.20.7#820007)