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 <[email protected]>
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);
}