You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Konstantin Kolinko <kn...@gmail.com> on 2014/10/26 23:40:05 UTC
Array handling in ELSupport.coerceToType() (Re: r1633806, r1607906)
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.
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
3) Bikeshed:
Maybe mention BZ 56425#c6 in changelog for r1607906.
I noted another unrelated difference vs specification in
coerceToType() method - filed BZ 57148.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Array handling in ELSupport.coerceToType() (Re: r1633806, r1607906)
Posted by Mark Thomas <ma...@apache.org>.
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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Array handling in ELSupport.coerceToType() (Re: r1633806, r1607906)
Posted by Mark Thomas <ma...@apache.org>.
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.
> 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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org