Bug 53333 - NPE is thrown for env-entry without env-entry-type but with injection-target specified
NPE is thrown for env-entry without env-entry-type but with injection-target ...
Status: RESOLVED FIXED
Product: Tomcat 7
Classification: Unclassified
Component: Catalina
7.0.27
All All
: P2 normal (vote)
: ---
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2012-05-30 19:28 UTC by Violeta Georgieva
Modified: 2012-06-03 15:21 UTC (History)
0 users



Attachments
Test web application (2.73 KB, application/x-zip-compressed)
2012-05-30 19:28 UTC, Violeta Georgieva
Details
Patch proposal (13.92 KB, patch)
2012-05-30 19:41 UTC, Violeta Georgieva
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Violeta Georgieva 2012-05-30 19:28:00 UTC
Created attachment 28863 [details]
Test web application

Hi,

I have a web application (attached) that specifies env-entry in the web.xml. The env-entry does not specify env-entry-type, but specifies injection-target. When deploying that web application, the exception below is thrown:


Caused by: java.lang.NullPointerException
	at java.util.Hashtable.put(Hashtable.java:394)
	at org.apache.catalina.deploy.NamingResources.addEnvironment(NamingResources.java:254)
	at org.apache.catalina.deploy.WebXml.configureContext(WebXml.java:1195)
	at org.apache.catalina.startup.ContextConfig.webConfig(ContextConfig.java:1294)
	at org.apache.catalina.startup.ContextConfig.configureStart(ContextConfig.java:855)
	at org.apache.catalina.startup.ContextConfig.lifecycleEvent(ContextConfig.java:345)
	at org.apache.catalina.util.LifecycleSupport.fireLifecycleEvent(LifecycleSupport.java:119)
	at org.apache.catalina.util.LifecycleBase.fireLifecycleEvent(LifecycleBase.java:90)
	at org.apache.catalina.core.StandardContext.startInternal(StandardContext.java:5161)
	at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:150)


Servlet Specification, 14.4 Deployment Descriptor Diagram, describes that use case and how it should be handled:
"
env-entry element
If an injection-target is specified for the environment entry, the enventry-
type may be ommitted or MUST match the type of the injection target. If
no injection-target is specified, the env-entry-type is required.
"


I would like to propose a patch (attached) that solves this issue.

I'm looking forward for your comments.

Thanks
Violeta


Steps to reproduce the problem:
1. Deploy the attached application
2. Request http://localhost:8080/test/TestServlet
3. NPE is thrown
4. Apply the provided patch
5. Request http://localhost:8080/test/TestServlet
6. The following response should be generated:
envEntry_1: 1 
envEntry_2: 2 
dataSource: org.apache.tomcat.dbcp.dbcp.BasicDataSource@506dd108
Comment 1 Violeta Georgieva 2012-05-30 19:41:54 UTC
Created attachment 28864 [details]
Patch proposal
Comment 2 Mark Thomas 2012-06-01 20:05:03 UTC
Thanks for pointing this out. It is definitely a bug.

As I started to look at this I found some clean-up that could be done. It changes the failure point although I think the patch is currently trying to identify the type at the correct point. However, I'm not sure that NamingResources is the best home for the actual functionality. I think some refactoring may be in order. I'm looking at the now.

I also think that the specification wording is ambiguous. There is
<quote>
type may be ommitted or MUST match the type of the injection target
</quote>
and
<quote>
type MUST be assignment compatible with the type of the injection target
</quote>

Clearly the wording is different but I am not convinced that the meaning is. It depends what is meant by "match". I am leaning towards implementing the more flexible "assignment compatible" in all cases.
Comment 3 Mark Thomas 2012-06-02 21:31:45 UTC
Thanks for the suggested patch. I used it as a basis for the committed solution although I tweaked the code a little and made it more relaxed regarding inputs. Generally, as long as the types are compatible - it will work.

The change has been applied to trunk and 7.0.x and will be included in 7.0.28 onwards.
Comment 4 Violeta Georgieva 2012-06-03 15:21:16 UTC
Thanks