[
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)
