You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@commons.apache.org by "efegokdemir (via GitHub)" <gi...@apache.org> on 2026/09/23 11:56:34 UTC

[PR] IO-879: Restore read-only state after failed deletion [commons-io]

efegokdemir opened a new pull request, #881:
URL: https://github.com/apache/commons-io/pull/881

## Summary

Fixes IO-879 by restoring a file's read-only state when an override-enabled deletion attempt fails.

## Changes

- Restore the read-only attribute in `PathUtils.deleteFile` when the file remains after a failed deletion.
- Apply the same fail-safe behavior to `DeletingPathVisitor` and `CleaningPathVisitor`.
- Add a Windows-focused regression test covering a read-only file in a read-only parent directory.

## Testing

- `mvn -pl . -Dtest=PathUtilsDeleteFileTest -Dsurefire.failIfNoSpecifiedTests=false test -q` — passed locally: 8 tests, 1 skipped (Windows-only regression skipped on macOS).
- `mvn -DskipTests compile -q` — passed.
- `git diff --check` — passed.
- Full `mvn test` — not completed: the suite continued running without producing new reports for several minutes on macOS/JDK 27, so the local process was stopped rather than reported as passing.
- The Windows-specific regression requires Windows CI and was not runnable on this Mac.

## Notes

The change preserves the original deletion exception and only restores the attribute when the path still exists. AI assistance was used during investigation and implementation; the submitter reviewed the changed lines and test results.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [IO-879] Restore read-only state after failed deletion [commons-io]

Posted by "garydgregory (via GitHub) via issues" <is...@commons.apache.org>.

garydgregory commented on PR #881:
URL: https://github.com/apache/commons-io/pull/881#issuecomment-6729645446

@efegokdemir
Thank you for working on IO-879. Restoring the read-only attribute after a failed deletion is the right direction. However, I’m closing this PR in favor of PR #927 that addresses several correctness and testing issues:

1) The Windows tests do not reliably cause deletion to fail. They mark the parent directory DOS read-only, but Windows [does not honor that attribute on directories](https://learn.microsoft.com/en-us/windows/win32/fileio/file-attribute-constants). That setup does not establish the failure needed to test restoration.

2) Restoration can change POSIX file permissions. The new `isReadOnly()` treats missing `OWNER_WRITE` as read-only, then calls `setReadOnly(file, true)`. That method also adds `OWNER_READ`, potentially granting a permission the file did not originally have.

3) Restoring parent permissions after each visitor deletion can interrupt traversal. A parent can become non-writable before a later child directory is deleted, causing `AccessDeniedException`. I reproduced this behavior and added a regression test.

4) Attribute-read failures can leave the original problem unresolved. The attribute-reading helpers swallow `IOException`, so deletion may clear the DOS flag without having saved its original value.

PR #927 reuses DOS restoration logic across the deletion paths, preserves existing POSIX traversal behavior, and avoids clearing DOS state when its original value cannot be read. The tests in PR #927 verify that the flag is cleared during the failing deletion attempt and restored afterward.

Thank you again for the contribution and for helping move this issue forward.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] IO-879: Restore read-only state after failed deletion [commons-io]

Posted by "dajiaohuang (via GitHub)" <gi...@apache.org>.

dajiaohuang commented on PR #881:
URL: https://github.com/apache/commons-io/pull/881#issuecomment-6393023676

   One failure case appears to change the file state: `readOnlyCleared` records that the override branch ran, not that the file was read-only beforehand. If a writable file is passed with `OVERRIDE_READ_ONLY` and deletion fails, the `finally` block calls `setReadOnly(file, true)`, making the previously writable file read-only. Could the original read-only state (or exact attributes) be captured and restored, with a regression test for a writable file whose deletion fails?
   
   AI assistance used.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] IO-879: Restore read-only state after failed deletion [commons-io]

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.

garydgregory commented on PR #881:
URL: https://github.com/apache/commons-io/pull/881#issuecomment-6401850581

   @efegokdemir Please rebase on git master.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [IO-879] Restore read-only state after failed deletion [commons-io]

Posted by "garydgregory (via GitHub) via issues" <is...@commons.apache.org>.

garydgregory closed pull request #881: [IO-879] Restore read-only state after failed deletion
URL: https://github.com/apache/commons-io/pull/881


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org