From 758f37b886e3d770b4fda548c41bd24e7d521436 Mon Sep 17 00:00:00 2001 From: Niels Andriesse Date: Mon, 17 Feb 2020 10:29:43 +1100 Subject: [PATCH 1/2] Proxy profile picture uploads --- .../src/Loki/API/LokiDotNetAPI.swift | 6 +++--- .../src/Loki/API/LokiFileServerAPI.swift | 17 ++++------------- .../src/Loki/API/LokiFileServerProxy.swift | 2 +- 3 files changed, 8 insertions(+), 17 deletions(-) diff --git a/SignalServiceKit/src/Loki/API/LokiDotNetAPI.swift b/SignalServiceKit/src/Loki/API/LokiDotNetAPI.swift index e56c58471..4cb21c49e 100644 --- a/SignalServiceKit/src/Loki/API/LokiDotNetAPI.swift +++ b/SignalServiceKit/src/Loki/API/LokiDotNetAPI.swift @@ -100,10 +100,10 @@ public class LokiDotNetAPI : NSObject { return seal.reject(error) } // Send the request - func parseResponse(_ response: Any) { + func parseResponse(_ responseObject: Any) { // Parse the server ID & download URL - guard let json = response as? JSON, let data = json["data"] as? JSON, let serverID = data["id"] as? UInt64, let downloadURL = data["url"] as? String else { - print("[Loki] Couldn't parse attachment from: \(response).") + guard let json = responseObject as? JSON, let data = json["data"] as? JSON, let serverID = data["id"] as? UInt64, let downloadURL = data["url"] as? String else { + print("[Loki] Couldn't parse attachment from: \(responseObject).") return seal.reject(LokiDotNetAPIError.parsingFailed) } // Update the attachment diff --git a/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift b/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift index cc70bb566..f9b60faca 100644 --- a/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift +++ b/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift @@ -150,24 +150,15 @@ public final class LokiFileServerAPI : LokiDotNetAPI { print("[Loki] Couldn't upload profile picture due to error: \(error).") throw error } - let task = AFURLSessionManager(sessionConfiguration: .default).uploadTask(withStreamedRequest: request as URLRequest, progress: nil, completionHandler: { response, responseObject, error in - if let error = error { - print("[Loki] Couldn't upload profile picture due to error: \(error).") - return seal.reject(error) - } - let statusCode = (response as! HTTPURLResponse).statusCode - let isSuccessful = (200...299) ~= statusCode - guard isSuccessful else { - print("[Loki] Couldn't upload profile picture.") - return seal.reject(LokiDotNetAPIError.generic) - } + let _ = LokiFileServerProxy(for: server).performLokiFileServerNSURLRequest(request as NSURLRequest).done { responseObject in guard let json = responseObject as? JSON, let data = json["data"] as? JSON, let profilePicture = data["avatar_image"] as? JSON, let downloadURL = profilePicture["url"] as? String else { print("[Loki] Couldn't parse profile picture from: \(responseObject).") return seal.reject(LokiDotNetAPIError.parsingFailed) } return seal.fulfill(downloadURL) - }) - task.resume() + }.catch { error in + seal.reject(error) + } }.catch { error in print("[Loki] Couldn't upload profile picture due to error: \(error).") seal.reject(error) diff --git a/SignalServiceKit/src/Loki/API/LokiFileServerProxy.swift b/SignalServiceKit/src/Loki/API/LokiFileServerProxy.swift index a4ce4c848..d3a007f3b 100644 --- a/SignalServiceKit/src/Loki/API/LokiFileServerProxy.swift +++ b/SignalServiceKit/src/Loki/API/LokiFileServerProxy.swift @@ -106,7 +106,7 @@ internal class LokiFileServerProxy : LokiHTTPClient { print("[Loki] Received an invalid response.") throw Error.proxyResponseParsingFailed } - let isSuccess = (200..<300).contains(statusCode) + let isSuccess = (200...299) ~= statusCode guard isSuccess else { throw HTTPError.networkError(code: statusCode, response: nil, underlyingError: Error.fileServerHTTPError(code: statusCode, message: nil)) } let uncheckedJSONAsData = try DiffieHellman.decrypt(cipherText, using: symmetricKey) if uncheckedJSONAsData.isEmpty { return () } From 6def911dbc3397adced440c77187b2035e40d0f4 Mon Sep 17 00:00:00 2001 From: Niels Andriesse Date: Mon, 17 Feb 2020 10:46:43 +1100 Subject: [PATCH 2/2] Enforce file size limit for profile pictures --- Signal/Signal-Info.plist | 2 +- .../src/Loki/View Controllers/DisplayNameVC.swift | 2 +- Signal/src/Loki/View Controllers/SettingsVC.swift | 10 ++++++++-- Signal/src/ViewControllers/ProfileViewController.m | 2 +- .../OnboardingProfileViewController.swift | 2 +- Signal/src/util/Backup/OWSBackupImportJob.m | 2 +- SignalMessaging/profiles/OWSProfileManager.h | 2 +- SignalMessaging/profiles/OWSProfileManager.m | 14 +++++++------- .../src/Loki/API/LokiFileServerAPI.swift | 3 +++ 9 files changed, 24 insertions(+), 15 deletions(-) diff --git a/Signal/Signal-Info.plist b/Signal/Signal-Info.plist index 378a5f3ca..342d37adb 100644 --- a/Signal/Signal-Info.plist +++ b/Signal/Signal-Info.plist @@ -5,7 +5,7 @@ BuildDetails CarthageVersion - 0.33.0 + 0.34.0 OSXVersion 10.15.3 WebRTCCommit diff --git a/Signal/src/Loki/View Controllers/DisplayNameVC.swift b/Signal/src/Loki/View Controllers/DisplayNameVC.swift index ffbadd74a..264a5fd3d 100644 --- a/Signal/src/Loki/View Controllers/DisplayNameVC.swift +++ b/Signal/src/Loki/View Controllers/DisplayNameVC.swift @@ -155,7 +155,7 @@ final class DisplayNameVC : UIViewController { return showError(title: NSLocalizedString("Please pick a shorter display name", comment: "")) } TSAccountManager.sharedInstance().didRegister() - OWSProfileManager.shared().updateLocalProfileName(displayName, avatarImage: nil, success: { }, failure: { }) // Try to save the user name but ignore the result + OWSProfileManager.shared().updateLocalProfileName(displayName, avatarImage: nil, success: { }, failure: { _ in }) // Try to save the user name but ignore the result let homeVC = HomeVC() navigationController!.setViewControllers([ homeVC ], animated: true) } diff --git a/Signal/src/Loki/View Controllers/SettingsVC.swift b/Signal/src/Loki/View Controllers/SettingsVC.swift index acf7b3ab2..4e92331db 100644 --- a/Signal/src/Loki/View Controllers/SettingsVC.swift +++ b/Signal/src/Loki/View Controllers/SettingsVC.swift @@ -274,10 +274,16 @@ final class SettingsVC : UIViewController, AvatarViewHelperDelegate { self.displayNameToBeUploaded = nil } } - }, failure: { + }, failure: { error in DispatchQueue.main.async { modalActivityIndicator.dismiss { - let alert = UIAlertController(title: NSLocalizedString("Couldn't Update Profile", comment: ""), message: NSLocalizedString("Please check your internet connection and try again", comment: ""), preferredStyle: .alert) + var isMaxFileSizeExceeded = false + if let error = error as? LokiDotNetAPI.LokiDotNetAPIError { + isMaxFileSizeExceeded = (error == .maxFileSizeExceeded) + } + let title = isMaxFileSizeExceeded ? "Maximum File Size Exceeded" : NSLocalizedString("Couldn't Update Profile", comment: "") + let message = isMaxFileSizeExceeded ? "Please select a smaller photo and try again" : NSLocalizedString("Please check your internet connection and try again", comment: "") + let alert = UIAlertController(title: title, message: message, preferredStyle: .alert) alert.addAction(UIAlertAction(title: NSLocalizedString("OK", comment: ""), style: .default, handler: nil)) self?.present(alert, animated: true, completion: nil) } diff --git a/Signal/src/ViewControllers/ProfileViewController.m b/Signal/src/ViewControllers/ProfileViewController.m index 7234eeea8..536a69d45 100644 --- a/Signal/src/ViewControllers/ProfileViewController.m +++ b/Signal/src/ViewControllers/ProfileViewController.m @@ -419,7 +419,7 @@ NSString *const kProfileView_LastPresentedDate = @"kProfileView_LastPresentedDat }]; }); } - failure:^{ + failure:^(NSError *error) { dispatch_async(dispatch_get_main_queue(), ^{ [modalActivityIndicator dismissWithCompletion:^{ [OWSAlerts showErrorAlertWithMessage:NSLocalizedString( diff --git a/Signal/src/ViewControllers/Registration/OnboardingProfileViewController.swift b/Signal/src/ViewControllers/Registration/OnboardingProfileViewController.swift index 2e94c34b9..795bfd7e5 100644 --- a/Signal/src/ViewControllers/Registration/OnboardingProfileViewController.swift +++ b/Signal/src/ViewControllers/Registration/OnboardingProfileViewController.swift @@ -170,7 +170,7 @@ public class OnboardingProfileViewController: OnboardingBaseViewController { self.onboardingController.profileDidComplete(fromView: self) }) } - }, failure: { + }, failure: { _ in DispatchQueue.main.async { modal.dismiss(completion: { OWSAlerts.showErrorAlert(message: NSLocalizedString("PROFILE_VIEW_ERROR_UPDATE_FAILED", diff --git a/Signal/src/util/Backup/OWSBackupImportJob.m b/Signal/src/util/Backup/OWSBackupImportJob.m index 6ccf6f5de..2d94a7bc5 100644 --- a/Signal/src/util/Backup/OWSBackupImportJob.m +++ b/Signal/src/util/Backup/OWSBackupImportJob.m @@ -336,7 +336,7 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe success:^{ resolve(@(1)); } - failure:^{ + failure:^(NSError *error) { // Ignore errors related to local profile. resolve(@(1)); }]; diff --git a/SignalMessaging/profiles/OWSProfileManager.h b/SignalMessaging/profiles/OWSProfileManager.h index a0b5a379b..69d991906 100644 --- a/SignalMessaging/profiles/OWSProfileManager.h +++ b/SignalMessaging/profiles/OWSProfileManager.h @@ -50,7 +50,7 @@ extern const NSUInteger kOWSProfileManager_MaxAvatarDiameter; - (void)updateLocalProfileName:(nullable NSString *)profileName avatarImage:(nullable UIImage *)avatarImage success:(void (^)(void))successBlock - failure:(void (^)(void))failureBlock; + failure:(void (^)(NSError *))failureBlock; - (BOOL)isProfileNameTooLong:(nullable NSString *)profileName; diff --git a/SignalMessaging/profiles/OWSProfileManager.m b/SignalMessaging/profiles/OWSProfileManager.m index 386247089..bc4376d20 100644 --- a/SignalMessaging/profiles/OWSProfileManager.m +++ b/SignalMessaging/profiles/OWSProfileManager.m @@ -229,13 +229,13 @@ typedef void (^ProfileManagerFailureBlock)(NSError *error); - (void)updateLocalProfileName:(nullable NSString *)profileName avatarImage:(nullable UIImage *)avatarImage success:(void (^)(void))successBlockParameter - failure:(void (^)(void))failureBlockParameter + failure:(void (^)(NSError *))failureBlockParameter { OWSAssertDebug(successBlockParameter); OWSAssertDebug(failureBlockParameter); // Ensure that the success and failure blocks are called on the main thread. - void (^failureBlock)(void) = ^{ + void (^failureBlock)(NSError *) = ^(NSError *error) { OWSLogError(@"Updating service with profile failed."); // We use a "self-only" contact sync to indicate to desktop @@ -247,7 +247,7 @@ typedef void (^ProfileManagerFailureBlock)(NSError *error); [[self.syncManager syncLocalContact] retainUntilComplete]; dispatch_async(dispatch_get_main_queue(), ^{ - failureBlockParameter(); + failureBlockParameter(error); }); }; void (^successBlock)(void) = ^{ @@ -288,7 +288,7 @@ typedef void (^ProfileManagerFailureBlock)(NSError *error); }]; } failure:^(NSError *error) { - failureBlock(); + failureBlock(error); }]; }; @@ -319,11 +319,11 @@ typedef void (^ProfileManagerFailureBlock)(NSError *error); tryToUpdateService(avatarUrlPath, fileName); } failure:^(NSError *error) { - failureBlock(); + failureBlock(error); }]; } failure:^(NSError *error) { - failureBlock(); + failureBlock(error); }]; } } else if (userProfile.avatarUrlPath) { @@ -333,7 +333,7 @@ typedef void (^ProfileManagerFailureBlock)(NSError *error); tryToUpdateService(nil, nil); } failure:^(NSError *error) { - failureBlock(); + failureBlock(error); }]; } else { OWSLogVerbose(@"Updating local profile on service with no avatar."); diff --git a/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift b/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift index f9b60faca..e33c9a43f 100644 --- a/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift +++ b/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift @@ -138,6 +138,9 @@ public final class LokiFileServerAPI : LokiDotNetAPI { // MARK: Profile Pictures (Public API) public static func setProfilePicture(_ profilePicture: Data) -> Promise { return Promise() { seal in + guard profilePicture.count < maxFileSize else { + return seal.reject(LokiDotNetAPIError.maxFileSizeExceeded) + } getAuthToken(for: server).done { token in let url = "\(server)/users/me/avatar" let parameters: JSON = [ "type" : attachmentType, "Content-Type" : "application/binary" ]