Bug 60431

Summary: EL function varargs with array
Product: Tomcat 6 Reporter: Ben Wolfe <apache>
Component: JasperAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal CC: apache
Priority: P2    
Version: unspecified   
Target Milestone: default   
Hardware: All   
OS: All   
Attachments: AstFunction patch

Description Ben Wolfe 2016-11-29 21:36:14 UTC
The varargs implementation of the ASTFunction treats arrays in the final parameter as a single parameter. 

Example jsp:

<%@ taglib uri="/myfunctions" prefix="myfn"%>
<c:set var="myarray" value="${myfn:split( myvar, '@' )}")
<c:set var="pageTitle" value="${myfn:getTitle( myobj, myarray )}"/>

myfunctions.java has:
public static String getTitle(String key, String... args) { }

The array passed in jsp should be treated as the whole argument to getTitle.  The current implementation converts the passed array to be the first element in a new array and passes that to getTitle.
Comment 1 Ben Wolfe 2016-11-29 21:42:17 UTC
Created attachment 34489 [details]
AstFunction patch
Comment 2 Ben Wolfe 2016-11-29 21:42:45 UTC
Attached patch against trunk to fix this.
Comment 3 Mark Thomas 2016-12-01 14:36:39 UTC
In addition to fixing this issue, the proposed patch also converts a List into a set of varargs parameters. What is the specification based justification for this? I can see why one would want to do this but Java doesn't do this and I couldn't find any reference to this conversion in the UEL spec.

I'm working on the array to varargs mapping which, in my view, is justified by how Java behaves even through the UEL spec doesn't mention it.
Comment 4 Ben Wolfe 2016-12-01 14:44:13 UTC
The java list --> array is not strictly necessary. I only added that in while writing the test cases.  I wanted a simple test case like "fn:echo(['a', 'b'])", but el converts raw [] to a List, not an array.  

In order for the param to remain an array I have to put it into a separate variable and pass that variable (which is actually my use-case).  See other test-case that does this.
Comment 5 Mark Thomas 2016-12-01 21:00:24 UTC
Thanks for the patch.

Fixed in:
- trunk for 9.0.0.M14 onwards
- 8.5.x for 8.5.9 onwards
- 8.0.x for 8.0.40 onwards
- 7.0.x for 7.0.74 onwards
- 6.0.x for 6.0.49 onwards