Bug 56652 - EL defineFunction does not support method signatures with arrays as arguments
Summary: EL defineFunction does not support method signatures with arrays as arguments
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: EL (show other bugs)
Version: 8.0.x-trunk
Hardware: PC All
: P2 normal (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-20 21:55 UTC by Arthur Fiedler
Modified: 2014-07-04 18:47 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Arthur Fiedler 2014-06-20 21:55:01 UTC
When defineFuncton in ELProcessor is called it uses class.getName() to compare class names, and the MethodSignature class used to parse the signatures will return whatever value is defined(in the method signature) as the class name when its fully qualified. 

So if the method signature is lets say "boolean sameSize(java.util.Collection[])" defineFunction wont find a matching method on the provided class, because class.getName() will return "[Ljava.util.Collection;" instead. 

To get around this I made some tweaks to unqualified type's also to check for array, and also to always get the correct class name, I changed it to always get the referenced class then use the getName() function there. 

I added most the functionality to the ImportHandler to reuse the cache and because it was contently there

Original Code in 8.0.8
[javax.el.ELProcessor- MethodSignature]

ImportHandler importHandler = context.getImportHandler();
for (int i = 0; i < parameterTypeNames.length; i++) {
    parameterTypeNames[i] = parameterTypeNames[i].trim();
    if (!parameterTypeNames[i].contains(".")) {
        Class<?> clazz = importHandler.resolveClass(
                parameterTypeNames[i]);
        if (clazz == null) {
            throw new NoSuchMethodException(Util.message(
                    context,
                    "elProcessor.defineFunctionInvalidParameterTypeName",
                    parameterTypeNames[i], methodName,
                    className));
        }
        parameterTypeNames[i] = clazz.getName();
    }
}

Replaced with:

ImportHandler importHandler = context.getImportHandler();
Class<?> clazz;
for (int i = 0; i < parameterTypeNames.length; i++) {
    clazz = importHandler.extendedResolveClass(parameterTypeNames[i].trim());
    if (clazz == null) {
        throw new NoSuchMethodException(Util.message(
                context,
                "elProcessor.defineFunctionInvalidParameterTypeName",
                parameterTypeNames[i], methodName,
                className));
    }
    parameterTypeNames[i] = clazz.getName();
}


[javax.el.ImportHandler] - Added the following methods

/**
 * This version of Class<?> resolveClass(String name) will allow qualified names, 
 * handles arrays, and for qualified names will ignore requiring the type is a public class
 * Note: added to this class to take advantage of the existing cache, and will cache 
 * original name for qualified names before parsing
 * @param name Classname with or without namespace
 * @return
 */
public java.lang.Class<?> extendedResolveClass(String name) {
    Class<?> result = clazzes.get(name);

    if (result == null) {
        int firstBracketIndex = name.indexOf('[');
        if (name.contains(".")) 
        {
            try {
                if (firstBracketIndex > -1)
                    result = Class.forName(convertArrayClassName(name, firstBracketIndex));
                else
                    result = Class.forName(name);
            } catch (ClassNotFoundException e) {
                return null;
            }
            
            Class<?> conflict = clazzes.get(name);

            if (conflict != null) {
                throw new ELException(Util.message(null,
                        "importHandler.ambiguousImport", name, conflict.getName()));
            }

            clazzes.put(name, result);
        }
        else {
            // Search the package imports - note there may be multiple matches
            // (which correctly triggers an error)
            for (String p : packages) {
                String className = p + '.' + name;
                if (firstBracketIndex > -1) 
                    className = convertArrayClassName(className, firstBracketIndex);
                result = findClass(className, true);
            }
        }
    }

    return result;
}

/**
 * Converts a class name in the format of java.util.Collection[] to [Ljava.util.Collection;
 * @param typeName Original class name
 * @param firstBracketIndex position of the first [
 * @return The class name in the format of [LclassName;
 */
private String convertArrayClassName(String typeName, int firstBracketIndex)
{
    final int length = typeName.length();
    StringBuilder sb = new StringBuilder(length + 1); // +1 is to account for ';'
    sb.append('[');
    for(int i = firstBracketIndex + 2; i < length; i++) 
    {
        if (typeName.charAt(i) == '[')
        {
            sb.append('[');
            i++;
        }
    }
    typeName = typeName.substring(0, firstBracketIndex).trim();
    switch(typeName) 
    {
        case "boolean": 
            sb.append('Z');
            break;
        case "byte":
            sb.append('B');
            break;
        case "char":
            sb.append('C');
            break;
        case "double":
            sb.append('D');
            break;
        case "float":
            sb.append('F');
            break;
        case "int":
            sb.append('I');
            break;
        case "long":
            sb.append('J');  
            break;
        case "short":
            sb.append('S');
            break;
        default:
            sb.append('L');
            sb.append(typeName);
            sb.append(';');
            break;
    }
    return sb.toString();
}
Comment 1 Mark Thomas 2014-07-03 16:04:23 UTC
Patches should be attached to issues in diff -u format. Providing the patch in other formats slows down the review process.
Comment 2 Mark Thomas 2014-07-04 10:54:23 UTC
The proposed solution can not be used. The EL API may not be changed. I'm working on a solution along similar lines but the work is done in the ELProcessor.
Comment 3 Mark Thomas 2014-07-04 18:47:28 UTC
This has been fixed in 8.0.x for 8.0.10 onwards. I also added support for varargs.