[
https://issues.apache.org/jira/browse/IO-751?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17420592#comment-17420592
]
Richard Cyganiak commented on IO-751:
-------------------------------------
[~ggregory] A fresh build from master does not work either.
I believe prefixing the tests with this line would make them be skipped when
run on non-POSIX OSes:
{code}
assumeTrue(FileSystems.getDefault().supportedFileAttributeViews().contains("posix"));
{code}
Looking into the code, I see at least two problems here:
# The intended semantics of {{PathUtils.setReadOnly(p, false)}} is not clear on
POSIX operating systems. They don't have a read-only flag that could be set to
false. When I look at calls to this method from within the Commons IO codebase,
the assumed semantics seems to be: “If the given file cannot be deleted due to
file permissions, then change the permissions so that the file can be deleted.”
But your proposed change above looks like you have a different semantics in
mind, because read permissions do not influence whether a file can be deleted.
# Regardless of the intended semantics, the current implementation is broken.
It ignores the {{readOnly}} flag on POSIX systems and just treats it as
{{true}}.
A possible way to address both problems:
# Make {{PathUtils.setReadOnly(...)}} have no effect at all on POSIX system.
After all, these systems don't have a read-only attribute that could be set or
unset.
# Update Javadocs around {{StandardDeleteOptions.OVERRIDE_READ_ONLY}} to make
clear the flag has no effect at all on systems that don't support a read-only
flag.
> When deleting symlinks, File/PathUtils.deleteDirectory() changes file
> permissions of the target
> -----------------------------------------------------------------------------------------------
>
> Key: IO-751
> URL: https://issues.apache.org/jira/browse/IO-751
> Project: Commons IO
> Issue Type: Bug
> Components: Utilities
> Affects Versions: 2.11.0
> Environment: macOS 11.5.2
> OpenJDK 11
> Reporter: Richard Cyganiak
> Priority: Major
> Attachments: DeleteDirectoryTest.java, commons-io.patch
>
>
> When {{FileUtils.deleteDirectory(...)}} and
> {{PathUtils.deleteDirectory(...)}} encounter a symlink while recursively
> deleting, the default behaviour is to delete the symlink, but leave the
> target of the symlink alone. This works for the most part: the symlink is
> correctly deleted, and the target is not deleted or recursed into.
> However, the methods _alter the file permissions of the target_:
> - {{FileUtils.deleteDirectory(file)}} _removes_ all write permissions from
> the target
> - {{PathUtils.deleteDirectory(path,
> StandardDeleteOption.OVERRIDE_READ_ONLY)}} _removes_ all write permissions,
> and _adds_ all execute permissions (even if the target is a file, not a
> directory)
> - {{PathUtils.deleteDirectory(path)}} works correctly and does not change the
> target's permissions
> A JUnit 4 test case that demonstrates the behaviour of all three methods is
> attached.
> The behaviour is unexpected (the Javadocs give no hint), inconvenient (it
> leaves the owner of the target without write permission) and potentially
> dangerous (it adds execute permissions for anyone).
> It appears the implementation assumes it can freely modify permissions
> because it is going to delete the file/directory anyway, and the case of
> symlinks was simply not considered. The handling of write permissions is
> particularly puzzling. I could understand why an implementation would _add_
> write permission, but why _remove_ it?
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
