Author: kkolinko
Date: Thu Oct 23 08:43:52 2014
New Revision: 1633769
URL: http://svn.apache.org/r1633769
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=57132
ImportHandler.resolveClass() fails to report conflicting import when called
repeatedly
Modified:
tomcat/trunk/java/javax/el/ImportHandler.java
tomcat/trunk/test/javax/el/TestImportHandler.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/javax/el/ImportHandler.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/javax/el/ImportHandler.java?rev=1633769&r1=1633768&r2=1633769&view=diff
==============================================================================
--- tomcat/trunk/java/javax/el/ImportHandler.java (original)
+++ tomcat/trunk/java/javax/el/ImportHandler.java Thu Oct 23 08:43:52 2014
@@ -51,7 +51,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(
@@ -108,12 +108,22 @@ public class ImportHandler {
null, "importHandler.invalidClassName", name));
}
- Class<?> clazz = findClass(name, true);
+ Class<?> clazz = findClass(name);
if (clazz == null) {
throw new ELException(Util.message(
null, "importHandler.classNotFound", name));
}
+
+ String simpleName = clazz.getSimpleName();
+ Class<?> conflict = clazzes.get(simpleName);
+
+ if (conflict != null) {
+ throw new ELException(Util.message(null,
+ "importHandler.ambiguousImport", name,
conflict.getName()));
+ }
+
+ clazzes.put(simpleName, clazz);
}
@@ -143,11 +153,19 @@ public class ImportHandler {
// (which correctly triggers an error)
for (String p : packages) {
String className = p + '.' + name;
- Class<?> clazz = findClass(className, true);
+ 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;
@@ -159,7 +177,7 @@ public class ImportHandler {
}
- private Class<?> findClass(String name, boolean cache) {
+ private Class<?> findClass(String name) {
Class<?> clazz;
try {
clazz = Class.forName(name);
@@ -175,18 +193,6 @@ public class ImportHandler {
null, "importHandler.invalidClass", name));
}
- 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()));
- }
-
- 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=1633769&r1=1633768&r2=1633769&view=diff
==============================================================================
--- tomcat/trunk/test/javax/el/TestImportHandler.java (original)
+++ tomcat/trunk/test/javax/el/TestImportHandler.java Thu Oct 23 08:43:52 2014
@@ -54,14 +54,22 @@ public class TestImportHandler {
/**
* Conflict on resolution.
*/
- @Test(expected=ELException.class)
+ @Test
public void testResolveClass03() {
ImportHandler handler = new ImportHandler();
handler.importPackage("org.apache.tomcat.util");
handler.importPackage("org.apache.jasper.util");
- handler.resolveClass("ExceptionUtils");
+ for (int i = 1; i <= 3; i++) {
+ try {
+ Class<?> clazz = handler.resolveClass("ExceptionUtils");
+ Assert.fail("Expected ELException but got [" + clazz.getName()
+ + "] on iteration " + i);
+ } catch (ELException ex) {
+ // Expected
+ }
+ }
}
@@ -111,12 +119,20 @@ public class TestImportHandler {
/**
* Import conflicting classes
*/
- @Test(expected=ELException.class)
+ @Test
public void testImportClass03() {
ImportHandler handler = new ImportHandler();
handler.importClass("org.apache.tomcat.util.ExceptionUtils");
- handler.importClass("org.apache.jasper.util.ExceptionUtils");
+ for (int i = 1; i <= 3; i++) {
+ try {
+ handler.importClass("org.apache.jasper.util.ExceptionUtils");
+ Assert.fail("Expected ELException but got none on iteration "
+ + i);
+ } catch (ELException ex) {
+ // Expected
+ }
+ }
}
@@ -175,11 +191,19 @@ public class TestImportHandler {
/**
* Import an invalid static field - conflict.
*/
- @Test(expected=ELException.class)
+ @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");
+ for (int i = 1; i <= 3; i++) {
+ try {
+
handler.importStatic("org.apache.tomcat.util.scan.Constants.Package");
+ Assert.fail("Expected ELException but got none on iteration "
+ + i);
+ } catch (ELException ex) {
+ // Expected
+ }
+ }
}
}
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1633769&r1=1633768&r2=1633769&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Thu Oct 23 08:43:52 2014
@@ -201,6 +201,10 @@
more than one package was imported and the desired class was not in the
last package imported. (markt)
</fix>
+ <fix>
+ <bug>57132</bug>: Fix import conflicts reporting in Expression
Language.
+ (kkolinko)
+ </fix>
</changelog>
</subsection>
<subsection name="Cluster">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
