[JENKINS-34755] fix read of 'SystemProperties' values before init of …

…the context (#2347) * fix read of 'SystemProperties' values before init of the context * fix constructor and test * revert help-spec_fr.html * revert help-spec_fr.html * revert help-spec_fr.html
jenkinsci · May 15, 2016 · 65f2a4a · 65f2a4a
1 parent 606794a
commit 65f2a4a
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 20 deletions.
diff --git a/core/src/main/java/hudson/WebAppMain.java b/core/src/main/java/hudson/WebAppMain.java
@@ -121,8 +121,6 @@ public Locale get() {
 
             installLogger();
 
-            SystemProperties.initialize( event.getServletContext() );
-
             final FileAndDescription describedHomeDir = getHomeDir(event);
             home = describedHomeDir.file.getAbsoluteFile();
             home.mkdirs();

diff --git a/core/src/main/java/jenkins/util/SystemProperties.java b/core/src/main/java/jenkins/util/SystemProperties.java
@@ -29,6 +29,8 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
 import org.apache.commons.lang.StringUtils;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
@@ -62,7 +64,9 @@
  */
 //TODO: Define a correct design of this engine later. Should be accessible in libs (remoting, stapler) and Jenkins modules too
 @Restricted(NoExternalUse.class)
-public class SystemProperties {
+public class SystemProperties implements ServletContextListener {
+    // this class implements ServletContextListener and is declared in WEB-INF/web.xml
+
     /**
      * The ServletContext to get the "init" parameters from.
      */
@@ -75,15 +79,22 @@ public class SystemProperties {
     private static final Logger LOGGER = Logger.getLogger(SystemProperties.class.getName());
 
     /**
-     * This class should never be instantiated.
+     * Public for the servlet container.
+     */
+    public SystemProperties() {}
+
+    /**
+     * Called by the servlet container to initialize the {@link ServletContext}.
      */
-    private SystemProperties() {}
+    @Override
+    public void contextInitialized(ServletContextEvent event) {
+        theContext = event.getServletContext();
+    }
 
     /**
      * Gets the system property indicated by the specified key.
      * This behaves just like {@link System#getProperty(java.lang.String)}, except that it
      * also consults the {@link ServletContext}'s "init" parameters.
-     * {@link ServletContext} check will be skipped if the context is not initialized.
      * 
      * @param      key   the name of the system property.
      * @return     the string value of the system property,
@@ -120,7 +131,6 @@ public static String getString(String key) {
      * Gets the system property indicated by the specified key, or a default value.
      * This behaves just like {@link System#getProperty(java.lang.String, java.lang.String)}, except
      * that it also consults the {@link ServletContext}'s "init" parameters.
-     * {@link ServletContext} check will be skipped if the context is not initialized.
      * 
      * @param      key   the name of the system property.
      * @param      def   a default value.
@@ -163,7 +173,6 @@ public static String getString(String key, @CheckForNull String def) {
       * 
       * This behaves just like {@link Boolean#getBoolean(java.lang.String)}, except that it
       * also consults the {@link ServletContext}'s "init" parameters.
-      * {@link ServletContext} check will be skipped if the context is not initialized.
       * 
       * @param   name   the system property name.
       * @return  the {@code boolean} value of the system property.
@@ -182,7 +191,6 @@ public static boolean getBoolean(String name) {
       * 
       * This behaves just like {@link Boolean#getBoolean(java.lang.String)} with a default
       * value, except that it also consults the {@link ServletContext}'s "init" parameters.
-      * {@link ServletContext} check will be skipped if the context is not initialized.
       * 
       * @param   name   the system property name.
       * @param   def   a default value.
@@ -203,7 +211,6 @@ public static boolean getBoolean(String name, boolean def) {
       * 
       * This behaves just like {@link Integer#getInteger(java.lang.String)}, except that it
       * also consults the {@link ServletContext}'s "init" parameters.
-      * {@link ServletContext} check will be skipped if the context is not initialized.
       * 
       * @param   name property name.
       * @return  the {@code Integer} value of the property.
@@ -243,15 +250,6 @@ public static Integer getInteger(String name, Integer def) {
         return def;
     }
 
-    /**
-     * Invoked by WebAppMain, tells us where to get the "init" parameters from.
-     * 
-     * @param context the <code>ServletContext</code> obtained from <code>contextInitialized</code>
-     */
-    public static void initialize(ServletContext context) {
-        theContext = context;
-    }
-
     @CheckForNull
     private static String tryGetValueFromContext(String key) {
         if (StringUtils.isNotBlank(key) && theContext != null) {
@@ -267,4 +265,9 @@ private static String tryGetValueFromContext(String key) {
         }
         return null;
     }
+
+    @Override
+    public void contextDestroyed(ServletContextEvent event) {
+        // nothing to do
+    }
 }
diff --git a/test/src/test/java/jenkins/util/SystemPropertiesTest.java b/test/src/test/java/jenkins/util/SystemPropertiesTest.java
@@ -24,6 +24,7 @@
 package jenkins.util;
 
 import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
 import net.sourceforge.htmlunit.corejs.javascript.RhinoSecurityManager;
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.assertThat;
@@ -43,7 +44,7 @@ public class SystemPropertiesTest {
 
     @Before
     public void setUp() {
-        SystemProperties.initialize(j.jenkins.servletContext);
+        new SystemProperties().contextInitialized(new ServletContextEvent(j.jenkins.servletContext));
     }
 
     @Test

diff --git a/war/src/main/webapp/WEB-INF/web.xml b/war/src/main/webapp/WEB-INF/web.xml
@@ -156,6 +156,10 @@ THE SOFTWARE.
     <url-pattern>/*</url-pattern>
   </filter-mapping>
 
+  <listener>
+    <!-- Must be before WebAppMain in order to initialize the context before the first use of this class. -->
+    <listener-class>jenkins.util.SystemProperties</listener-class>
+  </listener>
   <listener>
     <listener-class>hudson.WebAppMain</listener-class>
   </listener>