[JENKINS-19985] Prevent NPE and save if no publishers are added to th…

…e conditional step. This also prevents a NPE when the job loads.
jenkinsci · Jan 15, 2015 · c0618c4 · c0618c4
1 parent 02e403e
commit c0618c4
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 2 deletions.
diff --git a/src/main/java/org/jenkins_ci/plugins/flexible_publish/ConditionalPublisher.java b/src/main/java/org/jenkins_ci/plugins/flexible_publish/ConditionalPublisher.java
@@ -111,6 +111,10 @@ public ConditionalPublisher(final RunCondition condition, final BuildStep publis
     public ConditionalPublisher(final RunCondition condition, final List<BuildStep> publisherList, final BuildStepRunner runner,
             boolean configuredAggregation, final RunCondition aggregationCondition, final BuildStepRunner aggregationRunner) {
         this.condition = condition;
+        // Guard against inserting null publishers to the list.
+        if (publisherList == null || publisherList.contains(null)) {
+            throw new IllegalArgumentException("Must specifiy a publisher for a conditional publisher.");
+        }
         this.publisherList = publisherList;
         this.runner = runner;
         if (configuredAggregation) {
@@ -137,9 +141,19 @@ public BuildStep getPublisher() {
     }
 
     public List<BuildStep> getPublisherList() {
-        return publisherList != null?publisherList:Collections.<BuildStep>emptyList();
-    }
+        if (publisherList != null) {
+            // We also need to guard against any null values in the publisherList
+            // This can happen if the projects publisher list was removed, returning
+            // a list with one null element.
+            if (publisherList.contains(null)) {
+                publisherList.remove(null);
+            }
 
+            return publisherList;
+        } else {
+            return Collections.<BuildStep>emptyList();
+        }
+    }
     public BuildStepRunner getRunner() {
         return runner;
     }

diff --git a/src/test/java/org/jenkins_ci/plugins/flexible_publish/ConfigurationTest.java b/src/test/java/org/jenkins_ci/plugins/flexible_publish/ConfigurationTest.java
@@ -27,6 +27,7 @@
 import java.io.IOException;
 import java.lang.reflect.Constructor;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
 import hudson.Launcher;
@@ -53,6 +54,7 @@
 import org.jenkins_ci.plugins.run_condition.BuildStepRunner;
 import org.jenkins_ci.plugins.run_condition.core.AlwaysRun;
 import org.jenkins_ci.plugins.run_condition.core.NeverRun;
+import org.jvnet.hudson.test.Bug;
 import org.jvnet.hudson.test.HudsonTestCase;
 import org.jvnet.hudson.test.TestExtension;
 import org.kohsuke.stapler.DataBoundConstructor;
@@ -87,6 +89,21 @@ public void testWithoutPublishers() throws Exception {
         reconfigure(p);
     }
 
+    @Bug(19985)
+    public void testNullCondition() throws  Exception {
+      FreeStyleProject p = createFreeStyleProject();
+      try {
+        ConditionalPublisher conditionalPublisher = new ConditionalPublisher(new AlwaysRun(),
+                Collections.<BuildStep>singletonList(null),
+                null,
+                false,
+                null,
+                null);
+
+          fail("Should throw Exception when null publishers are added.");
+      } catch (IllegalArgumentException e) { }
+    }
+
     public void testSingleCondition() throws Exception {
         FreeStyleProject p = createFreeStyleProject();
         BuildTrigger trigger = new BuildTrigger("anotherProject", Result.SUCCESS);