From cff71772686033d97acd9fa2ecb4a92aa2d8acc0 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Sun, 14 Jan 2024 08:16:56 -0500 Subject: [PATCH 1/5] update test to reflect desired behavior of copying broken symlinks --- .../java/org/apache/commons/io/FileUtilsTest.java | 11 +++++------ 1 file changed, 5 insertions(+), 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 e7b3202eadc..8ccc2f74504 100644 --- a/src/test/java/org/apache/commons/io/FileUtilsTest.java +++ b/src/test/java/org/apache/commons/io/FileUtilsTest.java @@ -783,12 +783,11 @@ public void testCopyDirectory_brokenSymLink() throws IOException { // Now copy sourceDirectory, including the broken link, to another directory final File destination = new File(tempDirFile, "destination"); - final FileNotFoundException thrown = assertThrows( - FileNotFoundException.class, - () -> FileUtils.copyDirectory(sourceDirectory, destination), - "ignored broken link" - ); - assertTrue(thrown.getMessage().contains("linkfile' does not exist")); + FileUtils.copyDirectory(sourceDirectory, destination); + assertTrue(destination.exists()); + File copiedBrokenSymlink = new File(destination, "linkfile"); + assertTrue(Files.isSymbolicLink(copiedBrokenSymlink.toPath())); + assertFalse(Files.exists(copiedBrokenSymlink.toPath())); } @Test From 9089174059023e18265e1a9cd12e45dad8317980 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Sun, 14 Jan 2024 09:05:42 -0500 Subject: [PATCH 2/5] [IO-807] copy symbolic links as links --- .../java/org/apache/commons/io/FileUtils.java | 28 ++++++++++++++++--- .../org/apache/commons/io/FileUtilsTest.java | 11 ++++++-- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/apache/commons/io/FileUtils.java b/src/main/java/org/apache/commons/io/FileUtils.java index 2b3a554ab0e..0af7fb2ff97 100644 --- a/src/main/java/org/apache/commons/io/FileUtils.java +++ b/src/main/java/org/apache/commons/io/FileUtils.java @@ -57,6 +57,7 @@ import java.time.chrono.ChronoLocalDateTime; import java.time.chrono.ChronoZonedDateTime; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Date; @@ -297,7 +298,9 @@ private static void checkFileExists(final File file, final String name) throws F if (file.exists()) { throw new IllegalArgumentException("Parameter '" + name + "' is not a file: " + file); } - throw new FileNotFoundException("Source '" + file + "' does not exist"); + if (!Files.isSymbolicLink(file.toPath())) { + throw new FileNotFoundException("Source '" + file + "' does not exist"); + } } } @@ -504,6 +507,13 @@ public static File[] convertFileCollectionToFileArray(final Collection fil * not guaranteed that the operation will succeed. If the modification operation fails, it falls back to * {@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. + *

* * @param srcDir an existing directory to copy, must not be {@code null}. * @param destDir the new directory, must not be {@code null}. @@ -816,7 +826,7 @@ public static void copyFile(final File srcFile, final File destFile, final boole * @param srcFile an existing file to copy, must not be {@code null}. * @param destFile the new file, must not be {@code null}. * @param preserveFileDate true if the file date of the copy should be the same as the original. - * @param copyOptions options specifying how the copy should be done, for example {@link StandardCopyOption}.. + * @param copyOptions options specifying how the copy should be done, for example {@link StandardCopyOption}. * @throws NullPointerException if any of the given {@link File}s are {@code null}. * @throws FileNotFoundException if the source does not exist. * @throws IllegalArgumentException if source is not a file. @@ -825,7 +835,7 @@ public static void copyFile(final File srcFile, final File destFile, final boole * @see #copyFileToDirectory(File, File, boolean) * @since 2.8.0 */ - public static void copyFile(final File srcFile, final File destFile, final boolean preserveFileDate, final CopyOption... copyOptions) throws IOException { + public static void copyFile(final File srcFile, final File destFile, final boolean preserveFileDate, CopyOption... copyOptions) throws IOException { Objects.requireNonNull(destFile, "destination"); checkFileExists(srcFile, "srcFile"); requireCanonicalPathsNotEquals(srcFile, destFile); @@ -834,10 +844,20 @@ public static void copyFile(final File srcFile, final File destFile, final boole checkFileExists(destFile, "destFile"); requireCanWrite(destFile, "destFile"); } + + final boolean isSymLink = Files.isSymbolicLink(srcFile.toPath()); + if (isSymLink) { + if (!Arrays.asList(copyOptions).contains(LinkOption.NOFOLLOW_LINKS)) { + final List list = new ArrayList(Arrays.asList(copyOptions)); + list.add(LinkOption.NOFOLLOW_LINKS); + copyOptions = list.toArray(new CopyOption[0]); + } + } + Files.copy(srcFile.toPath(), destFile.toPath(), copyOptions); // On Windows, the last modified time is copied by default. - if (preserveFileDate && !setTimes(srcFile, destFile)) { + if (preserveFileDate && !isSymLink && !setTimes(srcFile, destFile)) { throw new IOException("Cannot set the file time."); } } diff --git a/src/test/java/org/apache/commons/io/FileUtilsTest.java b/src/test/java/org/apache/commons/io/FileUtilsTest.java index 8ccc2f74504..471a9e6c31c 100644 --- a/src/test/java/org/apache/commons/io/FileUtilsTest.java +++ b/src/test/java/org/apache/commons/io/FileUtilsTest.java @@ -785,9 +785,14 @@ public void testCopyDirectory_brokenSymLink() throws IOException { final File destination = new File(tempDirFile, "destination"); FileUtils.copyDirectory(sourceDirectory, destination); assertTrue(destination.exists()); - File copiedBrokenSymlink = new File(destination, "linkfile"); - assertTrue(Files.isSymbolicLink(copiedBrokenSymlink.toPath())); - assertFalse(Files.exists(copiedBrokenSymlink.toPath())); + final Path copiedBrokenSymlink = new File(destination, "linkfile").toPath(); + + // test for theb existence of the copied symbolic link as a link + assertTrue(Files.isSymbolicLink(copiedBrokenSymlink)); + + // shouldn't be able to read through to the source of the link. + // If we can, then the link points somewhere other than the deleted file + assertFalse(Files.exists(copiedBrokenSymlink)); } @Test From 8da94f6f8216de565f526484e2c877a97ab909f8 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Sun, 14 Jan 2024 09:27:50 -0500 Subject: [PATCH 3/5] pmd --- src/main/java/org/apache/commons/io/FileUtils.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/apache/commons/io/FileUtils.java b/src/main/java/org/apache/commons/io/FileUtils.java index 0af7fb2ff97..5a26fc2eb0f 100644 --- a/src/main/java/org/apache/commons/io/FileUtils.java +++ b/src/main/java/org/apache/commons/io/FileUtils.java @@ -846,12 +846,10 @@ public static void copyFile(final File srcFile, final File destFile, final boole } final boolean isSymLink = Files.isSymbolicLink(srcFile.toPath()); - if (isSymLink) { - if (!Arrays.asList(copyOptions).contains(LinkOption.NOFOLLOW_LINKS)) { - final List list = new ArrayList(Arrays.asList(copyOptions)); - list.add(LinkOption.NOFOLLOW_LINKS); - copyOptions = list.toArray(new CopyOption[0]); - } + if (isSymLink && !Arrays.asList(copyOptions).contains(LinkOption.NOFOLLOW_LINKS)) { + final List list = new ArrayList(Arrays.asList(copyOptions)); + list.add(LinkOption.NOFOLLOW_LINKS); + copyOptions = list.toArray(new CopyOption[0]); } Files.copy(srcFile.toPath(), destFile.toPath(), copyOptions); From 57e133684964c73cc486c1e971ca959d2ba723e4 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Sun, 14 Jan 2024 09:29:58 -0500 Subject: [PATCH 4/5] typo --- src/test/java/org/apache/commons/io/FileUtilsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/apache/commons/io/FileUtilsTest.java b/src/test/java/org/apache/commons/io/FileUtilsTest.java index 471a9e6c31c..46cfd0ce862 100644 --- a/src/test/java/org/apache/commons/io/FileUtilsTest.java +++ b/src/test/java/org/apache/commons/io/FileUtilsTest.java @@ -787,7 +787,7 @@ public void testCopyDirectory_brokenSymLink() throws IOException { assertTrue(destination.exists()); final Path copiedBrokenSymlink = new File(destination, "linkfile").toPath(); - // test for theb existence of the copied symbolic link as a link + // test for the existence of the copied symbolic link as a link assertTrue(Files.isSymbolicLink(copiedBrokenSymlink)); // shouldn't be able to read through to the source of the link. From a8e26933c1cc1b386a2ef82f3a5ac9ceadba0891 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Sun, 14 Jan 2024 10:09:57 -0500 Subject: [PATCH 5/5] add test for unbroken link --- .../org/apache/commons/io/FileUtilsTest.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/test/java/org/apache/commons/io/FileUtilsTest.java b/src/test/java/org/apache/commons/io/FileUtilsTest.java index 46cfd0ce862..71811ac8ec4 100644 --- a/src/test/java/org/apache/commons/io/FileUtilsTest.java +++ b/src/test/java/org/apache/commons/io/FileUtilsTest.java @@ -757,6 +757,33 @@ public void testContentEqualsIgnoreEOL() throws Exception { assertTrue(FileUtils.contentEqualsIgnoreEOL(file1, file2, null)); } + @Test + 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"); + + // Make a symlink to the file + 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, including the broken link, to another directory + final File destination = new File(tempDirFile, "destination"); + FileUtils.copyDirectory(sourceDirectory, destination); + assertTrue(destination.exists()); + 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)); + } + /** * Tests IO-807. */