Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IO-845] repoint sibling links #572

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

[IO-845] repoint sibling links #572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f435c71
repoint sibling links
elharo Jan 30, 2024
ecf09ac
dismabiguate
elharo Jan 31, 2024
8ba5b59
Merge branch 'master' into IO-845
elharo Feb 1, 2024
b1b2790
add test for subdirectory link repointing
elharo Feb 2, 2024
d45a4a9
preserve symbolic link graph inside copied directory
elharo Feb 2, 2024
92fcd72
doCopyDirectoryFromRoot
elharo Feb 2, 2024
8bfcf3e
javadoc
elharo Feb 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 71 additions & 8 deletions src/main/java/org/apache/commons/io/FileUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,11 +507,12 @@ public static File[] convertFileCollectionToFileArray(final Collection<File> fil
* {@link File#setLastModified(long)}. If that fails, the method throws IOException.
* </p>
* <p>
* 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}.
* </p>
*
* @param srcDir an existing directory to copy, must not be {@code null}.
Expand All @@ -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.
* </p>
* <p>
* 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}.
* </p>
* <p>
* <strong>Note:</strong> 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.
Expand Down Expand Up @@ -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.
* </p>
* <p>
* 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}.
* </p>
* <p>
* <strong>Note:</strong> 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.
Expand Down Expand Up @@ -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.
* </p>
* <p>
* 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}.
* </p>
* <p>
* <strong>Note:</strong> 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
Expand Down Expand Up @@ -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.
* </p>
* <p>
* 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}.
* </p>
* <p>
* <strong>Note:</strong> 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
Expand Down Expand Up @@ -720,7 +753,7 @@ public static void copyDirectory(final File srcDir, final File destDir, final Fi
}
}
}
doCopyDirectory(srcDir, destDir, fileFilter, exclusionList, preserveFileDate, copyOptions);
doCopyDirectoryFromRoot(srcDir, destDir, fileFilter, exclusionList, preserveFileDate, copyOptions);
}

/**
Expand All @@ -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.
* </p>
* <p>
* 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}.
* </p>
* <p>
* <strong>Note:</strong> 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
Expand Down Expand Up @@ -1323,6 +1364,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<String> 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.
Expand All @@ -1332,12 +1380,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<String> 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");
Expand All @@ -1347,7 +1397,20 @@ 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());
if (directoryContains(srcRoot, linkTarget.toFile())) {
// make a new link that points to the copy of the target
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);
}

} else {
copyFile(srcFile, dstFile, preserveDirDate, copyOptions);
}
Expand Down
84 changes: 76 additions & 8 deletions src/test/java/org/apache/commons/io/FileUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,79 @@ 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);
}

@Test
public void testCopyDirectory_symLinkInternalFile_nested() throws Exception {
// Make a directory
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
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 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(copiedDirectory, "B"), "link_to_file");
assertTrue(Files.isSymbolicLink(copiedLink.toPath()));

final Path source = Files.readSymbolicLink(copiedLink.toPath());
assertNotEquals(content.toPath(), source);
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);
}

/**
* See what happens when copyDirectory copies a directory that is a symlink
* to another directory containing non-symlinked files.
Expand All @@ -790,10 +863,9 @@ public void testCopyDirectory_symLinkExternalFile() throws Exception {
* and should not be relied on.
*/
@Test
public void testCopyDir_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");

Expand All @@ -818,7 +890,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();
Expand Down Expand Up @@ -890,17 +962,13 @@ 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");

// 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 to another directory
final File destination = new File(tempDirFile, "destination");
Expand All @@ -909,8 +977,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
Expand Down