From 11f8fcc80fe312ea5526056b7359cc538a2b1a9c Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 14 Nov 2018 14:45:11 -0500 Subject: [PATCH 1/4] Use persistent HTTP connection for UD requests. --- .../Network/API/Requests/OWSRequestFactory.m | 6 ++ .../src/Network/API/Requests/TSRequest.h | 1 + .../src/Network/API/TSNetworkManager.m | 101 ++++++++++++++++-- 3 files changed, 99 insertions(+), 9 deletions(-) diff --git a/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m b/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m index 745a4b9f6..2ef8041dc 100644 --- a/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m +++ b/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m @@ -419,6 +419,8 @@ NS_ASSUME_NONNULL_BEGIN // Don't bother with the default cookie store; // these cookies are ephemeral. + // + // NOTE: TSNetworkManager now separately disables default cookie handling for all requests. [request setHTTPShouldHandleCookies:NO]; return request; @@ -451,6 +453,8 @@ NS_ASSUME_NONNULL_BEGIN // Don't bother with the default cookie store; // these cookies are ephemeral. + // + // NOTE: TSNetworkManager now separately disables default cookie handling for all requests. [request setHTTPShouldHandleCookies:NO]; // Set the cookie header. OWSAssertDebug(request.allHTTPHeaderFields.count == 0); @@ -489,6 +493,8 @@ NS_ASSUME_NONNULL_BEGIN // Add UD auth header. [request setValue:[udAccessKey.keyData base64EncodedString] forHTTPHeaderField:@"Unidentified-Access-Key"]; + + request.isUDRequest = YES; } @end diff --git a/SignalServiceKit/src/Network/API/Requests/TSRequest.h b/SignalServiceKit/src/Network/API/Requests/TSRequest.h index b56e113dd..069a706d0 100644 --- a/SignalServiceKit/src/Network/API/Requests/TSRequest.h +++ b/SignalServiceKit/src/Network/API/Requests/TSRequest.h @@ -6,6 +6,7 @@ @interface TSRequest : NSMutableURLRequest +@property (nonatomic) BOOL isUDRequest; @property (nonatomic) BOOL shouldHaveAuthorizationHeaders; @property (atomic, nullable) NSString *authUsername; @property (atomic, nullable) NSString *authPassword; diff --git a/SignalServiceKit/src/Network/API/TSNetworkManager.m b/SignalServiceKit/src/Network/API/TSNetworkManager.m index 8cd7675ce..8be27e4df 100644 --- a/SignalServiceKit/src/Network/API/TSNetworkManager.m +++ b/SignalServiceKit/src/Network/API/TSNetworkManager.m @@ -25,12 +25,17 @@ BOOL IsNSErrorNetworkFailure(NSError *_Nullable error) @interface TSNetworkManager () +// This property should only be accessed on udSerialQueue. +@property (atomic, readonly) AFHTTPSessionManager *udSessionManager; + typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); @end @implementation TSNetworkManager +@synthesize udSessionManager = _udSessionManager; + #pragma mark Singleton implementation + (instancetype)sharedManager @@ -61,6 +66,17 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); return [self makeRequest:request completionQueue:dispatch_get_main_queue() success:success failure:failure]; } +- (dispatch_queue_t)udSerialQueue +{ + static dispatch_queue_t _serialQueue; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + _serialQueue = dispatch_queue_create("org.whispersystems.networkManager.udQueue", DISPATCH_QUEUE_SERIAL); + }); + + return _serialQueue; +} + - (void)makeRequest:(TSRequest *)request completionQueue:(dispatch_queue_t)completionQueue success:(TSNetworkManagerSuccess)successBlock @@ -70,9 +86,15 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); OWSAssertDebug(successBlock); OWSAssertDebug(failureBlock); - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - [self makeRequestSync:request completionQueue:completionQueue success:successBlock failure:failureBlock]; - }); + if (request.isUDRequest) { + dispatch_async(self.udSerialQueue, ^{ + [self makeUDRequestSync:request success:successBlock failure:failureBlock]; + }); + } else { + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ + [self makeRequestSync:request completionQueue:completionQueue success:successBlock failure:failureBlock]; + }); + } } - (void)makeRequestSync:(TSRequest *)request @@ -84,11 +106,11 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); OWSAssertDebug(successBlock); OWSAssertDebug(failureBlock); - OWSLogInfo(@"Making request: %@", request); + OWSLogInfo(@"Making Non-UD request: %@", request); // TODO: Remove this logging when the call connection issues have been resolved. TSNetworkManagerSuccess success = ^(NSURLSessionDataTask *task, _Nullable id responseObject) { - OWSLogInfo(@"request succeeded : %@", request); + OWSLogInfo(@"UD request succeeded : %@", request); if (request.shouldHaveAuthorizationHeaders) { [TSAccountManager.sharedInstance setIsDeregistered:NO]; @@ -110,10 +132,8 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); password:request.authPassword]; } - // Honor the request's preferences about default cookie handling. - // - // Default is YES. - sessionManager.requestSerializer.HTTPShouldHandleCookies = request.HTTPShouldHandleCookies; + // Disable default cookie handling for all requests. + sessionManager.requestSerializer.HTTPShouldHandleCookies = NO; // Honor the request's headers. for (NSString *headerField in request.allHTTPHeaderFields) { @@ -121,6 +141,69 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); [sessionManager.requestSerializer setValue:headerValue forHTTPHeaderField:headerField]; } + [self performRequest:request sessionManager:sessionManager success:success failure:failure]; +} + +// This method should only be invoked on udSerialQueue. +- (AFHTTPSessionManager *)udSessionManager +{ + if (!_udSessionManager) { + _udSessionManager = [OWSSignalService sharedInstance].signalServiceSessionManager; + _udSessionManager.completionQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); + // Disable default cookie handling for all requests. + _udSessionManager.requestSerializer.HTTPShouldHandleCookies = NO; + _udSessionManager.session.configuration.HTTPShouldSetCookies = NO; + _udSessionManager.session.configuration.HTTPShouldUsePipelining = YES; + } + + return _udSessionManager; +} + +- (void)makeUDRequestSync:(TSRequest *)request + success:(TSNetworkManagerSuccess)successBlock + failure:(TSNetworkManagerFailure)failureBlock +{ + OWSAssertDebug(request); + OWSAssert(!request.shouldHaveAuthorizationHeaders); + OWSAssertDebug(successBlock); + OWSAssertDebug(failureBlock); + + OWSLogInfo(@"Making UD request: %@", request); + uint64_t before = [NSDate ows_millisecondTimeStamp]; + + TSNetworkManagerSuccess success = ^(NSURLSessionDataTask *task, _Nullable id responseObject) { + OWSLogInfo(@"Non-UD request succeeded : %@", request); + + successBlock(task, responseObject); + + [OutageDetection.sharedManager reportConnectionSuccess]; + }; + TSNetworkManagerFailure failure = [TSNetworkManager errorPrettifyingForFailureBlock:failureBlock request:request]; + + AFHTTPSessionManager *sessionManager = self.udSessionManager; + + // Honor the request's headers. + for (NSString *headerField in sessionManager.requestSerializer.HTTPRequestHeaders.allKeys.copy) { + [sessionManager.requestSerializer setValue:nil forHTTPHeaderField:headerField]; + } + for (NSString *headerField in request.allHTTPHeaderFields) { + NSString *headerValue = request.allHTTPHeaderFields[headerField]; + [sessionManager.requestSerializer setValue:headerValue forHTTPHeaderField:headerField]; + } + + [self performRequest:request sessionManager:sessionManager success:success failure:failure]; +} + +- (void)performRequest:(TSRequest *)request + sessionManager:(AFHTTPSessionManager *)sessionManager + success:(TSNetworkManagerSuccess)success + failure:(TSNetworkManagerFailure)failure +{ + OWSAssertDebug(request); + OWSAssertDebug(sessionManager); + OWSAssertDebug(success); + OWSAssertDebug(failure); + if ([request.HTTPMethod isEqualToString:@"GET"]) { [sessionManager GET:request.URL.absoluteString parameters:request.parameters From c0e57bb35b82f78715c773941eebe0c8e8e6858a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 14 Nov 2018 14:48:47 -0500 Subject: [PATCH 2/4] Use persistent HTTP connection for UD requests. --- SignalServiceKit/src/Network/API/TSNetworkManager.m | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/SignalServiceKit/src/Network/API/TSNetworkManager.m b/SignalServiceKit/src/Network/API/TSNetworkManager.m index 8be27e4df..2aacd7749 100644 --- a/SignalServiceKit/src/Network/API/TSNetworkManager.m +++ b/SignalServiceKit/src/Network/API/TSNetworkManager.m @@ -152,8 +152,7 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); _udSessionManager.completionQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); // Disable default cookie handling for all requests. _udSessionManager.requestSerializer.HTTPShouldHandleCookies = NO; - _udSessionManager.session.configuration.HTTPShouldSetCookies = NO; - _udSessionManager.session.configuration.HTTPShouldUsePipelining = YES; + // NOTE: We could enable HTTPShouldUsePipelining here. } return _udSessionManager; From bbdeeffc76375d686a51bf6b5c955447349f4729 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 14 Nov 2018 14:54:37 -0500 Subject: [PATCH 3/4] Use persistent HTTP connection for UD requests. --- SignalServiceKit/src/Network/API/TSNetworkManager.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/SignalServiceKit/src/Network/API/TSNetworkManager.m b/SignalServiceKit/src/Network/API/TSNetworkManager.m index 2aacd7749..6ff1955da 100644 --- a/SignalServiceKit/src/Network/API/TSNetworkManager.m +++ b/SignalServiceKit/src/Network/API/TSNetworkManager.m @@ -110,7 +110,7 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); // TODO: Remove this logging when the call connection issues have been resolved. TSNetworkManagerSuccess success = ^(NSURLSessionDataTask *task, _Nullable id responseObject) { - OWSLogInfo(@"UD request succeeded : %@", request); + OWSLogInfo(@"Non-UD request succeeded : %@", request); if (request.shouldHaveAuthorizationHeaders) { [TSAccountManager.sharedInstance setIsDeregistered:NO]; @@ -171,7 +171,7 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); uint64_t before = [NSDate ows_millisecondTimeStamp]; TSNetworkManagerSuccess success = ^(NSURLSessionDataTask *task, _Nullable id responseObject) { - OWSLogInfo(@"Non-UD request succeeded : %@", request); + OWSLogInfo(@"UD request succeeded : %@", request); successBlock(task, responseObject); From 949225d52504a69ff091209bd67ddc02f1ab6007 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 14 Nov 2018 16:35:11 -0500 Subject: [PATCH 4/4] Respond to CR. --- .../src/Network/API/TSNetworkManager.m | 21 +++++-------------- .../src/Network/OWSSignalService.m | 4 ++++ 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/SignalServiceKit/src/Network/API/TSNetworkManager.m b/SignalServiceKit/src/Network/API/TSNetworkManager.m index 6ff1955da..03ef7f052 100644 --- a/SignalServiceKit/src/Network/API/TSNetworkManager.m +++ b/SignalServiceKit/src/Network/API/TSNetworkManager.m @@ -28,6 +28,8 @@ BOOL IsNSErrorNetworkFailure(NSError *_Nullable error) // This property should only be accessed on udSerialQueue. @property (atomic, readonly) AFHTTPSessionManager *udSessionManager; +@property (atomic, readonly) dispatch_queue_t udSerialQueue; + typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); @end @@ -35,6 +37,7 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); @implementation TSNetworkManager @synthesize udSessionManager = _udSessionManager; +@synthesize udSerialQueue = _udSerialQueue; #pragma mark Singleton implementation @@ -52,6 +55,8 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); return self; } + _udSerialQueue = dispatch_queue_create("org.whispersystems.networkManager.udQueue", DISPATCH_QUEUE_SERIAL); + OWSSingletonAssert(); return self; @@ -66,17 +71,6 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); return [self makeRequest:request completionQueue:dispatch_get_main_queue() success:success failure:failure]; } -- (dispatch_queue_t)udSerialQueue -{ - static dispatch_queue_t _serialQueue; - static dispatch_once_t onceToken; - dispatch_once(&onceToken, ^{ - _serialQueue = dispatch_queue_create("org.whispersystems.networkManager.udQueue", DISPATCH_QUEUE_SERIAL); - }); - - return _serialQueue; -} - - (void)makeRequest:(TSRequest *)request completionQueue:(dispatch_queue_t)completionQueue success:(TSNetworkManagerSuccess)successBlock @@ -132,9 +126,6 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); password:request.authPassword]; } - // Disable default cookie handling for all requests. - sessionManager.requestSerializer.HTTPShouldHandleCookies = NO; - // Honor the request's headers. for (NSString *headerField in request.allHTTPHeaderFields) { NSString *headerValue = request.allHTTPHeaderFields[headerField]; @@ -150,8 +141,6 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); if (!_udSessionManager) { _udSessionManager = [OWSSignalService sharedInstance].signalServiceSessionManager; _udSessionManager.completionQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); - // Disable default cookie handling for all requests. - _udSessionManager.requestSerializer.HTTPShouldHandleCookies = NO; // NOTE: We could enable HTTPShouldUsePipelining here. } diff --git a/SignalServiceKit/src/Network/OWSSignalService.m b/SignalServiceKit/src/Network/OWSSignalService.m index 0b203532e..fd8a2fc3d 100644 --- a/SignalServiceKit/src/Network/OWSSignalService.m +++ b/SignalServiceKit/src/Network/OWSSignalService.m @@ -168,6 +168,8 @@ NSString *const kNSNotificationName_IsCensorshipCircumventionActiveDidChange = sessionManager.securityPolicy = [OWSHTTPSecurityPolicy sharedPolicy]; sessionManager.requestSerializer = [AFJSONRequestSerializer serializer]; sessionManager.responseSerializer = [AFJSONResponseSerializer serializer]; + // Disable default cookie handling for all requests. + sessionManager.requestSerializer.HTTPShouldHandleCookies = NO; return sessionManager; } @@ -186,6 +188,8 @@ NSString *const kNSNotificationName_IsCensorshipCircumventionActiveDidChange = sessionManager.requestSerializer = [AFJSONRequestSerializer serializer]; [sessionManager.requestSerializer setValue:self.censorshipConfiguration.signalServiceReflectorHost forHTTPHeaderField:@"Host"]; sessionManager.responseSerializer = [AFJSONResponseSerializer serializer]; + // Disable default cookie handling for all requests. + sessionManager.requestSerializer.HTTPShouldHandleCookies = NO; return sessionManager; }