From 0549f75ae4f598cff08aebe9134ad4ba5ebf9ead Mon Sep 17 00:00:00 2001 From: Oleg Nenashev Date: Wed, 8 Nov 2017 18:39:58 +0100 Subject: [PATCH] [JENKINS-47901] - Stop using unchecked File#toPath() --- .../hudson/remoting/FileSystemJarCache.java | 8 ++- src/main/java/hudson/remoting/Launcher.java | 6 +- src/main/java/hudson/remoting/Util.java | 24 +++---- src/main/java/hudson/remoting/jnlp/Main.java | 19 +++++- .../remoting/engine/WorkDirManager.java | 18 +++++- .../jenkinsci/remoting/util/PathUtils.java | 62 +++++++++++++++++++ 6 files changed, 108 insertions(+), 29 deletions(-) create mode 100644 src/main/java/org/jenkinsci/remoting/util/PathUtils.java diff --git a/src/main/java/hudson/remoting/FileSystemJarCache.java b/src/main/java/hudson/remoting/FileSystemJarCache.java index 6c2a71252..48e0662dd 100644 --- 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 index e55a81cbb..c0109d0a4 100644 --- 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 index 41a08a232..455197033 100644 --- 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 index 747224bf4..f8d50f2e1 100644 --- 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 index 10dbaaf5a..497ef8d75 100644 --- 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 new file mode 100644 index 000000000..d2c31cd32 --- /dev/null +++ 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); + } + } +}