[JENKINS-39179] [JENKINS-36088] Always use NIO to create and detect symbolic...

[JENKINS-39179] [JENKINS-36088] Always use NIO to create and detect symbolic links and Windows junctions (#3133)

* Always use NIO to detect symlinks

* Make assertion failure message consistent

* Catch NoSuchFileException to keep tests passing

* Make method name more specific and simlify assumption

* Remove obsolete comment and reword the main comment in isSymlink

* Deprecate Kernel32Util#isJunctionOrSymlink

* Use assumptions for junction creation and add messages to assumptions

* Replace deprecated code with recommended alternative

* Add comment explaining call to DosFileAttributes#isOther

* Do not fall back to native code when creating symlinks

* Log FileSystemExceptions when creating symbolic links

* Catch InvalidPathException and rethrow as IOException

* Deprecate Kernel32Utils#createSymbolicLink and #getWin32FileAttributes

* Preserve original logging behavior on Windows and remove useless call to Util#displayIOException

core/src/main/java/hudson/Util.java

@@ -25,15 +25,12 @@ package hudson;
 
 import java.nio.file.InvalidPathException;
 import jenkins.util.SystemProperties;
-import com.sun.jna.Native;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
-import hudson.Proc.LocalProc;
 import hudson.model.TaskListener;
 import hudson.os.PosixAPI;
 import hudson.util.QuotedStringTokenizer;
 import hudson.util.VariableResolver;
-import hudson.util.jna.WinIOException;
 
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.time.FastDateFormat;
@@ -67,8 +64,12 @@ import java.nio.charset.CharsetEncoder;
 import java.nio.file.FileAlreadyExistsException;
 import java.nio.file.FileSystemException;
 import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.nio.file.attribute.DosFileAttributes;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.text.NumberFormat;
@@ -81,9 +82,6 @@ import java.util.logging.Logger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-import hudson.util.jna.Kernel32Utils;
-import static hudson.util.jna.GNUCLibrary.LIBC;
-
 import java.security.DigestInputStream;
 
 import javax.annotation.CheckForNull;
@@ -493,7 +491,6 @@ public class Util {
     /**
      * Checks if the given file represents a symlink.
      */
-    //Taken from http://svn.apache.org/viewvc/maven/shared/trunk/file-management/src/main/java/org/apache/maven/shared/model/fileset/util/FileSetManager.java?view=markup
     public static boolean isSymlink(@Nonnull File file) throws IOException {
         /*
          *  Windows Directory Junctions are effectively the same as Linux symlinks to directories.
@@ -502,44 +499,29 @@ public class Util {
          *  you have to go through BasicFileAttributes and do the following check:
          *     isSymbolicLink() || isOther()
          *  The isOther() call will include Windows reparse points, of which a directory junction is.
-         *
-         *  Since we already have a function that detects Windows junctions or symlinks and treats them
-         *  both as symlinks, let's use that function and always call it before calling down to the
-         *  NIO2 API.
-         *
+         *  It also includes includes devices, but reading the attributes of a device with NIO fails
+         *  or returns false for isOther(). (i.e. named pipes such as \\.\pipe\JenkinsTestPipe return
+         *  false for isOther(), and drives such as \\.\PhysicalDrive0 throw an exception when
+         *  calling readAttributes.
          */
-        if (Functions.isWindows()) {
-            try {
-                return Kernel32Utils.isJunctionOrSymlink(file);
-            } catch (UnsupportedOperationException | LinkageError e) {
-                // fall through
-            }
-        }
-        Boolean r = isSymlinkJava7(file);
-        if (r != null) {
-            return r;
-        }
-        String name = file.getName();
-        if (name.equals(".") || name.equals(".."))
-            return false;
-
-        File fileInCanonicalParent;
-        File parentDir = file.getParentFile();
-        if ( parentDir == null ) {
-            fileInCanonicalParent = file;
-        } else {
-            fileInCanonicalParent = new File( parentDir.getCanonicalPath(), name );
-        }
-        return !fileInCanonicalParent.getCanonicalFile().equals( fileInCanonicalParent.getAbsoluteFile() );
-    }
-
-    @SuppressFBWarnings("NP_BOOLEAN_RETURN_NULL")
-    private static Boolean isSymlinkJava7(@Nonnull File file) throws IOException {
         try {
             Path path = file.toPath();
-            return Files.isSymbolicLink(path);
-        } catch (Exception x) {
-            throw (IOException) new IOException(x.toString()).initCause(x);
+            BasicFileAttributes attrs = Files.readAttributes(path, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS);
+            if (attrs.isSymbolicLink()) {
+                return true;
+            } else if (attrs instanceof DosFileAttributes) {
+                /* Returns true for non-symbolic link reparse points and devices. We could call
+                 * WindowsFileAttributes#isReparsePoint with reflection instead to exclude devices,
+                 * but as mentioned in the above comment this does not appear to be an issue.
+                 */
+                return attrs.isOther();
+            } else {
+                return false;
+            }
+        } catch (InvalidPathException e) {
+            throw new IOException(e);
+        } catch (NoSuchFileException e) {
+            return false;
         }
     }
 
@@ -1341,78 +1323,6 @@ public class Util {
      */
     public static void createSymlink(@Nonnull File baseDir, @Nonnull String targetPath,
             @Nonnull String symlinkPath, @Nonnull TaskListener listener) throws InterruptedException {
-        try {
-            if (createSymlinkJava7(baseDir, targetPath, symlinkPath)) {
-                return;
-            }
-            if (NO_SYMLINK) {
-                return;
-            }
-
-            File symlinkFile = new File(baseDir, symlinkPath);
-            if (Functions.isWindows()) {
-                if (symlinkFile.exists()) {
-                    symlinkFile.delete();
-                }
-                File dst = new File(symlinkFile,"..\\"+targetPath);
-                try {
-                    Kernel32Utils.createSymbolicLink(symlinkFile,targetPath,dst.isDirectory());
-                } catch (WinIOException e) {
-                    if (e.getErrorCode()==1314) {/* ERROR_PRIVILEGE_NOT_HELD */
-                        warnWindowsSymlink();
-                        return;
-                    }
-                    throw e;
-                } catch (UnsatisfiedLinkError e) {
-                    // not available on this Windows
-                    return;
-                }
-            } else {
-                String errmsg = "";
-                // if a file or a directory exists here, delete it first.
-                // try simple delete first (whether exists() or not, as it may be symlink pointing
-                // to non-existent target), but fallback to "rm -rf" to delete non-empty dir.
-                if (!symlinkFile.delete() && symlinkFile.exists())
-                    // ignore a failure.
-                    new LocalProc(new String[]{"rm","-rf", symlinkPath},new String[0],listener.getLogger(), baseDir).join();
-
-                Integer r=null;
-                if (!SYMLINK_ESCAPEHATCH) {
-                    try {
-                        r = LIBC.symlink(targetPath,symlinkFile.getAbsolutePath());
-                        if (r!=0) {
-                            r = Native.getLastError();
-                            errmsg = LIBC.strerror(r);
-                        }
-                    } catch (LinkageError e) {
-                        // if JNA is unavailable, fall back.
-                        // we still prefer to try JNA first as PosixAPI supports even smaller platforms.
-                        POSIX posix = PosixAPI.jnr();
-                        if (posix.isNative()) {
-                            // TODO should we rethrow PosixException as IOException here?
-                            r = posix.symlink(targetPath,symlinkFile.getAbsolutePath());
-                        }
-                    }
-                }
-                if (r==null) {
-                    // if all else fail, fall back to the most expensive approach of forking a process
-                    // TODO is this really necessary? JavaPOSIX should do this automatically
-                    r = new LocalProc(new String[]{
-                        "ln","-s", targetPath, symlinkPath},
-                        new String[0],listener.getLogger(), baseDir).join();
-                }
-                if (r!=0)
-                    listener.getLogger().println(String.format("ln -s %s %s failed: %d %s",targetPath, symlinkFile, r, errmsg));
-            }
-        } catch (IOException e) {
-            PrintStream log = listener.getLogger();
-            log.printf("ln %s %s failed%n",targetPath, new File(baseDir, symlinkPath));
-            Util.displayIOException(e,listener);
-            Functions.printStackTrace(e, log);
-        }
-    }
-
-    private static boolean createSymlinkJava7(@Nonnull File baseDir, @Nonnull String targetPath, @Nonnull String symlinkPath) throws IOException {
         try {
             Path path = new File(baseDir, symlinkPath).toPath();
             Path target = Paths.get(targetPath, new String[0]);
@@ -1433,19 +1343,18 @@ public class Util {
                     throw fileAlreadyExistsException;
                 }
             }
-            return true;
         } catch (UnsupportedOperationException e) {
-                return true; // no symlinks on this platform
-        } catch (FileSystemException e) {
-            if (Functions.isWindows()) {
+            PrintStream log = listener.getLogger();
+            log.print("Symbolic links are not supported on this platform");
+            Functions.printStackTrace(e, log);
+        } catch (InvalidPathException | IOException e) {
+            if (Functions.isWindows() && e instanceof FileSystemException) {
                 warnWindowsSymlink();
-                return true;
+                return;
             }
-            return false;
-        } catch (IOException x) {
-            throw x;
-        } catch (Exception x) {
-            throw (IOException) new IOException(x.toString()).initCause(x);
+            PrintStream log = listener.getLogger();
+            log.printf("ln %s %s failed%n",targetPath, new File(baseDir, symlinkPath));
+            Functions.printStackTrace(e, log);
         }
     }
 

core/src/main/java/hudson/util/jna/Kernel32Utils.java

@@ -23,6 +23,8 @@
  */
 package hudson.util.jna;
 
+import hudson.Util;
+
 import java.io.*;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -58,6 +60,12 @@ public class Kernel32Utils {
         }
     }
 
+    /**
+     * @deprecated Use {@link java.nio.file.Files#readAttributes} with
+     * {@link java.nio.file.attribute.DosFileAttributes} and reflective calls to
+     * WindowsFileAttributes if necessary.
+     */
+    @Deprecated
     public static int getWin32FileAttributes(File file) throws IOException {
    	// allow lookup of paths longer than MAX_PATH
     	// http://msdn.microsoft.com/en-us/library/aa365247(v=VS.85).aspx
@@ -85,7 +93,9 @@ public class Kernel32Utils {
      *      If the function is not exported by kernel32.
      *      See http://msdn.microsoft.com/en-us/library/windows/desktop/aa363866(v=vs.85).aspx
      *      for compatibility info.
+     * @deprecated Use {@link Util#createSymlink} instead.
      */
+    @Deprecated
     public static void createSymbolicLink(File symlink, String target, boolean dirLink) throws IOException {
         if (!Kernel32.INSTANCE.CreateSymbolicLinkW(
                 new WString(symlink.getPath()), new WString(target),
@@ -94,8 +104,12 @@ public class Kernel32Utils {
         }
     }
 
+    /**
+     * @deprecated Use {@link Util#isSymlink} to detect symbolic links and junctions instead.
+     */
+    @Deprecated
     public static boolean isJunctionOrSymlink(File file) throws IOException {
-        return (file.exists() && (Kernel32.FILE_ATTRIBUTE_REPARSE_POINT & getWin32FileAttributes(file)) != 0);
+        return Util.isSymlink(file);
     }
 
     public static File getTempDir() {

core/src/test/java/hudson/FilePathTest.java

@@ -44,6 +44,7 @@ import java.net.URLConnection;
 import java.net.URLStreamHandler;
 import java.net.URLStreamHandlerFactory;
 import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
@@ -59,7 +60,9 @@ import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.output.NullOutputStream;
 import org.apache.tools.ant.Project;
 import org.apache.tools.ant.taskdefs.Chmod;
+import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.*;
+import static org.junit.Assume.assumeThat;
 import static org.junit.Assume.assumeTrue;
 import static org.junit.Assume.assumeFalse;
 import org.junit.Ignore;
@@ -736,4 +739,38 @@ public class FilePathTest {
         // and now fail when flush is bad!
         tmpDirPath.child("../" + archive.getName()).untar(outDir, TarCompression.NONE);
     }
+
+    @Test public void deleteRecursiveOnUnix() throws Exception {
+        assumeFalse("Uses Unix-specific features", Functions.isWindows());
+        Path targetDir = temp.newFolder("target").toPath();
+        Path targetContents = Files.createFile(targetDir.resolve("contents.txt"));
+        Path toDelete = temp.newFolder("toDelete").toPath();
+        Util.createSymlink(toDelete.toFile(), "../targetDir", "link", TaskListener.NULL);
+        Files.createFile(toDelete.resolve("foo"));
+        Files.createFile(toDelete.resolve("bar"));
+        FilePath f = new FilePath(toDelete.toFile());
+        f.deleteRecursive();
+        assertTrue("symlink target should not be deleted", Files.exists(targetDir));
+        assertTrue("symlink target contents should not be deleted", Files.exists(targetContents));
+        assertFalse("could not delete target", Files.exists(toDelete));
+    }
+
+    @Test public void deleteRecursiveOnWindows() throws Exception {
+        assumeTrue("Uses Windows-specific features", Functions.isWindows());
+        Path targetDir = temp.newFolder("targetDir").toPath();
+        Path targetContents = Files.createFile(targetDir.resolve("contents.txt"));
+        Path toDelete = temp.newFolder("toDelete").toPath();
+        Process p = new ProcessBuilder()
+                .directory(toDelete.toFile())
+                .command("cmd.exe", "/C", "mklink /J junction ..\\targetDir")
+                .start();
+        assumeThat("unable to create junction", p.waitFor(), is(0));
+        Files.createFile(toDelete.resolve("foo"));
+        Files.createFile(toDelete.resolve("bar"));
+        FilePath f = new FilePath(toDelete.toFile());
+        f.deleteRecursive();
+        assertTrue("junction target should not be deleted", Files.exists(targetDir));
+        assertTrue("junction target contents should not be deleted", Files.exists(targetContents));
+        assertFalse("could not delete target", Files.exists(toDelete));
+    }
 }

core/src/test/java/hudson/UtilTest.java

@@ -41,6 +41,7 @@ import static org.hamcrest.CoreMatchers.allOf;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.hasItem;
 import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
 import static org.junit.Assert.*;
 
@@ -259,6 +260,19 @@ public class UtilTest {
         }
     }
 
+    @Test
+    public void testIsSymlink_onWindows_junction() throws Exception {
+        Assume.assumeTrue("Uses Windows-specific features", Functions.isWindows());
+        tmp.newFolder("targetDir");
+        File d = tmp.newFolder("dir");
+        Process p = new ProcessBuilder()
+                .directory(d)
+                .command("cmd.exe", "/C", "mklink /J junction ..\\targetDir")
+                .start();
+        Assume.assumeThat("unable to create junction", p.waitFor(), is(0));
+        assertTrue(Util.isSymlink(new File(d, "junction")));
+    }
+
     @Test
     public void testDeleteFile() throws Exception {
         File f = tmp.newFile();