diff --git a/src/Devices/OWSReadReceiptsProcessor.m b/src/Devices/OWSReadReceiptsProcessor.m index ac671641a..a41bcc6d6 100644 --- a/src/Devices/OWSReadReceiptsProcessor.m +++ b/src/Devices/OWSReadReceiptsProcessor.m @@ -1,4 +1,6 @@ -// Copyright © 2016 Open Whisper Systems. All rights reserved. +// +// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// #import "OWSReadReceiptsProcessor.h" #import "OWSDisappearingMessagesJob.h" @@ -91,7 +93,7 @@ NSString *const OWSReadReceiptsProcessorMarkedMessageAsReadNotification = object:message]; } else { - DDLogDebug(@"%@ Received read receipt for an unkown message. Saving it for later.", self.tag); + DDLogDebug(@"%@ Received read receipt for an unknown message. Saving it for later.", self.tag); [readReceipt save]; } } diff --git a/src/Messages/Attachments/OWSAttachmentsProcessor.m b/src/Messages/Attachments/OWSAttachmentsProcessor.m index b0e75b914..3683e78d4 100644 --- a/src/Messages/Attachments/OWSAttachmentsProcessor.m +++ b/src/Messages/Attachments/OWSAttachmentsProcessor.m @@ -119,6 +119,9 @@ NS_ASSUME_NONNULL_BEGIN } }; + if (attachment.serverId < 100) { + DDLogError(@"%@ Suspicious attachment id: %llu", self.tag, (unsigned long long)attachment.serverId); + } TSAttachmentRequest *attachmentRequest = [[TSAttachmentRequest alloc] initWithId:attachment.serverId relay:attachment.relay]; [self.networkManager makeRequest:attachmentRequest @@ -139,18 +142,48 @@ NS_ASSUME_NONNULL_BEGIN dispatch_async([OWSDispatch attachmentsQueue], ^{ [self downloadFromLocation:location - pointer:attachment - success:^(NSData *_Nonnull encryptedData) { - [self decryptAttachmentData:encryptedData - pointer:attachment - success:markAndHandleSuccess - failure:markAndHandleFailure]; - } - failure:markAndHandleFailure]; + pointer:attachment + success:^(NSData *_Nonnull encryptedData) { + [self decryptAttachmentData:encryptedData + pointer:attachment + success:markAndHandleSuccess + failure:markAndHandleFailure]; + } + failure:^(NSURLSessionDataTask *_Nullable task, NSError *_Nonnull error) { + if (attachment.serverId < 100) { + // This looks like the symptom of the "frequent 404 + // downloading attachments with low server ids". + NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)task.response; + NSInteger statusCode = [httpResponse statusCode]; + DDLogError(@"%@ %d Failure with suspicious attachment id: %llu, %@", + self.tag, + (int)statusCode, + (unsigned long long)attachment.serverId, + error); + [DDLog flushLog]; + OWSAssert(0); + } + if (markAndHandleFailure) { + markAndHandleFailure(error); + } + }]; }); } failure:^(NSURLSessionDataTask *task, NSError *error) { DDLogError(@"Failed retrieval of attachment with error: %@", error); + if (attachment.serverId < 100) { + // This _shouldn't_ be the symptom of the "frequent 404 + // downloading attachments with low server ids". + NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)task.response; + NSInteger statusCode = [httpResponse statusCode]; + DDLogError(@"%@ %d Failure with suspicious attachment id: %llu, %@", + self.tag, + (int)statusCode, + (unsigned long long)attachment.serverId, + error); + [DDLog flushLog]; + OWSAssert(0); + } return markAndHandleFailure(error); }]; } @@ -184,7 +217,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)downloadFromLocation:(NSString *)location pointer:(TSAttachmentPointer *)pointer success:(void (^)(NSData *encryptedData))successHandler - failure:(void (^)(NSError *error))failureHandler + failure:(void (^)(NSURLSessionDataTask *_Nullable task, NSError *_Nonnull error))failureHandler { AFHTTPSessionManager *manager = [AFHTTPSessionManager manager]; manager.requestSerializer = [AFHTTPRequestSerializer serializer]; @@ -200,13 +233,13 @@ NS_ASSUME_NONNULL_BEGIN if (![responseObject isKindOfClass:[NSData class]]) { DDLogError(@"%@ Failed retrieval of attachment. Response had unexpected format.", self.tag); NSError *error = OWSErrorMakeUnableToProcessServerResponseError(); - return failureHandler(error); + return failureHandler(task, error); } successHandler((NSData *)responseObject); } failure:^(NSURLSessionDataTask *_Nullable task, NSError *_Nonnull error) { DDLogError(@"Failed to retrieve attachment with error: %@", error.description); - return failureHandler(error); + return failureHandler(task, error); }]; } diff --git a/src/Messages/Attachments/TSAttachment.h b/src/Messages/Attachments/TSAttachment.h index 67f1dd274..150fa7199 100644 --- a/src/Messages/Attachments/TSAttachment.h +++ b/src/Messages/Attachments/TSAttachment.h @@ -6,20 +6,42 @@ NS_ASSUME_NONNULL_BEGIN +@class TSAttachmentPointer; + @interface TSAttachment : TSYapDatabaseObject { @protected NSString *_contentType; } +// TSAttachment is a base class for TSAttachmentPointer (a yet-to-be-downloaded +// incoming attachment) and TSAttachmentStream (an outgoing or already-downloaded +// incoming attachment). +// +// The attachmentSchemaVersion and serverId properties only apply to +// TSAttachmentPointer, which can be distinguished by the isDownloaded +// property. @property (atomic, readwrite) UInt64 serverId; @property (atomic, readwrite) NSData *encryptionKey; @property (nonatomic, readonly) NSString *contentType; +// This property + +@property (atomic, readwrite) BOOL isDownloaded; +// This constructor is used for new instances of TSAttachmentPointer, +// i.e. undownloaded incoming attachments. - (instancetype)initWithServerId:(UInt64)serverId encryptionKey:(NSData *)encryptionKey contentType:(NSString *)contentType; +// This constructor is used for new instances of TSAttachmentStream +// that represent new, un-uploaded outgoing attachments. +- (instancetype)initWithContentType:(NSString *)contentType; + +// This constructor is used for new instances of TSAttachmentStream +// that represent downloaded incoming attachments. +- (instancetype)initWithPointer:(TSAttachmentPointer *)pointer; + - (void)upgradeFromAttachmentSchemaVersion:(NSUInteger)attachmentSchemaVersion; @end diff --git a/src/Messages/Attachments/TSAttachment.m b/src/Messages/Attachments/TSAttachment.m index c955fbff4..3fcbd3946 100644 --- a/src/Messages/Attachments/TSAttachment.m +++ b/src/Messages/Attachments/TSAttachment.m @@ -4,6 +4,7 @@ #import "TSAttachment.h" #import "MIMETypeUtil.h" +#import "TSAttachmentPointer.h" NS_ASSUME_NONNULL_BEGIN @@ -17,6 +18,8 @@ NSUInteger const TSAttachmentSchemaVersion = 3; @implementation TSAttachment +// This constructor is used for new instances of TSAttachmentPointer, +// i.e. undownloaded incoming attachments. - (instancetype)initWithServerId:(UInt64)serverId encryptionKey:(NSData *)encryptionKey contentType:(NSString *)contentType @@ -34,6 +37,44 @@ NSUInteger const TSAttachmentSchemaVersion = 3; return self; } +// This constructor is used for new instances of TSAttachmentStream +// that represent new, un-uploaded outgoing attachments. +- (instancetype)initWithContentType:(NSString *)contentType +{ + self = [super init]; + if (!self) { + return self; + } + + _contentType = contentType; + _attachmentSchemaVersion = TSAttachmentSchemaVersion; + + return self; +} + +// This constructor is used for new instances of TSAttachmentStream +// that represent downloaded incoming attachments. +- (instancetype)initWithPointer:(TSAttachmentPointer *)pointer +{ + // Once saved, this AttachmentStream will replace the AttachmentPointer in the attachments collection. + self = [super initWithUniqueId:pointer.uniqueId]; + if (!self) { + return self; + } + + _serverId = pointer.serverId; + _encryptionKey = pointer.encryptionKey; + _contentType = pointer.contentType; + _attachmentSchemaVersion = TSAttachmentSchemaVersion; + + return self; +} + +- (BOOL)isDecimalNumberText:(NSString *)text +{ + return [text componentsSeparatedByCharactersInSet:[NSCharacterSet decimalDigitCharacterSet]].count == 1; +} + - (nullable instancetype)initWithCoder:(NSCoder *)coder { self = [super initWithCoder:coder]; @@ -51,11 +92,22 @@ NSUInteger const TSAttachmentSchemaVersion = 3; - (void)upgradeFromAttachmentSchemaVersion:(NSUInteger)attachmentSchemaVersion { - if (attachmentSchemaVersion < 2) { + // TSAttachment is a base class for TSAttachmentPointer (a yet-to-be-downloaded + // incoming attachment) and TSAttachmentStream (an outgoing or already-downloaded + // incoming attachment). + // + // The attachmentSchemaVersion and serverId properties only apply to + // TSAttachmentPointer, which can be distinguished by the isDownloaded + // property. + if (!_isDownloaded && _attachmentSchemaVersion < 2) { if (!_serverId) { + OWSAssert([self isDecimalNumberText:self.uniqueId]); + if (![self isDecimalNumberText:self.uniqueId]) { + DDLogError(@"%@ invalid legacy attachment uniqueId: %@.", self.tag, self.uniqueId); + } _serverId = [self.uniqueId integerValue]; if (!_serverId) { - DDLogError(@"%@ failed to parse legacy uniqueId:%@ as integer.", self.tag, self.uniqueId); + DDLogError(@"%@ failed to parse legacy attachment uniqueId: %@.", self.tag, self.uniqueId); } } } diff --git a/src/Messages/Attachments/TSAttachmentPointer.m b/src/Messages/Attachments/TSAttachmentPointer.m index 426878483..8701335f4 100644 --- a/src/Messages/Attachments/TSAttachmentPointer.m +++ b/src/Messages/Attachments/TSAttachmentPointer.m @@ -28,10 +28,6 @@ NS_ASSUME_NONNULL_BEGIN return self; } -- (BOOL)isDownloaded { - return NO; -} - @end NS_ASSUME_NONNULL_END diff --git a/src/Messages/Attachments/TSAttachmentStream.h b/src/Messages/Attachments/TSAttachmentStream.h index 8bd2b6a2b..6e4439b52 100644 --- a/src/Messages/Attachments/TSAttachmentStream.h +++ b/src/Messages/Attachments/TSAttachmentStream.h @@ -17,8 +17,6 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)initWithContentType:(NSString *)contentType NS_DESIGNATED_INITIALIZER; - (instancetype)initWithPointer:(TSAttachmentPointer *)pointer NS_DESIGNATED_INITIALIZER; -@property (atomic, readwrite) BOOL isDownloaded; - // Though now required, `digest` may be null for pre-existing records or from // messages received from other clients @property (nullable, nonatomic) NSData *digest; diff --git a/src/Messages/Attachments/TSAttachmentStream.m b/src/Messages/Attachments/TSAttachmentStream.m index a3dd9f364..7ce6e5bf4 100644 --- a/src/Messages/Attachments/TSAttachmentStream.m +++ b/src/Messages/Attachments/TSAttachmentStream.m @@ -14,13 +14,12 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)initWithContentType:(NSString *)contentType { - self = [super init]; + self = [super initWithContentType:contentType]; if (!self) { return self; } - _contentType = contentType; - _isDownloaded = YES; + self.isDownloaded = YES; // TSAttachmentStream doesn't have any "incoming vs. outgoing" // state, but this constructor is used only for new outgoing // attachments which haven't been uploaded yet. @@ -32,13 +31,13 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)initWithPointer:(TSAttachmentPointer *)pointer { // Once saved, this AttachmentStream will replace the AttachmentPointer in the attachments collection. - self = [super initWithUniqueId:pointer.uniqueId]; + self = [super initWithPointer:pointer]; if (!self) { return self; } _contentType = pointer.contentType; - _isDownloaded = YES; + self.isDownloaded = YES; // TSAttachmentStream doesn't have any "incoming vs. outgoing" // state, but this constructor is used only for new incoming // attachments which don't need to be uploaded. diff --git a/src/Network/WebSockets/TSSocketManager.m b/src/Network/WebSockets/TSSocketManager.m index 6b6a412b5..6d8d6846f 100644 --- a/src/Network/WebSockets/TSSocketManager.m +++ b/src/Network/WebSockets/TSSocketManager.m @@ -114,6 +114,8 @@ NSString *const SocketConnectingNotification = @"SocketConnectingNotification"; } } + DDLogWarn(@"Creating new websocket"); + // If socket is not already open or connecting, connect now. self.status = kSocketStatusConnecting; } @@ -229,6 +231,7 @@ NSString *const SocketConnectingNotification = @"SocketConnectingNotification"; + (void)resignActivity { OWSAssert([NSThread isMainThread]); + DDLogWarn(@"resignActivity closing web socket"); [[self sharedManager] closeWebSocket]; } @@ -253,8 +256,8 @@ NSString *const SocketConnectingNotification = @"SocketConnectingNotification"; - (void)webSocket:(SRWebSocket *)webSocket didFailWithError:(NSError *)error { OWSAssert([NSThread isMainThread]); - - DDLogError(@"Error connecting to socket %@", error); + + DDLogError(@"Websocket did fail with error: %@", error); [self closeWebSocket]; @@ -352,6 +355,8 @@ NSString *const SocketConnectingNotification = @"SocketConnectingNotification"; wasClean:(BOOL)wasClean { OWSAssert([NSThread isMainThread]); + DDLogWarn(@"Websocket did close with code: %ld", (long)code); + [self closeWebSocket]; if (!wasClean && [self shouldKeepWebSocketAlive]) { @@ -370,6 +375,7 @@ NSString *const SocketConnectingNotification = @"SocketConnectingNotification"; [self scheduleRetry]; } } else { + DDLogWarn(@"webSocketHeartBeat closing web socket"); [self closeWebSocket]; } }