Log4j 2
  1. Log4j 2
  2. LOG4J2-359

Log4jServletContextListener does not work on Weblogic 12.1.1 (12c) with web-app version "2.5"

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0-beta8
    • Fix Version/s: 2.0-rc1
    • Component/s: None
    • Labels:
      None

      Description

      I have Weblogic 12c running. My web-app is version "2.5".

      Following is a snippet from my web.xml

      <web-app xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
      	xmlns="http://java.sun.com/xml/ns/javaee"
      	xsi:schemaLocation="http://java.sun.com/xml/ns/javaee http://java.sun.com/xml/ns/javaee/web-app_2_5.xsd"
      	id="WebApp_ID" version="2.5">
      	<display-name>pec-service</display-name>
      	<context-param>
      		<param-name>log4jConfiguration</param-name>
      		<param-value>file:/C:/log4j/dev.log4j.xml</param-value>
      	</context-param>
      
      	<listener> 
      		<listener-class>org.apache.logging.log4j.core.web.Log4jServletContextListener</listener-class> 
      	</listener>	
      
      	<filter>
      		<filter-name>log4jServletFilter</filter-name>
      		<filter-class>org.apache.logging.log4j.core.web.Log4jServletFilter</filter-class> 
      	</filter>
      	<filter-mapping>
      		<filter-name>log4jServletFilter</filter-name> 
      		<url-pattern>/*</url-pattern>
      		<dispatcher>REQUEST</dispatcher>
      		<dispatcher>FORWARD</dispatcher> 
      		<dispatcher>INCLUDE</dispatcher>
      		<dispatcher>ERROR</dispatcher>
      	</filter-mapping>
      	
      </web-app>
      

      However, on my server startup I am getting the following error -

      <Aug 16, 2013 3:12:32 PM PDT> <Warning> <HTTP> <BEA-101162> <User defined listener org.apache.logging.log4j.core.web.Log4jServletContextListener failed: java.lang.IllegalStateException: Context destroyed before it was initialized..
      java.lang.IllegalStateException: Context destroyed before it was initialized.
      	at org.apache.logging.log4j.core.web.Log4jServletContextListener.contextDestroyed(Log4jServletContextListener.java:51)
      	at weblogic.servlet.internal.EventsManager$FireContextListenerAction.run(EventsManager.java:583)
      	at weblogic.security.acl.internal.AuthenticatedSubject.doAs(AuthenticatedSubject.java:321)
      	at weblogic.security.service.SecurityManager.runAs(SecurityManager.java:120)
      	at weblogic.servlet.provider.WlsSubjectHandle.run(WlsSubjectHandle.java:57)
      	Truncated. see log file for complete stacktrace
      > 
      <Aug 16, 2013 3:12:32 PM PDT> <Error> <Deployer> <BEA-149265> <Failure occurred in the execution of deployment request with ID "1376691143681" for task "2". Error is: "weblogic.application.ModuleException"
      weblogic.application.ModuleException
      	at weblogic.servlet.internal.WebAppModule.startContexts(WebAppModule.java:1708)
      	at weblogic.servlet.internal.WebAppModule.start(WebAppModule.java:781)
      	at weblogic.application.internal.flow.ModuleStateDriver$3.next(ModuleStateDriver.java:213)
      	at weblogic.application.internal.flow.ModuleStateDriver$3.next(ModuleStateDriver.java:208)
      	at weblogic.application.utils.StateMachineDriver.nextState(StateMachineDriver.java:35)
      	Truncated. see log file for complete stacktrace
      Caused By: java.lang.NullPointerException
      	at org.apache.logging.log4j.core.web.Log4jServletContainerInitializer.onStartup(Log4jServletContainerInitializer.java:44)
      	at weblogic.servlet.internal.WebAppServletContext.initContainerInitializer(WebAppServletContext.java:1271)
      	at weblogic.servlet.internal.WebAppServletContext.initContainerInitializers(WebAppServletContext.java:1229)
      	at weblogic.servlet.internal.WebAppServletContext.preloadResources(WebAppServletContext.java:1726)
      	at weblogic.servlet.internal.WebAppServletContext.start(WebAppServletContext.java:2740)
      	Truncated. see log file for complete stacktrace
      > 
      <Aug 16, 2013 3:12:32 PM PDT> <Error> <Deployer> <BEA-149202> <Encountered an exception while attempting to commit the 7 task for the application "_auto_generated_ear_".> 
      <Aug 16, 2013 3:12:32 PM PDT> <Warning> <Deployer> <BEA-149004> <Failures were detected while initiating start task for application "_auto_generated_ear_".> 
      <Aug 16, 2013 3:12:32 PM PDT> <Warning> <Deployer> <BEA-149078> <Stack trace for message 149004
      weblogic.application.ModuleException
      	at weblogic.servlet.internal.WebAppModule.startContexts(WebAppModule.java:1708)
      	at weblogic.servlet.internal.WebAppModule.start(WebAppModule.java:781)
      	at weblogic.application.internal.flow.ModuleStateDriver$3.next(ModuleStateDriver.java:213)
      	at weblogic.application.internal.flow.ModuleStateDriver$3.next(ModuleStateDriver.java:208)
      	at weblogic.application.utils.StateMachineDriver.nextState(StateMachineDriver.java:35)
      	Truncated. see log file for complete stacktrace
      Caused By: java.lang.NullPointerException
      	at org.apache.logging.log4j.core.web.Log4jServletContainerInitializer.onStartup(Log4jServletContainerInitializer.java:44)
      	at weblogic.servlet.internal.WebAppServletContext.initContainerInitializer(WebAppServletContext.java:1271)
      	at weblogic.servlet.internal.WebAppServletContext.initContainerInitializers(WebAppServletContext.java:1229)
      	at weblogic.servlet.internal.WebAppServletContext.preloadResources(WebAppServletContext.java:1726)
      	at weblogic.servlet.internal.WebAppServletContext.start(WebAppServletContext.java:2740)
      	Truncated. see log file for complete stacktrace
      

      If I remove the listener & the filter, it works fine.


      I did some research and found that even though the web-app is version "2.5", the

      Log4jServletContainerInitializer
      is getting invoked.

      1. LOG4J2-359.patch
        3 kB
        Abhinav Shah
      2. Log4jServletContainerInitializerTest.java
        4 kB
        Abhinav Shah
      3. Log4jServletContainerInitializer.java
        2 kB
        Abhinav Shah

        Issue Links

          Activity

          Hide
          Nick Williams added a comment -

          This is fixed with r1561933. Unfortunately, WebLogic is simply not spec compliant here. There's no standard way other than getEffectiveMajorVersion to detect the Servlet version. However, a new setting being added (which will be documented) as part of a separate bug allows you to disable auto-initialization, which you can do as a work around in WebLogic with you set the Servlet version to 2.5. Meanwhile, I suggest you report this bug with getEffectiveMajorVersion to Oracle/WebLogic. I have seen others complaining about it online, but no responses from Oracle.

          Show
          Nick Williams added a comment - This is fixed with r1561933. Unfortunately, WebLogic is simply not spec compliant here. There's no standard way other than getEffectiveMajorVersion to detect the Servlet version. However, a new setting being added (which will be documented) as part of a separate bug allows you to disable auto-initialization, which you can do as a work around in WebLogic with you set the Servlet version to 2.5. Meanwhile, I suggest you report this bug with getEffectiveMajorVersion to Oracle/WebLogic. I have seen others complaining about it online, but no responses from Oracle.
          Hide
          Abhinav Shah added a comment - - edited

          This patch is not working with Weblogic 12c. It works with Glassfish 4.0.
          Weblogic for some reason does not give the right

           servletContext.getEffectiveMajorVersion() 

          .
          With web.xml version attribute = 2.5,

           servletContext.getEffectiveMajorVersion()  

          should have returned 2, but Weblogic gives 3.

          Glassfish on other hand returns 2 as expected.

          We need to find a new fix.

          Show
          Abhinav Shah added a comment - - edited This patch is not working with Weblogic 12c. It works with Glassfish 4.0. Weblogic for some reason does not give the right servletContext.getEffectiveMajorVersion() . With web.xml version attribute = 2.5, servletContext.getEffectiveMajorVersion() should have returned 2, but Weblogic gives 3. Glassfish on other hand returns 2 as expected. We need to find a new fix.
          Hide
          Abhinav Shah added a comment -

          Please check-in the patch I have attached.

          Show
          Abhinav Shah added a comment - Please check-in the patch I have attached.
          Hide
          Abhinav Shah added a comment -

          There was an issue with using servletContext.getMajorVersion(), as it gives the major version of the Servlet API that this servlet container supports.

          This returns 3 in a servlet 3 environment even though web.xml version attribute is 2.5.

          I changed it to use servletContext.getEffectiveMajorVersion() instead.
          I tested and it works as expected.

          Show
          Abhinav Shah added a comment - There was an issue with using servletContext.getMajorVersion(), as it gives the major version of the Servlet API that this servlet container supports. This returns 3 in a servlet 3 environment even though web.xml version attribute is 2.5. I changed it to use servletContext.getEffectiveMajorVersion() instead. I tested and it works as expected.
          Hide
          Ralph Goers added a comment -

          Sorry for editing the comments but I keep finding typos or need to add more detail. For example, when I said annotation in my last comment it really should have been "interface" as that is what @HandleTypes expects.

          First, I hate relying on behaviors that can change from release to release rather than something in the spec, so I applaude your effort to get that corrected.

          I disagree that it is invalid to define Log4j's listener in web.xml. It may be undesirable but it is not invalid. Again, logging a warning that says that it might not work correctly is preferable to aborting the application. Shouldn't it behave essentially as it does in 2.5?

          You are saying we can't reasonably expect users to create jars that they place in WEB-INF/lib? You can't be serious. I do that all the time.

          I don't think it is any more unreasonable to expect an application to provide an initializer for Log4j than it is for Spring, although it will occur less frequently.

          The problem with locating the "proper" configuration file is that you don't know whether it is json, xml or something else so you essentially have to run through all the same logic that ConfigurationFactory does. What if the user wants to place the Log4j jar in the container but then provide a custom configuration factory in one of the web apps? Right now we don't support that but it wouldn't be a big deal to add a new Configurator method that accepts a ConfigurationFactory as an argument. The user would then want to set this up in their own Log4jInitializer by doing:

          public void onStartup(ServletContext servletContext) {
              ConfigurationFactory factory = new MyConfigurationFactory(servletContext.getInitParam("MyInitData"));
              LoggerContext context = Configurator.initialize(servletContext.getServletContextName(), servletContext.getClassLoader(), factory, servletContext);
              servletContext.setAttribute(org.apache.logging.log4j.core.helpers.Constants.LOG4J_SERVLET_CONTEXT, context);
          }
          

          I strongly believe that if we are going to have a ServletContainerInitializer at all that it must support this kind of flexibility. In addition, we also need to allow the user to do the same sort of thing from a Spring WebApplicationInitializer. Notice that this could be handled by checking whether the ServletContext attribute for the LoggerContext has been set.

          Show
          Ralph Goers added a comment - Sorry for editing the comments but I keep finding typos or need to add more detail. For example, when I said annotation in my last comment it really should have been "interface" as that is what @HandleTypes expects. First, I hate relying on behaviors that can change from release to release rather than something in the spec, so I applaude your effort to get that corrected. I disagree that it is invalid to define Log4j's listener in web.xml. It may be undesirable but it is not invalid. Again, logging a warning that says that it might not work correctly is preferable to aborting the application. Shouldn't it behave essentially as it does in 2.5? You are saying we can't reasonably expect users to create jars that they place in WEB-INF/lib? You can't be serious. I do that all the time. I don't think it is any more unreasonable to expect an application to provide an initializer for Log4j than it is for Spring, although it will occur less frequently. The problem with locating the "proper" configuration file is that you don't know whether it is json, xml or something else so you essentially have to run through all the same logic that ConfigurationFactory does. What if the user wants to place the Log4j jar in the container but then provide a custom configuration factory in one of the web apps? Right now we don't support that but it wouldn't be a big deal to add a new Configurator method that accepts a ConfigurationFactory as an argument. The user would then want to set this up in their own Log4jInitializer by doing: public void onStartup(ServletContext servletContext) { ConfigurationFactory factory = new MyConfigurationFactory(servletContext.getInitParam( "MyInitData" )); LoggerContext context = Configurator.initialize(servletContext.getServletContextName(), servletContext.getClassLoader(), factory, servletContext); servletContext.setAttribute(org.apache.logging.log4j.core.helpers.Constants.LOG4J_SERVLET_CONTEXT, context); } I strongly believe that if we are going to have a ServletContainerInitializer at all that it must support this kind of flexibility. In addition, we also need to allow the user to do the same sort of thing from a Spring WebApplicationInitializer. Notice that this could be handled by checking whether the ServletContext attribute for the LoggerContext has been set.
          Hide
          Nick Williams added a comment -

          So. Many. Questions. And stop editing comments hours later!

          I'm going to address the ordering issue, first. You are correct that, according to the spec, the order that SCIs are executed in is unspecified. This is an oversight that I intend to lobby hard to have corrected in Servlet 3.2 (and judging by the Tomcat mailing list, I'll have some backup). However, in practice, this is not actually the case. If you read the responses to the message you link to, you will see that Tomcat orders SCIs according to the web-fragment ordering. This is legal, because the spec doesn't prohibit it, it just doesn't require it. Furthermore, five of the eight major containers (Tomcat, TomEE, JBoss, WebLogic, and WebSphere) order SCIs according to the web-fragment ordering. After further inspection, it appears that GlassFish, Jetty, and Resin do execute them in "random" order (ugh). However, with that said, this "random" order is actually alphabetic by JAR name, and log4j*.jar comes before spring*.jar. So, most users will never see a problem with ordering.

          (Yes, I'm operating under the assumption that Spring's SCI is the one we're most concerned about. I've yet to see any other SCIs in any other third-party libraries or in containers that could result in inadvertent initialization of Log4j. I am, nevertheless, filing improvement requests with GlassFish, Jetty, and Resin asking them to follow what other containers do.)

          With that out of the way, I'm not saying it's "invalid to specify a servlet context listener in the web.xml" in Servlet 3.0+. I'm saying it's invalid (but harmless) to re-define Log4j's listener in web.xml in Servlet 3.0+. I'm not completely opposed to changing the exception to a warning, but I don't agree with the idea, either. Added in the SCI, it ensures that the filter runs before any filters in web.xml. It also ensures that it runs before any filters in other SCIs (with the ordering issue kept in mind, of course). The exception lets the user know in no uncertain terms that Log4j might not work correctly because the filter wasn't defined early enough, and that they can very simply fix the issue by taking the filter out of their web.xml. That's my stance.

          Web applications cannot define their own ServletContainerInitializer implementations without putting them in a JAR file within WEB-INF/lib, so we can't reasonably expect a user to do this. An annotation is a potential idea, but I just don't see the necessity. The existing code is capable of initializing correctly. Just because there was a bug doesn't mean the whole concept is fatally flawed. I strongly stand behind the idea of the user not having to do anything other than add log4j2.xml to their application unless they need to customize behavior (which they can do using the context parameters; they don't need to override the existing initializer to do this). If they really need to stop Log4j from auto-initializing in Servlet 3.0+, we don't have to provide a mechanism to support this. They can use <absolute-ordering> in web.xml to exclude the Log4j web-fragment. I'm okay with adding an example of doing this to the documentation.

          I do agree that the initializer should not do anything if the Log4j JARs come from the container and the application isn't actually using Log4j. What is the correct way to detect this? I believe this is sufficient, but I could be wrong:

          • If the Log4j context parameters are specified, the application obviously intends to use Log4j, so initialize it.
          • If the Log4j context parameters are not specified but one of the standard configuration files is present, the application obviously intends to use Log4j, so initialize it.
          • Otherwise, don't initialize Log4j.

          If this is correct, this leads me to a question. How do I look for the "standard configuration" without Log4j initializing the null or default configuration if it doesn't find one?

          Show
          Nick Williams added a comment - So. Many. Questions. And stop editing comments hours later! I'm going to address the ordering issue, first. You are correct that, according to the spec , the order that SCIs are executed in is unspecified. This is an oversight that I intend to lobby hard to have corrected in Servlet 3.2 (and judging by the Tomcat mailing list, I'll have some backup). However, in practice, this is not actually the case. If you read the responses to the message you link to, you will see that Tomcat orders SCIs according to the web-fragment ordering. This is legal, because the spec doesn't prohibit it, it just doesn't require it. Furthermore, five of the eight major containers (Tomcat, TomEE, JBoss, WebLogic, and WebSphere) order SCIs according to the web-fragment ordering. After further inspection, it appears that GlassFish, Jetty, and Resin do execute them in "random" order ( ugh ). However, with that said, this "random" order is actually alphabetic by JAR name, and log4j*.jar comes before spring*.jar. So, most users will never see a problem with ordering. (Yes, I'm operating under the assumption that Spring's SCI is the one we're most concerned about. I've yet to see any other SCIs in any other third-party libraries or in containers that could result in inadvertent initialization of Log4j. I am, nevertheless, filing improvement requests with GlassFish, Jetty, and Resin asking them to follow what other containers do.) With that out of the way, I'm not saying it's "invalid to specify a servlet context listener in the web.xml " in Servlet 3.0+. I'm saying it's invalid (but harmless) to re-define Log4j's listener in web.xml in Servlet 3.0+. I'm not completely opposed to changing the exception to a warning, but I don't agree with the idea, either. Added in the SCI, it ensures that the filter runs before any filters in web.xml . It also ensures that it runs before any filters in other SCIs (with the ordering issue kept in mind, of course). The exception lets the user know in no uncertain terms that Log4j might not work correctly because the filter wasn't defined early enough, and that they can very simply fix the issue by taking the filter out of their web.xml . That's my stance. Web applications cannot define their own ServletContainerInitializer implementations without putting them in a JAR file within WEB-INF/lib, so we can't reasonably expect a user to do this. An annotation is a potential idea, but I just don't see the necessity. The existing code is capable of initializing correctly. Just because there was a bug doesn't mean the whole concept is fatally flawed. I strongly stand behind the idea of the user not having to do anything other than add log4j2.xml to their application unless they need to customize behavior (which they can do using the context parameters; they don't need to override the existing initializer to do this). If they really need to stop Log4j from auto-initializing in Servlet 3.0+, we don't have to provide a mechanism to support this. They can use <absolute-ordering> in web.xml to exclude the Log4j web-fragment. I'm okay with adding an example of doing this to the documentation. I do agree that the initializer should not do anything if the Log4j JARs come from the container and the application isn't actually using Log4j. What is the correct way to detect this? I believe this is sufficient, but I could be wrong: If the Log4j context parameters are specified, the application obviously intends to use Log4j, so initialize it. If the Log4j context parameters are not specified but one of the standard configuration files is present, the application obviously intends to use Log4j, so initialize it. Otherwise, don't initialize Log4j. If this is correct, this leads me to a question. How do I look for the "standard configuration" without Log4j initializing the null or default configuration if it doesn't find one?
          Hide
          Ralph Goers added a comment -

          I am inclined to think that we need to introduce a Log4jWebInitializer annotation and do exactly what Spring is doing to allow applications to wire in stuff at startup. However, we also need to allow for the whole thing to be bypassed if it has already been done, such as if a Spring WebApplicationInitializer sets up logging.

          Show
          Ralph Goers added a comment - I am inclined to think that we need to introduce a Log4jWebInitializer annotation and do exactly what Spring is doing to allow applications to wire in stuff at startup. However, we also need to allow for the whole thing to be bypassed if it has already been done, such as if a Spring WebApplicationInitializer sets up logging.
          Hide
          Ralph Goers added a comment - - edited

          So I found your questions and the answers on SCI ordering at http://www.mail-archive.com/users@tomcat.apache.org/msg106625.html and now I am really confused. From what I understand even if Log4j and Spring both provide initializers there is still no guarantee that Log4j will initialize before Spring. It would seem to me that the only way to solve that would be to provide a Spring WebApplicationInitializer that is guaranteed to be the first one called. If that is done then the Log4j ServletContainerInitializer is redundant.

          Am I misunderstanding something?

          Show
          Ralph Goers added a comment - - edited So I found your questions and the answers on SCI ordering at http://www.mail-archive.com/users@tomcat.apache.org/msg106625.html and now I am really confused. From what I understand even if Log4j and Spring both provide initializers there is still no guarantee that Log4j will initialize before Spring. It would seem to me that the only way to solve that would be to provide a Spring WebApplicationInitializer that is guaranteed to be the first one called. If that is done then the Log4j ServletContainerInitializer is redundant. Am I misunderstanding something?
          Hide
          Ralph Goers added a comment - - edited

          Further thoughts on this.

          I'd probably be more in favor of this if the initializer looked for a configuration being present and exited if it couldn't find one. But what if a user wants to create their own configuration factory? They can't override our initializer or replace it because it is embedded in the jar. Unless you provide extension points for users then this is just going to create a mess.

          I don't recall seeing it in the spec, but is there some way a user can insure that their servlet container initializer runs first? If we are going to keep this I would like to see examples of how users can perform customizations.

          Show
          Ralph Goers added a comment - - edited Further thoughts on this. I'd probably be more in favor of this if the initializer looked for a configuration being present and exited if it couldn't find one. But what if a user wants to create their own configuration factory? They can't override our initializer or replace it because it is embedded in the jar. Unless you provide extension points for users then this is just going to create a mess. I don't recall seeing it in the spec, but is there some way a user can insure that their servlet container initializer runs first? If we are going to keep this I would like to see examples of how users can perform customizations.
          Hide
          Ralph Goers added a comment - - edited

          First, I don't understand how it is invalid to specify a servlet context listener in the web.xml. I don't believe they were removed from the spec. Someone upgrading from 2.5 to 3 is quite likely to just change the version number and leave the rest alone.

          Although you are indeed preventing the error by now detecting that the filter is being added a second time, I don't understand why you are throwing an exception. Just log a warning. The application shouldn't fail because they manually specified it.

          I understand that the ServletContainerInitializer must run automatically. I just really don't like it being the default behavior of just having the log4j core jar being present. I am quite sure it is going to cause problems for some users. In Spring's case it only initializes WebApplicationInitializer classes. If the web app has none it is effectively a no-op. In our case if the log4j jar is in the container's class path then every app will initialize whether it actually uses log4j or not and whether it has a configuration or not.

          Show
          Ralph Goers added a comment - - edited First, I don't understand how it is invalid to specify a servlet context listener in the web.xml. I don't believe they were removed from the spec. Someone upgrading from 2.5 to 3 is quite likely to just change the version number and leave the rest alone. Although you are indeed preventing the error by now detecting that the filter is being added a second time, I don't understand why you are throwing an exception. Just log a warning. The application shouldn't fail because they manually specified it. I understand that the ServletContainerInitializer must run automatically. I just really don't like it being the default behavior of just having the log4j core jar being present. I am quite sure it is going to cause problems for some users. In Spring's case it only initializes WebApplicationInitializer classes. If the web app has none it is effectively a no-op. In our case if the log4j jar is in the container's class path then every app will initialize whether it actually uses log4j or not and whether it has a configuration or not.
          Hide
          Nick Williams added a comment - - edited

          1. No, it would not be valid to specify the listener in web.xml in a Servlet 3.0 application (the container doesn't matter, only the effective application version).
          2. It would also be completely unnecessary for them to manually specify the listener in web.xml in a Servlet 3.0 application. Doing so would not provide them any benefit, as they can customize the behavior using context parameters without manually specifying the listener.
          3. If they did manually specify the listener in web.xml, it would not have any negative impact on the application working correctly, because the second call to initialize the Log4jWebInitializer is ignored. The problem present in this bug was specifically that the filter was being added twice (which might not be immediately clear since the errors were all in the listener and the initializer). It's important for the filter to be added so that it executes before any filters defined in web.xml, so adding it in the initializer is the safest route. In Servlet 2.5 applications the filter won't be added in the initializer (and neither will the listener). In Servlet 3.0+ applications, Log4j makes in clear with a specific error message that the user must not specify the filter in web.xml.
          4. The Log4jWebInitializer must run automatically in a ServletContainerInitializer. This is not an option. The reason for this is that other ServletContainerInitializer implementations, such as the one provided by Spring Framework, may trigger behavior that causes logging to take place. If this happens before the initializer runs, all bets are off. Log4j will not start up correctly and will cause errors on shutdown.

          Show
          Nick Williams added a comment - - edited 1. No, it would not be valid to specify the listener in web.xml in a Servlet 3.0 application (the container doesn't matter, only the effective application version). 2. It would also be completely unnecessary for them to manually specify the listener in web.xml in a Servlet 3.0 application. Doing so would not provide them any benefit, as they can customize the behavior using context parameters without manually specifying the listener. 3. If they did manually specify the listener in web.xml , it would not have any negative impact on the application working correctly, because the second call to initialize the Log4jWebInitializer is ignored. The problem present in this bug was specifically that the filter was being added twice (which might not be immediately clear since the errors were all in the listener and the initializer). It's important for the filter to be added so that it executes before any filters defined in web.xml , so adding it in the initializer is the safest route. In Servlet 2.5 applications the filter won't be added in the initializer (and neither will the listener). In Servlet 3.0+ applications, Log4j makes in clear with a specific error message that the user must not specify the filter in web.xml . 4. The Log4jWebInitializer must run automatically in a ServletContainerInitializer . This is not an option. The reason for this is that other ServletContainerInitializer implementations, such as the one provided by Spring Framework, may trigger behavior that causes logging to take place. If this happens before the initializer runs, all bets are off. Log4j will not start up correctly and will cause errors on shutdown.
          Hide
          Ralph Goers added a comment - - edited

          I still don't think this is correct. In a servlet 3.0 environment it would still be valid for the user to specify a servlet context listener in their web.xml would it not? In that case the problem still exists.

          I also have to say that the more I think about this the less comfortable I am in having the initializer run automagically in a servlet 3.0 container. I am thinking perhaps the user should have to provide the initializer instead of us. If the log4j jar is outside the WEB-INF/lib of the application then the log4j initializer's onStartup method will be invoked when every new application is started. This is likely to be undesirable. Does Spring automatically start initialization just because the spring jars are present in a servlet 3.0 environment?

          Show
          Ralph Goers added a comment - - edited I still don't think this is correct. In a servlet 3.0 environment it would still be valid for the user to specify a servlet context listener in their web.xml would it not? In that case the problem still exists. I also have to say that the more I think about this the less comfortable I am in having the initializer run automagically in a servlet 3.0 container. I am thinking perhaps the user should have to provide the initializer instead of us. If the log4j jar is outside the WEB-INF/lib of the application then the log4j initializer's onStartup method will be invoked when every new application is started. This is likely to be undesirable. Does Spring automatically start initialization just because the spring jars are present in a servlet 3.0 environment?
          Hide
          Nick Williams added a comment -

          Thanks for the patch. I tweaked it slightly to provide more extensive testing, ensure the feature is used correctly, and ensure that it does nothing at all if the application version is not 3.0 or greater. This is fixed in trunk. Can you check out, build, verify and close?

          Show
          Nick Williams added a comment - Thanks for the patch. I tweaked it slightly to provide more extensive testing, ensure the feature is used correctly, and ensure that it does nothing at all if the application version is not 3.0 or greater. This is fixed in trunk. Can you check out, build, verify and close?
          Hide
          Nick Williams added a comment -

          Okay. I think I understand something that perhaps I didn't understand before. It sounds like in a Servlet 3.0 environment with version 2.5 in web.xml, the container calls onStartup on the initializer, but ignores the listener that it adds programmatically. This behavior is confusing and arbitrary, and I suspect other containers don't do it this exact same way.

          Because of what I perceive to be a high likelihood of uncertainty in this situation (Servlet 3.0+ container with 2.5- application), I think Abhinav's patch is essentially correct. However, I would extend it further: the initializer just needs to do nothing in onStartup if the Servlet version is less than 3. No adding a listener, no adding a filter. This way, the behavior is guaranteed to be consistent across containers.

          I'll make this change.

          Show
          Nick Williams added a comment - Okay. I think I understand something that perhaps I didn't understand before. It sounds like in a Servlet 3.0 environment with version 2.5 in web.xml, the container calls onStartup on the initializer, but ignores the listener that it adds programmatically. This behavior is confusing and arbitrary, and I suspect other containers don't do it this exact same way. Because of what I perceive to be a high likelihood of uncertainty in this situation (Servlet 3.0+ container with 2.5- application), I think Abhinav's patch is essentially correct. However, I would extend it further: the initializer just needs to do nothing in onStartup if the Servlet version is less than 3. No adding a listener, no adding a filter. This way, the behavior is guaranteed to be consistent across containers. I'll make this change.
          Hide
          Abhinav Shah added a comment - - edited

          Nick,
          This goes back to the original problem. In a servlet 3.0+ environment, but if your web.xml has version attribute 2.5, you would need a listener.

          I am running weblogic 12c on my desktop. However I am setting version attribute as 2.5 in my web.xml so that my code runs on the production servers which are running old version of weblogic. So I need the ability to have listener configured in web.xml.

          PS: this same problem exists in glassfish 4.0.

          Show
          Abhinav Shah added a comment - - edited Nick, This goes back to the original problem. In a servlet 3.0+ environment, but if your web.xml has version attribute 2.5, you would need a listener. I am running weblogic 12c on my desktop. However I am setting version attribute as 2.5 in my web.xml so that my code runs on the production servers which are running old version of weblogic. So I need the ability to have listener configured in web.xml. PS: this same problem exists in glassfish 4.0.
          Hide
          Nick Williams added a comment -

          The problem is purely in documentation. The documentation just needs to be changed to say that if you're running on a Servlet 3.0 or newer environment, you don't have to manually initialize Log4j; one should not put the Log4j listener and filter in web.xml in a Servlet 3.0 or newer environment.

          Show
          Nick Williams added a comment - The problem is purely in documentation. The documentation just needs to be changed to say that if you're running on a Servlet 3.0 or newer environment, you don't have to manually initialize Log4j; one should not put the Log4j listener and filter in web.xml in a Servlet 3.0 or newer environment.
          Hide
          Abhinav Shah added a comment -

          Ralph,
          Even if we remove the initialization code from the WebInitializer, there will still be two instances of the WebInitializer. One from web.xml and another which gets added from the Log4jServletContextInitializer. So the issue will still be there.

          Show
          Abhinav Shah added a comment - Ralph, Even if we remove the initialization code from the WebInitializer, there will still be two instances of the WebInitializer. One from web.xml and another which gets added from the Log4jServletContextInitializer. So the issue will still be there.
          Hide
          Ralph Goers added a comment -

          The problem with this patch is that nothing gets control when the servlet is undeployed or shutdown. That is why I suggested removing the initialization of the WebInitializer from the onStartup method.

          Show
          Ralph Goers added a comment - The problem with this patch is that nothing gets control when the servlet is undeployed or shutdown. That is why I suggested removing the initialization of the WebInitializer from the onStartup method.
          Hide
          Abhinav Shah added a comment -

          Nick. I just removed all the code formatting. Attaching new patch.

          Show
          Abhinav Shah added a comment - Nick. I just removed all the code formatting. Attaching new patch.
          Hide
          Nick Williams added a comment - - edited

          Your IDE has reformatted the JavaDoc and Java code in the file. This makes it very difficult to separate out the changes you made from the reformatting. Please correct this. Patches should not reformat code that doesn't change. You will likely need to disable this auto-reformatting in your IDE, or change the settings to reflect the Log4j coding standards (for example, 120-character lines, not 80-character lines as it appears your IDE is enforcing).

          Show
          Nick Williams added a comment - - edited Your IDE has reformatted the JavaDoc and Java code in the file. This makes it very difficult to separate out the changes you made from the reformatting. Please correct this. Patches should not reformat code that doesn't change. You will likely need to disable this auto-reformatting in your IDE, or change the settings to reflect the Log4j coding standards (for example, 120-character lines, not 80-character lines as it appears your IDE is enforcing).
          Hide
          Abhinav Shah added a comment -

          Attaching svn patch.

          Show
          Abhinav Shah added a comment - Attaching svn patch.
          Hide
          Nick Williams added a comment - - edited

          I shouldn't have closed this as duplicate. There is a separate issues here, which needs to be resolved in documentation.

          Show
          Nick Williams added a comment - - edited I shouldn't have closed this as duplicate. There is a separate issues here, which needs to be resolved in documentation.
          Hide
          Abhinav Shah added a comment -

          Issue fixed.

          Show
          Abhinav Shah added a comment - Issue fixed.
          Hide
          Abhinav Shah added a comment -

          Ok I fixed this issue. Since I am not a contributor to log4j2, I am just going to attach the source files here.

          Show
          Abhinav Shah added a comment - Ok I fixed this issue. Since I am not a contributor to log4j2, I am just going to attach the source files here.
          Hide
          Ralph Goers added a comment - - edited

          Nick, Log4jServletContainerInitializer is adding Log4jServletContextListener as a listener but then appears to act as if its contextInitialized method is not going to be called. From my reading and in looking at http://svn.apache.org/repos/asf/tomcat/trunk/test/org/apache/catalina/startup/TestListener.java that doesn't appear to be the case. Shouldn't the onStartup method look like:

              @Override
              public void onStartup(final Set<Class<?>> classes, final ServletContext servletContext) throws ServletException {
                  servletContext.log("Log4jServletContainerInitializer starting up Log4j in Servlet 3.0+ environment.");
                  servletContext.addListener(new Log4jServletContextListener());
          
                  final FilterRegistration.Dynamic filter = servletContext.addFilter("log4jServletFilter", new Log4jServletFilter());
                  if (filter != null) {
                      filter.addMappingForUrlPatterns(EnumSet.allOf(DispatcherType.class), false, "/*");
                  }
              }
          

          Of course, if the user also configures the ServletContextListener in web.xml also then it will still try to initialize and destroy twice, but that can be handled pretty easily.

          Show
          Ralph Goers added a comment - - edited Nick, Log4jServletContainerInitializer is adding Log4jServletContextListener as a listener but then appears to act as if its contextInitialized method is not going to be called. From my reading and in looking at http://svn.apache.org/repos/asf/tomcat/trunk/test/org/apache/catalina/startup/TestListener.java that doesn't appear to be the case. Shouldn't the onStartup method look like: @Override public void onStartup( final Set< Class <?>> classes, final ServletContext servletContext) throws ServletException { servletContext.log( "Log4jServletContainerInitializer starting up Log4j in Servlet 3.0+ environment." ); servletContext.addListener( new Log4jServletContextListener()); final FilterRegistration.Dynamic filter = servletContext.addFilter( "log4jServletFilter" , new Log4jServletFilter()); if (filter != null ) { filter.addMappingForUrlPatterns(EnumSet.allOf(DispatcherType.class), false , "/*" ); } } Of course, if the user also configures the ServletContextListener in web.xml also then it will still try to initialize and destroy twice, but that can be handled pretty easily.
          Hide
          Nick Williams added a comment -

          Oh. Haha. I missed that part, somehow. Thanks!

          Show
          Nick Williams added a comment - Oh. Haha. I missed that part, somehow. Thanks!
          Hide
          Abhinav Shah added a comment -

          Thanks Nick. I already mentioned that if I remove the listener & the filter, it works fine.

          Show
          Abhinav Shah added a comment - Thanks Nick. I already mentioned that if I remove the listener & the filter, it works fine.
          Hide
          Nick Williams added a comment - - edited

          Thanks for the thorough bug report. This may simply be a documentation issue. After re-reading the relevant part of the spec, it would seem that Servlet 3.0+ containers must invoke any Log4jServletContainerInitializer implementations even if the web-app version is 2.5. This seems like a bad decision on the part of the spec committee, but if it's the case the documentation will have to be refactored. I need to discuss it with some fellow Java EE experts and confirm my suspicions.

          In the meantime, can you take the <listener>, <filter>, and <filter-mapping> out of your deployment descriptor and see if it starts working? Note that you might starting seeing a different error (NullPointerException) due to bug LOG4J2-344.

          Show
          Nick Williams added a comment - - edited Thanks for the thorough bug report. This may simply be a documentation issue. After re-reading the relevant part of the spec, it would seem that Servlet 3.0+ containers must invoke any Log4jServletContainerInitializer implementations even if the web-app version is 2.5. This seems like a bad decision on the part of the spec committee, but if it's the case the documentation will have to be refactored. I need to discuss it with some fellow Java EE experts and confirm my suspicions. In the meantime, can you take the <listener> , <filter> , and <filter-mapping> out of your deployment descriptor and see if it starts working? Note that you might starting seeing a different error ( NullPointerException ) due to bug LOG4J2-344 .

            People

            • Assignee:
              Nick Williams
              Reporter:
              Abhinav Shah
            • Votes:
              3 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development