Do not submit an empty pre/post-send script for approval (#518)

* Do not submit an empty pre/post-send script for approval

* Fixing `ContentBuilderTest`

Author: jglick

src/main/java/hudson/plugins/emailext/ExtendedEmailPublisherDescriptor.java

@@ -4,6 +4,7 @@
 import com.cloudbees.plugins.credentials.common.StandardUsernamePasswordCredentials;
 import com.cloudbees.plugins.credentials.domains.DomainRequirement;
 import com.cloudbees.plugins.credentials.domains.HostnamePortRequirement;
+import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.Extension;
 import hudson.Util;
@@ -89,12 +90,12 @@ public final class ExtendedEmailPublisherDescriptor extends BuildStepDescriptor<
     /**
      * This is the global default pre-send script.
      */
-    private String defaultPresendScript = "";
+    private String defaultPresendScript;
 
     /**
      * This is the global default post-send script.
      */
-    private String defaultPostsendScript = "";
+    private String defaultPostsendScript;
 
     private List<GroovyScriptPath> defaultClasspath = new ArrayList<>();
 
@@ -689,34 +690,38 @@ public boolean isApplicable(Class<? extends AbstractProject> jobType) {
         return true;
     }
 
-    public String getDefaultPresendScript() {
+    public @CheckForNull String getDefaultPresendScript() {
         return defaultPresendScript;
     }
 
     @SuppressWarnings("unused")
     @DataBoundSetter
-    public void setDefaultPresendScript(String script) {
-        script = StringUtils.trim(script);
-        this.defaultPresendScript = ScriptApproval.get()
-                .configuring(
-                        script == null ? "" : script,
-                        GroovyLanguage.get(),
-                        ApprovalContext.create().withCurrentUser());
-    }
-
-    public String getDefaultPostsendScript() {
+    public void setDefaultPresendScript(@CheckForNull String script) {
+        script = StringUtils.trimToNull(script);
+        this.defaultPresendScript = script == null
+                ? null
+                : ScriptApproval.get()
+                        .configuring(
+                                script,
+                                GroovyLanguage.get(),
+                                ApprovalContext.create().withCurrentUser());
+    }
+
+    public @CheckForNull String getDefaultPostsendScript() {
         return defaultPostsendScript;
     }
 
     @SuppressWarnings("unused")
     @DataBoundSetter
-    public void setDefaultPostsendScript(String script) {
-        script = StringUtils.trim(script);
-        this.defaultPostsendScript = ScriptApproval.get()
-                .configuring(
-                        script == null ? "" : script,
-                        GroovyLanguage.get(),
-                        ApprovalContext.create().withCurrentUser());
+    public void setDefaultPostsendScript(@CheckForNull String script) {
+        script = StringUtils.trimToNull(script);
+        this.defaultPostsendScript = script == null
+                ? null
+                : ScriptApproval.get()
+                        .configuring(
+                                script,
+                                GroovyLanguage.get(),
+                                ApprovalContext.create().withCurrentUser());
     }
 
     public List<GroovyScriptPath> getDefaultClasspath() {

src/test/java/hudson/plugins/emailext/ExtendedEmailPublisherDescriptorTest.java

@@ -2,6 +2,7 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.oneOf;
 import static org.junit.Assert.assertArrayEquals;
@@ -66,6 +67,7 @@
 import org.htmlunit.html.HtmlSelect;
 import org.htmlunit.html.HtmlTextArea;
 import org.htmlunit.html.HtmlTextInput;
+import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval;
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.Issue;
@@ -1198,4 +1200,15 @@ public void persistedConfigurationWithCredentialId() {
                         UnstableTrigger.class.getName(),
                         XNthFailureTrigger.class.getName()));
     }
+
+    @Test
+    public void emptyScriptApproval() throws Exception {
+        j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
+        j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()
+                .grant(Jenkins.ADMINISTER)
+                .everywhere()
+                .to("admin"));
+        j.submit(j.createWebClient().login("admin").goTo("configure").getFormByName("config"));
+        assertThat(ScriptApproval.get().getPendingScripts(), is(empty()));
+    }
 }

src/test/java/hudson/plugins/emailext/plugins/ContentBuilderTest.java

@@ -19,6 +19,7 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import org.apache.commons.lang.StringUtils;
 import org.jenkinsci.plugins.tokenmacro.TokenMacro;
 import org.junit.Rule;
 import org.junit.Test;
@@ -115,20 +116,24 @@ public void before() throws Throwable {
     public void testTransformText_shouldExpand_$DEFAULT_PRESEND_SCRIPT() {
         assertEquals(
                 publisher.getDescriptor().getDefaultPresendScript(),
-                ContentBuilder.transformText("$DEFAULT_PRESEND_SCRIPT", publisher, build, listener));
+                StringUtils.trimToNull(
+                        ContentBuilder.transformText("$DEFAULT_PRESEND_SCRIPT", publisher, build, listener)));
         assertEquals(
                 publisher.getDescriptor().getDefaultPresendScript(),
-                ContentBuilder.transformText("${DEFAULT_PRESEND_SCRIPT}", publisher, build, listener));
+                StringUtils.trimToNull(
+                        ContentBuilder.transformText("${DEFAULT_PRESEND_SCRIPT}", publisher, build, listener)));
     }
 
     @Test
     public void testTransformText_shouldExpand_$DEFAULT_POSTSEND_SCRIPT() {
         assertEquals(
                 publisher.getDescriptor().getDefaultPostsendScript(),
-                ContentBuilder.transformText("$DEFAULT_POSTSEND_SCRIPT", publisher, build, listener));
+                StringUtils.trimToNull(
+                        ContentBuilder.transformText("$DEFAULT_POSTSEND_SCRIPT", publisher, build, listener)));
         assertEquals(
                 publisher.getDescriptor().getDefaultPostsendScript(),
-                ContentBuilder.transformText("${DEFAULT_POSTSEND_SCRIPT}", publisher, build, listener));
+                StringUtils.trimToNull(
+                        ContentBuilder.transformText("${DEFAULT_POSTSEND_SCRIPT}", publisher, build, listener)));
     }
 
     @Test