Bug 42547

Summary: Same env-entry (web.xml) and ResourceLink (context) names causes NPE
Product: Tomcat 5 Reporter: John Hutchnson <belwig>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P3    
Version: 5.5.23   
Target Milestone: ---   
Hardware: All   
OS: All   
Attachments: Demonstrates the issue

Description John Hutchnson 2007-05-30 11:29:37 UTC
Having the following in web.xml:

  <env-entry>
    <env-entry-name>testIt</env-entry-name>
    <env-entry-type>java.lang.String</env-entry-type>
    <env-entry-value>test</env-entry-value>
  </env-entry>

and the following in your context descriptor:

  <ResourceLink name="testIt"
                global="testIt"
                type="java.lang.String"/>

causes an NPE on context load.

java.lang.NullPointerException
	at
org.apache.catalina.deploy.NamingResources.addEnvironment(NamingResources.java:188)
	
This assumes you have an entry for 'testIt' in the <GlobalNamingResources>
section of server.xml to link to.

This worked in 5.5.20. No longer works in 5.5.23.
Comment 1 John Hutchnson 2007-05-30 11:36:46 UTC
More info on having duplicate names in the following scenarios

web.xml           context descriptor       winner
------------      --------------------     ------------
no entry          <Environment>            <Environment>
<env-entry>       <Environment>            <env-entry>
<env-entry>       no entry                 <env-entry>
no entry          <ResourceLink>           <ResourceLink>
<env-entry>       <ResourceLink>           blows up
Comment 2 John Hutchnson 2007-05-30 11:46:21 UTC
Created attachment 20291 [details]
Demonstrates the issue

Zip contains war and an external context descriptor. The configuration as it is
will genreate the issue. Assumes:

- war is exploded to c:/dev/sandbox/tomcat_bug_test/exploded
- The following is in <GlobalNamingResources> in conf/server.xml   
<Environment name="testIt"
	     value="GlobalNamingResources"
	     type="java.lang.String"/>

The war has only a web.xml and a jsp with a scriptlet to do a lookup on
java:comp/env/testIt.
Comment 3 John Hutchnson 2007-05-31 08:33:48 UTC
(In reply to comment #1)
> More info on having duplicate names in the following scenarios
> 
> web.xml           context descriptor       winner
> ------------      --------------------     ------------
> no entry          <Environment>            <Environment>
> <env-entry>       <Environment>            <env-entry>
> <env-entry>       no entry                 <env-entry>
> no entry          <ResourceLink>           <ResourceLink>
> <env-entry>       <ResourceLink>           blows up

This needs to be updated slightly to take into account the override property of
the <Enviroment> tag in the context descriptor. 

web.xml           context descriptor            winner
------------      --------------------          ------------
no entry          <Environment>                  <Environment>
<env-entry>       <Environment>                  <env-entry>     
<env-entry>       <Environment override="true">  <env-entry>   (same as previous)
<env-entry>       <Environment override="false"> <Environment>
<env-entry>       no entry                       <env-entry>
no entry          <ResourceLink>                 <ResourceLink>
<env-entry>       <ResourceLink>                 blows up
Comment 4 John Hutchnson 2007-05-31 09:12:08 UTC
This is related to a change made to fix the following bug:

http://issues.apache.org/bugzilla/show_bug.cgi?id=29727

First, it appears that contrary to the documentation (located at
http://tomcat.apache.org/tomcat-5.5-doc/config/globalresources.html#Enviroment%20Entries),
setting the 'override' attribute of a <Enviroment> tag inside
<GlobalNameingResources> to 'true' will never be (and perhaps never was) obeyed. 

Allowing <env-entry> overrides of global <Enviroment> entries (through
<ResourceLinks> in the context descriptor) may, it could be argued, be illegal
(not the mention confusing). The current behavior, with the NPE, makes it
illegal (intentional or not).

However, it could also be argued that, with an external context descriptor, you
should be able to override anything set via <env-entry> in web.xml if you wanted
to (and this is what the documentation seems to indicate). 

Finally, there's restoring the previous behavior, which never allows any
overriding and always (silently) takes the global value, regardless of any
corresponding <env-entry>s or any reloading.

Essentially, you can a) make it work as described by the documentation, b) blow
up, or c) silently take the global values on load (or reload) always. 

If b, I recommend blowing up with something more descriptive than the NPE we
have now and changing the doco.

If c, you could just check if (in
org.apache.catalina.deploy.NamingResources.java, line 188), findEnviroment()
retuns null. If it does, log a warning and return. If not, do the getOverride()
if. You should also update the doco in this case too.

    if (entries.containsKey(environment.getName())) {
	ContextEnvironment ce = findEnvironment(environment.getName());
	if (ce == null) {
	    // Log a warning: 
	    return;
	}
        if (findEnvironment(environment.getName()).getOverride()) {
            removeEnvironment(environment.getName());
        } else {
            return;
        }
    }
Comment 5 Mark Thomas 2007-06-23 08:03:11 UTC
Thanks for the report. This has been fixed so functionality agrees with the
current documentation (a).

The fix is in svn and will be included in 5.5.25 and 6.0.14.