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);
         }

Reply via email to