From ef44757f2ae28741f99e94eebe7c961799ef781b Mon Sep 17 00:00:00 2001 From: Alex Billingsley Date: Wed, 6 Apr 2016 15:23:10 -0400 Subject: [PATCH 1/5] More Verbose Logging during Install DyKnow/DyKnowMe#5647 --- Sparkle/SUFileManager.m | 15 +++++++++++++++ Sparkle/SUPlainInstaller.m | 17 +++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/Sparkle/SUFileManager.m b/Sparkle/SUFileManager.m index d0fc3b00d..12cdc86d9 100644 --- a/Sparkle/SUFileManager.m +++ b/Sparkle/SUFileManager.m @@ -12,6 +12,7 @@ #include #include #include +#import "SULog.h" static char SUAppleQuarantineIdentifier[] = "com.apple.quarantine"; @@ -201,6 +202,8 @@ - (BOOL)_removeXAttrWithAuthentication:(char *__nonnull)xattrName fromRootURL:(N return NO; } + SULog(@"Will _acquireAuthorizationWithError for _removeXAttrWithAuthentication"); + if (![self _acquireAuthorizationWithError:error]) { return NO; } @@ -388,6 +391,8 @@ - (BOOL)copyItemAtURL:(NSURL *)sourceURL toURL:(NSURL *)destinationURL error:(NS return NO; } + SULog(@"Will _acquireAuthorizationWithError for copyItemAtURL"); + if (![self _acquireAuthorizationWithError:error]) { return NO; } @@ -500,6 +505,8 @@ - (BOOL)moveItemAtURL:(NSURL *)sourceURL toURL:(NSURL *)destinationURL error:(NS } return NO; } + + SULog(@"Will _acquireAuthorizationWithError for moveItemAtURL"); if (![self _acquireAuthorizationWithError:error]) { return NO; @@ -658,6 +665,8 @@ - (BOOL)changeOwnerAndGroupOfItemAtRootURL:(NSURL *)targetURL toMatchURL:(NSURL if (written < 0 || written >= 100) { return NO; // No custom error, because it's too unlikely to ever happen } + + SULog(@"Will _acquireAuthorizationWithError for changeOwnerAndGroupOfItemAtRootURL"); if (![self _acquireAuthorizationWithError:error]) { return NO; @@ -705,6 +714,8 @@ - (BOOL)updateModificationAndAccessTimeOfItemAtURL:(NSURL *)targetURL error:(NSE } return NO; } + + SULog(@"Will _acquireAuthorizationWithError for updateModificationAndAccessTimeOfItemAtURL"); if (![self _acquireAuthorizationWithError:error]) { return NO; @@ -759,6 +770,8 @@ - (BOOL)_makeDirectoryAtURL:(NSURL *)url error:(NSError * __autoreleasing *)erro return NO; } + SULog(@"Will _acquireAuthorizationWithError for _makeDirectoryAtURL"); + if (![self _acquireAuthorizationWithError:error]) { return NO; } @@ -812,6 +825,8 @@ - (BOOL)removeItemAtURL:(NSURL *)url error:(NSError * __autoreleasing *)error } return NO; } + + SULog(@"Will _acquireAuthorizationWithError for removeItemAtURL"); if (![self _acquireAuthorizationWithError:error]) { return NO; diff --git a/Sparkle/SUPlainInstaller.m b/Sparkle/SUPlainInstaller.m index 70a7ded9e..9eccad055 100644 --- a/Sparkle/SUPlainInstaller.m +++ b/Sparkle/SUPlainInstaller.m @@ -45,6 +45,8 @@ + (BOOL)performInstallationToURL:(NSURL *)installationURL fromUpdateAtURL:(NSURL SUFileManager *fileManager = [SUFileManager fileManagerAllowingAuthorization:YES]; // Create a temporary directory for our new app that resides on our destination's volume + SULog(@"Will make Temprary directory for installationUrl %@", [installationURL absoluteString]); + NSURL *tempNewDirectoryURL = [fileManager makeTemporaryDirectoryWithPreferredName:[installationURL.lastPathComponent.stringByDeletingPathExtension stringByAppendingString:@" (Incomplete Update)"] appropriateForDirectoryURL:installationURL.URLByDeletingLastPathComponent error:error]; if (tempNewDirectoryURL == nil) { SULog(@"Failed to make new temp directory"); @@ -54,6 +56,7 @@ + (BOOL)performInstallationToURL:(NSURL *)installationURL fromUpdateAtURL:(NSURL // Move the new app to our temporary directory NSString *newURLLastPathComponent = newURL.lastPathComponent; NSURL *newTempURL = [tempNewDirectoryURL URLByAppendingPathComponent:newURLLastPathComponent]; + SULog(@"Will move new app at URL %@ to Temprary directory %@", [newURL absoluteString], [newTempURL absoluteString]); if (![fileManager moveItemAtURL:newURL toURL:newTempURL error:error]) { SULog(@"Failed to move the new app from %@ to its temp directory at %@", newURL.path, newTempURL.path); [fileManager removeItemAtURL:tempNewDirectoryURL error:NULL]; @@ -64,6 +67,7 @@ + (BOOL)performInstallationToURL:(NSURL *)installationURL fromUpdateAtURL:(NSURL // We must leave moving the app to its destination as the final step in installing it, so that // it's not possible our new app can be left in an incomplete state at the final destination + SULog(@"Will release new app from quarantine %@", [newTempURL absoluteString]); NSError *quarantineError = nil; if (![fileManager releaseItemFromQuarantineAtRootURL:newTempURL error:&quarantineError]) { // Not big enough of a deal to fail the entire installation @@ -80,6 +84,7 @@ + (BOOL)performInstallationToURL:(NSURL *)installationURL fromUpdateAtURL:(NSURL return NO; } + SULog(@"Will change owner and group of new app %@ to match ownership of old app %@", [newTempURL absoluteString], [oldURL absoluteString]); if (![fileManager changeOwnerAndGroupOfItemAtRootURL:newTempURL toMatchURL:oldURL error:error]) { // But this is big enough of a deal to fail SULog(@"Failed to change owner and group of new app at %@ to match old app at %@", newTempURL.path, oldURL.path); @@ -87,6 +92,7 @@ + (BOOL)performInstallationToURL:(NSURL *)installationURL fromUpdateAtURL:(NSURL return NO; } + SULog(@"Will update last modified date of new app %@", [newTempURL absoluteString]); if (![fileManager updateModificationAndAccessTimeOfItemAtURL:newTempURL error:error]) { // Not a fatal error, but a pretty unfortunate one SULog(@"Failed to update modification and access time of new app at %@", newTempURL.path); @@ -106,6 +112,7 @@ + (BOOL)performInstallationToURL:(NSURL *)installationURL fromUpdateAtURL:(NSURL NSString *oldDestinationNameWithPathExtension = [oldDestinationName stringByAppendingPathExtension:oldURLExtension]; // Create a temporary directory for our old app that resides on its volume + SULog(@"Will create a temporary directory for the old app %@", [oldURL absoluteString]); NSURL *tempOldDirectoryURL = [fileManager makeTemporaryDirectoryWithPreferredName:oldDestinationName appropriateForDirectoryURL:oldURL.URLByDeletingLastPathComponent error:error]; if (tempOldDirectoryURL == nil) { SULog(@"Failed to create temporary directory for old app at %@", oldURL.path); @@ -115,38 +122,48 @@ + (BOOL)performInstallationToURL:(NSURL *)installationURL fromUpdateAtURL:(NSURL // Move the old app to the temporary directory NSURL *oldTempURL = [tempOldDirectoryURL URLByAppendingPathComponent:oldDestinationNameWithPathExtension]; + SULog(@"Will move the old app %@ to the temp directory %@", [oldURL absoluteString], [oldTempURL absoluteString]); if (![fileManager moveItemAtURL:oldURL toURL:oldTempURL error:error]) { SULog(@"Failed to move the old app at %@ to a temporary location at %@", oldURL.path, oldTempURL.path); // Just forget about our updated app on failure + SULog(@"Will remove new temp directory %@", [tempNewDirectoryURL absoluteString]); [fileManager removeItemAtURL:tempNewDirectoryURL error:NULL]; + SULog(@"Will remove old temp directory %@", [tempOldDirectoryURL absoluteString]); [fileManager removeItemAtURL:tempOldDirectoryURL error:NULL]; return NO; } // Move the new app to its final destination + SULog(@"Will move the new app from temp directory %@ to install location %@", [newTempURL absoluteString], [installationURL absoluteString]); if (![fileManager moveItemAtURL:newTempURL toURL:installationURL error:error]) { SULog(@"Failed to move new app at %@ to final destination %@", newTempURL.path, installationURL.path); // Forget about our updated app on failure + SULog(@"Will remove new temp directory %@", [tempNewDirectoryURL absoluteString]); [fileManager removeItemAtURL:tempNewDirectoryURL error:NULL]; // Attempt to restore our old app back the way it was on failure + SULog(@"Will restore old app %@ to install location %@", [oldTempURL absoluteString], [oldURL absoluteString]); [fileManager moveItemAtURL:oldTempURL toURL:oldURL error:NULL]; + SULog(@"Will remove old temp directory %@", [tempOldDirectoryURL absoluteString]); [fileManager removeItemAtURL:tempOldDirectoryURL error:NULL]; return NO; } // Cleanup: move the old app to the trash + SULog(@"Will move the old app from temp directory %@ to trash", [oldTempURL absoluteString]); NSError *trashError = nil; if (![fileManager moveItemAtURLToTrash:oldTempURL error:&trashError]) { SULog(@"Failed to move %@ to trash with error %@", oldTempURL, trashError); } + SULog(@"Will remove old temp directory %@", [tempOldDirectoryURL absoluteString]); [fileManager removeItemAtURL:tempOldDirectoryURL error:NULL]; + SULog(@"Will remove new temp directory %@", [tempNewDirectoryURL absoluteString]); [fileManager removeItemAtURL:tempNewDirectoryURL error:NULL]; return YES; From d8db6ee3d7f03e90de0e978793dc907a50fad5fc Mon Sep 17 00:00:00 2001 From: Alex Billingsley Date: Thu, 7 Apr 2016 11:27:40 -0400 Subject: [PATCH 2/5] Add SULog to BinaryDelta target to fix build failure DyKnow/DyKnowMe#5647 --- Sparkle.xcodeproj/project.pbxproj | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Sparkle.xcodeproj/project.pbxproj b/Sparkle.xcodeproj/project.pbxproj index 18fbe5741..dc4cfb4ac 100644 --- a/Sparkle.xcodeproj/project.pbxproj +++ b/Sparkle.xcodeproj/project.pbxproj @@ -50,6 +50,7 @@ 14958C6E19AEBC950061B14F /* signed-test-file.txt in Resources */ = {isa = PBXBuildFile; fileRef = 14958C6B19AEBC530061B14F /* signed-test-file.txt */; }; 14958C6F19AEBC980061B14F /* test-pubkey.pem in Resources */ = {isa = PBXBuildFile; fileRef = 14958C6C19AEBC610061B14F /* test-pubkey.pem */; }; 3772FEA913DE0B6B00F79537 /* SUVersionDisplayProtocol.h in Headers */ = {isa = PBXBuildFile; fileRef = 3772FEA813DE0B6B00F79537 /* SUVersionDisplayProtocol.h */; settings = {ATTRIBUTES = (Public, ); }; }; + 392CD5D21CB6B3B200447781 /* SULog.m in Sources */ = {isa = PBXBuildFile; fileRef = 55C14F05136EF6DB00649790 /* SULog.m */; }; 55C14BD4136EEFCE00649790 /* Autoupdate.m in Sources */ = {isa = PBXBuildFile; fileRef = 55C14BD3136EEFCE00649790 /* Autoupdate.m */; }; 55C14BD9136EF00C00649790 /* SUStatus.xib in Resources */ = {isa = PBXBuildFile; fileRef = 55C14BD8136EF00C00649790 /* SUStatus.xib */; }; 55C14BEE136EF20D00649790 /* SUAutomaticUpdateAlert.xib in Resources */ = {isa = PBXBuildFile; fileRef = 55C14BDA136EF20D00649790 /* SUAutomaticUpdateAlert.xib */; }; @@ -1618,6 +1619,7 @@ 7223E7631AD1AEFF008E3161 /* sais.c in Sources */, 14652F7D19A9726700959E44 /* SUBinaryDeltaApply.m in Sources */, 14652F7C19A9725300959E44 /* SUBinaryDeltaCommon.m in Sources */, + 392CD5D21CB6B3B200447781 /* SULog.m in Sources */, 7268AC631AD634C200C3E0C1 /* SUBinaryDeltaCreate.m in Sources */, 5D06E8EA0FD68CDB005AE3F6 /* SUBinaryDeltaTool.m in Sources */, 726F2CE81BC9C48F001971A4 /* SUConstants.m in Sources */, From 7bbc5a0ab0c8b064550c5f6772219408360754bf Mon Sep 17 00:00:00 2001 From: Alex Billingsley Date: Thu, 7 Apr 2016 11:33:28 -0400 Subject: [PATCH 3/5] Add SULog to TestApp target to fix build failure DyKnow/DyKnowMe#5647 --- Sparkle.xcodeproj/project.pbxproj | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Sparkle.xcodeproj/project.pbxproj b/Sparkle.xcodeproj/project.pbxproj index dc4cfb4ac..fb351a0bf 100644 --- a/Sparkle.xcodeproj/project.pbxproj +++ b/Sparkle.xcodeproj/project.pbxproj @@ -51,6 +51,7 @@ 14958C6F19AEBC980061B14F /* test-pubkey.pem in Resources */ = {isa = PBXBuildFile; fileRef = 14958C6C19AEBC610061B14F /* test-pubkey.pem */; }; 3772FEA913DE0B6B00F79537 /* SUVersionDisplayProtocol.h in Headers */ = {isa = PBXBuildFile; fileRef = 3772FEA813DE0B6B00F79537 /* SUVersionDisplayProtocol.h */; settings = {ATTRIBUTES = (Public, ); }; }; 392CD5D21CB6B3B200447781 /* SULog.m in Sources */ = {isa = PBXBuildFile; fileRef = 55C14F05136EF6DB00649790 /* SULog.m */; }; + 392CD5D41CB6B51A00447781 /* SULog.m in Sources */ = {isa = PBXBuildFile; fileRef = 55C14F05136EF6DB00649790 /* SULog.m */; }; 55C14BD4136EEFCE00649790 /* Autoupdate.m in Sources */ = {isa = PBXBuildFile; fileRef = 55C14BD3136EEFCE00649790 /* Autoupdate.m */; }; 55C14BD9136EF00C00649790 /* SUStatus.xib in Resources */ = {isa = PBXBuildFile; fileRef = 55C14BD8136EF00C00649790 /* SUStatus.xib */; }; 55C14BEE136EF20D00649790 /* SUAutomaticUpdateAlert.xib in Resources */ = {isa = PBXBuildFile; fileRef = 55C14BDA136EF20D00649790 /* SUAutomaticUpdateAlert.xib */; }; @@ -1657,6 +1658,7 @@ 726F2CEB1BC9C733001971A4 /* SUConstants.m in Sources */, 5A6D31EE1BF53245009C5157 /* SUFileManager.m in Sources */, 5A6D31EF1BF5325F009C5157 /* SUOperatingSystem.m in Sources */, + 392CD5D41CB6B51A00447781 /* SULog.m in Sources */, 72E45CF31B640CDD005C701A /* SUTestApplicationDelegate.m in Sources */, A5BF4F1D1BC7668B007A052A /* SUTestWebServer.m in Sources */, 72E45CF71B640DAE005C701A /* SUUpdateSettingsWindowController.m in Sources */, From f2acc94731b12c27ba4ca55c249f998d13968139 Mon Sep 17 00:00:00 2001 From: Alex Billingsley Date: Thu, 7 Apr 2016 16:37:37 -0400 Subject: [PATCH 4/5] Skip chown when trashing the files DyKnow/DyKnowMe#5647 --- Sparkle/SUFileManager.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sparkle/SUFileManager.m b/Sparkle/SUFileManager.m index 12cdc86d9..d36578f0a 100644 --- a/Sparkle/SUFileManager.m +++ b/Sparkle/SUFileManager.m @@ -897,11 +897,11 @@ - (BOOL)moveItemAtURLToTrash:(NSURL *)url error:(NSError *__autoreleasing *)erro return NO; } - if (![self changeOwnerAndGroupOfItemAtRootURL:tempItemURL toMatchURL:trashURL error:error]) { + /*if (![self changeOwnerAndGroupOfItemAtRootURL:tempItemURL toMatchURL:trashURL error:error]) { // Removing the item inside of the temp directory is better than trying to move the item to the trash with incorrect ownership [self removeItemAtURL:tempDirectory error:NULL]; return NO; - } + }*/ // If we get here, we should be able to trash the item normally without authentication From aeb1397adb35f603dbd3c93dbebce10cd8430853 Mon Sep 17 00:00:00 2001 From: Joel Dart Date: Wed, 13 Apr 2016 11:06:07 -0400 Subject: [PATCH 5/5] (dyknow/dyknowme#5647) yet more logging (dyknow/dyknowme#5647) yet more logging --- Sparkle/SUFileManager.m | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Sparkle/SUFileManager.m b/Sparkle/SUFileManager.m index d36578f0a..efc27118c 100644 --- a/Sparkle/SUFileManager.m +++ b/Sparkle/SUFileManager.m @@ -627,6 +627,8 @@ - (BOOL)changeOwnerAndGroupOfItemAtRootURL:(NSURL *)targetURL toMatchURL:(NSURL // Assume they're the same even if we don't check every file recursively // Speeds up the common case return YES; + } else { + SULog(@"Owner and group not equal targetOwner: %@ and targetGroup %@ ownerId: %@ groupId: %@", targetOwnerID, targetGroupID, ownerID, groupID); } BOOL needsAuth = NO;