diff --git a/src/IMessage/Sources/IMessage/PlatformAPI.swift b/src/IMessage/Sources/IMessage/PlatformAPI.swift index 85e76767..9bcbfc6f 100644 --- a/src/IMessage/Sources/IMessage/PlatformAPI.swift +++ b/src/IMessage/Sources/IMessage/PlatformAPI.swift @@ -736,25 +736,54 @@ public final class PlatformAPI { ) async throws -> PlatformSDK.MessageSendResult { let sentMessageIDs = try await waitForSentMessageIDs(since: lastRowID, text: text, timeout: timeout) let sentThreadIDs = try await waitForSentThreadIDs(messageRowIDs: sentMessageIDs.map(\.rowID)) - let address = threadIDToAddress(threadID) - let sentThreadIsValid = try await withMessagesController { controller in - sentThreadIDs.allSatisfy { sentThreadID in - if sentThreadID == threadID { return true } - guard let sentThreadID else { return false } - return controller.isSameContact(address, threadIDToAddress(sentThreadID)) - } + let validSentMessageIDs = try await withMessagesController { controller in + Self.validSentMessageIDs( + targetThreadID: threadID, + sentMessageIDs: sentMessageIDs, + sentThreadIDs: sentThreadIDs, + isSameContact: controller.isSameContact + ) + } + + if validSentMessageIDs.count < sentMessageIDs.count { + let hashedTarget = Hasher.thread.tokenizeRemembering(pii: threadID) + let hashedSent = sentThreadIDs + .map { $0.map { Hasher.thread.tokenizeRemembering(pii: $0) } ?? "" } + .joined(separator: " ") + platformLog.error("imsg: imessage potentially sent messages to invalid thread (target=\(hashedTarget) sent=\(hashedSent))") + reportErrorMessage("imessage sent message thread validation failed for \(sentMessageIDs.count - validSentMessageIDs.count)/\(sentMessageIDs.count) messages") } - guard sentThreadIsValid else { - platformLog.error("imsg: imessage potentially sent messages to invalid thread") + guard !validSentMessageIDs.isEmpty else { return .boolean(true) } - let messages = try await sentMessages(sentMessageIDs) + let messages = try await sentMessages(validSentMessageIDs) validateLinkedMessageIDs(messages, expectedLinkedMessageID: expectedLinkedMessageID) return .messages(messages) } + // A sent row is claimed as ours when it joined the chat we targeted, a chat + // whose address is the same recipient, or (via the contacts database) a chat + // whose address is on the same contact card. Rows that match none of these — + // or never joined a chat — are dropped individually, so one foreign row + // (e.g. a concurrent send from another device) doesn't discard the whole batch. + nonisolated static func validSentMessageIDs( + targetThreadID: String, + sentMessageIDs: [(rowID: Int, guid: String)], + sentThreadIDs: [String?], + isSameContact: (String?, String?) -> Bool + ) -> [(rowID: Int, guid: String)] { + let targetAddress = threadIDToAddress(targetThreadID) + return zip(sentMessageIDs, sentThreadIDs).filter { _, sentThreadID in + guard let sentThreadID else { return false } + if sentThreadID == targetThreadID { return true } + let sentAddress = threadIDToAddress(sentThreadID) + return addressesReferToSameRecipient(targetAddress, sentAddress) + || isSameContact(targetAddress, sentAddress) + }.map(\.0) + } + private func waitForSentMessageIDs( since lastRowID: Int, text: String?, diff --git a/src/IMessage/Sources/IMessage/Utilities/ThreadID.swift b/src/IMessage/Sources/IMessage/Utilities/ThreadID.swift index 04da8c08..0c4de59a 100644 --- a/src/IMessage/Sources/IMessage/Utilities/ThreadID.swift +++ b/src/IMessage/Sources/IMessage/Utilities/ThreadID.swift @@ -23,3 +23,22 @@ func singleParticipantAddress(_ threadID: String) -> String? { func threadIDIsForGroup(_ id: String) -> Bool { splitThreadID(id).map { $0.1 == MessagesDeepLink.groupThreadType } == true } + +// suffix match so "+15035550123" pairs with "5035550123"; require enough +// digits that shortcodes can't collide with a full number's tail +private let minimumPhoneSuffixDigits = 7 + +// Messages merges conversations per contact, so a send targeted at one chat can +// land in a sibling chat whose address is the same recipient with different +// formatting (email casing, or a phone with/without the country prefix). This +// runs before the contacts-database comparison, which can't see such pairs when +// the sent handle isn't on the contact card. +func addressesReferToSameRecipient(_ a: String?, _ b: String?) -> Bool { + guard let a, let b, !a.isEmpty, !b.isEmpty else { return false } + if a.caseInsensitiveCompare(b) == .orderedSame { return true } + guard !a.contains("@"), !b.contains("@") else { return false } + let digitsA = a.filter(\.isNumber) + let digitsB = b.filter(\.isNumber) + guard min(digitsA.count, digitsB.count) >= minimumPhoneSuffixDigits else { return false } + return digitsA.hasSuffix(digitsB) || digitsB.hasSuffix(digitsA) +} diff --git a/src/IMessage/Sources/IMessageTests/SentThreadValidationTests.swift b/src/IMessage/Sources/IMessageTests/SentThreadValidationTests.swift new file mode 100644 index 00000000..f1781f02 --- /dev/null +++ b/src/IMessage/Sources/IMessageTests/SentThreadValidationTests.swift @@ -0,0 +1,87 @@ +@testable import IMessage +import Testing + +@Test func addressesReferToSameRecipientMatchesFormattingVariants() { + #expect(addressesReferToSameRecipient("+15035550123", "+15035550123")) + #expect(addressesReferToSameRecipient("+15035550123", "5035550123")) + #expect(addressesReferToSameRecipient("5035550123", "+1 (503) 555-0123")) + #expect(addressesReferToSameRecipient("a@example.com", "A@Example.com")) +} + +@Test func addressesReferToSameRecipientRejectsDifferentRecipients() { + #expect(!addressesReferToSameRecipient("+15035550123", "+15035550124")) + #expect(!addressesReferToSameRecipient("a@example.com", "b@example.com")) + // emails must match exactly, not by digit content + #expect(!addressesReferToSameRecipient("5550123@example.com", "5550123")) + // shortcodes may not tail-match a full number + #expect(!addressesReferToSameRecipient("50123", "+15035550123")) + #expect(!addressesReferToSameRecipient(nil, "+15035550123")) + #expect(!addressesReferToSameRecipient("", "")) +} + +private let sentMessage = (rowID: 1, guid: "AAAAAAAA-0000-0000-0000-000000000001") +private let otherSentMessage = (rowID: 2, guid: "AAAAAAAA-0000-0000-0000-000000000002") + +@Test func validSentMessageIDsAcceptsExactThreadMatch() { + let valid = PlatformAPI.validSentMessageIDs( + targetThreadID: "iMessage;-;+15035550123", + sentMessageIDs: [sentMessage], + sentThreadIDs: ["iMessage;-;+15035550123"], + isSameContact: { _, _ in false } + ) + #expect(valid.map(\.rowID) == [1]) +} + +@Test func validSentMessageIDsAcceptsServiceAndFormattingFlips() { + // same recipient under a different service or handle formatting must not + // require the contacts database (DESK-28166: isSameContact false-negative + // left the client's echo stuck on "still sending") + let valid = PlatformAPI.validSentMessageIDs( + targetThreadID: "iMessage;-;+15035550123", + sentMessageIDs: [sentMessage], + sentThreadIDs: ["SMS;-;5035550123"], + isSameContact: { _, _ in false } + ) + #expect(valid.map(\.rowID) == [1]) +} + +@Test func validSentMessageIDsFallsBackToContacts() { + let valid = PlatformAPI.validSentMessageIDs( + targetThreadID: "iMessage;-;a@example.com", + sentMessageIDs: [sentMessage], + sentThreadIDs: ["iMessage;-;+15035550123"], + isSameContact: { a, b in a == "a@example.com" && b == "+15035550123" } + ) + #expect(valid.map(\.rowID) == [1]) +} + +@Test func validSentMessageIDsDropsForeignRowsIndividually() { + // a concurrent send to somebody else must not discard the whole batch + let valid = PlatformAPI.validSentMessageIDs( + targetThreadID: "iMessage;-;+15035550123", + sentMessageIDs: [sentMessage, otherSentMessage], + sentThreadIDs: ["iMessage;-;+15035550123", "iMessage;-;+19995550000"], + isSameContact: { _, _ in false } + ) + #expect(valid.map(\.rowID) == [1]) +} + +@Test func validSentMessageIDsDropsUnjoinedRows() { + let valid = PlatformAPI.validSentMessageIDs( + targetThreadID: "iMessage;-;+15035550123", + sentMessageIDs: [sentMessage, otherSentMessage], + sentThreadIDs: [nil, "iMessage;-;+15035550123"], + isSameContact: { _, _ in false } + ) + #expect(valid.map(\.rowID) == [2]) +} + +@Test func validSentMessageIDsReturnsEmptyWhenNothingMatches() { + let valid = PlatformAPI.validSentMessageIDs( + targetThreadID: "iMessage;-;+15035550123", + sentMessageIDs: [sentMessage], + sentThreadIDs: ["iMessage;-;+19995550000"], + isSameContact: { _, _ in false } + ) + #expect(valid.isEmpty) +}