Merge pull request #88 from jenkinsci/npe

Prevent NPE when createValue returns null

Author: res0nance

src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterParser.java

@@ -1,17 +1,14 @@
 package org.jenkinsci.plugins.parameterizedscheduler;
 
+import com.google.common.base.Splitter;
 import hudson.model.ParametersDefinitionProperty;
+import org.apache.commons.lang.StringUtils;
 
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
 
-import org.apache.commons.lang.StringUtils;
-import org.jenkinsci.plugins.parameterizedscheduler.Messages;
-
-import com.google.common.base.Splitter;
-
 public class ParameterParser {
 	/**
 	 * if ever changed, documentation and messages will need to be updated as well

src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedTimerTrigger.java

@@ -1,5 +1,6 @@
 package org.jenkinsci.plugins.parameterizedscheduler;
 
+import antlr.ANTLRException;
 import hudson.model.AbstractProject;
 import hudson.model.Cause;
 import hudson.model.CauseAction;
@@ -10,6 +11,8 @@
 import hudson.model.ParametersDefinitionProperty;
 import hudson.scheduler.Hash;
 import hudson.triggers.Trigger;
+import org.jenkinsci.plugins.workflow.job.WorkflowJob;
+import org.kohsuke.stapler.DataBoundConstructor;
 
 import java.util.ArrayList;
 import java.util.Calendar;
@@ -18,11 +21,6 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import org.jenkinsci.plugins.workflow.job.WorkflowJob;
-import org.kohsuke.stapler.DataBoundConstructor;
-
-import antlr.ANTLRException;
-
 /**
  * {@link Trigger} that runs a job periodically with support for parameters.
  * 
@@ -64,9 +62,15 @@ private List<ParameterValue> configurePropertyValues(Map<String, String> paramet
 			if (parameterValues.containsKey(paramDefinition.getName())) {
 				ParameterizedStaplerRequest request = new ParameterizedStaplerRequest(
 						parameterValues.get(paramDefinition.getName()));
-				defValues.add(paramDefinition.createValue(request));
-			} else if (defaultValue != null)
+				ParameterValue value = paramDefinition.createValue(request);
+				if (value!= null) {
+					defValues.add(value);
+				} else {
+					LOGGER.warning("Cannot create value for " + paramDefinition.getName());
+				}
+			} else if (defaultValue != null) {
 				defValues.add(defaultValue);
+			}
 		}
 
 		return defValues;

src/test/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedSchedulerTest.java

@@ -2,16 +2,24 @@
 
 import hudson.model.FreeStyleProject;
 import hudson.model.Job;
+import hudson.model.ParameterDefinition;
+import hudson.model.ParameterValue;
 import hudson.model.ParametersAction;
 import hudson.model.ParametersDefinitionProperty;
 import hudson.model.StringParameterDefinition;
 import hudson.triggers.Trigger;
+import net.sf.json.JSONObject;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
 import org.junit.Rule;
 import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.kohsuke.stapler.StaplerRequest;
+
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
@@ -97,4 +105,39 @@ public void declarative() throws Exception {
         assertThat(p.getLastCompletedBuild(), is(not(wfr)));
         assertThat((String) p.getLastCompletedBuild().getAction(ParametersAction.class).getParameter("foo").getValue(), is("bar"));
     }
+
+    @Test
+    @Issue("JENKINS-49372")
+    public void nullValueCreated() throws Exception {
+        FreeStyleProject p = r.createFreeStyleProject();
+        p.addProperty(new ParametersDefinitionProperty(new NullParameterDefinition("foo")));
+        assertThat(p.getLastCompletedBuild(), is(nullValue()));
+        Trigger<Job> t = new ParameterizedTimerTrigger("* * * * *%foo=test");
+        t.start(p, true);
+        p.addTrigger(t);
+        new Cron().doRun();
+        assertThat(p.isInQueue(), is(true));
+        r.waitUntilNoActivity();
+        // Build should complete successfully but will not have any value
+        assertThat(p.getLastCompletedBuild(), is(notNullValue()));
+    }
+
+    private static class NullParameterDefinition extends ParameterDefinition {
+
+        public NullParameterDefinition(@Nonnull String name) {
+            super(name, null);
+        }
+
+        @CheckForNull
+        @Override
+        public ParameterValue createValue(StaplerRequest staplerRequest, JSONObject jsonObject) {
+            return null;
+        }
+
+        @CheckForNull
+        @Override
+        public ParameterValue createValue(StaplerRequest staplerRequest) {
+            return null;
+        }
+    }
 }