Merge pull request #483 from jenkinsci/check-file-params

validate file based arguments point to existing files / directories

.mvn/jvm.config

@@ -0,0 +1 @@
+-Xmx128m -XX:+HeapDumpOnOutOfMemoryError -XX:+TieredCompilation -XX:TieredStopAtLevel=1

pom.xml

@@ -30,6 +30,7 @@
     <picocli.version>4.7.1</picocli.version>
     <slf4j.version>2.0.6</slf4j.version>
     <spotbugs.effort>Max</spotbugs.effort>
+    <argLine />
   </properties>
 
   <dependencyManagement>
@@ -296,9 +297,18 @@
         <artifactId>maven-surefire-plugin</artifactId>
         <!-- Version specified in parent POM -->
         <configuration>
+          <argLine>@{argLine} -Xmx128m -XX:+HeapDumpOnOutOfMemoryError -XX:+TieredCompilation -XX:TieredStopAtLevel=1</argLine>
           <systemPropertyVariables>
             <java.util.logging.config.class>org.jenkins.tools.test.logging.LoggingConfiguration</java.util.logging.config.class>
           </systemPropertyVariables>
+          <environmentVariables>
+            <!--
+              PluginCompatTesterTest runs the PCT which will run various maven.  if the JVM is not configured (its not for the text-finder)
+               then we run a JVM that seems the host/container memory and assumes it is the only thing running leading to it being OOMKilled
+               -->
+            <MAVEN_OPTS>-Xmx256m -XX:+TieredCompilation -XX:TieredStopAtLevel=1</MAVEN_OPTS>
+          </environmentVariables>
+          <reuseForks>false</reuseForks>
           <runOrder>alphabetical</runOrder>
         </configuration>
       </plugin>

src/main/java/org/jenkins/tools/test/PluginCompatTesterCli.java

@@ -37,6 +37,7 @@
 import org.jenkins.tools.test.exception.PluginCompatibilityTesterException;
 import org.jenkins.tools.test.logging.LoggingConfiguration;
 import org.jenkins.tools.test.model.PluginCompatTesterConfig;
+import org.jenkins.tools.test.picocli.ExistingFileTypeConverter;
 import picocli.CommandLine;
 
 @CommandLine.Command(
@@ -57,7 +58,8 @@ public class PluginCompatTesterCli implements Callable<Integer> {
     @CommandLine.Option(
             names = {"-w", "--war"},
             required = true,
-            description = "Path to the WAR file to be used by the PCT.")
+            description = "Path to the WAR file to be used by the PCT.",
+            converter = ExistingFileTypeConverter.class)
     private File war;
 
     @CommandLine.Option(
@@ -104,13 +106,17 @@ public class PluginCompatTesterCli implements Callable<Integer> {
     private String fallbackGitHubOrganization;
 
     @CheckForNull
-    @CommandLine.Option(names = "--mvn", description = "The path to the Maven executable.")
+    @CommandLine.Option(
+            names = "--mvn",
+            description = "The path to the Maven executable.",
+            converter = ExistingFileTypeConverter.class)
     private File externalMaven;
 
     @CheckForNull
     @CommandLine.Option(
             names = "--maven-settings",
-            description = "Settings file to use when executing Maven.")
+            description = "Settings file to use when executing Maven.",
+            converter = ExistingFileTypeConverter.class)
     private File mavenSettings;
 
     @CheckForNull
@@ -136,14 +142,16 @@ public class PluginCompatTesterCli implements Callable<Integer> {
             split = ",",
             arity = "1",
             paramLabel = "jar",
-            description = "Comma-separated list of paths to external hooks JARs.")
+            description = "Comma-separated list of paths to external hooks JARs.",
+            converter = ExistingFileTypeConverter.class)
     private List<File> externalHooksJars;
 
     @CheckForNull
     @CommandLine.Option(
             names = "--local-checkout-dir",
             description =
-                    "Folder containing either a local (possibly modified) clone of a plugin repository or a set of local clones of different plugins.")
+                    "Folder containing either a local (possibly modified) clone of a plugin repository or a set of local clones of different plugins.",
+            converter = ExistingFileTypeConverter.class)
     private File localCheckoutDir;
 
     @CommandLine.Option(

src/main/java/org/jenkins/tools/test/picocli/ExistingFileTypeConverter.java

@@ -0,0 +1,22 @@
+package org.jenkins.tools.test.picocli;
+
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import java.io.File;
+import picocli.CommandLine.ITypeConverter;
+import picocli.CommandLine.TypeConversionException;
+
+/** Converter that converts to a File that must exist (either as a directory or a file) */
+public class ExistingFileTypeConverter implements ITypeConverter<File> {
+
+    @SuppressFBWarnings(
+            value = "PATH_TRAVERSAL_IN",
+            justification = "by deign, we are converting an argument from the CLI")
+    @Override
+    public File convert(String value) throws Exception {
+        File f = new File(value);
+        if (!f.exists()) {
+            throw new TypeConversionException("Specified file " + value + " does not exist");
+        }
+        return f;
+    }
+}

src/test/java/org/jenkins/tools/test/picocli/ExistingFileTypeConverterTest.java

@@ -0,0 +1,31 @@
+package org.jenkins.tools.test.picocli;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.io.File;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import picocli.CommandLine.TypeConversionException;
+
+class ExistingFileTypeConverterTest {
+
+    @Test
+    void testValidFile(@TempDir File f) throws Exception {
+        ExistingFileTypeConverter converter = new ExistingFileTypeConverter();
+        File converted = converter.convert(f.getPath());
+        assertThat(converted, is(f));
+    }
+
+    @Test
+    void testMissingFile(@TempDir File f) throws Exception {
+        ExistingFileTypeConverter converter = new ExistingFileTypeConverter();
+        TypeConversionException tce =
+                assertThrows(
+                        TypeConversionException.class,
+                        () -> converter.convert(new File(f, "whatever").getPath()));
+        assertThat(tce.getMessage(), containsString("whatever"));
+    }
+}