CopyDirectoryVisitor creates incorrect file names when copying between

different file systems that use different file system separators ("/"
versus "\")

- Fixes PathUtils.copyDirectory(Path, Path, CopyOption...)

Author: garydgregory

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>

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

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

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