Bug 57309 - Custom type conversion sometimes bypassed
Custom type conversion sometimes bypassed
Status: RESOLVED FIXED
Product: Tomcat 8
Classification: Unclassified
Component: EL
8.0.x-trunk
All All
: P2 normal with 5 votes (vote)
: ----
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2014-12-04 14:14 UTC by Mark Thomas
Modified: 2014-12-06 16:43 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Thomas 2014-12-04 14:14:44 UTC
As per section 1.23 of the EL spec, the current ELResolver should be given an opportunity to apply custom type conversion before falling back on the standard rules.

See also http://tomcat.markmail.org/thread/utk3nsn4aakjl4gj
Comment 1 Mark Thomas 2014-12-04 15:07:40 UTC
Proposed fix:
https://github.com/markt-asf/tomcat/commit/53e9f7b44e1cd450ab27025a0202dbf4d383851d
https://github.com/markt-asf/tomcat/commit/d075c49460df3bcfbda3e6ed9f3e357003bad5df

I plan to commit this to trunk and back-port to 8.0.x once svn returns to read-write.
Comment 2 Mark Thomas 2014-12-05 17:39:18 UTC
This has been fixed in trunk and 8.0.x (for 8.0.16 onwards).
Comment 3 Konstantin Kolinko 2014-12-06 16:43:04 UTC
Reviewing r1643367
Reviewing ELSupport.compare(Object, Object) and coerceToNumber(ctx, Object, BigDecimal.class) calls there:

1. The first branch in compare() method:
        if (isBigDecimalOp(obj0, obj1)) {
            BigDecimal bd0 = (BigDecimal) coerceToNumber(ctx, obj0, BigDecimal.class);
            BigDecimal bd1 = (BigDecimal) coerceToNumber(ctx, obj1, BigDecimal.class);
            return bd0.compareTo(bd1);
        }

In this branch isBigDecimalOp check returned true, so one of operands is already a BigDecimal.

QUESTION: Does coerceToNumber() need to call ELResolver to convert it? Can it skip conversion if the object is already of the expected type.


2. Minor
                if (ctx.isPropertyResolved()) {
                    return (Number) result;
                }
It could be type.cast(result).