Fixing:

Bug #389869: "Sparkle runs thread-unsafe code on secondary threads"
Bug #312995: "Canceling authentication request causes crash on next update"
Bug #388793: "Need to notify SUUpdateDriverFinishedNotification on main thread"

The unfortunate side-effect of this fix is that all the file-handling code is now CoreServices-based, since NSFileManager is not thread-safe. This is disgusting and will be stricken from all records when installation is performed by relaunch in Next Major, as it should have been in the first place.

SUDiskImageUnarchiver.m

@@ -9,6 +9,7 @@
 #import "SUDiskImageUnarchiver.h"
 #import "SUUnarchiver_Private.h"
 #import "NTSynchronousTask.h"
+#import <CoreServices/CoreServices.h>
 
 @implementation SUDiskImageUnarchiver
 
@@ -28,18 +29,23 @@ - (void)_extractDMG
 	BOOL mountedSuccessfully = NO;
 
 	// get a unique mount point path
-	NSString *mountPoint = [@"/Volumes" stringByAppendingPathComponent:[[NSProcessInfo processInfo] globallyUniqueString]];
-
-	if ([[NSFileManager defaultManager] fileExistsAtPath:mountPoint]) goto reportError;
-
-	// create mount point folder
-#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_5
-    [[NSFileManager defaultManager] createDirectoryAtPath:mountPoint attributes:nil];
+	NSString *mountPointName = nil;
+	NSString *mountPoint = nil;
+	FSRef tmpRef;
+	do
+	{
+		CFUUIDRef uuid = CFUUIDCreate(NULL);
+		mountPointName = (NSString *)CFUUIDCreateString(NULL, uuid);
+		CFRelease(uuid);
+#if MAC_OS_X_VERSION_MAX_ALLOWED > MAC_OS_X_VERSION_10_4
+		[NSMakeCollectable((CFStringRef)mountPointName) autorelease];
 #else
-	[[NSFileManager defaultManager] createDirectoryAtPath:mountPoint withIntermediateDirectories:YES attributes:nil error:NULL];
+		[mountPointName autorelease];
 #endif
-	if (![[NSFileManager defaultManager] fileExistsAtPath:mountPoint]) goto reportError;
-
+		mountPoint = [@"/Volumes" stringByAppendingPathComponent:mountPointName];
+	}
+	while (noErr == FSPathMakeRefWithOptions((UInt8 *)[mountPoint fileSystemRepresentation], kFSPathMakeRefDoNotFollowLeafSymlink, &tmpRef, NULL));
+
 	NSArray* arguments = [NSArray arrayWithObjects:@"attach", archivePath, @"-mountpoint", mountPoint, @"-noverify", @"-nobrowse", @"-noautoopen", nil];
 	// set up a pipe and push "yes" (y works too), this will accept any license agreement crap
 	// not every .dmg needs this, but this will make sure it works with everyone
@@ -50,33 +56,16 @@ - (void)_extractDMG
 	mountedSuccessfully = YES;
 
 	// Now that we've mounted it, we need to copy out its contents.
-	NSString *targetPath = [[archivePath stringByDeletingLastPathComponent] stringByAppendingPathComponent:[mountPoint lastPathComponent]];
-#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_5
-    if (![[NSFileManager defaultManager] createDirectoryAtPath:targetPath attributes:nil]) goto reportError;
-#else
-	if (![[NSFileManager defaultManager] createDirectoryAtPath:targetPath withIntermediateDirectories:YES attributes:nil error:NULL]) goto reportError;
-#endif
+	FSRef srcRef, dstRef;
+	OSErr err;
+	err = FSPathMakeRef((UInt8 *)[mountPoint fileSystemRepresentation], &srcRef, NULL);
+	if (err != noErr) goto reportError;
+	err = FSPathMakeRef((UInt8 *)[[archivePath stringByDeletingLastPathComponent] fileSystemRepresentation], &dstRef, NULL);
+	if (err != noErr) goto reportError;
+
+	err = FSCopyObjectSync(&srcRef, &dstRef, (CFStringRef)mountPointName, NULL, kFSFileOperationSkipSourcePermissionErrors);
+	if (err != noErr) goto reportError;
 
-	// We can't just copyPath: from the volume root because that always fails. Seems to be a bug.
-#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_5
-    id subpathEnumerator = [[[NSFileManager defaultManager] directoryContentsAtPath:mountPoint] objectEnumerator];
-#else
-	id subpathEnumerator = [[[NSFileManager defaultManager] contentsOfDirectoryAtPath:mountPoint error:NULL] objectEnumerator];
-#endif
-	NSString *currentSubpath;
-	while ((currentSubpath = [subpathEnumerator nextObject]))
-	{
-		NSString *currentFullPath = [mountPoint stringByAppendingPathComponent:currentSubpath];
-		// Don't bother trying (and failing) to copy out files we can't read. That's not going to be the app anyway.
-		if (![[NSFileManager defaultManager] isReadableFileAtPath:currentFullPath]) continue;
-#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_5
-        if (![[NSFileManager defaultManager] copyPath:currentFullPath toPath:[targetPath stringByAppendingPathComponent:currentSubpath] handler:nil])
-#else
-		if (![[NSFileManager defaultManager] copyItemAtPath:currentFullPath toPath:[targetPath stringByAppendingPathComponent:currentSubpath] error:NULL])
-#endif
-			goto reportError;
-	}
-
 	[self performSelectorOnMainThread:@selector(_notifyDelegateOfSuccess) withObject:nil waitUntilDone:NO];
 	goto finally;
 
@@ -86,12 +75,6 @@ - (void)_extractDMG
 finally:
 	if (mountedSuccessfully)
 		[NSTask launchedTaskWithLaunchPath:@"/usr/bin/hdiutil" arguments:[NSArray arrayWithObjects:@"detach", mountPoint, @"-force", nil]];
-	else
-#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_5
-        [[NSFileManager defaultManager] removeFileAtPath:mountPoint handler:nil];
-#else
-		[[NSFileManager defaultManager] removeItemAtPath:mountPoint error:NULL];
-#endif
 	[pool drain];
 }
 

SUPlainInstaller.m

@@ -8,45 +8,53 @@
 
 #import "SUPlainInstaller.h"
 #import "SUPlainInstallerInternals.h"
-#import "SUHost.h"
 
 NSString *SUInstallerPathKey = @"SUInstallerPath";
+NSString *SUInstallerTargetPathKey = @"SUInstallerTargetPath";
+NSString *SUInstallerTempNameKey = @"SUInstallerTempName";
 NSString *SUInstallerHostKey = @"SUInstallerHost";
 NSString *SUInstallerDelegateKey = @"SUInstallerDelegate";
-NSString *SUInstallerVersionComparatorKey = @"SUInstallerVersionComparator";
+NSString *SUInstallerResultKey = @"SUInstallerResult";
+NSString *SUInstallerErrorKey = @"SUInstallerError";
 
 @implementation SUPlainInstaller
 
-+ (void)installPath:(NSString *)path overHost:(SUHost *)bundle delegate:delegate versionComparator:(id <SUVersionComparison>)comparator
++ (void)_finishInstallationWithInfo:(NSDictionary *)info
 {
-	NSError *error;
-	BOOL result = YES;
-
-	// Prevent malicious downgrades:
-	if ([comparator compareVersion:[bundle version] toVersion:[[NSBundle bundleWithPath:path] objectForInfoDictionaryKey:@"CFBundleVersion"]] == NSOrderedDescending)
-	{
-		NSString * errorMessage = [NSString stringWithFormat:@"Sparkle Updater: Possible attack in progress! Attempting to \"upgrade\" from %@ to %@. Aborting update.", [bundle version], [[NSBundle bundleWithPath:path] objectForInfoDictionaryKey:@"CFBundleVersion"]];
-		result = NO;
-		error = [NSError errorWithDomain:SUSparkleErrorDomain code:SUDowngradeError userInfo:[NSDictionary dictionaryWithObject:errorMessage forKey:NSLocalizedDescriptionKey]];
-	}
-
-	if (result)
-		result = [self copyPathWithAuthentication:path overPath:[bundle bundlePath] error:&error];
-	[self _finishInstallationWithResult:result host:bundle error:error delegate:delegate];
+	[self _finishInstallationWithResult:[[info objectForKey:SUInstallerResultKey] boolValue] host:[info objectForKey:SUInstallerHostKey] error:[info objectForKey:SUInstallerErrorKey] delegate:[info objectForKey:SUInstallerDelegateKey]];
 }
 
 + (void)_performInstallationWithInfo:(NSDictionary *)info
 {
 	NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
 
-	[self installPath:[info objectForKey:SUInstallerPathKey] overHost:[info objectForKey:SUInstallerHostKey] delegate:[info objectForKey:SUInstallerDelegateKey] versionComparator:[info objectForKey:SUInstallerVersionComparatorKey]];
+	NSError *error = nil;
 
+	BOOL result = [self copyPathWithAuthentication:[info objectForKey:SUInstallerPathKey] overPath:[info objectForKey:SUInstallerTargetPathKey] temporaryName:[info objectForKey:SUInstallerTempNameKey] error:&error];
+
+	NSMutableDictionary *mutableInfo = [[info mutableCopy] autorelease];
+	[mutableInfo setObject:[NSNumber numberWithBool:result] forKey:SUInstallerResultKey];
+	if (!result && error)
+		[mutableInfo setObject:error forKey:SUInstallerErrorKey];
+	[self performSelectorOnMainThread:@selector(_finishInstallationWithInfo:) withObject:mutableInfo waitUntilDone:NO];
+
 	[pool drain];
 }
 
 + (void)performInstallationWithPath:(NSString *)path host:(SUHost *)host delegate:delegate synchronously:(BOOL)synchronously versionComparator:(id <SUVersionComparison>)comparator
 {
-	NSDictionary *info = [NSDictionary dictionaryWithObjectsAndKeys:path, SUInstallerPathKey, host, SUInstallerHostKey, delegate, SUInstallerDelegateKey, comparator, SUInstallerVersionComparatorKey, nil];
+	// Prevent malicious downgrades:
+	if ([comparator compareVersion:[host version] toVersion:[[NSBundle bundleWithPath:path] objectForInfoDictionaryKey:@"CFBundleVersion"]] == NSOrderedDescending)
+	{
+		NSString * errorMessage = [NSString stringWithFormat:@"Sparkle Updater: Possible attack in progress! Attempting to \"upgrade\" from %@ to %@. Aborting update.", [host version], [[NSBundle bundleWithPath:path] objectForInfoDictionaryKey:@"CFBundleVersion"]];
+		NSError *error = [NSError errorWithDomain:SUSparkleErrorDomain code:SUDowngradeError userInfo:[NSDictionary dictionaryWithObject:errorMessage forKey:NSLocalizedDescriptionKey]];
+		[self _finishInstallationWithResult:NO host:host error:error delegate:delegate];
+		return;
+	}
+
+    NSString *targetPath = [host bundlePath];
+    NSString *tempName = [self temporaryNameForPath:targetPath];
+	NSDictionary *info = [NSDictionary dictionaryWithObjectsAndKeys:path, SUInstallerPathKey, targetPath, SUInstallerTargetPathKey, tempName, SUInstallerTempNameKey, host, SUInstallerHostKey, delegate, SUInstallerDelegateKey, nil];
 	if (synchronously)
 		[self _performInstallationWithInfo:info];
 	else

SUPlainInstallerInternals.h

@@ -12,7 +12,8 @@
 #import "SUPlainInstaller.h"
 
 @interface SUPlainInstaller (Internals)
-+ (BOOL)copyPathWithAuthentication:(NSString *)src overPath:(NSString *)dst error:(NSError **)error;
++ (NSString *)temporaryNameForPath:(NSString *)path;
++ (BOOL)copyPathWithAuthentication:(NSString *)src overPath:(NSString *)dst temporaryName:(NSString *)tmp error:(NSError **)error;
 @end
 
 #endif

SUPlainInstallerInternals.m

@@ -9,6 +9,7 @@
 #import "Sparkle.h"
 #import "SUPlainInstallerInternals.h"
 
+#import <CoreServices/CoreServices.h>
 #import <Security/Security.h>
 #import <sys/stat.h>
 #import <sys/wait.h>
@@ -60,42 +61,7 @@ static BOOL AuthorizationExecuteWithPrivilegesAndWait(AuthorizationRef authoriza
 
 @implementation SUPlainInstaller (Internals)
 
-+ (BOOL)currentUserOwnsPath:(NSString *)oPath
-{
-	const char *path = [oPath fileSystemRepresentation];
-	uid_t uid = getuid();
-	bool res = false;
-	struct stat sb;
-	if(stat(path, &sb) == 0)
-	{
-		if(sb.st_uid == uid)
-		{
-			res = true;
-			if(sb.st_mode & S_IFDIR)
-			{
-				DIR* dir = opendir(path);
-				struct dirent* entry = NULL;
-				while(res && (entry = readdir(dir)))
-				{
-					if(strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0)
-						continue;
-
-					size_t len = strlen(path) + 1 + entry->d_namlen + 1;
-					char descend[len];
-					strlcpy(descend, path, len);
-					strlcat(descend, "/", len);
-					strlcat(descend, entry->d_name, len);
-					NSString* newPath = [[NSFileManager defaultManager] stringWithFileSystemRepresentation:descend length:strlen(descend)];
-					res = [self currentUserOwnsPath:newPath];
-				}
-				closedir(dir);
-			}
-		}
-	}
-	return res;
-}
-
-+ (NSString *)_temporaryInstallationPathForPath:(NSString *)path
++ (NSString *)temporaryNameForPath:(NSString *)path
 {
 	// Let's try to read the version number so the filename will be more meaningful.
 	NSString *postFix;
@@ -116,12 +82,11 @@ + (NSString *)_temporaryInstallationPathForPath:(NSString *)path
 	int cnt=2;
 	while ([[NSFileManager defaultManager] fileExistsAtPath:tempDir] && cnt <= 999)
 		tempDir = [NSString stringWithFormat:@"%@ %d.%@", prefix, cnt++, [path pathExtension]];
-	return tempDir;
+	return [tempDir lastPathComponent];
 }
 
-+ (BOOL)_copyPathWithForcedAuthentication:(NSString *)src toPath:(NSString *)dst error:(NSError **)error
++ (BOOL)_copyPathWithForcedAuthentication:(NSString *)src toPath:(NSString *)dst temporaryPath:(NSString *)tmp error:(NSError **)error
 {
-	NSString *tmp = [self _temporaryInstallationPathForPath:dst];
 	const char* srcPath = [src fileSystemRepresentation];
 	const char* tmpPath = [tmp fileSystemRepresentation];
 	const char* dstPath = [dst fileSystemRepresentation];
@@ -190,7 +155,7 @@ + (BOOL)_copyPathWithForcedAuthentication:(NSString *)src toPath:(NSString *)dst
 		// change will almost certainly make it impossible to change
 		// attributes to release the files from the quarantine.
 		if (res) {
-			[self releaseFromQuarantine:dst];
+			[self performSelectorOnMainThread:@selector(releaseFromQuarantine:) withObject:dst waitUntilDone:YES];
 		}
 
 		// Now move past the NULL we found and continue executing
@@ -220,44 +185,46 @@ + (BOOL)_copyPathWithForcedAuthentication:(NSString *)src toPath:(NSString *)dst
 	return res;
 }
 
-+ (BOOL)copyPathWithAuthentication:(NSString *)src overPath:(NSString *)dst error:(NSError **)error
++ (BOOL)copyPathWithAuthentication:(NSString *)src overPath:(NSString *)dst temporaryName:(NSString *)tmp error:(NSError **)error
 {
-	if (![[NSFileManager defaultManager] fileExistsAtPath:dst])
+	FSRef srcRef, dstRef, targetRef, movedRef;
+	OSErr err;
+
+	err = FSPathMakeRefWithOptions((UInt8 *)[dst fileSystemRepresentation], kFSPathMakeRefDoNotFollowLeafSymlink, &dstRef, NULL);
+	if (err != noErr)
 	{
 		NSString *errorMessage = [NSString stringWithFormat:@"Couldn't copy %@ over %@ because there is no file at %@.", src, dst, dst];
 		if (error != NULL)
 			*error = [NSError errorWithDomain:SUSparkleErrorDomain code:SUFileCopyFailure userInfo:[NSDictionary dictionaryWithObject:errorMessage forKey:NSLocalizedDescriptionKey]];
 		return NO;
 	}
-
-	if (![[NSFileManager defaultManager] isWritableFileAtPath:dst] || ![[NSFileManager defaultManager] isWritableFileAtPath:[dst stringByDeletingLastPathComponent]])
-		return [self _copyPathWithForcedAuthentication:src toPath:dst error:error];
-
-	NSString *tmpPath = [self _temporaryInstallationPathForPath:dst];
-#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_5
-    if (![[NSFileManager defaultManager] movePath:dst toPath:tmpPath handler:nil])
-#else
-	if (![[NSFileManager defaultManager] moveItemAtPath:dst toPath:tmpPath error:NULL])
-#endif
+	
+	NSString *tmpPath = [[dst stringByDeletingLastPathComponent] stringByAppendingPathComponent:tmp];
+
+	if (0 != access([dst fileSystemRepresentation], W_OK) || 0 != access([[dst stringByDeletingLastPathComponent] fileSystemRepresentation], W_OK))
+		return [self _copyPathWithForcedAuthentication:src toPath:dst temporaryPath:tmpPath error:error];
+	
+	err = FSPathMakeRef((UInt8 *)[[dst stringByDeletingLastPathComponent] fileSystemRepresentation], &targetRef, NULL);
+	if (err == noErr)
+		err = FSMoveObjectSync(&dstRef, &targetRef, (CFStringRef)tmp, &movedRef, 0);
+	if (err != noErr)
 	{
 		if (error != NULL)
 			*error = [NSError errorWithDomain:SUSparkleErrorDomain code:SUFileCopyFailure userInfo:[NSDictionary dictionaryWithObject:[NSString stringWithFormat:@"Couldn't move %@ to %@.", dst, tmpPath] forKey:NSLocalizedDescriptionKey]];
 		return NO;			
 	}
-#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_5
-    if (![[NSFileManager defaultManager] copyPath:src toPath:dst handler:nil])
-#else
-	if (![[NSFileManager defaultManager] copyItemAtPath:src toPath:dst error:NULL])
-#endif
+	err = FSPathMakeRef((UInt8 *)[src fileSystemRepresentation], &srcRef, NULL);
+	if (err == noErr)
+		err = FSCopyObjectSync(&srcRef, &targetRef, (CFStringRef)[dst lastPathComponent], NULL, 0);
+	if (err != noErr)
 	{
 		if (error != NULL)
 			*error = [NSError errorWithDomain:SUSparkleErrorDomain code:SUFileCopyFailure userInfo:[NSDictionary dictionaryWithObject:[NSString stringWithFormat:@"Couldn't copy %@ to %@.", src, dst] forKey:NSLocalizedDescriptionKey]];
 		return NO;			
 	}
 
 	// Trash the old copy of the app.
-	NSInteger tag = 0;
-	if (![[NSWorkspace sharedWorkspace] performFileOperation:NSWorkspaceRecycleOperation source:[tmpPath stringByDeletingLastPathComponent] destination:@"" files:[NSArray arrayWithObject:[tmpPath lastPathComponent]] tag:&tag])
+	if (noErr != FSMoveObjectToTrashSync(&movedRef, NULL, 0))
 		NSLog(@"Sparkle error: couldn't move %@ to the trash. This is often a sign of a permissions error.", tmpPath);
 
 	// If the currently-running application is trusted, the new
@@ -269,7 +236,7 @@ + (BOOL)copyPathWithAuthentication:(NSString *)src overPath:(NSString *)dst erro
 	// new home in case it's moved across filesystems: if that
 	// happens, the move is actually a copy, and it may result
 	// in the application being quarantined.
-	[self releaseFromQuarantine:dst];
+	[self performSelectorOnMainThread:@selector(releaseFromQuarantine:) withObject:dst waitUntilDone:YES];
 
 	return YES;
 }
@@ -327,11 +294,7 @@ + (void)releaseFromQuarantine:(NSString*)root
 	// Only recurse if it's actually a directory.  Don't recurse into a
 	// root-level symbolic link.
 	NSDictionary* rootAttributes =
-#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_5
-    [[NSFileManager defaultManager] fileAttributesAtPath:root traverseLink:NO];
-#else
-	[[NSFileManager defaultManager] attributesOfItemAtPath:root error:NULL];
-#endif
+	[[NSFileManager defaultManager] fileAttributesAtPath:root traverseLink:NO];
 	NSString* rootType = [rootAttributes objectForKey:NSFileType];
 
 	if (rootType == NSFileTypeDirectory) {

SUUIBasedUpdateDriver.m

@@ -157,6 +157,7 @@ - (void)installUpdate
 	{
 		[statusController close];
 		[statusController autorelease];
+		statusController = nil;
 	}
 }
 
@@ -173,6 +174,7 @@ - (void)abortUpdate
 	{
 		[statusController close];
 		[statusController autorelease];
+		statusController = nil;
 	}
 	[super abortUpdate];
 }