From 799949e546fb7f439667d41336df0cf1e9b79db0 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 2 Feb 2018 10:56:16 -0500 Subject: [PATCH] Refine sync messages. --- .../src/Contacts/Threads/TSGroupThread.h | 1 - .../src/Contacts/Threads/TSGroupThread.m | 11 ---- .../src/Devices/OWSGroupsOutputStream.h | 7 +- .../src/Devices/OWSGroupsOutputStream.m | 9 ++- .../Messages/Attachments/TSAttachmentStream.h | 3 +- .../Messages/Attachments/TSAttachmentStream.m | 66 ++++++++++++------- .../DeviceSyncing/OWSSyncGroupsMessage.m | 4 +- .../Messages/Interactions/TSOutgoingMessage.m | 16 +++-- .../src/Messages/OWSMessageManager.m | 13 ++++ 9 files changed, 78 insertions(+), 52 deletions(-) diff --git a/SignalServiceKit/src/Contacts/Threads/TSGroupThread.h b/SignalServiceKit/src/Contacts/Threads/TSGroupThread.h index 8d11b4ab7..d5338ac64 100644 --- a/SignalServiceKit/src/Contacts/Threads/TSGroupThread.h +++ b/SignalServiceKit/src/Contacts/Threads/TSGroupThread.h @@ -22,7 +22,6 @@ NS_ASSUME_NONNULL_BEGIN + (instancetype)getOrCreateThreadWithGroupId:(NSData *)groupId transaction:(YapDatabaseReadWriteTransaction *)transaction; -+ (nullable instancetype)threadWithGroupId:(NSData *)groupId; + (nullable instancetype)threadWithGroupId:(NSData *)groupId transaction:(YapDatabaseReadTransaction *)transaction; + (NSString *)threadIdFromGroupId:(NSData *)groupId; diff --git a/SignalServiceKit/src/Contacts/Threads/TSGroupThread.m b/SignalServiceKit/src/Contacts/Threads/TSGroupThread.m index a61f675ac..7fbdd9493 100644 --- a/SignalServiceKit/src/Contacts/Threads/TSGroupThread.m +++ b/SignalServiceKit/src/Contacts/Threads/TSGroupThread.m @@ -58,17 +58,6 @@ NS_ASSUME_NONNULL_BEGIN return self; } -+ (nullable instancetype)threadWithGroupId:(NSData *)groupId; -{ - OWSAssert(groupId.length > 0); - - __block TSGroupThread *thread; - [[self dbReadWriteConnection] readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - thread = [self threadWithGroupId:groupId transaction:transaction]; - }]; - return thread; -} - + (nullable instancetype)threadWithGroupId:(NSData *)groupId transaction:(YapDatabaseReadTransaction *)transaction { OWSAssert(groupId.length > 0); diff --git a/SignalServiceKit/src/Devices/OWSGroupsOutputStream.h b/SignalServiceKit/src/Devices/OWSGroupsOutputStream.h index 9d5ce70e8..f8909bc70 100644 --- a/SignalServiceKit/src/Devices/OWSGroupsOutputStream.h +++ b/SignalServiceKit/src/Devices/OWSGroupsOutputStream.h @@ -1,14 +1,17 @@ -// Copyright © 2016 Open Whisper Systems. All rights reserved. +// +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. +// #import "OWSChunkedOutputStream.h" NS_ASSUME_NONNULL_BEGIN @class TSGroupModel; +@class YapDatabaseReadTransaction; @interface OWSGroupsOutputStream : OWSChunkedOutputStream -- (void)writeGroup:(TSGroupModel *)group; +- (void)writeGroup:(TSGroupModel *)group transaction:(YapDatabaseReadTransaction *)transaction; @end diff --git a/SignalServiceKit/src/Devices/OWSGroupsOutputStream.m b/SignalServiceKit/src/Devices/OWSGroupsOutputStream.m index 4c8f5abd6..ac9750049 100644 --- a/SignalServiceKit/src/Devices/OWSGroupsOutputStream.m +++ b/SignalServiceKit/src/Devices/OWSGroupsOutputStream.m @@ -14,8 +14,11 @@ NS_ASSUME_NONNULL_BEGIN @implementation OWSGroupsOutputStream -- (void)writeGroup:(TSGroupModel *)group +- (void)writeGroup:(TSGroupModel *)group transaction:(YapDatabaseReadTransaction *)transaction { + OWSAssert(group); + OWSAssert(transaction); + OWSSignalServiceProtosGroupDetailsBuilder *groupBuilder = [OWSSignalServiceProtosGroupDetailsBuilder new]; [groupBuilder setId:group.groupId]; [groupBuilder setName:group.groupName]; @@ -37,10 +40,10 @@ NS_ASSUME_NONNULL_BEGIN [self.delegateStream writeRawVarint32:groupDataLength]; [self.delegateStream writeRawData:groupData]; - TSGroupThread *_Nullable groupThread = [TSGroupThread threadWithGroupId:group.groupId]; + TSGroupThread *_Nullable groupThread = [TSGroupThread threadWithGroupId:group.groupId transaction:transaction]; if (groupThread) { OWSDisappearingMessagesConfiguration *_Nullable disappearingMessagesConfiguration = - [OWSDisappearingMessagesConfiguration fetchObjectWithUniqueID:groupThread.uniqueId]; + [OWSDisappearingMessagesConfiguration fetchObjectWithUniqueID:groupThread.uniqueId transaction:transaction]; if (disappearingMessagesConfiguration && disappearingMessagesConfiguration.isEnabled) { [groupBuilder setExpireTimer:disappearingMessagesConfiguration.durationSeconds]; diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h index 21cd8eb98..acfd9b203 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h @@ -1,5 +1,5 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // #import "DataSource.h" @@ -53,6 +53,7 @@ NS_ASSUME_NONNULL_BEGIN + (void)deleteAttachments; + (NSString *)attachmentsFolder; +- (BOOL)shouldHaveImageSize; - (CGSize)imageSize; - (CGFloat)audioDurationSeconds; diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index e07c06613..dd98b1902 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -19,9 +19,11 @@ NS_ASSUME_NONNULL_BEGIN // changes in the file path generation logic don't break existing attachments. @property (nullable, nonatomic) NSString *localRelativeFilePath; -// These properties should only be accessed on the main thread. +// These properties should only be accessed while synchronized on self. @property (nullable, nonatomic) NSNumber *cachedImageWidth; @property (nullable, nonatomic) NSNumber *cachedImageHeight; + +// This property should only be accessed on the main thread. @property (nullable, nonatomic) NSNumber *cachedAudioDurationSeconds; @end @@ -105,8 +107,11 @@ NS_ASSUME_NONNULL_BEGIN if (attachmentSchemaVersion < 4) { // Legacy image sizes don't correctly reflect image orientation. - self.cachedImageWidth = nil; - self.cachedImageHeight = nil; + @synchronized(self) + { + self.cachedImageWidth = nil; + self.cachedImageHeight = nil; + } } } @@ -390,35 +395,46 @@ NS_ASSUME_NONNULL_BEGIN } } +- (BOOL)shouldHaveImageSize +{ + return ([self isVideo] || [self isImage] || [self isAnimated]); +} + - (CGSize)imageSize { - OWSAssertIsOnMainThread(); + OWSAssert(self.shouldHaveImageSize); - if (self.cachedImageWidth && self.cachedImageHeight) { - return CGSizeMake(self.cachedImageWidth.floatValue, self.cachedImageHeight.floatValue); - } + @synchronized(self) + { + if (self.cachedImageWidth && self.cachedImageHeight) { + return CGSizeMake(self.cachedImageWidth.floatValue, self.cachedImageHeight.floatValue); + } - CGSize imageSize = [self calculateImageSize]; - self.cachedImageWidth = @(imageSize.width); - self.cachedImageHeight = @(imageSize.height); + CGSize imageSize = [self calculateImageSize]; + if (imageSize.width <= 0 || imageSize.height <= 0) { + return CGSizeZero; + } + self.cachedImageWidth = @(imageSize.width); + self.cachedImageHeight = @(imageSize.height); - [self.dbReadWriteConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [self.dbReadWriteConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - NSString *collection = [[self class] collection]; - TSAttachmentStream *latestInstance = [transaction objectForKey:self.uniqueId inCollection:collection]; - if (latestInstance) { - latestInstance.cachedImageWidth = @(imageSize.width); - latestInstance.cachedImageHeight = @(imageSize.height); - [latestInstance saveWithTransaction:transaction]; - } else { - // This message has not yet been saved or has been deleted; do nothing. - // This isn't an error per se, but these race conditions should be - // _very_ rare. - OWSFail(@"%@ Attachment not yet saved.", self.logTag); - } - }]; + NSString *collection = [[self class] collection]; + TSAttachmentStream *latestInstance = [transaction objectForKey:self.uniqueId inCollection:collection]; + if (latestInstance) { + latestInstance.cachedImageWidth = @(imageSize.width); + latestInstance.cachedImageHeight = @(imageSize.height); + [latestInstance saveWithTransaction:transaction]; + } else { + // This message has not yet been saved or has been deleted; do nothing. + // This isn't an error per se, but these race conditions should be + // _very_ rare. + OWSFail(@"%@ Attachment not yet saved.", self.logTag); + } + }]; - return imageSize; + return imageSize; + } } - (CGFloat)calculateAudioDurationSeconds diff --git a/SignalServiceKit/src/Messages/DeviceSyncing/OWSSyncGroupsMessage.m b/SignalServiceKit/src/Messages/DeviceSyncing/OWSSyncGroupsMessage.m index 651a7edb7..fcf11fdeb 100644 --- a/SignalServiceKit/src/Messages/DeviceSyncing/OWSSyncGroupsMessage.m +++ b/SignalServiceKit/src/Messages/DeviceSyncing/OWSSyncGroupsMessage.m @@ -1,5 +1,5 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // #import "OWSSyncGroupsMessage.h" @@ -57,7 +57,7 @@ NS_ASSUME_NONNULL_BEGIN return; } TSGroupModel *group = ((TSGroupThread *)obj).groupModel; - [groupsOutputStream writeGroup:group]; + [groupsOutputStream writeGroup:group transaction:transaction]; }]; [groupsOutputStream flush]; diff --git a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m index b7003f105..fe60d0dfa 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m @@ -535,13 +535,15 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec [builder setDigest:attachmentStream.digest]; [builder setFlags:(self.isVoiceMessage ? OWSSignalServiceProtosAttachmentPointerFlagsVoiceMessage : 0)]; - CGSize imageSize = [attachmentStream imageSize]; - if (imageSize.width < NSIntegerMax && imageSize.height < NSIntegerMax) { - NSInteger imageWidth = (NSInteger)round(imageSize.width); - NSInteger imageHeight = (NSInteger)round(imageSize.height); - if (imageWidth > 0 && imageHeight > 0) { - [builder setWidth:(UInt32)imageWidth]; - [builder setHeight:(UInt32)imageHeight]; + if ([attachmentStream shouldHaveImageSize]) { + CGSize imageSize = [attachmentStream imageSize]; + if (imageSize.width < NSIntegerMax && imageSize.height < NSIntegerMax) { + NSInteger imageWidth = (NSInteger)round(imageSize.width); + NSInteger imageHeight = (NSInteger)round(imageSize.height); + if (imageWidth > 0 && imageHeight > 0) { + [builder setWidth:(UInt32)imageWidth]; + [builder setHeight:(UInt32)imageHeight]; + } } } diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 4835c3fe5..29c20a60f 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -312,6 +312,19 @@ NS_ASSUME_NONNULL_BEGIN OWSAssert(dataMessage); OWSAssert(transaction); + if (dataMessage.hasTimestamp) { + if (dataMessage.timestamp <= 0) { + DDLogError(@"%@ Ignoring message with invalid data message timestamp: %@", self.logTag, envelope.source); + return; + } + // This prevents replay attacks by the service. + if (dataMessage.timestamp != envelope.timestamp) { + DDLogError( + @"%@ Ignoring message with non-matching data message timestamp: %@", self.logTag, envelope.source); + return; + } + } + if ([dataMessage hasProfileKey]) { NSData *profileKey = [dataMessage profileKey]; NSString *recipientId = envelope.source;