Bug 52511 - Exception logged in annotation scanning for web apps without /WEB-INF/classes
Summary: Exception logged in annotation scanning for web apps without /WEB-INF/classes
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 7.0.25
Hardware: PC All
: P2 regression (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-23 21:02 UTC by Jess Holle
Modified: 2012-01-27 22:10 UTC (History)
1 user (show)



Attachments
error and exception trace (1.83 KB, text/plain)
2012-01-23 21:02 UTC, Jess Holle
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jess Holle 2012-01-23 21:02:08 UTC
Created attachment 28196 [details]
error and exception trace

Any web app not containing a /WEB-INF/classes directory appears to cause an error and exception trace to be logged during web app startup.  This is new Tomcat 7.0.25 (or 7.0.24) and did not occur in 7.0.23.
Comment 1 Jess Holle 2012-01-23 21:06:50 UTC
This appears to be due to a change in ContextConfig from:

                        webinfClasses = context.getServletContext().getResource(
                                "/WEB-INF/classes");
                        processAnnotationsUrl(webinfClasses, webXml);

to

                        NamingEnumeration<Binding>  listBindings =
                            context.getResources().listBindings("/WEB-INF/classes");
                        while (listBindings.hasMoreElements()) {
                            Binding binding = listBindings.nextElement();
                            if (binding.getObject() instanceof FileDirContext) {
                                File webInfCLassDir =
                                    new File(
                                        ((FileDirContext) binding.getObject()).getDocBase());
                                processAnnotationsFile(webInfCLassDir, webXml);
                            }
                        }

which is all well and good except for the differences in behavior when /WEB-INF/classes is not found.  The old code did not consider this an error and just moved on.  The new code considers this an error to be logged including an exception trace.

I'd patch this myself, but I'm not familiar with the code.  This leaves open questions like: should listBindings() throw an exception upon finding no bindings -- or should it really just return an empty enumeration?  What other side-effects will this have?  Or should this ContextConfig code be changed.

Overall this makes any perfectly fine web app without /WEB-INF/classes look suspect/unreliable, which is not appropriate.
Comment 2 David Rees 2012-01-23 22:45:17 UTC
I'm seeing this, too.
Comment 3 Konstantin Kolinko 2012-01-24 08:48:37 UTC
Several notes.

First, this exception would not happen if either of the following two is true:

1. The web.xml specifies earlier version of servlet specification than 3.0
2. The web.xml has metadata-complete="true"

Note that using metadata-complete="true" is generally recommended to speed up web application startup. That is if you do not need features disabled by that option. All applications in default Tomcat distributive have metadata-complete="true".


Second, this exception is harmless.

Scanning of WEB-INF/classes is skipped, as it should be.

The exception is caught and logged. It does not impede further annotation scanning. Execution in ContextConfig#webConfig() continues.



This issue was introduced in r1209686

> 1210 	log.error(sm.getString(
> 1211 	"contextConfig.webinfClassesUrl"), e);

BTW, the exception handling was changed in r1209686 ,but the log message template above has not been updated.
Comment 4 Jess Holle 2012-01-24 12:54:54 UTC
Clearly the exception is harmless -- except that it causes unnecessary confusion, consternation, and loss of trust from those administering, maintaining, and troubleshooting applications on Tomcat 7.0.25 that produce this error.

I'm trying to understand the best fix here so I can patch this onto my 7.0.25 -- as I want other fixes in 7.0.25, but don't want all the downsides of inappropriate error/exception messages, nor do I want to accidentally hide any real errors.
Comment 5 Mark Thomas 2012-01-27 22:10:39 UTC
Fixed in trunk and 7.0.x and will be included in 7.0.26 onwards.