Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 27, 2025

Fix UnArchiver read-only file extraction issue

This PR addresses the issue where UnArchiver fails to extract files when the target file exists and is read-only.

Checklist:

  • Understand the issue and codebase
  • Run existing tests to verify current state
  • Implement fix: Delete existing file before extracting (in extractFile method)
  • Add test to verify read-only files can be overwritten during extraction
  • Run tests to ensure no regressions (341 tests pass)
  • Address code review feedback: Keep REPLACE_EXISTING as fallback
  • Fix Windows compatibility: Make file writable before delete
  • All tests pass (32 UnArchiver tests)

Summary:

The issue occurs in AbstractUnArchiver.extractFile() where Files.copy with REPLACE_EXISTING may fail on read-only files on some platforms. The fix deletes the existing file before copying, similar to how tar and unzip handle this scenario.

Changes Made:

  1. Modified AbstractUnArchiver.extractFile() to delete the existing file before calling Files.copy()
  2. Added setWritable(true) before delete() to handle Windows where read-only files cannot be deleted
  3. Kept REPLACE_EXISTING as a fallback for robustness (belt and suspenders approach)
  4. Added a test case shouldExtractReadOnlyFile() to verify the fix works correctly
  5. All tests pass successfully

Implementation Details:

The solution uses a robust approach:

  • First, make the file writable (required on Windows for read-only files)
  • Then, delete the existing file (handles read-only files on all platforms)
  • Finally, copy with REPLACE_EXISTING flag (handles edge cases where delete might fail)

This provides maximum compatibility across different platforms (Linux, macOS, Windows) and permission scenarios.

Original prompt

This section details on the original issue you should resolve

<issue_title>UnArchiver fails to extract files if the target exists and is read-only</issue_title>
<issue_description>In my maven build the unpack extension is used to extract some packages like zulu or postgres, which contain read-only files (e.g. -r-xr-xr-x pgsql/lib/libssl.so.1.0.0). When re-building, I get the WARNING Unable to expand to file .../pgsql/lib/libssl.so.1.0.0 . (I created https://stackoverflow.com/questions/62836984/how-to-make-maven-unpack-overwrite-read-only-files for that)

I followed the code to the plexus archiver, where the extractFile method in AbstractUnArchiver.java is just overwriting the file without deleting it first if it exists.

Now, I'm not sure if it is a bug, an enhancement or even desired behavior, that read-only target files are not overwritten? The directory that contains it is writable, so deleting and creating a new file would work.

Would you consider something like adding ' if (f.exists()) { f.delete() } ' at around line 350? Or would that be undesired?</issue_description>

Comments on the Issue (you are @copilot in this section)

@plamentotev From what I can gather tools like `tar` and `unzip` first delete the existing file when overriding files. I think first deleting the file should be ok for Plexus Archiver as well.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits October 27, 2025 01:49
Co-authored-by: slachiewicz <6705942+slachiewicz@users.noreply.github.com>
Co-authored-by: slachiewicz <6705942+slachiewicz@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix unarchiver to overwrite read-only target files Fix extraction failure for read-only files during archive extraction Oct 27, 2025
Copilot AI requested a review from slachiewicz October 27, 2025 01:55
Copilot finished work on behalf of slachiewicz October 27, 2025 01:55
@slachiewicz
Copy link
Member

@copilot gha cicd fails to delete files

Co-authored-by: slachiewicz <6705942+slachiewicz@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Oct 27, 2025

@copilot gha cicd fails to delete files

Fixed in 242cfe9. Added setWritable(true) before delete() to handle Windows systems where read-only files cannot be deleted without first making them writable.

Copilot finished work on behalf of slachiewicz October 27, 2025 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UnArchiver fails to extract files if the target exists and is read-only

2 participants