Fix leftover output files and file handle leak when extraction fails

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

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;
 }