Skip to content

Commit

Permalink
JENKINS-70066: Fix null check of parameters
Browse files Browse the repository at this point in the history
The `@DataBoundConstructor` for `ScriptlerBuilder` takes a list of
parameters for the given script. It was annotated as `@NotNull`, but
`Descriptor.bindJSON` could occasionally pass `null` for the parameters
list. Fix this by annotating the parameter as `@CheckForNull` and
performing the appropriate null checks.
  • Loading branch information
mtughan committed Dec 4, 2024
1 parent 88a8919 commit 440b59b
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@
import java.io.IOException;
import java.io.Serial;
import java.io.Serializable;
import java.util.*;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.logging.Level;
import java.util.logging.Logger;
Expand Down Expand Up @@ -91,19 +96,19 @@ public ScriptlerBuilder(
@CheckForNull String builderId,
@CheckForNull String scriptId,
boolean propagateParams,
Parameter[] parameters) {
this(builderId, scriptId, propagateParams, Arrays.asList(Objects.requireNonNull(parameters)));
@CheckForNull Parameter[] parameters) {
this(builderId, scriptId, propagateParams, parameters == null ? List.of() : List.of(parameters));

Check warning on line 100 in src/main/java/org/jenkinsci/plugins/scriptler/builder/ScriptlerBuilder.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 100 is not covered by tests
}

@DataBoundConstructor
public ScriptlerBuilder(
@CheckForNull String builderId,
@CheckForNull String scriptId,
boolean propagateParams,
@NonNull List<Parameter> parameters) {
@CheckForNull List<Parameter> parameters) {
this.builderId = builderId;
this.scriptId = scriptId;
this.parameters = new ArrayList<>(parameters);
this.parameters = parameters == null ? List.of() : List.copyOf(parameters);

Check warning on line 111 in src/main/java/org/jenkinsci/plugins/scriptler/builder/ScriptlerBuilder.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 111 is only partially covered, one branch is missing
this.propagateParams = propagateParams;
}

Expand Down Expand Up @@ -175,7 +180,7 @@ public Parameter[] getParameters() {

@NonNull
public List<Parameter> getParametersList() {
return Collections.unmodifiableList(parameters);
return parameters;
}

public String getBuilderId() {
Expand Down Expand Up @@ -233,7 +238,7 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListen

// expand the parameters before passing these to the execution, this is to allow any token macro to resolve
// parameter values
List<Parameter> expandedParams = new LinkedList<>();
List<Parameter> expandedParams = new ArrayList<>();

if (propagateParams) {
final ParametersAction paramsAction = build.getAction(ParametersAction.class);
Expand Down Expand Up @@ -371,17 +376,16 @@ public ScriptlerBuilder newInstance(StaplerRequest2 req, JSONObject formData) {
}

if (builder == null) {
builder = new ScriptlerBuilder(builderId, null, false, Collections.emptyList());
builder = new ScriptlerBuilder(builderId, null, false, List.of());
}

return builder.recreateBuilderWithBuilderIdIfRequired();
}

public List<Script> getScripts() {
// TODO currently only script for RUN_SCRIPT permissions are returned?

Check warning on line 386 in src/main/java/org/jenkinsci/plugins/scriptler/builder/ScriptlerBuilder.java

View check run for this annotation

ci.jenkins.io / Open Tasks Scanner

TODO

NORMAL: currently only script for RUN_SCRIPT permissions are returned?
Set<Script> scripts = getConfig().getScripts();
List<Script> scriptsForBuilder = new ArrayList<>();
for (Script script : scripts) {
for (Script script : getConfig().getScripts()) {

Check warning on line 388 in src/main/java/org/jenkinsci/plugins/scriptler/builder/ScriptlerBuilder.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 241-388 are not covered by tests
if (script.nonAdministerUsing) {
scriptsForBuilder.add(script);
}
Expand Down
2 changes: 2 additions & 0 deletions src/main/resources/META-INF/hudson.remoting.ClassFilter
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
java.util.ImmutableCollections$ListN
java.util.ImmutableCollections$List12

0 comments on commit 440b59b

Please sign in to comment.