From d047a669fa0e418b83b354fe1776b9b0af85de8f Mon Sep 17 00:00:00 2001 From: Max Radermacher Date: Wed, 29 Mar 2023 18:40:30 -0700 Subject: [PATCH] Remove `throws_` handling for incoming messages --- .../src/Messages/OWSMessageManager.h | 28 ++++++-- .../src/Messages/OWSMessageManager.m | 71 +++++-------------- .../Receiving/GroupsV2MessageProcessor.swift | 16 ++--- .../tests/Messages/OWSMessageManagerTest.m | 36 +++------- 4 files changed, 56 insertions(+), 95 deletions(-) diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.h b/SignalServiceKit/src/Messages/OWSMessageManager.h index 7d9cab995d..2ef6cfc34b 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.h +++ b/SignalServiceKit/src/Messages/OWSMessageManager.h @@ -11,7 +11,9 @@ NS_ASSUME_NONNULL_BEGIN @class MessageManagerRequest; @class SDSAnyWriteTransaction; +@class SSKProtoDataMessage; @class SSKProtoEnvelope; +@class SSKProtoSyncMessage; typedef NS_CLOSED_ENUM(NSUInteger, OWSMessageManagerMessageType) { @@ -29,17 +31,14 @@ typedef NS_CLOSED_ENUM(NSUInteger, OWSMessageManagerMessageType) @interface OWSMessageManager : OWSMessageHandler -// processEnvelope: can be called from any thread. -// -// Returns YES on success. -- (BOOL)processEnvelope:(SSKProtoEnvelope *)envelope +- (void)processEnvelope:(SSKProtoEnvelope *)envelope plaintextData:(NSData *_Nullable)plaintextData wasReceivedByUD:(BOOL)wasReceivedByUD serverDeliveryTimestamp:(uint64_t)serverDeliveryTimestamp shouldDiscardVisibleMessages:(BOOL)shouldDiscardVisibleMessages transaction:(SDSAnyWriteTransaction *)transaction; -- (BOOL)handleRequest:(MessageManagerRequest *)request +- (void)handleRequest:(MessageManagerRequest *)request context:(id)context transaction:(SDSAnyWriteTransaction *)transaction; @@ -60,6 +59,25 @@ typedef NS_CLOSED_ENUM(NSUInteger, OWSMessageManagerMessageType) context:(id)context transaction:(SDSAnyWriteTransaction *)transaction; +#if TESTABLE_BUILD +// exposed for testing +- (void)handleIncomingEnvelope:(SSKProtoEnvelope *)envelope + withSyncMessage:(SSKProtoSyncMessage *)syncMessage + plaintextData:(NSData *)plaintextData + wasReceivedByUD:(BOOL)wasReceivedByUD + serverDeliveryTimestamp:(uint64_t)serverDeliveryTimestamp + transaction:(SDSAnyWriteTransaction *)transaction; + +// exposed for testing +- (void)handleIncomingEnvelope:(SSKProtoEnvelope *)envelope + withDataMessage:(SSKProtoDataMessage *)dataMessage + plaintextData:(NSData *)plaintextData + wasReceivedByUD:(BOOL)wasReceivedByUD + serverDeliveryTimestamp:(uint64_t)serverDeliveryTimestamp + shouldDiscardVisibleMessages:(BOOL)shouldDiscardVisibleMessages + transaction:(SDSAnyWriteTransaction *)transaction; +#endif + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index abe1c1cfc2..5fe1c2a935 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -160,27 +160,6 @@ NS_ASSUME_NONNULL_BEGIN #pragma mark - message handling -- (BOOL)processEnvelope:(SSKProtoEnvelope *)envelope - plaintextData:(NSData *_Nullable)plaintextData - wasReceivedByUD:(BOOL)wasReceivedByUD - serverDeliveryTimestamp:(uint64_t)serverDeliveryTimestamp - shouldDiscardVisibleMessages:(BOOL)shouldDiscardVisibleMessages - transaction:(SDSAnyWriteTransaction *)transaction -{ - @try { - [self throws_processEnvelope:envelope - plaintextData:plaintextData - wasReceivedByUD:wasReceivedByUD - serverDeliveryTimestamp:serverDeliveryTimestamp - shouldDiscardVisibleMessages:shouldDiscardVisibleMessages - transaction:transaction]; - return YES; - } @catch (NSException *exception) { - OWSFailDebug(@"Received an invalid envelope: %@", exception.debugDescription); - return NO; - } -} - - (BOOL)canProcessEnvelope:(SSKProtoEnvelope *)envelope transaction:(SDSAnyWriteTransaction *)transaction { if (!envelope) { @@ -223,8 +202,7 @@ NS_ASSUME_NONNULL_BEGIN return YES; } - -- (void)throws_processEnvelope:(SSKProtoEnvelope *)envelope +- (void)processEnvelope:(SSKProtoEnvelope *)envelope plaintextData:(NSData *_Nullable)plaintextData wasReceivedByUD:(BOOL)wasReceivedByUD serverDeliveryTimestamp:(uint64_t)serverDeliveryTimestamp @@ -247,7 +225,7 @@ NS_ASSUME_NONNULL_BEGIN OWSFailDebug(@"missing decrypted data for envelope: %@", [self descriptionForEnvelope:envelope]); return; } - [self throws_handleEnvelope:envelope + [self handleEnvelope:envelope plaintextData:plaintextData wasReceivedByUD:wasReceivedByUD serverDeliveryTimestamp:serverDeliveryTimestamp @@ -428,7 +406,7 @@ NS_ASSUME_NONNULL_BEGIN OWSProdInfoWEnvelope([OWSAnalyticsEvents messageManagerErrorEnvelopeNoActionablePayload], envelope); } -- (void)throws_handleEnvelope:(SSKProtoEnvelope *)envelope +- (void)handleEnvelope:(SSKProtoEnvelope *)envelope plaintextData:(NSData *)plaintextData wasReceivedByUD:(BOOL)wasReceivedByUD serverDeliveryTimestamp:(uint64_t)serverDeliveryTimestamp @@ -446,27 +424,12 @@ NS_ASSUME_NONNULL_BEGIN return; } - [self throws_handleRequest:request - context:[[PassthroughDeliveryReceiptContext alloc] init] - transaction:transaction]; + [self handleRequest:request context:[[PassthroughDeliveryReceiptContext alloc] init] transaction:transaction]; } -- (BOOL)handleRequest:(MessageManagerRequest *)request +- (void)handleRequest:(MessageManagerRequest *)request context:(id)context transaction:(SDSAnyWriteTransaction *)transaction -{ - @try { - [self throws_handleRequest:request context:context transaction:transaction]; - return YES; - } @catch (NSException *exception) { - OWSFailDebug(@"Received an invalid envelope: %@", exception.debugDescription); - return NO; - } -} - -- (void)throws_handleRequest:(MessageManagerRequest *)request - context:(id)context - transaction:(SDSAnyWriteTransaction *)transaction { SSKProtoContent *contentProto = request.protoContent; if (contentProto == nil) { @@ -476,12 +439,12 @@ NS_ASSUME_NONNULL_BEGIN switch (request.messageType) { case OWSMessageManagerMessageTypeSyncMessage: - [self throws_handleIncomingEnvelope:request.envelope - withSyncMessage:contentProto.syncMessage - plaintextData:request.plaintextData - wasReceivedByUD:request.wasReceivedByUD - serverDeliveryTimestamp:request.serverDeliveryTimestamp - transaction:transaction]; + [self handleIncomingEnvelope:request.envelope + withSyncMessage:contentProto.syncMessage + plaintextData:request.plaintextData + wasReceivedByUD:request.wasReceivedByUD + serverDeliveryTimestamp:request.serverDeliveryTimestamp + transaction:transaction]; [[OWSDeviceManager shared] setHasReceivedSyncMessage]; break; @@ -1597,12 +1560,12 @@ NS_ASSUME_NONNULL_BEGIN }]; } -- (void)throws_handleIncomingEnvelope:(SSKProtoEnvelope *)envelope - withSyncMessage:(SSKProtoSyncMessage *)syncMessage - plaintextData:(NSData *)plaintextData - wasReceivedByUD:(BOOL)wasReceivedByUD - serverDeliveryTimestamp:(uint64_t)serverDeliveryTimestamp - transaction:(SDSAnyWriteTransaction *)transaction +- (void)handleIncomingEnvelope:(SSKProtoEnvelope *)envelope + withSyncMessage:(SSKProtoSyncMessage *)syncMessage + plaintextData:(NSData *)plaintextData + wasReceivedByUD:(BOOL)wasReceivedByUD + serverDeliveryTimestamp:(uint64_t)serverDeliveryTimestamp + transaction:(SDSAnyWriteTransaction *)transaction { if (!envelope) { OWSFailDebug(@"Missing envelope."); diff --git a/SignalServiceKit/src/Network/Receiving/GroupsV2MessageProcessor.swift b/SignalServiceKit/src/Network/Receiving/GroupsV2MessageProcessor.swift index b6ee74ebc3..e33736b4fc 100644 --- a/SignalServiceKit/src/Network/Receiving/GroupsV2MessageProcessor.swift +++ b/SignalServiceKit/src/Network/Receiving/GroupsV2MessageProcessor.swift @@ -572,14 +572,14 @@ internal class GroupsMessageProcessor: MessageProcessingPipelineStage, Dependenc continue } let shouldDiscardVisibleMessages = discardMode == .discardVisibleMessages - if !self.messageManager.processEnvelope(envelope, - plaintextData: job.plaintextData, - wasReceivedByUD: job.wasReceivedByUD, - serverDeliveryTimestamp: job.serverDeliveryTimestamp, - shouldDiscardVisibleMessages: shouldDiscardVisibleMessages, - transaction: transaction) { - reportFailure(transaction) - } + self.messageManager.processEnvelope( + envelope, + plaintextData: job.plaintextData, + wasReceivedByUD: job.wasReceivedByUD, + serverDeliveryTimestamp: job.serverDeliveryTimestamp, + shouldDiscardVisibleMessages: shouldDiscardVisibleMessages, + transaction: transaction + ) } processedJobs.append(job) diff --git a/SignalServiceKit/tests/Messages/OWSMessageManagerTest.m b/SignalServiceKit/tests/Messages/OWSMessageManagerTest.m index 934a63f8cd..bf09a5cb47 100644 --- a/SignalServiceKit/tests/Messages/OWSMessageManagerTest.m +++ b/SignalServiceKit/tests/Messages/OWSMessageManagerTest.m @@ -23,26 +23,6 @@ NS_ASSUME_NONNULL_BEGIN NSString *const kLocalE164 = @"+13215550198"; NSString *const kLocalUuidString = @"B0D19730-950B-462C-84E7-60421F879EEF"; -@interface OWSMessageManager (Testing) - -// private method we are testing -- (void)throws_handleIncomingEnvelope:(SSKProtoEnvelope *)envelope - withSyncMessage:(SSKProtoSyncMessage *)syncMessage - plaintextData:(NSData *)plaintextData - wasReceivedByUD:(BOOL)wasReceivedByUD - serverDeliveryTimestamp:(uint64_t)serverDeliveryTimestamp - transaction:(SDSAnyWriteTransaction *)transaction; - -- (void)handleIncomingEnvelope:(SSKProtoEnvelope *)envelope - withDataMessage:(SSKProtoDataMessage *)dataMessage - plaintextData:(NSData *)plaintextData - wasReceivedByUD:(BOOL)wasReceivedByUD - serverDeliveryTimestamp:(uint64_t)serverDeliveryTimestamp - shouldDiscardVisibleMessages:(BOOL)shouldDiscardVisibleMessages - transaction:(SDSAnyWriteTransaction *)transaction; - -@end - #pragma mark - @interface OWSMessageManagerTest : SSKBaseTestObjC @@ -87,12 +67,12 @@ NSString *const kLocalUuidString = @"B0D19730-950B-462C-84E7-60421F879EEF"; [envelopeBuilder setSourceDevice:1]; [self writeWithBlock:^(SDSAnyWriteTransaction *transaction) { - [self.messageManager throws_handleIncomingEnvelope:[envelopeBuilder buildIgnoringErrors] - withSyncMessage:[messageBuilder buildIgnoringErrors] - plaintextData:nil - wasReceivedByUD:NO - serverDeliveryTimestamp:0 - transaction:transaction]; + [self.messageManager handleIncomingEnvelope:[envelopeBuilder buildIgnoringErrors] + withSyncMessage:[messageBuilder buildIgnoringErrors] + plaintextData:[NSData data] + wasReceivedByUD:NO + serverDeliveryTimestamp:0 + transaction:transaction]; }]; [self waitForExpectationsWithTimeout:5 @@ -125,7 +105,7 @@ NSString *const kLocalUuidString = @"B0D19730-950B-462C-84E7-60421F879EEF"; [self writeWithBlock:^(SDSAnyWriteTransaction *transaction) { [self.messageManager handleIncomingEnvelope:[envelopeBuilder buildIgnoringErrors] withDataMessage:[messageBuilder buildIgnoringErrors] - plaintextData:nil + plaintextData:[NSData data] wasReceivedByUD:NO serverDeliveryTimestamp:0 shouldDiscardVisibleMessages:NO @@ -170,7 +150,7 @@ NSString *const kLocalUuidString = @"B0D19730-950B-462C-84E7-60421F879EEF"; [self writeWithBlock:^(SDSAnyWriteTransaction *transaction) { [self.messageManager handleIncomingEnvelope:[envelopeBuilder buildIgnoringErrors] withDataMessage:[messageBuilder buildIgnoringErrors] - plaintextData:nil + plaintextData:[NSData data] wasReceivedByUD:NO serverDeliveryTimestamp:0 shouldDiscardVisibleMessages:NO