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

Reply via email to