Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[FIXED JENKINS-15094] - Handle null parameter values in BuildCommand …
…(CLI) and Parameter classes (CLI).

The change adds missing checks for null ParameterValue, which may be passed from the CLI.
It prevents the death of Jenkins executors during the canTake() check of node labels.

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
  • Loading branch information
oleg-nenashev committed May 31, 2014
1 parent cd23fac commit 7b894f5
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 12 deletions.
17 changes: 14 additions & 3 deletions core/src/main/java/hudson/cli/BuildCommand.java
Expand Up @@ -51,6 +51,7 @@
import java.util.Map.Entry;
import java.io.FileNotFoundException;
import java.io.PrintStream;
import javax.annotation.Nonnull;

import jenkins.model.Jenkins;

Expand Down Expand Up @@ -101,15 +102,21 @@ protected int run() throws Exception {
if (pdp==null)
throw new AbortException(job.getFullDisplayName()+" is not parameterized but the -p option was specified");

//TODO: switch to type annotations after the migration to Java 1.8
List<ParameterValue> values = new ArrayList<ParameterValue>();

for (Entry<String, String> e : parameters.entrySet()) {
String name = e.getKey();
ParameterDefinition pd = pdp.getParameterDefinition(name);
if (pd==null)
if (pd==null) {
throw new AbortException(String.format("\'%s\' is not a valid parameter. Did you mean %s?",
name, EditDistance.findNearest(name, pdp.getParameterDefinitionNames())));
values.add(pd.createValue(this, Util.fixNull(e.getValue())));
}
ParameterValue val = pd.createValue(this, Util.fixNull(e.getValue()));
if (val == null) {
throw new AbortException(String.format("Cannot resolve the value for the parameter \'%s\'.",name));
}
values.add(val);
}

// handle missing parameters by adding as default values ISSUE JENKINS-7162
Expand All @@ -118,7 +125,11 @@ protected int run() throws Exception {
continue;

// not passed in use default
values.add(pd.getDefaultParameterValue());
ParameterValue defaultValue = pd.getDefaultParameterValue();
if (defaultValue == null) {
throw new AbortException(String.format("No default value for the parameter \'%s\'.",pd.getName()));
}
values.add(defaultValue);
}

a = new ParametersAction(values);
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/hudson/model/ParameterDefinition.java
Expand Up @@ -33,6 +33,7 @@
import java.io.Serializable;
import java.io.IOException;
import java.util.logging.Logger;
import javax.annotation.CheckForNull;

import jenkins.model.Jenkins;
import net.sf.json.JSONObject;
Expand Down Expand Up @@ -165,6 +166,7 @@ public ParameterDescriptor getDescriptor() {
* This method is invoked when the user fills in the parameter values in the HTML form
* and submits it to the server.
*/
@CheckForNull
public abstract ParameterValue createValue(StaplerRequest req, JSONObject jo);

/**
Expand All @@ -183,6 +185,7 @@ public ParameterDescriptor getDescriptor() {
* @throws IllegalStateException
* If the parameter is deemed required but was missing in the submission.
*/
@CheckForNull
public abstract ParameterValue createValue(StaplerRequest req);


Expand All @@ -200,6 +203,7 @@ public ParameterDescriptor getDescriptor() {
* the command exits with an error code.
* @since 1.334
*/
@CheckForNull
public ParameterValue createValue(CLICommand command, String value) throws IOException, InterruptedException {
throw new AbortException("CLI parameter submission is not supported for the "+getClass()+" type. Please file a bug report for this");
}
Expand All @@ -210,6 +214,7 @@ public ParameterValue createValue(CLICommand command, String value) throws IOExc
* @return default parameter value or null if no defaults are available
* @since 1.253
*/
@CheckForNull
@Exported
public ParameterValue getDefaultParameterValue() {
return null;
Expand Down
36 changes: 28 additions & 8 deletions core/src/main/java/hudson/model/ParametersAction.java
Expand Up @@ -45,6 +45,8 @@

import static com.google.common.collect.Lists.newArrayList;
import static com.google.common.collect.Sets.newHashSet;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;

/**
* Records the parameter values used for a build.
Expand Down Expand Up @@ -74,14 +76,17 @@ public ParametersAction(ParameterValue... parameters) {

public void createBuildWrappers(AbstractBuild<?,?> build, Collection<? super BuildWrapper> result) {
for (ParameterValue p : parameters) {
if (p == null) continue;
BuildWrapper w = p.createBuildWrapper(build);
if(w!=null) result.add(w);
}
}

public void buildEnvVars(AbstractBuild<?,?> build, EnvVars env) {
for (ParameterValue p : parameters)
p.buildEnvironment(build, env);
for (ParameterValue p : parameters) {
if (p == null) continue;
p.buildEnvironment(build, env);
}
}

// TODO do we need an EnvironmentContributingAction variant that takes Run so this can implement it?
Expand All @@ -102,14 +107,16 @@ public String substitute(AbstractBuild<?,?> build, String text) {
public VariableResolver<String> createVariableResolver(AbstractBuild<?,?> build) {
VariableResolver[] resolvers = new VariableResolver[parameters.size()+1];
int i=0;
for (ParameterValue p : parameters)
for (ParameterValue p : parameters) {
if (p == null) continue;
resolvers[i++] = p.createVariableResolver(build);

}

resolvers[i] = build.getBuildVariableResolver();

return new VariableResolver.Union<String>(resolvers);
}

public Iterator<ParameterValue> iterator() {
return parameters.iterator();
}
Expand All @@ -120,14 +127,17 @@ public List<ParameterValue> getParameters() {
}

public ParameterValue getParameter(String name) {
for (ParameterValue p : parameters)
for (ParameterValue p : parameters) {
if (p == null) continue;
if (p.getName().equals(name))
return p;
}
return null;
}

public Label getAssignedLabel(SubTask task) {
for (ParameterValue p : parameters) {
if (p == null) continue;
Label l = p.getAssignedLabel(task);
if (l!=null) return l;
}
Expand Down Expand Up @@ -166,7 +176,9 @@ public boolean shouldSchedule(List<Action> actions) {
/**
* Creates a new {@link ParametersAction} that contains all the parameters in this action
* with the overrides / new values given as parameters.
* @return New {@link ParametersAction}. The result may contain null {@link ParameterValue}s
*/
@Nonnull
public ParametersAction createUpdated(Collection<? extends ParameterValue> overrides) {
if(overrides == null) {
return new ParametersAction(parameters);
Expand All @@ -175,10 +187,12 @@ public ParametersAction createUpdated(Collection<? extends ParameterValue> overr
Set<String> names = newHashSet();

for(ParameterValue v : overrides) {
if (v == null) continue;
names.add(v.getName());
}

for (ParameterValue v : parameters) {
if (v == null) continue;
if (!names.contains(v.getName())) {
combinedParameters.add(v);
}
Expand All @@ -187,8 +201,14 @@ public ParametersAction createUpdated(Collection<? extends ParameterValue> overr
return new ParametersAction(combinedParameters);
}

public ParametersAction merge(ParametersAction overrides) {
if(overrides == null) {
/*
* Creates a new {@link ParametersAction} that contains all the parameters in this action
* with the overrides / new values given as another {@link ParametersAction}.
* @return New {@link ParametersAction}. The result may contain null {@link ParameterValue}s
*/
@Nonnull
public ParametersAction merge(@CheckForNull ParametersAction overrides) {
if (overrides == null) {
return new ParametersAction(parameters);
}
return createUpdated(overrides.getParameters());
Expand Down
Expand Up @@ -147,7 +147,11 @@ public void _doBuild(StaplerRequest req, StaplerResponse rsp, @QueryParameter Ti
if(d==null)
throw new IllegalArgumentException("No such parameter definition: " + name);
ParameterValue parameterValue = d.createValue(req, jo);
values.add(parameterValue);
if (parameterValue != null) {
values.add(parameterValue);
} else {
throw new IllegalArgumentException("Cannot retrieve the parameter value: " + name);
}
}

WaitingItem item = Jenkins.getInstance().getQueue().schedule(
Expand Down

0 comments on commit 7b894f5

Please sign in to comment.