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: [email protected]
For additional commands, e-mail: [email protected]
