Merge pull request #211 from oleg-nenashev/bug/JENKINS-47901

[JENKINS-47901] - Stop using unchecked File#toPath()
jenkinsci · Nov 10, 2017 · 7c28abe · 7c28abe
2 parents 49c67ee + 0549f75
commit 7c28abe
Show file tree

Hide file tree

Showing 6 changed files with 108 additions and 29 deletions.
diff --git a/src/main/java/hudson/remoting/FileSystemJarCache.java b/src/main/java/hudson/remoting/FileSystemJarCache.java
@@ -1,5 +1,7 @@
 package hudson.remoting;
 
+import org.jenkinsci.remoting.util.PathUtils;
+
 import javax.annotation.Nonnull;
 import javax.annotation.concurrent.GuardedBy;
 import java.io.File;
@@ -72,7 +74,7 @@ protected URL lookInCache(Channel channel, long sum1, long sum2) throws IOExcept
         if (jar.exists()) {
             LOGGER.log(Level.FINER, String.format("Jar file cache hit %16X%16X",sum1,sum2));
             if (touch)  {
-                Files.setLastModifiedTime(jar.toPath(), FileTime.fromMillis(System.currentTimeMillis()));
+                Files.setLastModifiedTime(PathUtils.fileToPath(jar), FileTime.fromMillis(System.currentTimeMillis()));
             }
             if (notified.add(new Checksum(sum1,sum2))) {
                 getJarLoader(channel).notifyJarPresence(sum1,sum2);
@@ -98,7 +100,7 @@ protected URL retrieve(Channel channel, long sum1, long sum2) throws IOException
                     "Cached file checksum mismatch: %s%nExpected: %s%n Actual: %s",
                     target.getAbsolutePath(), expected, actual
             ));
-            Files.delete(target.toPath());
+            Files.delete(PathUtils.fileToPath(target));
             synchronized (checksumsByPath) {
                 checksumsByPath.remove(target.getCanonicalPath());
             }
@@ -142,7 +144,7 @@ protected URL retrieve(Channel channel, long sum1, long sum2) throws IOException
 
                 return target.toURI().toURL();
             } finally {
-                Files.deleteIfExists(tmp.toPath());
+                Files.deleteIfExists(PathUtils.fileToPath(tmp));
             }
         } catch (IOException e) {
             throw (IOException)new IOException("Failed to write to "+target).initCause(e);

diff --git a/src/main/java/hudson/remoting/Launcher.java b/src/main/java/hudson/remoting/Launcher.java
@@ -27,7 +27,6 @@
 import hudson.remoting.Channel.Mode;
 import java.io.FileInputStream;
 import java.io.UnsupportedEncodingException;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import java.security.KeyStore;
 import java.security.KeyStoreException;
@@ -41,6 +40,7 @@
 
 import org.jenkinsci.remoting.engine.WorkDirManager;
 import org.jenkinsci.remoting.util.IOUtils;
+import org.jenkinsci.remoting.util.PathUtils;
 import org.w3c.dom.Document;
 import org.w3c.dom.NodeList;
 import org.xml.sax.SAXException;
@@ -59,7 +59,6 @@
 import javax.net.ssl.X509TrustManager;
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLSession;
-import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -70,7 +69,6 @@
 import java.io.ByteArrayOutputStream;
 import java.io.Closeable;
 import java.io.FileWriter;
-import java.io.PrintStream;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.net.URL;
@@ -295,7 +293,7 @@ public void run() throws Exception {
         if (slaveLog != null) {
             workDirManager.disable(WorkDirManager.DirType.LOGS_DIR);
         }
-        workDirManager.setupLogging(internalDirPath, slaveLog != null ? slaveLog.toPath() : null);
+        workDirManager.setupLogging(internalDirPath, slaveLog != null ? PathUtils.fileToPath(slaveLog) : null);
 
         if(auth!=null) {
             final int idx = auth.indexOf(':');

diff --git a/src/main/java/hudson/remoting/Util.java b/src/main/java/hudson/remoting/Util.java
@@ -1,5 +1,9 @@
 package hudson.remoting;
 
+import org.jenkinsci.remoting.util.PathUtils;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileOutputStream;
@@ -57,8 +61,8 @@ public static void copy(InputStream in, OutputStream out) throws IOException {
     static File makeResource(String name, byte[] image) throws IOException {
         Path tmpDir = Files.createTempDirectory("resource-");
         File resource = new File(tmpDir.toFile(), name);
-        Files.createDirectories(fileToPath(resource.getParentFile()));
-        Files.createFile(fileToPath(resource));
+        Files.createDirectories(PathUtils.fileToPath(resource.getParentFile()));
+        Files.createFile(PathUtils.fileToPath(resource));
 
         try(FileOutputStream fos = new FileOutputStream(resource)) {
             fos.write(image);
@@ -203,21 +207,7 @@ static URLConnection openURLConnection(URL url) throws IOException {
     @Deprecated
     static void mkdirs(@Nonnull File file) throws IOException {
         if (file.isDirectory()) return;
-        Files.createDirectories(fileToPath(file));
+        Files.createDirectories(PathUtils.fileToPath(file));
     }
 
-    /**
-     * Converts {@link File} to {@link Path} and checks runtime exceptions.
-     * @param file File
-     * @return Resulting path
-     * @throws IOException Conversion error caused by {@link InvalidPathException}
-     */
-    @Nonnull
-    private static Path fileToPath(@Nonnull File file) throws IOException {
-        try {
-            return file.toPath();
-        } catch (InvalidPathException ex) {
-            throw new IOException(ex);
-        }
-    }
 }
diff --git a/src/main/java/hudson/remoting/jnlp/Main.java b/src/main/java/hudson/remoting/jnlp/Main.java
@@ -35,6 +35,7 @@
 
 import org.jenkinsci.remoting.engine.WorkDirManager;
 import org.jenkinsci.remoting.util.IOUtils;
+import org.jenkinsci.remoting.util.PathUtils;
 import org.kohsuke.args4j.Option;
 import org.kohsuke.args4j.CmdLineParser;
 import org.kohsuke.args4j.Argument;
@@ -245,10 +246,18 @@ public Engine createEngine() {
 
         // TODO: ideally logging should be initialized before the "Setting up slave" entry
         if (agentLog != null) {
-            engine.setAgentLog(agentLog.toPath());
+            try {
+                engine.setAgentLog(PathUtils.fileToPath(agentLog));
+            } catch (IOException ex) {
+                throw new IllegalStateException("Cannot retrieve custom log destination", ex);
+            }
         }
         if (loggingConfigFile != null) {
-            engine.setLoggingConfigFile(loggingConfigFile.toPath());
+            try {
+                engine.setLoggingConfigFile(PathUtils.fileToPath(loggingConfigFile));
+            } catch (IOException ex) {
+                throw new IllegalStateException("Logging config file is invalid", ex);
+            }
         }
 
         if (candidateCertificates != null && !candidateCertificates.isEmpty()) {
@@ -321,7 +330,11 @@ public Engine createEngine() {
 
         // Working directory settings
         if (workDir != null) {
-            engine.setWorkDir(workDir.toPath());
+            try {
+                engine.setWorkDir(PathUtils.fileToPath(workDir));
+            } catch (IOException ex) {
+                throw new IllegalStateException("Work directory path is invalid", ex);
+            }
         }
         engine.setInternalDir(internalDir);
         engine.setFailIfWorkDirIsMissing(failIfWorkDirIsMissing);

diff --git a/src/main/java/org/jenkinsci/remoting/engine/WorkDirManager.java b/src/main/java/org/jenkinsci/remoting/engine/WorkDirManager.java
@@ -26,6 +26,7 @@
 package org.jenkinsci.remoting.engine;
 
 import hudson.remoting.TeeOutputStream;
+import org.jenkinsci.remoting.util.PathUtils;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 
@@ -124,6 +125,19 @@ public void disable(@Nonnull DirType dir) {
     public File getLocation(@Nonnull DirType type) {
         return directories.get(type);
     }
+
+    /**
+     * Get {@link Path} of the directory.
+     * @param type Type of the directory
+     * @return Path
+     * @since TODO
+     * @throws IOException Invalid path, e.g. ig the root directory is incorrect
+     */
+    @CheckForNull
+    public Path getLocationPath(@Nonnull DirType type) throws IOException {
+        File location = directories.get(type);
+        return location != null ? PathUtils.fileToPath(location) : null;
+    }
 
     /**
      * Sets path to the Logging JUL property file with logging settings.
@@ -191,7 +205,7 @@ public Path initializeWorkDir(final @CheckForNull File workDir, final @Nonnull S
             directories.put(DirType.INTERNAL_DIR, internalDirFile);
 
             // Create the directory on-demand
-            final Path internalDirPath = internalDirFile.toPath();
+            final Path internalDirPath = PathUtils.fileToPath(internalDirFile);
             Files.createDirectories(internalDirPath);
             LOGGER.log(Level.INFO, "Using {0} as a remoting work directory", internalDirPath);
 
@@ -208,7 +222,7 @@ private void createInternalDirIfRequired(File internalDir, DirType type)
         if (!disabledDirectories.contains(type)) {
             final File directory = new File(internalDir, type.getDefaultLocation());
             verifyDirectory(directory, type, false);
-            Files.createDirectories(directory.toPath());
+            Files.createDirectories(PathUtils.fileToPath(directory));
             directories.put(type, directory);
         } else {
             LOGGER.log(Level.FINE, "Skipping the disabled directory: {0}", type.getName());

diff --git a/src/main/java/org/jenkinsci/remoting/util/PathUtils.java b/src/main/java/org/jenkinsci/remoting/util/PathUtils.java
@@ -0,0 +1,62 @@
+/*
+ *
+ * The MIT License
+ *
+ * Copyright (c) 2017 CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ *  The above copyright notice and this permission notice shall be included in
+ *  all copies or substantial portions of the Software.
+ *
+ *  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ *  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ *  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ *  AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ *  LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ *  OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ *  THE SOFTWARE.
+ *
+ */
+
+package org.jenkinsci.remoting.util;
+
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+
+import javax.annotation.Nonnull;
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.InvalidPathException;
+import java.nio.file.Path;
+
+/**
+ * Utilities for {@link Path} handling.
+ */
+@Restricted(NoExternalUse.class)
+public class PathUtils {
+
+    private PathUtils() {}
+
+    /**
+     * Converts {@link File} to {@link Path} and checks runtime exceptions.
+     * @param file File
+     * @return Resulting path
+     * @throws IOException Conversion error caused by {@link InvalidPathException}
+     * @since TODO
+     */
+    @Nonnull
+    @Restricted(NoExternalUse.class)
+    public static Path fileToPath(@Nonnull File file) throws IOException {
+        try {
+            return file.toPath();
+        } catch (InvalidPathException ex) {
+            throw new IOException(ex);
+        }
+    }
+}