This is an automated email from the ASF dual-hosted git repository. ggregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-io.git
The following commit(s) were added to refs/heads/master by this push: new 94da6dec7 CopyDirectoryVisitor creates incorrect file names when copying between different file systems that use different file system separators ("/" versus "\") 94da6dec7 is described below commit 94da6dec7476abf7bc6929537637e2f4ed640cc5 Author: Gary Gregory <garydgreg...@gmail.com> AuthorDate: Sat Apr 5 15:06:11 2025 -0400 CopyDirectoryVisitor creates incorrect file names when copying between different file systems that use different file system separators ("/" versus "\") - Fixes PathUtils.copyDirectory(Path, Path, CopyOption...) --- src/changes/changes.xml | 1 + .../commons/io/file/CopyDirectoryVisitor.java | 25 ++++++++++++++++----- .../java/org/apache/commons/io/file/PathUtils.java | 4 ++-- .../io/file/PathUtilsContentEqualsTest.java | 26 +++++++++++++++++----- 4 files changed, 42 insertions(+), 14 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 0784b5533..1684f86a7 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -73,6 +73,7 @@ The <action> type attribute can be add,update,fix,remove. <action dev="ggregory" type="fix" due-to="Gary Gregory">Fix Javadoc for ChunkedOutputStream.Builder.</action> <action dev="ggregory" type="fix" due-to="Gary Gregory">General Javadoc improvements.</action> <action dev="ggregory" type="fix" due-to="Gary Gregory">Calling QueueInputStream.QueueInputStream(null) maps to the same kind of default blocking queue as QueueInputStream.Builder.setBlockingQueue(null).</action> + <action dev="ggregory" type="fix" due-to="Gary Gregory">CopyDirectoryVisitor creates incorrect file names when copying between different file systems that use different file system separators ("/" versus "\"); fixes PathUtils.copyDirectory(Path, Path, CopyOption...).</action> <!-- ADD --> <action dev="ggregory" type="add" issue="IO-860" due-to="Nico Strecker, Gary Gregory">Add ThrottledInputStream.Builder.setMaxBytes(long, ChronoUnit).</action> <action dev="ggregory" type="add" due-to="Gary Gregory">Add IOIterable.</action> diff --git a/src/main/java/org/apache/commons/io/file/CopyDirectoryVisitor.java b/src/main/java/org/apache/commons/io/file/CopyDirectoryVisitor.java index 41c1d6bba..0fa09f200 100644 --- a/src/main/java/org/apache/commons/io/file/CopyDirectoryVisitor.java +++ b/src/main/java/org/apache/commons/io/file/CopyDirectoryVisitor.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.nio.file.CopyOption; +import java.nio.file.FileSystem; import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.Path; @@ -141,8 +142,7 @@ public int hashCode() { final int prime = 31; int result = super.hashCode(); result = prime * result + Arrays.hashCode(copyOptions); - result = prime * result + Objects.hash(sourceDirectory, targetDirectory); - return result; + return prime * result + Objects.hash(sourceDirectory, targetDirectory); } @Override @@ -155,17 +155,30 @@ public FileVisitResult preVisitDirectory(final Path directory, final BasicFileAt return super.preVisitDirectory(directory, attributes); } + private Path resolve(final Path otherPath) { + final FileSystem fileSystemTarget = targetDirectory.getFileSystem(); + final FileSystem fileSystemSource = sourceDirectory.getFileSystem(); + if (fileSystemTarget == fileSystemSource) { + return targetDirectory.resolve(otherPath); + } + final String separatorSource = fileSystemSource.getSeparator(); + final String separatorTarget = fileSystemTarget.getSeparator(); + final String otherString = otherPath.toString(); + return targetDirectory.resolve(Objects.equals(separatorSource, separatorTarget) ? otherString : otherString.replace(separatorSource, separatorTarget)); + } + /** * Relativizes against {@code sourceDirectory}, then resolves against {@code targetDirectory}. - * - * We have to call {@link Path#toString()} relative value because we cannot use paths belonging to different - * FileSystems in the Path methods, usually this leads to {@link ProviderMismatchException}. + * <p> + * We call {@link Path#toString()} on the relativized value because we cannot use paths from different FileSystems which throws + * {@link ProviderMismatchException}. Special care is taken to handle differences in file system separators. + * </p> * * @param directory the directory to relativize. * @return a new path, relativized against sourceDirectory, then resolved against targetDirectory. */ private Path resolveRelativeAsString(final Path directory) { - return targetDirectory.resolve(sourceDirectory.relativize(directory).toString()); + return resolve(sourceDirectory.relativize(directory)); } @Override diff --git a/src/main/java/org/apache/commons/io/file/PathUtils.java b/src/main/java/org/apache/commons/io/file/PathUtils.java index e67ed7f4c..3ad5db134 100644 --- a/src/main/java/org/apache/commons/io/file/PathUtils.java +++ b/src/main/java/org/apache/commons/io/file/PathUtils.java @@ -129,9 +129,9 @@ private static boolean equalsIgnoreFileSystem(final Path path1, final Path path2 final String separator2 = fileSystem2.getSeparator(); final String string1 = path1.toString(); final String string2 = path2.toString(); - if (separator1.equals(separator2)) { + if (Objects.equals(separator1, separator2)) { // Separators are the same, so we can use toString comparison - return string1.equals(string2); + return Objects.equals(string1, string2); } // Compare paths from different file systems component by component. return extractKey(separator1, string1).equals(extractKey(separator2, string2)); diff --git a/src/test/java/org/apache/commons/io/file/PathUtilsContentEqualsTest.java b/src/test/java/org/apache/commons/io/file/PathUtilsContentEqualsTest.java index 4bae7bfd4..f4a0e181d 100644 --- a/src/test/java/org/apache/commons/io/file/PathUtilsContentEqualsTest.java +++ b/src/test/java/org/apache/commons/io/file/PathUtilsContentEqualsTest.java @@ -44,7 +44,7 @@ */ public class PathUtilsContentEqualsTest { - static Configuration[] testContentEqualsFileSystemsMemVsZip() { + static Configuration[] testConfigurations() { // @formatter:off return new Configuration[] { Configuration.osX().toBuilder().setWorkingDirectory("/").build(), @@ -104,13 +104,27 @@ private String getName() { } @ParameterizedTest - @MethodSource + @MethodSource("testConfigurations") + public void testContentEqualsFileSystemsMemVsMem(final Configuration configuration) throws Exception { + final Path refDir = Paths.get("src/test/resources/dir-equals-tests"); + try (FileSystem fileSystem1 = Jimfs.newFileSystem(configuration); + FileSystem fileSystem2 = Jimfs.newFileSystem(configuration)) { + final Path fsDir1 = fileSystem1.getPath(refDir.getFileName().toString()); + final Path fsDir2 = fileSystem2.getPath(refDir.getFileName().toString()); + assertTrue(PathUtils.copyDirectory(refDir, fsDir1).getByteCounter().get() > 0); + assertTrue(PathUtils.copyDirectory(refDir, fsDir2).getByteCounter().get() > 0); + assertContentEquals(fileSystem1, fileSystem2); + } + } + + @ParameterizedTest + @MethodSource("testConfigurations") public void testContentEqualsFileSystemsMemVsZip(final Configuration configuration) throws Exception { - final Path dir1 = Paths.get("src/test/resources/dir-equals-tests"); + final Path refDir = Paths.get("src/test/resources/dir-equals-tests"); try (FileSystem fileSystem1 = Jimfs.newFileSystem(configuration); - FileSystem fileSystem2 = FileSystems.newFileSystem(dir1.resolveSibling(dir1.getFileName() + ".zip"), null)) { - final Path dir2 = fileSystem1.getPath(dir1.getFileName().toString()); - final PathCounters copyDirectory = PathUtils.copyDirectory(dir1, dir2); + FileSystem fileSystem2 = FileSystems.newFileSystem(refDir.resolveSibling(refDir.getFileName() + ".zip"), null)) { + final Path fsDir1 = fileSystem1.getPath(refDir.getFileName().toString()); + final PathCounters copyDirectory = PathUtils.copyDirectory(refDir, fsDir1); assertTrue(copyDirectory.getByteCounter().get() > 0); assertContentEquals(fileSystem1, fileSystem2); }