You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2023/04/18 20:13:02 UTC
[commons-io] branch master updated: [IO-769] FileUtils copyDirectory() should NOT use COPY_ATTRIBUTES (#377)
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 22f65255 [IO-769] FileUtils copyDirectory() should NOT use COPY_ATTRIBUTES (#377)
22f65255 is described below
commit 22f6525588afe563a895d6c7a70e03ede86610d1
Author: Tres Finocchiaro <tr...@gmail.com>
AuthorDate: Tue Apr 18 16:12:56 2023 -0400
[IO-769] FileUtils copyDirectory() should NOT use COPY_ATTRIBUTES (#377)
* Initial commit: Don't copy all attributes, only timestamp information
* Fix docs, formatting, add unit tests
* Format: "catch(" -> "catch ("
---------
Co-authored-by: Gary Gregory <ga...@users.noreply.github.com>
---
src/main/java/org/apache/commons/io/FileUtils.java | 167 ++++++++++-----------
.../java/org/apache/commons/io/FileUtilsTest.java | 14 ++
2 files changed, 92 insertions(+), 89 deletions(-)
diff --git a/src/main/java/org/apache/commons/io/FileUtils.java b/src/main/java/org/apache/commons/io/FileUtils.java
index b5a80e5a..51746461 100644
--- a/src/main/java/org/apache/commons/io/FileUtils.java
+++ b/src/main/java/org/apache/commons/io/FileUtils.java
@@ -43,6 +43,8 @@ import java.nio.file.LinkOption;
import java.nio.file.NotDirectoryException;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
+import java.nio.file.attribute.BasicFileAttributeView;
+import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.FileTime;
import java.time.Duration;
import java.time.Instant;
@@ -54,7 +56,6 @@ import java.time.chrono.ChronoLocalDate;
import java.time.chrono.ChronoLocalDateTime;
import java.time.chrono.ChronoZonedDateTime;
import java.util.ArrayList;
-import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
@@ -199,23 +200,6 @@ public class FileUtils {
*/
public static final File[] EMPTY_FILE_ARRAY = {};
- /**
- * Copies the given array and adds StandardCopyOption.COPY_ATTRIBUTES.
- *
- * @param copyOptions sorted copy options.
- * @return a new array.
- */
- private static CopyOption[] addCopyAttributes(final CopyOption... copyOptions) {
- // Make a copy first since we don't want to sort the call site's version.
- final CopyOption[] actual = Arrays.copyOf(copyOptions, copyOptions.length + 1);
- Arrays.sort(actual, 0, copyOptions.length);
- if (Arrays.binarySearch(copyOptions, 0, copyOptions.length, StandardCopyOption.COPY_ATTRIBUTES) >= 0) {
- return copyOptions;
- }
- actual[actual.length - 1] = StandardCopyOption.COPY_ATTRIBUTES;
- return actual;
- }
-
/**
* Returns a human-readable version of the file size, where the input represents a specific number of bytes.
* <p>
@@ -489,9 +473,10 @@ public class FileUtils {
* method merges the source with the destination, with the source taking precedence.
* </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 methods throws IOException.
+ * <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 will fallback to
+ * {@link File#setLastModified(long)} and if that fails, the methods throws IOException.
* </p>
*
* @param srcDir an existing directory to copy, must not be {@code null}.
@@ -594,9 +579,10 @@ public class FileUtils {
* method merges the source with the destination, with the source taking precedence.
* </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 methods throws IOException.
+ * <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 will fallback to
+ * {@link File#setLastModified(long)} and if that fails, the methods throws IOException.
* </p>
* <b>Example: Copy directories only</b>
*
@@ -643,9 +629,10 @@ public class FileUtils {
* method merges the source with the destination, with the source taking precedence.
* </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 methods throws IOException.
+ * <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 will fallback to
+ * {@link File#setLastModified(long)} and if that fails, the methods throws IOException.
* </p>
* <b>Example: Copy directories only</b>
*
@@ -698,7 +685,7 @@ public class FileUtils {
}
}
}
- doCopyDirectory(srcDir, destDir, fileFilter, exclusionList, preserveFileDate, preserveFileDate ? addCopyAttributes(copyOptions) : copyOptions);
+ doCopyDirectory(srcDir, destDir, fileFilter, exclusionList, preserveFileDate, copyOptions);
}
/**
@@ -712,9 +699,10 @@ public class FileUtils {
* method merges the source with the destination, with the source taking precedence.
* </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 methods throws IOException.
+ * <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 will fallback to
+ * {@link File#setLastModified(long)} and if that fails, the methods throws IOException.
* </p>
*
* @param sourceDir an existing directory to copy, must not be {@code null}.
@@ -740,8 +728,9 @@ public class FileUtils {
* </p>
* <p>
* <strong>Note:</strong> This method tries to preserve the file's last modified date/times using
- * {@link StandardCopyOption#COPY_ATTRIBUTES}, however it is not guaranteed that the operation will succeed. If the
- * modification operation fails, the methods throws IOException.
+ * {@link BasicFileAttributeView#setTimes(FileTime, FileTime, FileTime)}, however it is not guaranteed that the
+ * operation will succeed. If the modification operation fails it will fallback to
+ * {@link File#setLastModified(long)} and if that fails, the methods throws IOException.
* </p>
*
* @param srcFile an existing file to copy, must not be {@code null}.
@@ -754,7 +743,7 @@ public class FileUtils {
* @see #copyFile(File, File, boolean)
*/
public static void copyFile(final File srcFile, final File destFile) throws IOException {
- copyFile(srcFile, destFile, StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING);
+ copyFile(srcFile, destFile, StandardCopyOption.REPLACE_EXISTING);
}
/**
@@ -766,8 +755,9 @@ public class FileUtils {
* </p>
* <p>
* <strong>Note:</strong> Setting {@code preserveFileDate} to {@code true} tries to preserve the file's last
- * modified date/times using {@link StandardCopyOption#COPY_ATTRIBUTES}, however it is not guaranteed that the operation
- * will succeed. If the modification operation fails, the methods throws IOException.
+ * 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 will fallback to
+ * {@link File#setLastModified(long)} and if that fails, the methods throws IOException.
* </p>
*
* @param srcFile an existing file to copy, must not be {@code null}.
@@ -781,9 +771,7 @@ public class FileUtils {
*/
public static void copyFile(final File srcFile, final File destFile, final boolean preserveFileDate) throws IOException {
// @formatter:off
- copyFile(srcFile, destFile, preserveFileDate
- ? new CopyOption[] {StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING}
- : new CopyOption[] {StandardCopyOption.REPLACE_EXISTING});
+ copyFile(srcFile, destFile, preserveFileDate, new CopyOption[] {StandardCopyOption.REPLACE_EXISTING});
// @formatter:on
}
@@ -796,8 +784,9 @@ public class FileUtils {
* </p>
* <p>
* <strong>Note:</strong> Setting {@code preserveFileDate} to {@code true} tries to preserve the file's last
- * modified date/times using {@link StandardCopyOption#COPY_ATTRIBUTES}, however it is not guaranteed that the operation
- * will succeed. If the modification operation fails, the methods throws IOException.
+ * 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 will fallback to
+ * {@link File#setLastModified(long)} and if that fails, the methods throws IOException.
* </p>
*
* @param srcFile an existing file to copy, must not be {@code null}.
@@ -813,7 +802,20 @@ public class FileUtils {
* @since 2.8.0
*/
public static void copyFile(final File srcFile, final File destFile, final boolean preserveFileDate, final CopyOption... copyOptions) throws IOException {
- copyFile(srcFile, destFile, preserveFileDate ? addCopyAttributes(copyOptions) : copyOptions);
+ requireFileCopy(srcFile, destFile);
+ requireFile(srcFile, "srcFile");
+ requireCanonicalPathsNotEquals(srcFile, destFile);
+ createParentDirectories(destFile);
+ requireFileIfExists(destFile, "destFile");
+ if (destFile.exists()) {
+ requireCanWrite(destFile, "destFile");
+ }
+ Files.copy(srcFile.toPath(), destFile.toPath(), copyOptions);
+
+ // On Windows, the last modified time is copied by default.
+ if(preserveFileDate) {
+ setTimes(srcFile, destFile);
+ }
}
/**
@@ -835,16 +837,7 @@ public class FileUtils {
* @since 2.9.0
*/
public static void copyFile(final File srcFile, final File destFile, final CopyOption... copyOptions) throws IOException {
- requireFileCopy(srcFile, destFile);
- requireFile(srcFile, "srcFile");
- requireCanonicalPathsNotEquals(srcFile, destFile);
- createParentDirectories(destFile);
- requireFileIfExists(destFile, "destFile");
- if (destFile.exists()) {
- requireCanWrite(destFile, "destFile");
- }
- // On Windows, the last modified time is copied by default.
- Files.copy(srcFile.toPath(), destFile.toPath(), copyOptions);
+ copyFile(srcFile, destFile, true, copyOptions);
}
/**
@@ -876,8 +869,9 @@ public class FileUtils {
* </p>
* <p>
* <strong>Note:</strong> This method tries to preserve the file's last modified date/times using
- * {@link StandardCopyOption#COPY_ATTRIBUTES}, however it is not guaranteed that the operation will succeed. If the
- * modification operation fails, the methods throws IOException.
+ * {@link BasicFileAttributeView#setTimes(FileTime, FileTime, FileTime)}, however it is not guaranteed that the
+ * operation will succeed. If the modification operation fails it will fallback to
+ * {@link File#setLastModified(long)} and if that fails, the methods throws IOException.
* </p>
*
* @param srcFile an existing file to copy, must not be {@code null}.
@@ -900,8 +894,9 @@ public class FileUtils {
* </p>
* <p>
* <strong>Note:</strong> Setting {@code preserveFileDate} to {@code true} tries to preserve the file's last
- * modified date/times using {@link StandardCopyOption#COPY_ATTRIBUTES}, however it is not guaranteed that the operation
- * will succeed. If the modification operation fails, the methods throws IOException.
+ * 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 will fallback to
+ * {@link File#setLastModified(long)} and if that fails, the methods throws IOException.
* </p>
*
* @param sourceFile an existing file to copy, must not be {@code null}.
@@ -957,10 +952,10 @@ public class FileUtils {
* merges the source with the destination, with the source taking precedence.
* </p>
* <p>
- * <strong>Note:</strong> This method tries to preserve the files' last modified date/times using
- * {@link StandardCopyOption#COPY_ATTRIBUTES} or {@link File#setLastModified(long)} depending on the input, however it
- * is not guaranteed that those operations will succeed. If the modification operation fails, the methods throws
- * IOException.
+ * <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 will fallback to
+ * {@link File#setLastModified(long)} and if that fails, the methods throws IOException.
* </p>
*
* @param sourceFile an existing file or directory to copy, must not be {@code null}.
@@ -993,10 +988,10 @@ public class FileUtils {
* If the destination file exists, then this method will overwrite it.
* </p>
* <p>
- * <strong>Note:</strong> This method tries to preserve the file's last
- * modified date/times using {@link File#setLastModified(long)}, however
- * it is not guaranteed that the operation will succeed.
- * If the modification operation fails, the methods throws IOException.
+ * <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 will fallback to
+ * {@link File#setLastModified(long)} and if that fails, the methods throws IOException.
* </p>
*
* @param sourceIterable existing files to copy, must not be {@code null}.
@@ -1310,13 +1305,13 @@ public class FileUtils {
if (srcFile.isDirectory()) {
doCopyDirectory(srcFile, dstFile, fileFilter, exclusionList, preserveDirDate, copyOptions);
} else {
- copyFile(srcFile, dstFile, copyOptions);
+ copyFile(srcFile, dstFile, preserveDirDate, copyOptions);
}
}
}
// Do this last, as the above has probably affected directory metadata
if (preserveDirDate) {
- setLastModified(srcDir, destDir);
+ setTimes(srcDir, destDir);
}
}
@@ -2386,7 +2381,8 @@ public class FileUtils {
requireAbsent(destFile, "destFile");
final boolean rename = srcFile.renameTo(destFile);
if (!rename) {
- copyFile(srcFile, destFile, copyOptions);
+ // Don't interfere with file date on move, handled by StandardCopyOption.COPY_ATTRIBUTES
+ copyFile(srcFile, destFile, false, copyOptions);
if (!srcFile.delete()) {
FileUtils.deleteQuietly(destFile);
throw new IOException("Failed to delete original file '" + srcFile + "' after copy to '" + destFile + "'");
@@ -2836,7 +2832,7 @@ public class FileUtils {
}
/**
- * Sets the given {@code targetFile}'s last modified date to the value from {@code sourceFile}.
+ * Set file lastModifiedTime, lastAccessTime and creationTime to match source file
*
* @param sourceFile The source file to query.
* @param targetFile The target file or directory to set.
@@ -2844,29 +2840,22 @@ public class FileUtils {
* @throws NullPointerException if targetFile is {@code null}.
* @throws IOException if setting the last-modified time failed.
*/
- private static void setLastModified(final File sourceFile, final File targetFile) throws IOException {
+ private static void setTimes(final File sourceFile, final File targetFile) throws IOException {
Objects.requireNonNull(sourceFile, "sourceFile");
Objects.requireNonNull(targetFile, "targetFile");
- if (targetFile.isFile()) {
- PathUtils.setLastModifiedTime(targetFile.toPath(), sourceFile.toPath());
- } else {
- setLastModified(targetFile, lastModified(sourceFile));
- }
- }
-
- /**
- * Sets the given {@code targetFile}'s last modified date to the given value.
- *
- * @param file The source file to query.
- * @param timeMillis The new last-modified time, measured in milliseconds since the epoch 01-01-1970 GMT.
- * @throws NullPointerException if file is {@code null}.
- * @throws IOException if setting the last-modified time failed.
- */
- private static void setLastModified(final File file, final long timeMillis) throws IOException {
- Objects.requireNonNull(file, "file");
- if (!file.setLastModified(timeMillis)) {
- throw new IOException(String.format("Failed setLastModified(%s) on '%s'", timeMillis, file));
- }
+ try {
+ // Set creation, modified, last accessed to match source file
+ final BasicFileAttributes srcAttr = Files.readAttributes(sourceFile.toPath(), BasicFileAttributes.class);
+ final BasicFileAttributeView destAttrView = Files.getFileAttributeView(targetFile.toPath(), BasicFileAttributeView.class);
+ // null guards are not needed; BasicFileAttributes.setTimes(...) is null safe
+ destAttrView.setTimes(srcAttr.lastModifiedTime(), srcAttr.lastAccessTime(), srcAttr.creationTime());
+ } catch (IOException unused) {
+ // Fallback: Only set modified time to match source file
+ targetFile.setLastModified(sourceFile.lastModified());
+ }
+
+ // TODO: (Help!) Determine historically why setLastModified(File, File) needed PathUtils.setLastModifiedTime() if
+ // sourceFile.isFile() was true, but needed setLastModifiedTime(File, long) if sourceFile.isFile() was false
}
/**
diff --git a/src/test/java/org/apache/commons/io/FileUtilsTest.java b/src/test/java/org/apache/commons/io/FileUtilsTest.java
index 1c5474f6..dae19644 100644
--- a/src/test/java/org/apache/commons/io/FileUtilsTest.java
+++ b/src/test/java/org/apache/commons/io/FileUtilsTest.java
@@ -44,7 +44,9 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
+import java.nio.file.attribute.AclFileAttributeView;
import java.nio.file.attribute.FileTime;
+import java.nio.file.attribute.PosixFilePermission;
import java.time.Instant;
import java.time.LocalDate;
import java.time.LocalDateTime;
@@ -855,6 +857,18 @@ public class FileUtilsTest extends AbstractTempDirTest {
if (!SystemUtils.IS_OS_WINDOWS) {
assertNotEquals(DATE3, getLastModifiedMillis(targetFile));
}
+
+ // Test permission of copied file match destination folder
+ if (!SystemUtils.IS_OS_WINDOWS) {
+ final Set<PosixFilePermission> parentPerms = Files.getPosixFilePermissions(target.getParentFile().toPath());
+ final Set<PosixFilePermission> targetPerms = Files.getPosixFilePermissions(target.toPath());
+ assertEquals(parentPerms, targetPerms);
+ } else {
+ final AclFileAttributeView parentView = Files.getFileAttributeView(target.getParentFile().toPath(), AclFileAttributeView.class);
+ final AclFileAttributeView targetView = Files.getFileAttributeView(target.toPath(), AclFileAttributeView.class);
+ assertEquals(parentView.getAcl(), targetView.getAcl());
+ }
+
FileUtils.deleteDirectory(target);
// Test with preserveFileDate enabled