Merge pull request #84 from jglick/empty-ClasspathEntry-JENKINS-37599

[JENKINS-37599] Prevent empty classpath entries from being saved accidentally
jenkinsci · Sep 21, 2016 · 07d1063 · 07d1063
2 parents feb65ce + 06e88a9
commit 07d1063
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 15 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java
@@ -58,10 +58,18 @@ public ClasspathEntry(@Nonnull String path) throws MalformedURLException {
     }
 
     static URL pathToURL(String path) throws MalformedURLException {
+        if (path.isEmpty()) {
+            throw new MalformedURLException("JENKINS-37599: empty classpath entries not allowed");
+        }
         try {
             return new URL(path);
         } catch (MalformedURLException x) {
-            return new File(path).toURI().toURL();
+            File f = new File(path);
+            if (f.isAbsolute()) {
+                return f.toURI().toURL();
+            } else {
+                throw new MalformedURLException("Classpath entry ‘" + path + "’ does not look like either a URL or an absolute file path");
+            }
         }
     }
 

diff --git a/...test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java b/...test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java
@@ -28,7 +28,6 @@
 import com.gargoylesoftware.htmlunit.html.HtmlButton;
 import com.gargoylesoftware.htmlunit.html.HtmlCheckBoxInput;
 import com.gargoylesoftware.htmlunit.html.HtmlElement;
-import com.gargoylesoftware.htmlunit.html.HtmlRadioButtonInput;
 import groovy.lang.Binding;
 import hudson.remoting.Which;
 import org.apache.tools.ant.AntClassLoader;
@@ -46,7 +45,6 @@
 import hudson.tasks.BuildStepDescriptor;
 import hudson.tasks.Publisher;
 import java.io.File;
-import java.io.FileFilter;
 import java.io.IOException;
 import java.net.URISyntaxException;
 import java.util.ArrayList;
@@ -57,7 +55,6 @@
 import org.apache.commons.lang.StringUtils;
 import org.apache.tools.ant.DirectoryScanner;
 import org.apache.tools.ant.taskdefs.Expand;
-import org.apache.tools.ant.taskdefs.Touch;
 import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval;
 import org.jenkinsci.plugins.scriptsecurity.scripts.UnapprovedUsageException;
 import static org.junit.Assert.*;
@@ -287,20 +284,16 @@ private List<File> getAllUpdatedJarFiles() throws URISyntaxException {
         for (File jarfile: getAllJarFiles()) {
             classpath.add(new ClasspathEntry(jarfile.getAbsolutePath()));
         }
-
-        FreeStyleProject p = r.createFreeStyleProject();
-        p.getPublishersList().add(new TestGroovyRecorder(new SecureGroovyScript(
+        TestGroovyRecorder recorder = new TestGroovyRecorder(new SecureGroovyScript(
                 "whatever",
                 true,
                 classpath
-        )));
-
-        JenkinsRule.WebClient wc = r.createWebClient();
-        r.submit(wc.getPage(p, "configure").getFormByName("config"));
-
-        p = r.jenkins.getItemByFullName(p.getFullName(), FreeStyleProject.class);
-        TestGroovyRecorder recorder = (TestGroovyRecorder)p.getPublishersList().get(0);
-        assertEquals(classpath, recorder.getScript().getClasspath());
+        ));
+        TestGroovyRecorder recorder2 = r.configRoundtrip(recorder);
+        r.assertEqualDataBoundBeans(recorder, recorder2);
+        classpath.clear();
+        recorder2 = r.configRoundtrip(recorder);
+        r.assertEqualDataBoundBeans(recorder, recorder2);
     }
 
     @Test public void testClasspathInSandbox() throws Exception {

diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntryTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntryTest.java
@@ -27,13 +27,15 @@
 import hudson.Functions;
 
 import java.io.File;
+import java.net.MalformedURLException;
 import java.net.URL;
 
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
 import static org.junit.Assert.*;
+import org.jvnet.hudson.test.Issue;
 
 public class ClasspathEntryTest {
     @Rule public TemporaryFolder rule = new TemporaryFolder();
@@ -65,4 +67,29 @@ private static void assertRoundTrip(String path, String url) throws Exception {
         assertFalse("Generic URLs ending in / are not considered class directories", ClasspathEntry.isClassDirectoryURL(new URL("http://example.com/file")));
     }
 
+    @Issue("JENKINS-37599")
+    @Test public void pathToURL() throws Exception {
+        ClasspathEntry ignore = new ClasspathEntry("http://nowhere.net/");
+        ignore = new ClasspathEntry(rule.newFile("x.jar").getAbsolutePath());
+        ignore = new ClasspathEntry(rule.newFolder().getAbsolutePath());
+        try {
+            ignore = new ClasspathEntry("");
+            fail();
+        } catch (MalformedURLException x) {
+            // good
+        }
+        try {
+            ignore = new ClasspathEntry(" ");
+            fail();
+        } catch (MalformedURLException x) {
+            // good
+        }
+        try {
+            ignore = new ClasspathEntry("relative");
+            fail();
+        } catch (MalformedURLException x) {
+            // good
+        }
+    }
+
 }