[IO-751] When deleting symlinks, File/PathUtils.deleteDirectory()
changes file permissions of the target.
POSIX: Deleting a file requires write and execute on the parent. Listing
a file also requires reading.
Looking for a fix to the Ubuntu failure in the GitHub builds.
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 01877d0..13d304d 100644
--- a/src/main/java/org/apache/commons/io/file/PathUtils.java
+++ b/src/main/java/org/apache/commons/io/file/PathUtils.java
@@ -53,7 +53,6 @@
import java.time.Duration;
import java.time.Instant;
import java.time.chrono.ChronoZonedDateTime;
-import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@@ -68,7 +67,6 @@
import org.apache.commons.io.Charsets;
import org.apache.commons.io.FileUtils;
-import org.apache.commons.io.IOExceptionList;
import org.apache.commons.io.IOUtils;
import org.apache.commons.io.UncheckedIOExceptions;
import org.apache.commons.io.file.Counters.PathCounters;
@@ -512,7 +510,7 @@ public static PathCounters deleteFile(final Path file, final LinkOption[] linkOp
pathCounts.getByteCounter().add(size);
return pathCounts;
}
- } catch (AccessDeniedException e) {
+ } catch (final AccessDeniedException e) {
// Ignore and try again below.
}
final Path parent = file.getParent();
@@ -538,7 +536,7 @@ public static PathCounters deleteFile(final Path file, final LinkOption[] linkOp
}
private static <R> R withPosixFileAttributes(final Path path, final LinkOption[] linkOptions, final boolean overrideReadOnly,
- IOFunction<PosixFileAttributes, R> function) throws IOException {
+ final IOFunction<PosixFileAttributes, R> function) throws IOException {
final PosixFileAttributes posixFileAttributes = overrideReadOnly ? readPosixFileAttributes(path, linkOptions) : null;
try {
return function.apply(posixFileAttributes);
@@ -1305,67 +1303,53 @@ public static void setLastModifiedTime(final Path sourceFile, final Path targetF
* @since 2.8.0
*/
public static Path setReadOnly(final Path path, final boolean readOnly, final LinkOption... linkOptions) throws IOException {
- final List<Exception> causeList = new ArrayList<>(3);
try {
// Windows is simplest
if (setDosReadOnly(path, readOnly, linkOptions)) {
return path;
}
} catch (final IOException e) {
- // Remember and retry with POSIX
- causeList.add(e);
+ // Retry with POSIX below.
}
// POSIX
if (readOnly) {
// RO
// File, then parent dir (if any).
- final boolean set = setPosixReadOnlyFile(path, readOnly, linkOptions);
+ setPosixReadOnlyFile(path, readOnly, linkOptions);
setPosixDeletePermissions(path.getParent(), false, linkOptions);
- if (set) {
- return path;
- }
} else {
// RE
// Parent dir (if any), then file.
- if (setPosixDeletePermissions(path.getParent(), true, linkOptions)) {
- return path;
- }
+ setPosixDeletePermissions(path.getParent(), true, linkOptions);
}
- IOExceptionList.checkEmpty(causeList, path);
- throw new IOException(String.format("No DosFileAttributeView or PosixFileAttributeView for '%s' (linkOptions=%s)", path, Arrays.toString(linkOptions)));
+ return path;
}
- private static boolean setPosixReadOnlyFile(final Path path, final boolean readOnly, final LinkOption... linkOptions) throws IOException {
- final PosixFileAttributeView posixFileAttributeView = getPosixFileAttributeView(path, linkOptions);
- if (posixFileAttributeView != null) {
- // Not Windows 10
- final PosixFileAttributes readAttributes = posixFileAttributeView.readAttributes();
- final Set<PosixFilePermission> permissions = readAttributes.permissions();
- // @formatter:off
- final List<PosixFilePermission> readPermissions = Arrays.asList(
- PosixFilePermission.OWNER_READ
- //PosixFilePermission.GROUP_READ,
- //PosixFilePermission.OTHERS_READ
- );
- final List<PosixFilePermission> writePermissions = Arrays.asList(
- PosixFilePermission.OWNER_WRITE
- //PosixFilePermission.GROUP_WRITE,
- //PosixFilePermission.OTHERS_WRITE
- );
- // @formatter:on
- if (readOnly) {
- // RO: We can read, we cannot write.
- permissions.addAll(readPermissions);
- permissions.removeAll(writePermissions);
- } else {
- // Not RO: We can read, we can write.
- permissions.addAll(readPermissions);
- permissions.addAll(writePermissions);
- }
- Files.setPosixFilePermissions(path, permissions);
- return true;
+ private static void setPosixReadOnlyFile(final Path path, final boolean readOnly, final LinkOption... linkOptions) throws IOException {
+ // Not Windows 10
+ final Set<PosixFilePermission> permissions = Files.getPosixFilePermissions(path, linkOptions);
+ // @formatter:off
+ final List<PosixFilePermission> readPermissions = Arrays.asList(
+ PosixFilePermission.OWNER_READ
+ //PosixFilePermission.GROUP_READ,
+ //PosixFilePermission.OTHERS_READ
+ );
+ final List<PosixFilePermission> writePermissions = Arrays.asList(
+ PosixFilePermission.OWNER_WRITE
+ //PosixFilePermission.GROUP_WRITE,
+ //PosixFilePermission.OTHERS_WRITE
+ );
+ // @formatter:on
+ if (readOnly) {
+ // RO: We can read, we cannot write.
+ permissions.addAll(readPermissions);
+ permissions.removeAll(writePermissions);
+ } else {
+ // Not RO: We can read, we can write.
+ permissions.addAll(readPermissions);
+ permissions.addAll(writePermissions);
}
- return false;
+ Files.setPosixFilePermissions(path, permissions);
}
private static boolean setDosReadOnly(final Path path, final boolean readOnly, final LinkOption... linkOptions) throws IOException {
@@ -1381,26 +1365,24 @@ private static boolean setDosReadOnly(final Path path, final boolean readOnly, f
* To delete a file in POSIX, you need Write and Execute permissions on its parent directory.
*
* @param parent The parent path for a file element to delete which needs RW permissions.
- * @param enableDelete true to set permissions to delete.
+ * @param enableDeleteChildren true to set permissions to delete.
* @param linkOptions options indicating how handle symbolic links.
* @return true if the operation was attempted and succeeded, false if parent is null.
* @throws IOException if an I/O error occurs.
*/
- private static boolean setPosixDeletePermissions(final Path parent, final boolean enableDelete, final LinkOption... linkOptions) throws IOException {
- if (parent != null) {
- // To delete a file in POSIX, you need write and execute permissions on its parent directory.
- // @formatter:off
- return setPosixPermissions(parent, enableDelete, Arrays.asList(
- PosixFilePermission.OWNER_WRITE,
- //PosixFilePermission.GROUP_WRITE,
- //PosixFilePermission.OTHERS_WRITE,
- PosixFilePermission.OWNER_EXECUTE
- //PosixFilePermission.GROUP_EXECUTE,
- //PosixFilePermission.OTHERS_EXECUTE
- ), linkOptions);
- // @formatter:on
- }
- return false;
+ private static boolean setPosixDeletePermissions(final Path parent, final boolean enableDeleteChildren, final LinkOption... linkOptions)
+ throws IOException {
+ // To delete a file in POSIX, you need write and execute permissions on its parent directory.
+ // @formatter:off
+ return setPosixPermissions(parent, enableDeleteChildren, Arrays.asList(
+ PosixFilePermission.OWNER_WRITE,
+ //PosixFilePermission.GROUP_WRITE,
+ //PosixFilePermission.OTHERS_WRITE,
+ PosixFilePermission.OWNER_EXECUTE
+ //PosixFilePermission.GROUP_EXECUTE,
+ //PosixFilePermission.OTHERS_EXECUTE
+ ), linkOptions);
+ // @formatter:on
}
/**
@@ -1416,17 +1398,14 @@ private static boolean setPosixDeletePermissions(final Path parent, final boolea
private static boolean setPosixPermissions(final Path path, final boolean addPermissions, final List<PosixFilePermission> updatePermissions,
final LinkOption... linkOptions) throws IOException {
if (path != null) {
- final PosixFileAttributeView posixFileAttributeView = getPosixFileAttributeView(path, linkOptions);
- if (posixFileAttributeView != null) {
- final Set<PosixFilePermission> permissions = posixFileAttributeView.readAttributes().permissions();
- if (addPermissions) {
- permissions.addAll(updatePermissions);
- } else {
- permissions.removeAll(updatePermissions);
- }
- Files.setPosixFilePermissions(path, permissions);
- return true;
+ final Set<PosixFilePermission> permissions = Files.getPosixFilePermissions(path, linkOptions);
+ if (addPermissions) {
+ permissions.addAll(updatePermissions);
+ } else {
+ permissions.removeAll(updatePermissions);
}
+ Files.setPosixFilePermissions(path, permissions);
+ return true;
}
return false;
}
diff --git a/src/test/java/org/apache/commons/io/DeleteDirectoryTest.java b/src/test/java/org/apache/commons/io/DeleteDirectoryTest.java
index 1bc2420..866195c 100644
--- a/src/test/java/org/apache/commons/io/DeleteDirectoryTest.java
+++ b/src/test/java/org/apache/commons/io/DeleteDirectoryTest.java
@@ -100,8 +100,10 @@ public void testDeleteFileCheckParentAccess() throws IOException {
// A file is RO in POSIX if the parent is not W and not E.
PathUtils.setReadOnly(file, true);
final Set<PosixFilePermission> permissions = Files.getPosixFilePermissions(testDir);
- assertFalse(Files.isWritable(testDir), () -> String.format("Directory '%s' should NOT be Writable, permissions are %s ", testDir, permissions));
- assertFalse(Files.isExecutable(testDir), () -> String.format("Directory '%s' should NOT be Executable, permissions are %s ", testDir, permissions));
+ assertFalse(Files.isWritable(testDir),
+ () -> String.format("Parent directory '%s' of '%s' should NOT be Writable, permissions are %s ", testDir, file, permissions));
+ assertFalse(Files.isExecutable(testDir),
+ () -> String.format("Parent directory '%s' of '%s' should NOT be Executable, permissions are %s ", testDir, file, permissions));
assertThrows(IOException.class, () -> PathUtils.delete(file));
// Nothing happened, we're not even allowed to test attributes, so the file seems deleted, but it is not.
