From f435c719f6d8609a4ce1b62ccc93fdd4ef6bf92c Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Tue, 30 Jan 2024 07:54:48 -0500 Subject: [PATCH 1/6] repoint sibling links --- .../java/org/apache/commons/io/FileUtils.java | 14 ++++++ .../org/apache/commons/io/FileUtilsTest.java | 43 +++++++++++++++++-- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/apache/commons/io/FileUtils.java b/src/main/java/org/apache/commons/io/FileUtils.java index c7bbbdfcbcf..57d68818224 100644 --- a/src/main/java/org/apache/commons/io/FileUtils.java +++ b/src/main/java/org/apache/commons/io/FileUtils.java @@ -1348,6 +1348,20 @@ private static void doCopyDirectory(final File srcDir, final File destDir, final if (exclusionList == null || !exclusionList.contains(srcFile.getCanonicalPath())) { if (srcFile.isDirectory()) { doCopyDirectory(srcFile, dstFile, fileFilter, exclusionList, preserveDirDate, copyOptions); + } else if (Files.isSymbolicLink(srcFile.toPath())) { + final Path linkTarget = Files.readSymbolicLink(srcFile.toPath()); + // TODO handle ancestors/grandchildren + if (directoryContains(srcDir, linkTarget.toFile())) { + // make a new link that points to the copy of the target + final String srcFileName = srcFile.getName(); + final String linkTargetName = linkTarget.toFile().getName(); + final Path newLink = destDir.toPath().resolve(srcFileName); + final Path newTarget = destDir.toPath().resolve(linkTargetName); + Files.createSymbolicLink(newLink, newTarget); + } else { + copyFile(srcFile, dstFile, preserveDirDate, copyOptions); + } + } else { copyFile(srcFile, dstFile, preserveDirDate, copyOptions); } diff --git a/src/test/java/org/apache/commons/io/FileUtilsTest.java b/src/test/java/org/apache/commons/io/FileUtilsTest.java index 173f7a3d9cb..8c306f51a6a 100644 --- a/src/test/java/org/apache/commons/io/FileUtilsTest.java +++ b/src/test/java/org/apache/commons/io/FileUtilsTest.java @@ -782,6 +782,43 @@ public void testCopyDirectory_symLinkExternalFile() throws Exception { assertEquals(content.toPath(), source); } + /** + * Test what happens when copyDirectory copies a directory that contains a symlink + * to a file inside the copied directory. + */ + @Test + public void testCopyDirectory_symLinkInternalFile() throws Exception { + // Make a directory + final File realDirectory = new File(tempDirFile, "real_directory"); + realDirectory.mkdir(); + + // make a file + final File content = new File(realDirectory, "hello.txt"); + FileUtils.writeStringToFile(content, "HELLO WORLD", "UTF8"); + + // Make a symlink to the file + final Path linkPath = realDirectory.toPath().resolve("link_to_file"); + Files.createSymbolicLink(linkPath, content.toPath()); + + // Now copy the directory + final File destination = new File(tempDirFile, "destination"); + FileUtils.copyDirectory(realDirectory, destination); + + // delete the original + assumeTrue(content.delete()); + + // test that the copied directory contains a link to the copied file + final File copiedLink = new File(destination, "link_to_file"); + assertTrue(Files.isSymbolicLink(copiedLink.toPath())); + final String actual = FileUtils.readFileToString(copiedLink, "UTF8"); + assertEquals("HELLO WORLD", actual); + + final Path source = Files.readSymbolicLink(copiedLink.toPath()); + assertNotEquals(content.toPath(), source); + final File copiedContent = new File(destination, "hello.txt"); + assertEquals(copiedContent.toPath(), source); + } + /** * See what happens when copyDirectory copies a directory that is a symlink * to another directory containing non-symlinked files. @@ -790,7 +827,7 @@ public void testCopyDirectory_symLinkExternalFile() throws Exception { * and should not be relied on. */ @Test - public void testCopyDir_symLink() throws Exception { + public void testCopyDirectory_symLink() throws Exception { // Make a directory final File realDirectory = new File(tempDirFile, "real_directory"); realDirectory.mkdir(); @@ -818,7 +855,7 @@ public void testCopyDir_symLink() throws Exception { } @Test - public void testCopyDir_symLinkCycle() throws Exception { + public void testCopyDirectory_symLinkCycle() throws Exception { // Make a directory final File topDirectory = new File(tempDirFile, "topDirectory"); topDirectory.mkdir(); @@ -909,8 +946,8 @@ public void testCopyDirectory_symLink() throws IOException { final Path copiedSymlink = new File(destination, "linkfile").toPath(); // test for the existence of the copied symbolic link as a link - assertTrue(Files.isSymbolicLink(copiedSymlink)); assertTrue(Files.exists(copiedSymlink)); + assertTrue(Files.isSymbolicLink(copiedSymlink)); } @Test From ecf09ac7b4ac60cf290fcca2e803c02ff3752cbe Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Wed, 31 Jan 2024 07:22:04 -0500 Subject: [PATCH 2/6] dismabiguate --- src/test/java/org/apache/commons/io/FileUtilsTest.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/test/java/org/apache/commons/io/FileUtilsTest.java b/src/test/java/org/apache/commons/io/FileUtilsTest.java index 8c306f51a6a..1b34e6c47ee 100644 --- a/src/test/java/org/apache/commons/io/FileUtilsTest.java +++ b/src/test/java/org/apache/commons/io/FileUtilsTest.java @@ -827,10 +827,9 @@ public void testCopyDirectory_symLinkInternalFile() throws Exception { * and should not be relied on. */ @Test - public void testCopyDirectory_symLink() throws Exception { + public void testCopyDirectory_symLink2() throws Exception { // Make a directory final File realDirectory = new File(tempDirFile, "real_directory"); - realDirectory.mkdir(); final File content = new File(realDirectory, "hello.txt"); FileUtils.writeStringToFile(content, "HELLO WORLD", "UTF8"); @@ -927,7 +926,6 @@ public void testCopyDirectory_brokenSymLink() throws IOException { public void testCopyDirectory_symLink() throws IOException { // Make a file final File sourceDirectory = new File(tempDirFile, "source_directory"); - sourceDirectory.mkdir(); final File targetFile = new File(sourceDirectory, "hello.txt"); FileUtils.writeStringToFile(targetFile, "HELLO WORLD", "UTF8"); @@ -935,9 +933,6 @@ public void testCopyDirectory_symLink() throws IOException { final Path targetPath = targetFile.toPath(); final Path linkPath = sourceDirectory.toPath().resolve("linkfile"); Files.createSymbolicLink(linkPath, targetPath); - assumeTrue(Files.isSymbolicLink(linkPath), () -> "Expected a symlink here: " + linkPath); - assumeTrue(Files.exists(linkPath)); - assumeTrue(Files.exists(linkPath, LinkOption.NOFOLLOW_LINKS)); // Now copy sourceDirectory to another directory final File destination = new File(tempDirFile, "destination"); From b1b27902bc9a9f6a7eeed75e03f4a495bbd077d6 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Fri, 2 Feb 2024 07:55:07 -0500 Subject: [PATCH 3/6] add test for subdirectory link repointing --- .../org/apache/commons/io/FileUtilsTest.java | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/test/java/org/apache/commons/io/FileUtilsTest.java b/src/test/java/org/apache/commons/io/FileUtilsTest.java index 79e310e299a..2a0e21e344a 100644 --- a/src/test/java/org/apache/commons/io/FileUtilsTest.java +++ b/src/test/java/org/apache/commons/io/FileUtilsTest.java @@ -819,6 +819,41 @@ public void testCopyDirectory_symLinkInternalFile() throws Exception { assertEquals(copiedContent.toPath(), source); } + @Test + public void testCopyDirectory_symLinkInternalFile_nested() throws Exception { + // Make a directory + final File realDirectory = new File(tempDirFile, "real_directory"); + final File subDirectoryA = new File(realDirectory, "A"); + final File subDirectoryB = new File(realDirectory, "B"); + subDirectoryB.mkdirs(); + + // make a file + final File content = new File(subDirectoryA, "hello.txt"); + FileUtils.writeStringToFile(content, "HELLO WORLD", "UTF8"); + + // Make a symlink to the file + final Path linkPath = subDirectoryB.toPath().resolve("link_to_file"); + Files.createSymbolicLink(linkPath, content.toPath()); + + // Now copy the directory + final File destination = new File(tempDirFile, "destination"); + FileUtils.copyDirectory(realDirectory, destination); + + // delete the original + content.delete(); + + // test that the copied directory contains a link to the copied file + final File copiedLink = new File(new File(destination, "B"), "link_to_file"); + assertTrue(Files.isSymbolicLink(copiedLink.toPath())); + final String actual = FileUtils.readFileToString(copiedLink, "UTF8"); + assertEquals("HELLO WORLD", actual); + + final Path source = Files.readSymbolicLink(copiedLink.toPath()); + assertNotEquals(content.toPath(), source); + final File copiedContent = new File(new File(destination, "A"), "hello.txt"); + assertEquals(copiedContent.toPath(), source); + } + /** * See what happens when copyDirectory copies a directory that is a symlink * to another directory containing non-symlinked files. From d45a4a9f25ae997d53a68d44a6deebfd4d9c4271 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Fri, 2 Feb 2024 08:46:32 -0500 Subject: [PATCH 4/6] preserve symbolic link graph inside copied directory --- .../java/org/apache/commons/io/FileUtils.java | 20 ++++++++++--------- .../org/apache/commons/io/FileUtilsTest.java | 19 +++++++++--------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/apache/commons/io/FileUtils.java b/src/main/java/org/apache/commons/io/FileUtils.java index f296483bd0e..82c1e6833c1 100644 --- a/src/main/java/org/apache/commons/io/FileUtils.java +++ b/src/main/java/org/apache/commons/io/FileUtils.java @@ -720,7 +720,8 @@ public static void copyDirectory(final File srcDir, final File destDir, final Fi } } } - doCopyDirectory(srcDir, destDir, fileFilter, exclusionList, preserveFileDate, copyOptions); + // TODO add a new doCopyDirectory or copyFromRoot method that sets the src and dest roots and calls this to avoid doubling up on args + doCopyDirectory(srcDir, destDir, fileFilter, exclusionList, preserveFileDate, srcDir, destDir, copyOptions); } /** @@ -1332,12 +1333,14 @@ public static boolean directoryContains(final File directory, final File child) * @param fileFilter the filter to apply, null means copy all directories and files. * @param exclusionList List of files and directories to exclude from the copy, may be null. * @param preserveDirDate preserve the directories last modified dates. + * @param srcRoot the top directory being copied from, preserved during recursion + * @param destRoot the top directory being copied to, preserved during recursion * @param copyOptions options specifying how the copy should be done, see {@link StandardCopyOption}. * @throws IOException if the directory was not created along with all its parent directories. * @throws SecurityException See {@link File#mkdirs()}. */ private static void doCopyDirectory(final File srcDir, final File destDir, final FileFilter fileFilter, final List exclusionList, - final boolean preserveDirDate, final CopyOption... copyOptions) throws IOException { + final boolean preserveDirDate, final File srcRoot, final File destRoot, final CopyOption... copyOptions) throws IOException { // recurse dirs, copy files. final File[] srcFiles = listFiles(srcDir, fileFilter); requireDirectoryIfExists(destDir, "destDir"); @@ -1347,16 +1350,15 @@ private static void doCopyDirectory(final File srcDir, final File destDir, final final File dstFile = new File(destDir, srcFile.getName()); if (exclusionList == null || !exclusionList.contains(srcFile.getCanonicalPath())) { if (srcFile.isDirectory()) { - doCopyDirectory(srcFile, dstFile, fileFilter, exclusionList, preserveDirDate, copyOptions); + doCopyDirectory(srcFile, dstFile, fileFilter, exclusionList, preserveDirDate, srcRoot, destRoot, copyOptions); } else if (Files.isSymbolicLink(srcFile.toPath())) { final Path linkTarget = Files.readSymbolicLink(srcFile.toPath()); - // TODO handle ancestors/grandchildren - if (directoryContains(srcDir, linkTarget.toFile())) { + if (directoryContains(srcRoot, linkTarget.toFile())) { // make a new link that points to the copy of the target - final String srcFileName = srcFile.getName(); - final String linkTargetName = linkTarget.toFile().getName(); - final Path newLink = destDir.toPath().resolve(srcFileName); - final Path newTarget = destDir.toPath().resolve(linkTargetName); + final Path srcFileRelativePath = srcRoot.toPath().relativize(srcFile.toPath()); + final Path linkTargetRelativePath = srcRoot.toPath().relativize(linkTarget); + final Path newLink = destRoot.toPath().resolve(srcFileRelativePath); + final Path newTarget = destRoot.toPath().resolve(linkTargetRelativePath); Files.createSymbolicLink(newLink, newTarget); } else { copyFile(srcFile, dstFile, preserveDirDate, copyOptions); diff --git a/src/test/java/org/apache/commons/io/FileUtilsTest.java b/src/test/java/org/apache/commons/io/FileUtilsTest.java index 2a0e21e344a..66cec142b2b 100644 --- a/src/test/java/org/apache/commons/io/FileUtilsTest.java +++ b/src/test/java/org/apache/commons/io/FileUtilsTest.java @@ -822,9 +822,9 @@ public void testCopyDirectory_symLinkInternalFile() throws Exception { @Test public void testCopyDirectory_symLinkInternalFile_nested() throws Exception { // Make a directory - final File realDirectory = new File(tempDirFile, "real_directory"); - final File subDirectoryA = new File(realDirectory, "A"); - final File subDirectoryB = new File(realDirectory, "B"); + final File originalDirectory = new File(tempDirFile, "original_directory"); + final File subDirectoryA = new File(originalDirectory, "A"); + final File subDirectoryB = new File(originalDirectory, "B"); subDirectoryB.mkdirs(); // make a file @@ -836,22 +836,23 @@ public void testCopyDirectory_symLinkInternalFile_nested() throws Exception { Files.createSymbolicLink(linkPath, content.toPath()); // Now copy the directory - final File destination = new File(tempDirFile, "destination"); - FileUtils.copyDirectory(realDirectory, destination); + final File copiedDirectory = new File(tempDirFile, "copied_directory"); + FileUtils.copyDirectory(originalDirectory, copiedDirectory); // delete the original content.delete(); // test that the copied directory contains a link to the copied file - final File copiedLink = new File(new File(destination, "B"), "link_to_file"); + final File copiedLink = new File(new File(copiedDirectory, "B"), "link_to_file"); assertTrue(Files.isSymbolicLink(copiedLink.toPath())); - final String actual = FileUtils.readFileToString(copiedLink, "UTF8"); - assertEquals("HELLO WORLD", actual); final Path source = Files.readSymbolicLink(copiedLink.toPath()); assertNotEquals(content.toPath(), source); - final File copiedContent = new File(new File(destination, "A"), "hello.txt"); + final File copiedContent = new File(new File(copiedDirectory, "A"), "hello.txt"); assertEquals(copiedContent.toPath(), source); + + final String actual = FileUtils.readFileToString(copiedLink, "UTF8"); + assertEquals("HELLO WORLD", actual); } /** From 92fcd72ae8b6e9b1636c345cc364b77523c3f74c Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Fri, 2 Feb 2024 09:50:29 -0500 Subject: [PATCH 5/6] doCopyDirectoryFromRoot --- src/main/java/org/apache/commons/io/FileUtils.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/commons/io/FileUtils.java b/src/main/java/org/apache/commons/io/FileUtils.java index 82c1e6833c1..cf160a804be 100644 --- a/src/main/java/org/apache/commons/io/FileUtils.java +++ b/src/main/java/org/apache/commons/io/FileUtils.java @@ -720,8 +720,7 @@ public static void copyDirectory(final File srcDir, final File destDir, final Fi } } } - // TODO add a new doCopyDirectory or copyFromRoot method that sets the src and dest roots and calls this to avoid doubling up on args - doCopyDirectory(srcDir, destDir, fileFilter, exclusionList, preserveFileDate, srcDir, destDir, copyOptions); + doCopyDirectoryFromRoot(srcDir, destDir, fileFilter, exclusionList, preserveFileDate, copyOptions); } /** @@ -1324,6 +1323,13 @@ public static boolean directoryContains(final File directory, final File child) return FilenameUtils.directoryContains(directory.getCanonicalPath(), child.getCanonicalPath()); } + // Splits src and dest directories into two variables each: one that is modified as we descend + // through the directory tree and one that isn't + private static void doCopyDirectoryFromRoot(final File srcDir, final File destDir, final FileFilter fileFilter, final List exclusionList, + final boolean preserveDirDate, final CopyOption... copyOptions) throws IOException { + doCopyDirectory(srcDir, destDir, fileFilter, exclusionList, preserveDirDate, srcDir, destDir, copyOptions); + } + /** * Internal copy directory method. Creates all destination parent directories, * including any necessary but non-existent parent directories. From 8bfcf3e3d9789588c5f1638e46f820f601c76ac9 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Fri, 2 Feb 2024 18:26:01 -0500 Subject: [PATCH 6/6] javadoc --- .../java/org/apache/commons/io/FileUtils.java | 51 +++++++++++++++++-- 1 file changed, 46 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/apache/commons/io/FileUtils.java b/src/main/java/org/apache/commons/io/FileUtils.java index cf160a804be..6053962a318 100644 --- a/src/main/java/org/apache/commons/io/FileUtils.java +++ b/src/main/java/org/apache/commons/io/FileUtils.java @@ -507,11 +507,12 @@ public static File[] convertFileCollectionToFileArray(final Collection fil * {@link File#setLastModified(long)}. If that fails, the method throws IOException. *

*

- * Symbolic links in the source directory are copied to new symbolic links in the destination - * directory that point to the original target. The target of the link is not copied unless - * it is also under the source directory. Even if it is under the source directory, the new symbolic - * link in the destination points to the original target in the source directory, not to the - * newly created copy of the target. + * Treatment of symbolic links inside {@code srcDir} depends on the target of the link. + * If the link points to a target outside of {@code srcDir} (and therefore is not copied), + * then {@code destDir} will contain link to the original, uncopied file. + * However, if the symbolic links point to a file inside {@code srcDir}, + * the new link inside {@code destDir} will point to the copy of the target inside + * {@code destDir}. *

* * @param srcDir an existing directory to copy, must not be {@code null}. @@ -536,6 +537,14 @@ public static void copyDirectory(final File srcDir, final File destDir) throws I * method merges the source with the destination, with the source taking precedence. *

*

+ * Treatment of symbolic links inside {@code srcDir} depends on the target of the link. + * If the link points to a target outside of {@code srcDir} (and therefore is not copied), + * then {@code destDir} will contain link to the original, uncopied file. + * However, if the symbolic links point to a file inside {@code srcDir}, + * the new link inside {@code destDir} will point to the copy of the target inside + * {@code destDir}. + *

+ *

* Note: Setting {@code preserveFileDate} to {@code true} tries to preserve the files' last * modified date/times using {@link File#setLastModified(long)}. However it is not guaranteed that those operations * will succeed. If the modification operation fails, the method throws IOException. @@ -565,6 +574,14 @@ public static void copyDirectory(final File srcDir, final File destDir, final bo * method merges the source with the destination, with the source taking precedence. *

*

+ * Treatment of symbolic links inside {@code srcDir} depends on the target of the link. + * If the link points to a target outside of {@code srcDir} (and therefore is not copied), + * then {@code destDir} will contain link to the original, uncopied file. + * However, if the symbolic links point to a file inside {@code srcDir}, + * the new link inside {@code destDir} will point to the copy of the target inside + * {@code destDir}. + *

+ *

* Note: This method tries to preserve the files' last modified date/times using * {@link File#setLastModified(long)}. However it is not guaranteed that those operations will succeed. If the * modification operation fails, the method throws IOException. @@ -614,6 +631,14 @@ public static void copyDirectory(final File srcDir, final File destDir, final Fi * method merges the source with the destination, with the source taking precedence. *

*

+ * Treatment of symbolic links inside {@code srcDir} depends on the target of the link. + * If the link points to a target outside of {@code srcDir} (and therefore is not copied), + * then {@code destDir} will contain link to the original, uncopied file. + * However, if the symbolic links point to a file inside {@code srcDir}, + * the new link inside {@code destDir} will point to the copy of the target inside + * {@code destDir}. + *

+ *

* Note: Setting {@code preserveFileDate} to {@code true} tries to preserve the file's last * modified date/times using {@link BasicFileAttributeView#setTimes(FileTime, FileTime, FileTime)}. However, it is * not guaranteed that the operation will succeed. If the modification operation fails it falls back to @@ -664,6 +689,14 @@ public static void copyDirectory(final File srcDir, final File destDir, final Fi * method merges the source with the destination, with the source taking precedence. *

*

+ * Treatment of symbolic links inside {@code srcDir} depends on the target of the link. + * If the link points to a target outside of {@code srcDir} (and therefore is not copied), + * then {@code destDir} will contain link to the original, uncopied file. + * However, if the symbolic links point to a file inside {@code srcDir}, + * the new link inside {@code destDir} will point to the copy of the target inside + * {@code destDir}. + *

+ *

* Note: Setting {@code preserveFileDate} to {@code true} tries to preserve the file's last * modified date/times using {@link BasicFileAttributeView#setTimes(FileTime, FileTime, FileTime)}. However, it is * not guaranteed that the operation will succeed. If the modification operation fails it falls back to @@ -734,6 +767,14 @@ public static void copyDirectory(final File srcDir, final File destDir, final Fi * method merges the source with the destination, with the source taking precedence. *

*

+ * Treatment of symbolic links inside {@code srcDir} depends on the target of the link. + * If the link points to a target outside of {@code srcDir} (and therefore is not copied), + * then {@code destDir} will contain link to the original, uncopied file. + * However, if the symbolic links point to a file inside {@code srcDir}, + * the new link inside {@code destDir} will point to the copy of the target inside + * {@code destDir}. + *

+ *

* Note: Setting {@code preserveFileDate} to {@code true} tries to preserve the file's last * modified date/times using {@link BasicFileAttributeView#setTimes(FileTime, FileTime, FileTime)}. However, it is * not guaranteed that the operation will succeed. If the modification operation fails it falls back to