Author: markt Date: Wed Dec 10 21:08:54 2014 New Revision: 1644523 URL: http://svn.apache.org/r1644523 Log: Fix ImportHandler issues identified in review of fix for BZ 57142 - delay resolution of class name to Class until it is used - Use the TCCL to load classes by name
Modified: tomcat/trunk/java/javax/el/ImportHandler.java tomcat/trunk/test/javax/el/TestImportHandler.java Modified: tomcat/trunk/java/javax/el/ImportHandler.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/javax/el/ImportHandler.java?rev=1644523&r1=1644522&r2=1644523&view=diff ============================================================================== --- tomcat/trunk/java/javax/el/ImportHandler.java (original) +++ tomcat/trunk/java/javax/el/ImportHandler.java Wed Dec 10 21:08:54 2014 @@ -21,18 +21,19 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.net.URL; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; /** * @since EL 3.0 */ public class ImportHandler { - private List<String> packages = new ArrayList<>(); - private Map<String,Class<?>> clazzes = new HashMap<>(); - private Map<String,Class<?>> statics = new HashMap<>(); + private List<String> packageNames = new ArrayList<>(); + private Map<String,String> classNames = new ConcurrentHashMap<>(); + private Map<String,Class<?>> clazzes = new ConcurrentHashMap<>(); + private Map<String,Class<?>> statics = new ConcurrentHashMap<>(); public ImportHandler() { @@ -51,7 +52,7 @@ public class ImportHandler { String className = name.substring(0, lastPeriod); String fieldOrMethodName = name.substring(lastPeriod + 1); - Class<?> clazz = findClass(className, false); + Class<?> clazz = findClass(className); if (clazz == null) { throw new ELException(Util.message( @@ -103,33 +104,20 @@ public class ImportHandler { public void importClass(String name) throws javax.el.ELException { - if (!name.contains(".")) { - throw new ELException(Util.message( - null, "importHandler.invalidClassName", name)); - } - - Class<?> clazz = findClass(name, false); + int lastPeriodIndex = name.lastIndexOf('.'); - if (clazz == null) { + if (lastPeriodIndex < 0) { throw new ELException(Util.message( - null, "importHandler.classNotFound", name)); + null, "importHandler.invalidClassName", name)); } - String simpleName = clazz.getSimpleName(); - Class<?> conflict = clazzes.get(simpleName); + String unqualifiedName = name.substring(lastPeriodIndex + 1); + String currentName = classNames.putIfAbsent(unqualifiedName, name); - if (conflict == null) { - // No conflict - add it - clazzes.put(simpleName, clazz); - } else { - // Check for a duplicate - if (conflict.equals(clazz)) { - // This is a duplicate. - // NO-OP - } else { - throw new ELException(Util.message(null, - "importHandler.ambiguousImport", name, conflict.getName())); - } + if (currentName != null && !currentName.equals(name)) { + // Conflict. Same unqualifiedName, different fully qualified names + throw new ELException(Util.message(null, + "importHandler.ambiguousImport", name, currentName)); } } @@ -148,7 +136,7 @@ public class ImportHandler { null, "importHandler.invalidPackage", name)); } } - packages.add(name); + packageNames.add(name); } @@ -157,27 +145,40 @@ public class ImportHandler { return null; } + // Has it been previously resolved? Class<?> result = clazzes.get(name); - if (result == null) { - // Search the package imports - note there may be multiple matches - // (which correctly triggers an error) - for (String p : packages) { - String className = p + '.' + name; - Class<?> clazz = findClass(className, false); - if (clazz != null) { - if (result != null) { - throw new ELException(Util.message(null, - "importHandler.ambiguousImport", className, - result.getName())); - } - result = clazz; - } + if (result != null) { + return result; + } + + // Search the class imports + String className = classNames.get(name); + if (className != null) { + Class<?> clazz = findClass(className); + if (clazz != null) { + clazzes.put(className, clazz); + return clazz; } - if (result != null) { - clazzes.put(name, result); + } + + // Search the package imports - note there may be multiple matches + // (which correctly triggers an error) + for (String p : packageNames) { + className = p + '.' + name; + Class<?> clazz = findClass(className); + if (clazz != null) { + if (result != null) { + throw new ELException(Util.message(null, + "importHandler.ambiguousImport", className, + result.getName())); + } + result = clazz; } } + if (result != null) { + clazzes.put(name, result); + } return result; } @@ -188,28 +189,19 @@ public class ImportHandler { } - /* - * This method is used for importing and resolving. Resolving has strict - * criteria for what may be returned. Import does not. Further, as a result - * of the clarification in https://java.net/jira/browse/JSP-44 any class or - * package imported using a JSP page directive will also be imported in to - * the EL environment. The validate flag is used to bypass the validation - * criteria required by resolving when importing else a typical JSP import - * (e.g. <%@page import="java.util.List"%>) would trigger an ELException and - * a 500 response which is clearly not correct. - */ - private Class<?> findClass(String name, boolean validate) { + private Class<?> findClass(String name) { Class<?> clazz; + ClassLoader cl = Thread.currentThread().getContextClassLoader(); try { - clazz = Class.forName(name); + clazz = cl.loadClass(name); } catch (ClassNotFoundException e) { return null; } // Class must be public, non-abstract and not an interface int modifiers = clazz.getModifiers(); - if (validate && (!Modifier.isPublic(modifiers) || Modifier.isAbstract(modifiers) || - Modifier.isInterface(modifiers))) { + if (!Modifier.isPublic(modifiers) || Modifier.isAbstract(modifiers) || + Modifier.isInterface(modifiers)) { throw new ELException(Util.message( null, "importHandler.invalidClass", name)); } Modified: tomcat/trunk/test/javax/el/TestImportHandler.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/javax/el/TestImportHandler.java?rev=1644523&r1=1644522&r2=1644523&view=diff ============================================================================== --- tomcat/trunk/test/javax/el/TestImportHandler.java (original) +++ tomcat/trunk/test/javax/el/TestImportHandler.java Wed Dec 10 21:08:54 2014 @@ -138,11 +138,12 @@ public class TestImportHandler { /** * Import an invalid class. */ - @Test(expected=ELException.class) + @Test public void testImportClass02() { ImportHandler handler = new ImportHandler(); - handler.importClass("org.apache.tomcat.util.res.StringManagerX"); + Class<?> result = handler.resolveClass("StringManagerX"); + Assert.assertNull(result); } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org