You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2013/07/08 15:04:31 UTC

svn commit: r1500707 - in /tomcat/trunk: java/javax/el/ImportHandler.java test/javax/el/TestImportHandler.java

Author: markt
Date: Mon Jul  8 13:04:31 2013
New Revision: 1500707

URL: http://svn.apache.org/r1500707
Log:
Extend unit test for ImportHandler and fix some bugs identifed:
- off-by-one error extracting class name
- not limiting static imports to public static fields/methods
- incorrectly flagging non-conflicting imports from classes with the same name as conflicting

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=1500707&r1=1500706&r2=1500707&view=diff
==============================================================================
--- tomcat/trunk/java/javax/el/ImportHandler.java (original)
+++ tomcat/trunk/java/javax/el/ImportHandler.java Mon Jul  8 13:04:31 2013
@@ -48,10 +48,10 @@ public class ImportHandler {
                     null, "importHandler.invalidStaticName", name));
         }
 
-        String className = name.substring(0, lastPeriod - 1);
+        String className = name.substring(0, lastPeriod);
         String fieldOrMethodName = name.substring(lastPeriod + 1);
 
-        Class<?> clazz = findClass(className);
+        Class<?> clazz = findClass(className, false);
 
         if (clazz == null) {
             throw new ELException(Util.message(
@@ -63,16 +63,24 @@ public class ImportHandler {
 
         for (Field field : clazz.getFields()) {
             if (field.getName().equals(fieldOrMethodName)) {
-                found = true;
-                break;
+                int modifiers = field.getModifiers();
+                if (Modifier.isStatic(modifiers) &&
+                        Modifier.isPublic(modifiers)) {
+                    found = true;
+                    break;
+                }
             }
         }
 
         if (!found) {
             for (Method method : clazz.getMethods()) {
                 if (method.getName().equals(fieldOrMethodName)) {
-                    found = true;
-                    break;
+                    int modifiers = method.getModifiers();
+                    if (Modifier.isStatic(modifiers) &&
+                            Modifier.isPublic(modifiers)) {
+                        found = true;
+                        break;
+                    }
                 }
             }
         }
@@ -100,7 +108,7 @@ public class ImportHandler {
                     null, "importHandler.invalidClassName", name));
         }
 
-        Class<?> clazz = findClass(name);
+        Class<?> clazz = findClass(name, true);
 
         if (clazz == null) {
             throw new ELException(Util.message(
@@ -135,7 +143,7 @@ public class ImportHandler {
             // (which correctly triggers an error)
             for (String p : packages) {
                 String className = p + '.' + name;
-                result = findClass(className);
+                result = findClass(className, true);
             }
         }
 
@@ -148,7 +156,7 @@ public class ImportHandler {
     }
 
 
-    private Class<?> findClass(String name) {
+    private Class<?> findClass(String name, boolean cache) {
         Class<?> clazz;
         try {
              clazz = Class.forName(name);
@@ -164,16 +172,18 @@ public class ImportHandler {
                     null, "importHandler.invalidClass", name));
         }
 
-        String simpleName = clazz.getSimpleName();
-        Class<?> conflict = clazzes.get(simpleName);
+        if (cache) {
+            String simpleName = clazz.getSimpleName();
+            Class<?> conflict = clazzes.get(simpleName);
+
+            if (conflict != null) {
+                throw new ELException(Util.message(null,
+                        "importHandler.ambiguousImport", name, conflict.getName()));
+            }
 
-        if (conflict != null) {
-            throw new ELException(Util.message(null,
-                    "importHandler.ambiguousImport", name, conflict.getName()));
+            clazzes.put(simpleName, clazz);
         }
 
-        clazzes.put(simpleName, clazz);
-
         return clazz;
     }
 }

Modified: tomcat/trunk/test/javax/el/TestImportHandler.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/javax/el/TestImportHandler.java?rev=1500707&r1=1500706&r2=1500707&view=diff
==============================================================================
--- tomcat/trunk/test/javax/el/TestImportHandler.java (original)
+++ tomcat/trunk/test/javax/el/TestImportHandler.java Mon Jul  8 13:04:31 2013
@@ -102,7 +102,7 @@ public class TestImportHandler {
 
 
     /**
-     * Import an invalid [ackage.
+     * Import an invalid package.
      */
     @Test(expected=ELException.class)
     public void testImportPackage01() {
@@ -110,4 +110,57 @@ public class TestImportHandler {
 
         handler.importPackage("org.apache.tomcat.foo");
     }
+
+
+    /**
+     * Import a valid static field.
+     */
+    @Test
+    public void testImportStatic01() {
+        ImportHandler handler = new ImportHandler();
+
+        handler.importStatic("org.apache.tomcat.util.buf.Constants.Package");
+
+        Class<?> result = handler.resolveStatic("Package");
+
+        Assert.assertEquals(org.apache.tomcat.util.buf.Constants.class, result);
+    }
+
+
+    /**
+     * Import an invalid static field - does not exist.
+     */
+    @Test(expected=ELException.class)
+    public void testImportStatic02() {
+        ImportHandler handler = new ImportHandler();
+
+        handler.importStatic("org.apache.tomcat.util.buf.Constants.PackageXX");
+    }
+
+
+    /**
+     * Import an invalid static field - non-public.
+     */
+    @Test
+    public void testImportStatic03() {
+        ImportHandler handler = new ImportHandler();
+
+        handler.importStatic("org.apache.tomcat.util.buf.Ascii.toLower");
+
+        Class<?> result = handler.resolveStatic("toLower");
+
+        Assert.assertEquals(org.apache.tomcat.util.buf.Ascii.class, result);
+    }
+
+
+    /**
+     * Import an invalid static field - conflict.
+     */
+    @Test
+    public void testImportStatic04() {
+        ImportHandler handler = new ImportHandler();
+
+        handler.importStatic("org.apache.tomcat.util.buf.Constants.Package");
+        handler.importStatic("org.apache.tomcat.util.scan.Constants.Package");
+    }
 }



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org