You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2014/10/22 23:56:39 UTC
[Bug 57132] New: ImportHandler.resolveClass() fails to report
conflicting import when called repeatedly
https://issues.apache.org/bugzilla/show_bug.cgi?id=57132
Bug ID: 57132
Summary: ImportHandler.resolveClass() fails to report
conflicting import when called repeatedly
Product: Tomcat 8
Version: 8.0.14
Hardware: PC
Status: NEW
Severity: minor
Priority: P2
Component: EL
Assignee: dev@tomcat.apache.org
Reporter: knst.kolinko@gmail.com
I noted this issue when reviewing ImportHandler code after r1633440. I have
test case that demonstrates it, but I do not have a fix yet.
javax.el.ImportHandler.resolveClass(String name) should throw an ELException if
the same class can be ambiguously found in several packages.
To do so, it relies on calling findClass(className, true) and on "clazzes"
field that is a cache of classes keyed by their "simple class names". A simple
class name is the name without its package.
If resolveClass("foo") finds class foo in several packages then the first call
to findClass() updates the clazzes cache and the second call finds the
duplicate when trying to update the cache with a different class.
If resolveClass("foo") call is repeated, expected result is to report the error
again. The actual result is that the first thing it does is to look into the
cache. It finds the first class there and returns it without any errors.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57132] ImportHandler.resolveClass() fails to report conflicting
import when called repeatedly
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57132
--- Comment #5 from Christopher Schultz <ch...@christopherschultz.net> ---
(In reply to Konstantin Kolinko from comment #2)
> Also noted the third issue:
>
> ImportHandler.importClass() does not allow to import the exactly same class
> twice.
>
> I think that such imports shall be silently swallowed. It boils down to
> adding a check whether conflicting Class instances are the same, (conflict
> == clazz).
I haven't read through the code to see how class object references are
maintained, but I have been bitten in the past when
Foo.class.getMethod("bar").getName() == Foo.class.getMethod("bar").getName()
yields false.
Perhaps clazz.equals(conflict) would be better than == in this case?
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57132] ImportHandler.resolveClass() fails to report conflicting
import when called repeatedly
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57132
--- Comment #3 from Mark Thomas <ma...@apache.org> ---
Reading the Javadoc for the second issue, I think returning null is a better
response here. I've committed a couple of test cases and a fix for this.
Looking at the third issue now...
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57132] ImportHandler.resolveClass() fails to report conflicting
import when called repeatedly
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57132
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED
--- Comment #4 from Mark Thomas <ma...@apache.org> ---
Third issue fixed in 8.0.x for 8.0.15 onwards.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57132] ImportHandler.resolveClass() fails to report conflicting
import when called repeatedly
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57132
--- Comment #6 from Konstantin Kolinko <kn...@gmail.com> ---
(In reply to Christopher Schultz from comment #5)
>
> I haven't read through the code to see how class object references are
> maintained, but I have been bitten in the past when
> Foo.class.getMethod("bar").getName() == Foo.class.getMethod("bar").getName()
> yields false.
>
> Perhaps clazz.equals(conflict) would be better than == in this case?
Mark's fix in r1633810 used conflict.equals(clazz), so it is OK.
The JVM specification (Java 8 edition) in chapter 5.3 Creation and Loading
and Java Language Specification (Java 8 edition) in chapter 12.2 Loading of
Classes and Interfaces both contain the same phrase:
"Given the same name, a good class loader should always return the same Class
object."
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57132] ImportHandler.resolveClass() fails to report conflicting
import when called repeatedly
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57132
Konstantin Kolinko <kn...@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
OS| |All
--- Comment #2 from Konstantin Kolinko <kn...@gmail.com> ---
The first (main) reported issue fixed by r1633769 and will be in Tomcat 8.0.15.
>
> BTW, there is no control that the class name argument in resolveClass() is
> actually a simple name without a package. JavaEE javadoc says that it should
> be simple name, but there is no control of that fact. JavaEE does not say
> whether it is ELException or IAE to be thrown if such a check were added. I
> think it is an ELException. [1]
>
> [1] http://docs.oracle.com/javaee/7/api/javax/el/ImportHandler.html
The above second issue is still pending. BTW, it can simply return "null"
without reporting any errors.
Also noted the third issue:
ImportHandler.importClass() does not allow to import the exactly same class
twice.
I think that such imports shall be silently swallowed. It boils down to adding
a check whether conflicting Class instances are the same, (conflict == clazz).
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57132] ImportHandler.resolveClass() fails to report conflicting
import when called repeatedly
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57132
--- Comment #1 from Konstantin Kolinko <kn...@gmail.com> ---
Created attachment 32139
--> https://issues.apache.org/bugzilla/attachment.cgi?id=32139&action=edit
2014-10-22_tc8_57132_testcases.patch
Updated testcases for TestImportHandler to test that duplicate calls still
report an error.
A comment in ImportHandler class shows my first idea on fixing this. If I
remove first duplicate from the cache it fixes duplicate check in
resolveClass() but breaks duplicate check in importClass().
I think that if I move duplicate removal from the cache into resolveClass()
method itself, it will be fixed. But maybe there is a more clean solution.
BTW, there is no control that the class name argument in resolveClass() is
actually a simple name without a package. JavaEE javadoc says that it should be
simple name, but there is no control of that fact. JavaEE does not say whether
it is ELException or IAE to be thrown if such a check were added. I think it is
an ELException. [1]
[1] http://docs.oracle.com/javaee/7/api/javax/el/ImportHandler.html
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org