From 440b59b9a73f06a3e5b366ef7c9c9a4d37364218 Mon Sep 17 00:00:00 2001
From: Michael Tughan <mtughan@gmail.com>
Date: Wed, 4 Dec 2024 14:48:14 -0500
Subject: [PATCH] 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.
---
 .../scriptler/builder/ScriptlerBuilder.java   | 24 +++++++++++--------
 .../META-INF/hudson.remoting.ClassFilter      |  2 ++
 2 files changed, 16 insertions(+), 10 deletions(-)
 create mode 100644 src/main/resources/META-INF/hudson.remoting.ClassFilter

diff --git a/src/main/java/org/jenkinsci/plugins/scriptler/builder/ScriptlerBuilder.java b/src/main/java/org/jenkinsci/plugins/scriptler/builder/ScriptlerBuilder.java
index da020838..d6191021 100644
--- a/src/main/java/org/jenkinsci/plugins/scriptler/builder/ScriptlerBuilder.java
+++ b/src/main/java/org/jenkinsci/plugins/scriptler/builder/ScriptlerBuilder.java
@@ -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;
@@ -91,8 +96,8 @@ 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));
     }
 
     @DataBoundConstructor
@@ -100,10 +105,10 @@ 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);
         this.propagateParams = propagateParams;
     }
 
@@ -175,7 +180,7 @@ public Parameter[] getParameters() {
 
     @NonNull
     public List<Parameter> getParametersList() {
-        return Collections.unmodifiableList(parameters);
+        return parameters;
     }
 
     public String getBuilderId() {
@@ -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);
@@ -371,7 +376,7 @@ 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();
@@ -379,9 +384,8 @@ public ScriptlerBuilder newInstance(StaplerRequest2 req, JSONObject formData) {
 
         public List<Script> getScripts() {
             // TODO 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()) {
                 if (script.nonAdministerUsing) {
                     scriptsForBuilder.add(script);
                 }
diff --git a/src/main/resources/META-INF/hudson.remoting.ClassFilter b/src/main/resources/META-INF/hudson.remoting.ClassFilter
new file mode 100644
index 00000000..d6da0060
--- /dev/null
+++ b/src/main/resources/META-INF/hudson.remoting.ClassFilter
@@ -0,0 +1,2 @@
+java.util.ImmutableCollections$ListN
+java.util.ImmutableCollections$List12