Balzanka / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

reflect.ClassPath.from() can throw StackOverflowError #1351

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
My company uses Weblogic, and one of the provided JARs is 
net.sf.antcontrib_1.1.0.0_1-0b2.  (Can be found here: 
http://sourceforge.net/projects/ant-contrib/files/ant-contrib/ant-contrib-1.0b2/
 )

This particular JAR references itself in its MANIFEST.MF 'Class-Path', which 
causes Guava's ClassPath.from() to throw a StackOverflowError.

Could some circular-dependency protection be built into 
getClassPathFromManifest() to prevent returning a URI that matches the current 
file?  For example:

try {
    uri = getClassPathEntry(jarFile, path);
} catch (URISyntaxException e) {
    // Ignore bad entry
    logger.warning("Invalid Class-Path entry: " + path);
    continue;
}
// New code
if (jarFile.toURI().equals(uri)) {
    logger.warning("Invalid Class-Path entry: " + path + " references itself.");
    continue;
}

A more complicated but more robust scheme could be in browseJar(), where the 
returned URIs from the manifest could be compared against the in-progress 
ImmutableSet of resources.  This would help guard against hierarchical 
dependency loops, if such a travesty existed.  There would be a bit more work 
involved to make this work, since currently the jarFile itself isn't processed 
until all of its Manifest has been.

I realize this is the fault of the antcontrib project (or perhaps even 
Weblogic) and that it isn't the goal of Guava to try to handle odd edge cases 
like this.  My hope is that the market share of Weblogic might be enough to 
grant this some consideration.  Thanks!

Original issue reported on code.google.com by stephen....@gmail.com on 25 Mar 2013 at 7:16

GoogleCodeExporter commented 9 years ago
What's the use case for using ClassPath on WebLogic?

In this particular case it's circular reference but there could be not so 
obvious cases where ClassPath silently ignores classes.

We've only tested ClassPath in simple J2SE environment and the main use cases 
so far are for testing. ClassPath uses heuristics so it's possible that some 
application server uses uncommon ClassLoader implementation that ClassPath 
can't scan.

Original comment by be...@google.com on 25 Mar 2013 at 9:54

GoogleCodeExporter commented 9 years ago

Original comment by be...@google.com on 25 Mar 2013 at 9:56

GoogleCodeExporter commented 9 years ago
I think I've found an alternate way to accomplish what I was trying to do. 
Specifically, I was curious if I could find a way to offer an omnibox-style 
autocomplete recommendation on a field where an admin user would be typing 
either a package or fully qualified classname.

I remembered ClassPath, so that was where I initially focused my energies. I 
think I can get a subset of loaded classes through another means that will 
serve my purpose nearly as effectively.

If you think the guard I recommended might introduce unexpected results, 
perhaps the best solution would be to expand the javadocs to explain that 
ClassPath's use is recommended for non-JavaEE and non-Production uses.

It's always possible there are other JARs that have the same self-referential 
problem, and even in a J2SE/Testing situation you'd be unable to use ClassPath 
without repackaging the JAR yourself. So although my original plea was due to 
my Weblogic constraint, it still seems like fixing this might not be a bad idea 
for non-Weblogic/JavaEE users.

Original comment by stephen....@gmail.com on 26 Mar 2013 at 2:02

GoogleCodeExporter commented 9 years ago
Yes. I agree that doing the circular reference check is reasonable and thank 
you for reporting the bug.

I was just worried that if you use ClassPath for mission critical work on 
WebLogic such that ClassPath needs to return *all* classes at all cases, we 
didn't test it on WebLogic or other application servers to make sure it will 
offer the level of guarantee.

It sounds like it's more for a tool, not for production critical things, which 
sounds a lot less risky.

I'll add the fix.

Original comment by be...@google.com on 26 Mar 2013 at 2:14

GoogleCodeExporter commented 9 years ago
Great! Thanks for the quick reply. Correct - I'd plan to use it in a tool that 
was not mission critical. I also appreciate that you took the time to make sure 
I wasn't using it improperly.

Thanks again!

Original comment by stephen....@gmail.com on 26 Mar 2013 at 2:22

GoogleCodeExporter commented 9 years ago
In Guava 15

Original comment by be...@google.com on 28 Mar 2013 at 3:55

GoogleCodeExporter commented 9 years ago

Original comment by cpov...@google.com on 28 Mar 2013 at 3:57

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<issue id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:12

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:08