From 060e0fd062de9845bc59fed010d9fded4721ee13 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 23 Jul 2018 16:40:01 -0400 Subject: [PATCH 1/6] Fix NPE using mock for unknown database objects. --- SignalServiceKit/src/Storage/OWSStorage.m | 42 ++++++++++++++++++----- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/SignalServiceKit/src/Storage/OWSStorage.m b/SignalServiceKit/src/Storage/OWSStorage.m index 459e10606..872b56f08 100644 --- a/SignalServiceKit/src/Storage/OWSStorage.m +++ b/SignalServiceKit/src/Storage/OWSStorage.m @@ -192,7 +192,7 @@ NSString *const kNSUserDefaults_DatabaseExtensionVersionMap = @"kNSUserDefaults_ #pragma mark - -@interface OWSUnknownDBObject : NSObject +@interface OWSUnknownDBObject : TSYapDatabaseObject @end @@ -205,17 +205,42 @@ NSString *const kNSUserDefaults_DatabaseExtensionVersionMap = @"kNSUserDefaults_ */ @implementation OWSUnknownDBObject -- (nullable instancetype)initWithCoder:(NSCoder *)aDecoder +- (void)encodeWithCoder:(NSCoder *)aCoder +{ + OWSProdLogAndFail(@"%@ Tried to save object from unknown collection", self.logTag); + + return [super encodeWithCoder:aCoder]; +} + +- (nullable instancetype)initWithCoder:(NSCoder *)coder { - // We return self instead of, e.g. nil, to avoid a crash when YapDB enumerates - // all old objects when building a DB extension. + self = [super initWithCoder:coder]; + if (!self) { + return self; + } + return self; } -- (void)encodeWithCoder:(NSCoder *)aCoder +- (void)saveWithTransaction:(YapDatabaseReadWriteTransaction *)transaction { - OWSRaiseException( - @"OWSStorageExceptionName_SaveToUnknownCollection", @"Tried to save object from unknown collection"); + OWSProdLogAndFail(@"%@ Tried to save unknown object", self.logTag); + + // No-op. +} + +- (void)touchWithTransaction:(YapDatabaseReadWriteTransaction *)transaction +{ + OWSProdLogAndFail(@"%@ Tried to touch unknown object", self.logTag); + + // No-op. +} + +- (void)removeWithTransaction:(YapDatabaseReadWriteTransaction *)transaction +{ + OWSProdLogAndFail(@"%@ Tried to remove unknown object", self.logTag); + + // No-op. } @end @@ -457,7 +482,8 @@ NSString *const kNSUserDefaults_DatabaseExtensionVersionMap = @"kNSUserDefaults_ return ^id(NSString __unused *collection, NSString __unused *key, NSData *data) { if (!data || data.length <= 0) { - return nil; + OWSProdLogAndFail(@"%@ can't deserializing null object: %@", self.logTag, collection); + return [OWSUnknownDBObject new]; } @try { From 708ef6f7dd48409addfc81f37ef640fc27f1e510 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 23 Jul 2018 16:42:11 -0400 Subject: [PATCH 2/6] Fix NPE using mock for unknown database objects. --- SignalServiceKit/src/Storage/OWSStorage.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SignalServiceKit/src/Storage/OWSStorage.m b/SignalServiceKit/src/Storage/OWSStorage.m index 872b56f08..c7f589c17 100644 --- a/SignalServiceKit/src/Storage/OWSStorage.m +++ b/SignalServiceKit/src/Storage/OWSStorage.m @@ -482,7 +482,7 @@ NSString *const kNSUserDefaults_DatabaseExtensionVersionMap = @"kNSUserDefaults_ return ^id(NSString __unused *collection, NSString __unused *key, NSData *data) { if (!data || data.length <= 0) { - OWSProdLogAndFail(@"%@ can't deserializing null object: %@", self.logTag, collection); + OWSProdLogAndFail(@"%@ can't deserialize null object: %@", self.logTag, collection); return [OWSUnknownDBObject new]; } From 723691400f257b8e26c5f462215a82013f9b5d48 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 23 Jul 2018 16:50:12 -0400 Subject: [PATCH 3/6] Fix NPE using mock for unknown database objects. --- SignalServiceKit/src/Contacts/TSThread.m | 27 +++++++++++------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/SignalServiceKit/src/Contacts/TSThread.m b/SignalServiceKit/src/Contacts/TSThread.m index a8504165c..e86820e67 100644 --- a/SignalServiceKit/src/Contacts/TSThread.m +++ b/SignalServiceKit/src/Contacts/TSThread.m @@ -97,12 +97,14 @@ NS_ASSUME_NONNULL_BEGIN NSMutableArray *interactionIds = [NSMutableArray new]; YapDatabaseViewTransaction *interactionsByThread = [transaction ext:TSMessageDatabaseViewExtensionName]; OWSAssert(interactionsByThread); - [interactionsByThread - enumerateKeysInGroup:self.uniqueId - usingBlock:^( - NSString *_Nonnull collection, NSString *_Nonnull key, NSUInteger index, BOOL *_Nonnull stop) { - [interactionIds addObject:key]; - }]; + [interactionsByThread enumerateKeysInGroup:self.uniqueId + usingBlock:^(NSString *collection, NSString *key, NSUInteger index, BOOL *stop) { + if (key.length < 1) { + OWSProdLogAndFail(@"%@ invalid key in thread interactions.", self.logTag); + return; + } + [interactionIds addObject:key]; + }]; for (NSString *interactionId in interactionIds) { // We need to fetch each interaction, since [TSInteraction removeWithTransaction:] does important work. @@ -157,13 +159,8 @@ NS_ASSUME_NONNULL_BEGIN usingBlock:(void (^)(TSInteraction *interaction, YapDatabaseReadTransaction *transaction))block { - void (^interactionBlock)(NSString *, NSString *, id, id, NSUInteger, BOOL *) = ^void(NSString *_Nonnull collection, - NSString *_Nonnull key, - id _Nonnull object, - id _Nonnull metadata, - NSUInteger index, - BOOL *_Nonnull stop) { - + void (^interactionBlock)(NSString *, NSString *, id, id, NSUInteger, BOOL *) = ^void( + NSString *collection, NSString *key, id _Nonnull object, id _Nonnull metadata, NSUInteger index, BOOL *stop) { TSInteraction *interaction = object; block(interaction, transaction); }; @@ -194,7 +191,7 @@ NS_ASSUME_NONNULL_BEGIN - (NSArray *)allInteractions { NSMutableArray *interactions = [NSMutableArray new]; - [self enumerateInteractionsUsingBlock:^(TSInteraction *_Nonnull interaction) { + [self enumerateInteractionsUsingBlock:^(TSInteraction *interaction) { [interactions addObject:interaction]; }]; @@ -219,7 +216,7 @@ NS_ASSUME_NONNULL_BEGIN - (NSUInteger)numberOfInteractions { __block NSUInteger count; - [[self dbReadConnection] readWithBlock:^(YapDatabaseReadTransaction *_Nonnull transaction) { + [[self dbReadConnection] readWithBlock:^(YapDatabaseReadTransaction *transaction) { YapDatabaseViewTransaction *interactionsByThread = [transaction ext:TSMessageDatabaseViewExtensionName]; count = [interactionsByThread numberOfItemsInGroup:self.uniqueId]; }]; From 2c973782c4f500bc0b180727a134cf3465e8a4d2 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 23 Jul 2018 16:52:00 -0400 Subject: [PATCH 4/6] Fix NPE using mock for unknown database objects. --- SignalServiceKit/src/Contacts/TSThread.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SignalServiceKit/src/Contacts/TSThread.m b/SignalServiceKit/src/Contacts/TSThread.m index e86820e67..d6bcdb087 100644 --- a/SignalServiceKit/src/Contacts/TSThread.m +++ b/SignalServiceKit/src/Contacts/TSThread.m @@ -99,7 +99,7 @@ NS_ASSUME_NONNULL_BEGIN OWSAssert(interactionsByThread); [interactionsByThread enumerateKeysInGroup:self.uniqueId usingBlock:^(NSString *collection, NSString *key, NSUInteger index, BOOL *stop) { - if (key.length < 1) { + if (![key isKindOfClass:[NSString class]] || key.length < 1) { OWSProdLogAndFail(@"%@ invalid key in thread interactions.", self.logTag); return; } From 7a898f5e9936108daface2ce8437e558c9b8ed3a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 23 Jul 2018 16:53:02 -0400 Subject: [PATCH 5/6] Fix NPE using mock for unknown database objects. --- SignalServiceKit/src/Contacts/TSThread.m | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/SignalServiceKit/src/Contacts/TSThread.m b/SignalServiceKit/src/Contacts/TSThread.m index d6bcdb087..4ec7946fe 100644 --- a/SignalServiceKit/src/Contacts/TSThread.m +++ b/SignalServiceKit/src/Contacts/TSThread.m @@ -100,7 +100,10 @@ NS_ASSUME_NONNULL_BEGIN [interactionsByThread enumerateKeysInGroup:self.uniqueId usingBlock:^(NSString *collection, NSString *key, NSUInteger index, BOOL *stop) { if (![key isKindOfClass:[NSString class]] || key.length < 1) { - OWSProdLogAndFail(@"%@ invalid key in thread interactions.", self.logTag); + OWSProdLogAndFail(@"%@ invalid key in thread interactions: %@, %@.", + self.logTag, + key, + [key class]); return; } [interactionIds addObject:key]; From 5530b8d701c7061dda2a2d6f1837c3823749dec7 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 23 Jul 2018 17:12:58 -0400 Subject: [PATCH 6/6] Respond to CR. --- SignalServiceKit/src/Contacts/TSThread.m | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/SignalServiceKit/src/Contacts/TSThread.m b/SignalServiceKit/src/Contacts/TSThread.m index 4ec7946fe..187eb2206 100644 --- a/SignalServiceKit/src/Contacts/TSThread.m +++ b/SignalServiceKit/src/Contacts/TSThread.m @@ -97,6 +97,7 @@ NS_ASSUME_NONNULL_BEGIN NSMutableArray *interactionIds = [NSMutableArray new]; YapDatabaseViewTransaction *interactionsByThread = [transaction ext:TSMessageDatabaseViewExtensionName]; OWSAssert(interactionsByThread); + __block BOOL didDetectCorruption = NO; [interactionsByThread enumerateKeysInGroup:self.uniqueId usingBlock:^(NSString *collection, NSString *key, NSUInteger index, BOOL *stop) { if (![key isKindOfClass:[NSString class]] || key.length < 1) { @@ -104,11 +105,17 @@ NS_ASSUME_NONNULL_BEGIN self.logTag, key, [key class]); + didDetectCorruption = YES; return; } [interactionIds addObject:key]; }]; + if (didDetectCorruption) { + DDLogWarn(@"%@ incrementing version of: %@", self.logTag, TSMessageDatabaseViewExtensionName); + [OWSPrimaryStorage incrementVersionOfDatabaseExtension:TSMessageDatabaseViewExtensionName]; + } + for (NSString *interactionId in interactionIds) { // We need to fetch each interaction, since [TSInteraction removeWithTransaction:] does important work. TSInteraction *_Nullable interaction =