Skip to content

Commit

Permalink
[JENKINS-39179] [JENKINS-36088] Always use NIO to create and detect s…
Browse files Browse the repository at this point in the history 
…ymbolic 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
  • Loading branch information
@dwnusbaum @oleg-nenashev
dwnusbaum authored and oleg-nenashev committed Nov 16, 2017
1 parent 7d29d4d commit 52fa4d9
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 125 deletions.
157 changes: 33 additions & 124 deletions core/src/main/java/hudson/Util.java
View file
Expand Up @@ -25,15 +25,12 @@

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;
Expand Down Expand Up @@ -67,8 +64,12 @@
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;
Expand All @@ -81,9 +82,6 @@
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;
Expand Down Expand Up @@ -493,7 +491,6 @@ private static String deleteFailExceptionMessage(File whatWeWereTryingToRemove,
/**
* 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.
Expand All @@ -502,44 +499,29 @@ public static boolean isSymlink(@Nonnull File file) throws IOException {
* 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;
}
}

Expand Down Expand Up @@ -1341,78 +1323,6 @@ public static FileSet createFileSet(@Nonnull File baseDir, @Nonnull String inclu
*/
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]);
Expand All @@ -1433,19 +1343,18 @@ private static boolean createSymlinkJava7(@Nonnull File baseDir, @Nonnull String
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);
}
}

Expand Down
16 changes: 15 additions & 1 deletion core/src/main/java/hudson/util/jna/Kernel32Utils.java
View file
Expand Up @@ -23,6 +23,8 @@
*/
package hudson.util.jna;

import hudson.Util;

import java.io.*;
import java.util.logging.Level;
import java.util.logging.Logger;
Expand Down Expand Up @@ -58,6 +60,12 @@ public static int waitForExitProcess(Pointer hProcess) throws InterruptedExcepti
}
}

/**
* @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
Expand Down Expand Up @@ -85,7 +93,9 @@ public static int getWin32FileAttributes(File file) throws IOException {
* 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),
Expand All @@ -94,8 +104,12 @@ public static void createSymbolicLink(File symlink, String target, boolean dirLi
}
}

/**
* @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() {
Expand Down
37 changes: 37 additions & 0 deletions core/src/test/java/hudson/FilePathTest.java
View file
Expand Up @@ -44,6 +44,7 @@
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;
Expand All @@ -59,7 +60,9 @@
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;
Expand Down Expand Up @@ -736,4 +739,38 @@ public void testEOFbrokenFlush() throws IOException, InterruptedException {
// 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));
}
}
14 changes: 14 additions & 0 deletions core/src/test/java/hudson/UtilTest.java
View file
Expand Up @@ -41,6 +41,7 @@
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.*;

Expand Down Expand Up @@ -259,6 +260,19 @@ public void testIsSymlink() throws IOException, InterruptedException {
}
}

@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();
Expand Down

0 comments on commit 52fa4d9

Please sign in to comment.