Skip to content

Commit b5cbaf7

Browse files
committed
JENKINS-70066: Fix null check of parameters
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. (cherry picked from commit adce718)
1 parent 24ff60c commit b5cbaf7

File tree

2 files changed

+16
-10
lines changed

2 files changed

+16
-10
lines changed

src/main/java/org/jenkinsci/plugins/scriptler/builder/ScriptlerBuilder.java

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,12 @@
2929
import java.io.IOException;
3030
import java.io.Serial;
3131
import java.io.Serializable;
32-
import java.util.*;
32+
import java.util.ArrayList;
33+
import java.util.HashMap;
34+
import java.util.List;
35+
import java.util.Map;
36+
import java.util.Objects;
37+
import java.util.Optional;
3338
import java.util.concurrent.atomic.AtomicInteger;
3439
import java.util.logging.Level;
3540
import java.util.logging.Logger;
@@ -91,19 +96,19 @@ public ScriptlerBuilder(
9196
@CheckForNull String builderId,
9297
@CheckForNull String scriptId,
9398
boolean propagateParams,
94-
Parameter[] parameters) {
95-
this(builderId, scriptId, propagateParams, Arrays.asList(Objects.requireNonNull(parameters)));
99+
@CheckForNull Parameter[] parameters) {
100+
this(builderId, scriptId, propagateParams, parameters == null ? List.of() : List.of(parameters));
96101
}
97102

98103
@DataBoundConstructor
99104
public ScriptlerBuilder(
100105
@CheckForNull String builderId,
101106
@CheckForNull String scriptId,
102107
boolean propagateParams,
103-
@NonNull List<Parameter> parameters) {
108+
@CheckForNull List<Parameter> parameters) {
104109
this.builderId = builderId;
105110
this.scriptId = scriptId;
106-
this.parameters = new ArrayList<>(parameters);
111+
this.parameters = parameters == null ? List.of() : List.copyOf(parameters);
107112
this.propagateParams = propagateParams;
108113
}
109114

@@ -175,7 +180,7 @@ public Parameter[] getParameters() {
175180

176181
@NonNull
177182
public List<Parameter> getParametersList() {
178-
return Collections.unmodifiableList(parameters);
183+
return parameters;
179184
}
180185

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

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

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

373378
if (builder == null) {
374-
builder = new ScriptlerBuilder(builderId, null, false, Collections.emptyList());
379+
builder = new ScriptlerBuilder(builderId, null, false, List.of());
375380
}
376381

377382
return builder.recreateBuilderWithBuilderIdIfRequired();
378383
}
379384

380385
public List<Script> getScripts() {
381386
// TODO currently only script for RUN_SCRIPT permissions are returned?
382-
Set<Script> scripts = getConfig().getScripts();
383387
List<Script> scriptsForBuilder = new ArrayList<>();
384-
for (Script script : scripts) {
388+
for (Script script : getConfig().getScripts()) {
385389
if (script.nonAdministerUsing) {
386390
scriptsForBuilder.add(script);
387391
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
java.util.ImmutableCollections$ListN
2+
java.util.ImmutableCollections$List12

0 commit comments

Comments
 (0)