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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
