diff --git a/XADMasterTests/Stuffit/StuffitTests.m b/XADMasterTests/Stuffit/StuffitTests.m index 7b3a4ecf..d95321f6 100644 --- a/XADMasterTests/Stuffit/StuffitTests.m +++ b/XADMasterTests/Stuffit/StuffitTests.m @@ -97,7 +97,39 @@ - (void)testArchiveCanBeExtractedWithMoreThan8CharactersInPassword XCTAssertEqual(error, XADNoError, @"Error unarchiving: %@", [XADException describeXADError:error]); XCTAssertFalse(_delegate.extractionFailed, @"Extraction failed: %@", [XADException describeXADError:_delegate.error]); - + +} + +- (void)testExtractionWithWrongPasswordDoesNotLeaveFilesOnDisk +{ + // Arrange + NSURL *fixtureURL = [[NSBundle bundleForClass:[XADStuffitTests class]] + URLForResource:@"1234567" withExtension:@"sit.bin" subdirectory:@"StuffitFixtures"]; + XCTAssertNotNil(fixtureURL, @"Fixture file not found"); + + XADError createError; + XADSimpleUnarchiver *unarchiver = [XADSimpleUnarchiver simpleUnarchiverForPath:fixtureURL.path + error:&createError]; + XCTAssertEqual(createError, XADNoError); + XCTAssertNotNil(unarchiver); + + [unarchiver setDelegate:self.delegate]; + [unarchiver setPassword:@"wrongpassword"]; + [unarchiver setDestination:self.tempDirURL.path]; + + XADError parseError = [unarchiver parse]; + XCTAssertEqual(parseError, XADNoError, @"Parse should succeed regardless of password"); + + // Act + [unarchiver unarchive]; + + // Assert — wrong password was actually the failure cause + XCTAssertTrue(self.delegate.extractionFailed); + XCTAssertEqual(self.delegate.error, XADPasswordError); + + // Assert — no leftover files on disk + NSArray *items = [[NSFileManager defaultManager] contentsOfDirectoryAtPath:self.tempDirURL.path error:nil]; + XCTAssertEqualObjects(items, @[], @"Leftover files found after failed extraction: %@", items); } @end diff --git a/XADUnarchiver.m b/XADUnarchiver.m index 3da64cc4..b4e58e3e 100644 --- a/XADUnarchiver.m +++ b/XADUnarchiver.m @@ -414,6 +414,13 @@ -(XADError)_extractFileEntryWithDictionary:(NSDictionary *)dict as:(NSString *)d [fh close]; + // Remove the output file on any failure. This includes XADBreakError, which in + // most cases produces a partial file (cancellation is checked per 256KB chunk). + if(err!=XADNoError) + { + [XADPlatform removeItemAtPath:destpath]; + } + return err; } @@ -498,20 +505,30 @@ -(XADError)_extractResourceForkEntryWithDictionary:(NSDictionary *)dict asAppleD NSDictionary *extattrs=[parser extendedAttributesForDictionary:dict]; + XADError error=XADNoError; @try { // TODO: Should this function handle exceptions itself? - [XADAppleDouble writeAppleDoubleHeaderToHandle:fh resourceForkSize:(int)ressize - extendedAttributes:extattrs]; + [XADAppleDouble writeAppleDoubleHeaderToHandle:fh + resourceForkSize:(int)ressize + extendedAttributes:extattrs]; + if(ressize) + { + error=[self runExtractorWithDictionary:dict outputHandle:fh]; + } + } @catch(id e) { + error=[XADException parseException:e]; } - @catch(id e) { return [XADException parseException:e]; } - - // Write resource fork. - XADError error=XADNoError; - if(ressize) error=[self runExtractorWithDictionary:dict outputHandle:fh]; [fh close]; + // Remove the output file on any failure. This includes XADBreakError, which in + // most cases produces a partial file (cancellation is checked per 256KB chunk). + if(error!=XADNoError) + { + [XADPlatform removeItemAtPath:destpath]; + } + return error; }