On 28/10/2014 21:27, Mark Thomas wrote: > On 26/10/2014 22:40, Konstantin Kolinko wrote: >> Hi! >> >> This is a comment on the following commits: >> >> URL: http://svn.apache.org/r1633806 >> Log: >> When coercing an object to a given type, only attempt coercion to an >> array if both the object type and the target type are an array type. >> >> URL: http://svn.apache.org/r1607906 >> Log: >> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56652 >> Add support for method parameters that use arrays and varargs to >> ELProcessor.defineFunction() >> >> >> Issues: >> >> 1) General issue: >> In r1607906 there was added a conversion support for arrays into >> ELSupport.coerceToType() and r1633806 fixed a bug in it. >> >> My understanding is that the method ELSupport.coerceToType() >> implements conversion rules from EL specification chapter "Type >> Conversion" (ch.1.23 in EL 3.0). >> >> My concern is that EL specification does not specify such conversion >> for array elements. >> >> As such, the varargs support fix needs a different implementation that >> does not change the ELSupport.coerceToType() method. > > It is a grey area. Those coercion rules get used in multiple places and > some parts of the EL spec explicitly or implicitly require array support. > > Unless we get a complaint that this feature actually breaks something > (which I view as unlikely) I'm of the view we treat this as a Tomcat > specific extension and leave it in. Meanwhile when the EL.next EG starts > up, I'll raise the issue of array support for coercion. > >> 2) Technical issue: >> The ELSupport.coerceToArray() method does not support arrays of >> primitives. It class-casts its argument "(Object[]) obj", but that >> will fail for arrays of primitives. The correct way is to use the >> following method to access array elements: >> java.lang.reflect.Array.get(Object, int): Object > > Fair point. I'll take a look at a fix.
Fixed. Thanks, Mark > > >> 3) Bikeshed: >> Maybe mention BZ 56425#c6 in changelog for r1607906. > > Feel free to paint that particular bikeshed if you wish. > >> I noted another unrelated difference vs specification in >> coerceToType() method - filed BZ 57148. > > Thanks. > > Mark > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
