From 1ef170e9d883c18af4354d6fc382c2e2378b7b1d Mon Sep 17 00:00:00 2001 From: Purav Manot Date: Mon, 6 Jul 2026 20:07:21 +0530 Subject: [PATCH] Claim sent messages per-row and log thread validation failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Messages.app delivers a send under a different chat GUID than the one targeted (service or handle-formatting flip for the same contact), waitForMessageSend rejected the whole batch and resolved the send as a bare boolean(true). The client then had no message to reconcile its optimistic echo against, leaving it stuck on "Still sending…" indefinitely (DESK-28166). - Validate each sent row individually instead of allSatisfy over the batch, so one foreign or unjoined row doesn't discard the rest - Accept same-recipient addresses (email casing, phone with/without country prefix) before falling back to the contacts-database comparison, which returns false when the sent handle isn't on the contact card - Log hashed target/sent thread GUIDs and report to Sentry when validation still fails, so future rageshakes show which mismatch flavor occurred Fixes DESK-28166 --- .../Sources/IMessage/PlatformAPI.swift | 49 ++++++++--- .../Sources/IMessage/Utilities/ThreadID.swift | 19 ++++ .../SentThreadValidationTests.swift | 87 +++++++++++++++++++ 3 files changed, 145 insertions(+), 10 deletions(-) create mode 100644 src/IMessage/Sources/IMessageTests/SentThreadValidationTests.swift 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) +}