From c35f3ba75ce8a5a33b7efc8f760ec7e5bf8c9228 Mon Sep 17 00:00:00 2001
From: Matt Sicker <boards@gmail.com>
Date: Fri, 11 Jan 2019 10:35:57 -0600
Subject: [PATCH] Add more tests for JENKINS-55448

This also refactors some test calls to mklink into a package-private Util method due to additional tests needing to create junction points.

Signed-off-by: Matt Sicker <boards@gmail.com>
---
 core/src/main/java/hudson/Util.java           | 33 +++++++++++++
 core/src/test/java/hudson/FilePathTest.java   |  7 +--
 .../RemoveWindowsDirectoryJunctionTest.java   | 12 +----
 core/src/test/java/hudson/UtilTest.java       | 46 ++++++++++++++++---
 .../java/jenkins/util/io/PathRemoverTest.java | 33 +++++++++++++
 5 files changed, 108 insertions(+), 23 deletions(-)

diff --git a/core/src/main/java/hudson/Util.java b/core/src/main/java/hudson/Util.java
index 45d8f3f5e2..a55b90d05e 100644
--- a/core/src/main/java/hudson/Util.java
+++ b/core/src/main/java/hudson/Util.java
@@ -23,6 +23,7 @@
  */
 package hudson;
 
+import com.google.common.annotations.VisibleForTesting;
 import hudson.model.TaskListener;
 import jenkins.util.MemoryReductionUtil;
 import hudson.util.QuotedStringTokenizer;
@@ -1277,6 +1278,38 @@ public class Util {
         }
     }
 
+    /**
+     * Creates an NTFS junction point if supported. Similar to symbolic links, NTFS provides junction points which
+     * provide different features than symbolic links. This method is primarily useful for unit testing on Windows.
+     * @param junction NTFS junction point to create
+     * @param target target directory to junction
+     * @return the newly created junction point
+     * @throws IOException if the call to mklink exits with a non-zero status code
+     * @throws InterruptedException if the call to mklink is interrupted before completing
+     * @throws AssertionError if this method is called on a non-Windows platform
+     */
+    static @Nonnull File createJunction(@Nonnull File junction, @Nonnull File target) throws IOException, InterruptedException {
+        if (!Functions.isWindows()) throw new AssertionError("Cannot create junctions on non-Windows platforms");
+        String command = "mklink /J " + escapeCmdArgument(junction.getAbsolutePath()) + ' ' + escapeCmdArgument(target.getAbsolutePath());
+        Process process = new ProcessBuilder("cmd.exe", "/C", command).start();
+        int result = process.waitFor();
+        if (result != 0) throw new IOException("Command `" + command + "` failed with status code " + result);
+        return junction;
+    }
+
+    private static final Pattern CMD_METACHARS = Pattern.compile("[()%!^\"<>&|]");
+
+    /**
+     * Escapes a Windows cmd argument. Cmd supports escaping characters with carats which simplifies nested escapes.
+     * @param arg argument to escape
+     * @return the quoted argument for use in cmd.exe
+     * @see <a href="https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/>Everyone quotes command line arguments the wrong way</a>
+     */
+    @VisibleForTesting
+    static String escapeCmdArgument(String arg) {
+        return CMD_METACHARS.matcher('"' + arg + '"').replaceAll("^$0");
+    }
+
     /**
      * Encodes the URL by RFC 2396.
      *
diff --git a/core/src/test/java/hudson/FilePathTest.java b/core/src/test/java/hudson/FilePathTest.java
index 593d30ce4a..cf4e163b9d 100644
--- a/core/src/test/java/hudson/FilePathTest.java
+++ b/core/src/test/java/hudson/FilePathTest.java
@@ -836,17 +836,14 @@ public class FilePathTest {
         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));
+        File junction = Util.createJunction(toDelete.resolve("junction").toFile(), targetDir.toFile());
         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 junction", junction.exists());
         assertFalse("could not delete target", Files.exists(toDelete));
     }
 
diff --git a/core/src/test/java/hudson/RemoveWindowsDirectoryJunctionTest.java b/core/src/test/java/hudson/RemoveWindowsDirectoryJunctionTest.java
index a48c9d833c..dc44db7974 100644
--- a/core/src/test/java/hudson/RemoveWindowsDirectoryJunctionTest.java
+++ b/core/src/test/java/hudson/RemoveWindowsDirectoryJunctionTest.java
@@ -3,7 +3,6 @@
  */
 package hudson;
 
-import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assume.assumeTrue;
@@ -33,19 +32,10 @@ public class RemoveWindowsDirectoryJunctionTest {
         File subdir1 = tmp.newFolder("notJunction");
         File f1 = new File(subdir1, "testfile1.txt");
         assertTrue("Unable to create temporary file in notJunction directory", f1.createNewFile());
-        File j1 = makeJunction(tmp.getRoot(), subdir1);
+        File j1 = Util.createJunction(tmp.getRoot(), subdir1);
         Util.deleteRecursive(j1);
         assertFalse("Windows Junction should have been removed", j1.exists());
         assertTrue("Contents of Windows Junction should not be removed", f1.exists());
     }
 
-    private File makeJunction(File baseDir, File pointToDir) throws Exception {
-       File junc = new File(baseDir, "test Junction");
-       String cmd = "mklink /J \"" + junc.getPath() + "\" \"" + pointToDir.getPath() + "\"";
-       ProcessBuilder pb = new ProcessBuilder("cmd.exe", "/C", cmd);
-       pb.inheritIO();
-       Process p = pb.start();
-       assertEquals("Running mklink failed (cmd=" + cmd + ")", 0, p.waitFor());
-       return junc;
-    }
 }
diff --git a/core/src/test/java/hudson/UtilTest.java b/core/src/test/java/hudson/UtilTest.java
index 36586408e5..aeb7aaf9e8 100644
--- a/core/src/test/java/hudson/UtilTest.java
+++ b/core/src/test/java/hudson/UtilTest.java
@@ -24,6 +24,8 @@
  */
 package hudson;
 
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
 import java.util.Date;
@@ -255,17 +257,47 @@ public class UtilTest {
         }
     }
 
+    @Test
+    public void testEscapeCmdArgument() {
+        String raw = "hello \"\\world&";
+        String expected = "^\"hello ^\"\\world^&^\"";
+        String actual = Util.escapeCmdArgument(raw);
+        assertEquals(expected, actual);
+    }
+
     @Test
     public void testIsSymlink_onWindows_junction() throws Exception {
         Assume.assumeTrue("Uses Windows-specific features", Functions.isWindows());
-        tmp.newFolder("targetDir");
+        File targetDir = 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")));
+        File junction = Util.createJunction(new File(d, "junction"), targetDir);
+        assertTrue(Util.isSymlink(junction));
+    }
+
+    @Test
+    @Issue("JENKINS-55448")
+    public void testIsSymlink_ParentIsJunction() throws IOException, InterruptedException {
+        Assume.assumeTrue("Uses Windows-specific features", Functions.isWindows());
+        File targetDir = tmp.newFolder();
+        File file = new File(targetDir, "test-file");
+        new FilePath(file).touch(System.currentTimeMillis());
+        File dir = tmp.newFolder();
+        File junction = Util.createJunction(new File(dir, "junction"), targetDir);
+
+        assertTrue(Util.isSymlink(junction));
+        assertFalse(Util.isSymlink(file));
+    }
+
+    @Test
+    @Issue("JENKINS-55448")
+    public void testIsSymlink_ParentIsSymlink() throws IOException, InterruptedException {
+        File folder = tmp.newFolder();
+        File file = new File(folder, "test-file");
+        new FilePath(file).touch(System.currentTimeMillis());
+        Path link = tmp.getRoot().toPath().resolve("sym-link");
+        Path pathWithSymlinkParent = Files.createSymbolicLink(link, folder.toPath()).resolve("test-file");
+        assertTrue(Util.isSymlink(link));
+        assertFalse(Util.isSymlink(pathWithSymlinkParent));
     }
 
     @Test
diff --git a/core/src/test/java/jenkins/util/io/PathRemoverTest.java b/core/src/test/java/jenkins/util/io/PathRemoverTest.java
index 4a40684c0d..860315f6fc 100644
--- a/core/src/test/java/jenkins/util/io/PathRemoverTest.java
+++ b/core/src/test/java/jenkins/util/io/PathRemoverTest.java
@@ -159,6 +159,39 @@ public class PathRemoverTest {
         assertTrue("Should not have deleted target file: " + file, file.exists());
     }
 
+    @Test
+    @Issue("JENKINS-55448")
+    public void testForceRemoveFile_DotsInPath() throws IOException {
+        Path folder = tmp.newFolder().toPath();
+        File test = tmp.newFile("test");
+        touchWithFileName(test);
+        Path path = folder.resolve("../test");
+
+        PathRemover remover = PathRemover.newSimpleRemover();
+        remover.forceRemoveFile(path);
+
+        assertTrue("Unable to delete file: " + path, Files.notExists(path));
+        assertFalse(test.exists());
+        assertTrue("Should not have deleted directory: " + folder, Files.exists(folder));
+    }
+
+    @Test
+    @Issue("JENKINS-55448")
+    public void testForceRemoveFile_ParentIsSymbolicLink() throws IOException {
+        Path realParent = tmp.newFolder().toPath();
+        Path path = realParent.resolve("test-file");
+        touchWithFileName(path.toFile());
+        Path symParent = Files.createSymbolicLink(tmp.getRoot().toPath().resolve("sym-parent"), realParent);
+        Path toDelete = symParent.resolve("test-file");
+
+        PathRemover remover = PathRemover.newSimpleRemover();
+        remover.forceRemoveFile(toDelete);
+
+        assertTrue("Unable to delete file: " + toDelete, Files.notExists(toDelete));
+        assertTrue("Should not have deleted directory: " + realParent, Files.exists(realParent));
+        assertTrue("Should not have deleted symlink: " + symParent, Files.exists(symParent, LinkOption.NOFOLLOW_LINKS));
+    }
+
     @Test
     public void testForceRemoveDirectoryContents() throws IOException {
         File dir = tmp.newFolder();