Handle FormException from SecureGroovyScript (#726)

src/main/java/org/jenkins/plugins/lockableresources/LockableResourcesManager.java

@@ -18,6 +18,7 @@
 import hudson.Extension;
 import hudson.Util;
 import hudson.console.ModelHyperlinkNote;
+import hudson.model.Descriptor;
 import hudson.model.Run;
 import java.io.IOException;
 import java.io.PrintStream;
@@ -482,7 +483,12 @@ public List<LockableResource> tryQueue(
                 return null;
             }
 
-            final SecureGroovyScript systemGroovyScript = requiredResources.getResourceMatchScript();
+            final SecureGroovyScript systemGroovyScript;
+            try {
+                systemGroovyScript = requiredResources.getResourceMatchScript();
+            } catch (Descriptor.FormException x) {
+                throw new ExecutionException(x);
+            }
             boolean candidatesByScript = (systemGroovyScript != null);
             List<LockableResource> candidates = requiredResources.required; // default candidates
 

src/main/java/org/jenkins/plugins/lockableresources/RequiredResourcesProperty.java

@@ -14,6 +14,7 @@
 import hudson.Util;
 import hudson.model.AbstractProject;
 import hudson.model.AutoCompletionCandidates;
+import hudson.model.Descriptor;
 import hudson.model.Item;
 import hudson.model.Job;
 import hudson.model.JobProperty;
@@ -45,7 +46,8 @@ public RequiredResourcesProperty(
             String resourceNamesVar,
             String resourceNumber,
             String labelName,
-            @CheckForNull SecureGroovyScript resourceMatchScript) {
+            @CheckForNull SecureGroovyScript resourceMatchScript)
+            throws Descriptor.FormException {
         super();
 
         if (resourceNames == null || resourceNames.trim().isEmpty()) {
@@ -84,11 +86,12 @@ public RequiredResourcesProperty(
     @Deprecated
     @ExcludeFromJacocoGeneratedReport
     public RequiredResourcesProperty(
-            String resourceNames, String resourceNamesVar, String resourceNumber, String labelName) {
+            String resourceNames, String resourceNamesVar, String resourceNumber, String labelName)
+            throws Descriptor.FormException {
         this(resourceNames, resourceNamesVar, resourceNumber, labelName, null);
     }
 
-    private Object readResolve() {
+    private Object readResolve() throws Descriptor.FormException {
         // SECURITY-368 migration logic
         if (resourceMatchScript == null
                 && labelName != null

src/main/java/org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction.java

@@ -12,6 +12,7 @@
 import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.Extension;
 import hudson.model.Api;
+import hudson.model.Descriptor;
 import hudson.model.RootAction;
 import hudson.model.Run;
 import hudson.security.AccessDeniedException3;
@@ -297,7 +298,7 @@ private void informPerformanceIssue() {
 
     // ---------------------------------------------------------------------------
     @Restricted(NoExternalUse.class) // used by jelly
-    public Queue getQueue() {
+    public Queue getQueue() throws Descriptor.FormException {
         List<QueuedContextStruct> currentQueueContext =
                 List.copyOf(LockableResourcesManager.get().getCurrentQueuedContext());
         Queue queue = new Queue();
@@ -325,7 +326,8 @@ public Queue() {
 
         // -------------------------------------------------------------------------
         @Restricted(NoExternalUse.class) // used by jelly
-        public void add(final LockableResourcesStruct resourceStruct, final QueuedContextStruct context) {
+        public void add(final LockableResourcesStruct resourceStruct, final QueuedContextStruct context)
+                throws Descriptor.FormException {
             QueueStruct queueStruct = new QueueStruct(resourceStruct, context);
             queue.add(queueStruct);
             if (resourceStruct.queuedAt == 0) {
@@ -362,7 +364,8 @@ public static class QueueStruct {
             String id = null;
             Run<?, ?> build;
 
-            public QueueStruct(final LockableResourcesStruct resourceStruct, final QueuedContextStruct context) {
+            public QueueStruct(final LockableResourcesStruct resourceStruct, final QueuedContextStruct context)
+                    throws Descriptor.FormException {
                 this.requiredResources = resourceStruct.required;
                 this.requiredLabel = resourceStruct.label;
                 this.requiredNumber = resourceStruct.requiredNumber;

src/main/java/org/jenkins/plugins/lockableresources/queue/LockRunListener.java

@@ -49,7 +49,7 @@ public void onStarted(Run<?, ?> build, TaskListener listener) {
                 if (resources != null) {
                     if (resources.requiredNumber != null
                             || !resources.label.isEmpty()
-                            || resources.getResourceMatchScript() != null) {
+                            || resources.getResourceMatchScriptText() != null) {
                         required.addAll(lrm.getResourcesFromProject(proj.getFullName()));
                     } else {
                         required.addAll(resources.required);

src/main/java/org/jenkins/plugins/lockableresources/queue/LockableResourcesQueueTaskDispatcher.java

@@ -30,7 +30,6 @@
 import java.util.logging.Logger;
 import org.jenkins.plugins.lockableresources.LockableResource;
 import org.jenkins.plugins.lockableresources.LockableResourcesManager;
-import org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SecureGroovyScript;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 
@@ -55,7 +54,7 @@ public CauseOfBlockage canRun(Queue.Item item) {
         if (resources == null
                 || (resources.required.isEmpty()
                         && resources.label.isEmpty()
-                        && resources.getResourceMatchScript() == null)) {
+                        && resources.getResourceMatchScriptText() == null)) {
             return null;
         }
 
@@ -68,7 +67,7 @@ public CauseOfBlockage canRun(Queue.Item item) {
 
         LOGGER.finest(project.getName() + " trying to get resources with these details: " + resources);
 
-        if (resourceNumber > 0 || !resources.label.isEmpty() || resources.getResourceMatchScript() != null) {
+        if (resourceNumber > 0 || !resources.label.isEmpty() || resources.getResourceMatchScriptText() != null) {
             Map<String, Object> params = new HashMap<>();
 
             // Inject Build Parameters, if possible and applicable to the "item" type
@@ -153,11 +152,11 @@ public String getShortDescription() {
                 if (!this.rscStruct.required.isEmpty()) {
                     return "Waiting for resource instances " + rscStruct.required;
                 } else {
-                    final SecureGroovyScript systemGroovyScript = this.rscStruct.getResourceMatchScript();
+                    final String systemGroovyScript = this.rscStruct.getResourceMatchScriptText();
                     if (systemGroovyScript != null) {
                         // Empty or not... just keep the logic in sync
                         // with tryQueue() in LockableResourcesManager
-                        if (systemGroovyScript.getScript().isEmpty()) {
+                        if (systemGroovyScript.isEmpty()) {
                             return "Waiting for resources identified by custom script (which is empty)";
                         } else {
                             return "Waiting for resources identified by custom script";

src/main/java/org/jenkins/plugins/lockableresources/queue/LockableResourcesStruct.java

@@ -12,6 +12,7 @@
 import edu.umd.cs.findbugs.annotations.Nullable;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.EnvVars;
+import hudson.model.Descriptor;
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Date;
@@ -126,14 +127,19 @@ public LockableResourcesStruct(@Nullable List<String> resources, @Nullable Strin
      * @since 2.1
      */
     @CheckForNull
-    public SecureGroovyScript getResourceMatchScript() {
+    public SecureGroovyScript getResourceMatchScript() throws Descriptor.FormException {
         if (resourceMatchScript == null && serializableResourceMatchScript != null) {
             // this is probably high defensive code, because
             resourceMatchScript = serializableResourceMatchScript.rehydrate();
         }
         return resourceMatchScript;
     }
 
+    @CheckForNull
+    public String getResourceMatchScriptText() {
+        return serializableResourceMatchScript != null ? serializableResourceMatchScript.getScript() : null;
+    }
+
     @Override
     public String toString() {
         String str = "";

src/main/java/org/jenkins/plugins/lockableresources/util/SerializableSecureGroovyScript.java

@@ -26,6 +26,7 @@
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import edu.umd.cs.findbugs.annotations.Nullable;
+import hudson.model.Descriptor;
 import hudson.util.FormValidation;
 import java.io.Serializable;
 import java.net.MalformedURLException;
@@ -77,7 +78,12 @@ public SerializableSecureGroovyScript(@CheckForNull SecureGroovyScript secureScr
     }
 
     @CheckForNull
-    public SecureGroovyScript rehydrate() {
+    public String getScript() {
+        return script;
+    }
+
+    @CheckForNull
+    public SecureGroovyScript rehydrate() throws Descriptor.FormException {
         if (script == null) {
             return null;
         }

src/test/java/org/jenkins/plugins/lockableresources/FreeStyleProjectTest.java

@@ -24,7 +24,6 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 import jenkins.model.Jenkins;
@@ -216,7 +215,7 @@ public void approvalRequired() throws Exception {
     }
 
     @Test
-    public void autoCreateResource() throws IOException, InterruptedException, ExecutionException {
+    public void autoCreateResource() throws Exception {
         FreeStyleProject f = j.createFreeStyleProject("f");
         assertNull(LockableResourcesManager.get().fromName("resource1"));
         f.addProperty(new RequiredResourcesProperty("resource1", null, null, null, null));
@@ -230,7 +229,7 @@ public void autoCreateResource() throws IOException, InterruptedException, Execu
     }
 
     @Test
-    public void competingLabelLocks() throws IOException, InterruptedException, ExecutionException {
+    public void competingLabelLocks() throws Exception {
         LockableResourcesManager.get().createResourceWithLabel("resource1", "group1");
         LockableResourcesManager.get().createResourceWithLabel("resource2", "group2");
         LockableResourcesManager.get().createResource("shared");

src/test/java/org/jenkins/plugins/lockableresources/util/SerializableSecureGroovyScriptTest.java

@@ -14,7 +14,7 @@ public class SerializableSecureGroovyScriptTest {
     public JenkinsRule r = new JenkinsRule();
 
     @Test
-    public void testRehydrate() {
+    public void testRehydrate() throws Exception {
         SerializableSecureGroovyScript nullCheck = new SerializableSecureGroovyScript(null);
         assertNull("SerializableSecureGroovyScript null check", nullCheck.rehydrate());