Fix leftover output files and file handle leak when extraction fails#190
Merged
alek-prykhodko merged 2 commits intoJun 5, 2026
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves extraction failure handling in XADUnarchiver by ensuring partially created output files are cleaned up and a file-handle leak is avoided when AppleDouble header writing fails.
Changes:
- Delete the destination file after extraction when
_extractFileEntryWithDictionary:as:returns a non-zeroXADError. - Refactor
_extractResourceForkEntryWithDictionary:asAppleDoubleFile:to consolidate error handling so the file handle is always closed. - Delete the AppleDouble output file on any non-zero
XADErrorfrom header writing or resource-fork extraction.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
PaulTaykalo
approved these changes
Jun 4, 2026
Contributor
PaulTaykalo
left a comment
There was a problem hiding this comment.
Fix removes leftover output files and a file-handle leak when extraction fails. Author shipped XADMasterTests/Stuffit/StuffitTests.m::testExtractionWithWrongPasswordDoesNotLeaveFilesOnDisk covering the primary path with the 1234567.sit.bin fixture. The resource-fork (AppleDouble) error path is unchanged in behavior but untested; suggested follow-up, not blocking.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When file or resource fork extraction fails (due to a wrong password, cancellation, etc.) the output file created before extraction started was left on disk as empty or partial.
Additionally, a failed AppleDouble header write in
_extractResourceForkEntryWithDictionary:asAppleDoubleFile:returned early without calling[fh close], leaking the file handle.Changes
_extractFileEntryWithDictionary:as:— remove the output file on any non-zero error after[fh close];_extractResourceForkEntryWithDictionary:asAppleDoubleFile:— unify the error path so[fh close]always runs, and remove the output file on any non-zero error.P.S. Both fixes removes files for
XADBreakErroras well — cancellation is checked per 256KB chunk inside the read loop, so the file is partial in most cases at that point.