From 438357486f84a539cb585675d3218adac95f92e3 Mon Sep 17 00:00:00 2001 From: islam <2553451+qalandarov@users.noreply.github.com> Date: Thu, 18 Sep 2025 01:49:37 +0100 Subject: [PATCH] Introduce `Peer` for safer handling of `peerID` --- bitchat/Models/BitchatMessage.swift | 64 +++++++++++++--- bitchat/Models/Peer.swift | 75 +++++++++++++++++++ bitchat/Services/PrivateChatManager.swift | 6 +- bitchat/ViewModels/ChatViewModel.swift | 40 +++++----- bitchat/Views/ContentView.swift | 2 +- .../EndToEnd/PublicChatE2ETests.swift | 6 +- .../Integration/IntegrationTests.swift | 4 +- bitchatTests/Mocks/MockBLEService.swift | 2 +- .../Protocol/BinaryProtocolTests.swift | 2 +- 9 files changed, 160 insertions(+), 41 deletions(-) create mode 100644 bitchat/Models/Peer.swift diff --git a/bitchat/Models/BitchatMessage.swift b/bitchat/Models/BitchatMessage.swift index 1ae62040..25c8efe0 100644 --- a/bitchat/Models/BitchatMessage.swift +++ b/bitchat/Models/BitchatMessage.swift @@ -21,7 +21,7 @@ final class BitchatMessage: Codable { let originalSender: String? let isPrivate: Bool let recipientNickname: String? - let senderPeerID: String? + let senderPeer: Peer? let mentions: [String]? // Array of mentioned nicknames var deliveryStatus: DeliveryStatus? // Delivery tracking @@ -39,10 +39,23 @@ final class BitchatMessage: Codable { // Codable implementation enum CodingKeys: String, CodingKey { case id, sender, content, timestamp, isRelay, originalSender - case isPrivate, recipientNickname, senderPeerID, mentions, deliveryStatus + case isPrivate, recipientNickname, mentions, deliveryStatus + case senderPeer = "senderPeerID" // backwards compatibility } - init(id: String? = nil, sender: String, content: String, timestamp: Date, isRelay: Bool, originalSender: String? = nil, isPrivate: Bool = false, recipientNickname: String? = nil, senderPeerID: String? = nil, mentions: [String]? = nil, deliveryStatus: DeliveryStatus? = nil) { + init( + id: String? = nil, + sender: String, + content: String, + timestamp: Date, + isRelay: Bool, + originalSender: String? = nil, + isPrivate: Bool = false, + recipientNickname: String? = nil, + senderPeer: Peer?, + mentions: [String]? = nil, + deliveryStatus: DeliveryStatus? = nil + ) { self.id = id ?? UUID().uuidString self.sender = sender self.content = content @@ -51,12 +64,43 @@ final class BitchatMessage: Codable { self.originalSender = originalSender self.isPrivate = isPrivate self.recipientNickname = recipientNickname - self.senderPeerID = senderPeerID + self.senderPeer = senderPeer self.mentions = mentions self.deliveryStatus = deliveryStatus ?? (isPrivate ? .sending : nil) } } +extension BitchatMessage { + // Temp solution to minimize the code-change + convenience init( + id: String? = nil, + sender: String, + content: String, + timestamp: Date, + isRelay: Bool, + originalSender: String? = nil, + isPrivate: Bool = false, + recipientNickname: String? = nil, + senderPeerID: String? = nil, + mentions: [String]? = nil, + deliveryStatus: DeliveryStatus? = nil + ) { + self.init( + id: id, + sender: sender, + content: content, + timestamp: timestamp, + isRelay: isRelay, + originalSender: originalSender, + isPrivate: isPrivate, + recipientNickname: recipientNickname, + senderPeer: senderPeerID != nil ? Peer(str: senderPeerID!) : nil, + mentions: mentions, + deliveryStatus: deliveryStatus + ) + } +} + // MARK: - Equatable Conformance extension BitchatMessage: Equatable { @@ -69,7 +113,7 @@ extension BitchatMessage: Equatable { lhs.originalSender == rhs.originalSender && lhs.isPrivate == rhs.isPrivate && lhs.recipientNickname == rhs.recipientNickname && - lhs.senderPeerID == rhs.senderPeerID && + lhs.senderPeer == rhs.senderPeer && lhs.mentions == rhs.mentions && lhs.deliveryStatus == rhs.deliveryStatus } @@ -101,7 +145,7 @@ extension BitchatMessage { if isPrivate { flags |= 0x02 } if originalSender != nil { flags |= 0x04 } if recipientNickname != nil { flags |= 0x08 } - if senderPeerID != nil { flags |= 0x10 } + if senderPeer != nil { flags |= 0x10 } if mentions != nil && !mentions!.isEmpty { flags |= 0x20 } data.append(flags) @@ -151,7 +195,7 @@ extension BitchatMessage { data.append(recipData.prefix(255)) } - if let senderPeerID = senderPeerID, let peerData = senderPeerID.data(using: .utf8) { + if let peerData = senderPeer?.data { data.append(UInt8(min(peerData.count, 255))) data.append(peerData.prefix(255)) } @@ -264,11 +308,11 @@ extension BitchatMessage { } } - var senderPeerID: String? + var senderPeer: Peer? if hasSenderPeerID && offset < dataCopy.count { let length = Int(dataCopy[offset]); offset += 1 if offset + length <= dataCopy.count { - senderPeerID = String(data: dataCopy[offset.. +// + +import Foundation + +struct Peer: Equatable, Hashable { + let id: String +} + +extension Peer { + var data: Data? { + id.data(using: .utf8) + } + + var isNostr: Bool { + id.hasPrefix("nostr") + } + + var isNostrColon: Bool { + id.hasPrefix("nostr:") + } +} + +// MARK: - ExpressibleByStringLiteral + +extension Peer: ExpressibleByStringLiteral { + init(stringLiteral value: String) { + self.init(str: value) + } +} + +// MARK: - ExpressibleByStringInterpolation + +extension Peer: ExpressibleByStringInterpolation { + init(extendedGraphemeClusterLiteral value: String) { + self.init(str: value) + } +} + +// MARK: - Codable + +extension Peer: Codable { + init(from decoder: any Decoder) throws { + id = try decoder.singleValueContainer().decode(String.self) + } + + func encode(to encoder: any Encoder) throws { + var container = encoder.singleValueContainer() + try container.encode(id) + } +} + +// MARK: - Convenience Inits + +extension Peer { + init(str: String) { + id = str.lowercased() + } + + init(str: String.SubSequence) { + self.init(str: String(str)) + } + + init?(data: Data) { + guard let str = String(data: data, encoding: .utf8) else { + return nil + } + self.init(str: str) + } +} diff --git a/bitchat/Services/PrivateChatManager.swift b/bitchat/Services/PrivateChatManager.swift index 825d24f1..6e52e989 100644 --- a/bitchat/Services/PrivateChatManager.swift +++ b/bitchat/Services/PrivateChatManager.swift @@ -94,7 +94,7 @@ final class PrivateChatManager: ObservableObject { /// Handle incoming private message func handleIncomingMessage(_ message: BitchatMessage) { - guard let senderPeerID = message.senderPeerID else { return } + guard let senderPeerID = message.senderPeer?.id else { return } // Initialize chat if needed if privateChats[senderPeerID] == nil { @@ -161,7 +161,7 @@ final class PrivateChatManager: ObservableObject { // Send read receipts for unread messages that haven't been sent yet if let messages = privateChats[peerID] { for message in messages { - if message.senderPeerID == peerID && !message.isRelay && !sentReadReceipts.contains(message.id) { + if message.senderPeer?.id == peerID && !message.isRelay && !sentReadReceipts.contains(message.id) { sendReadReceipt(for: message) } } @@ -214,7 +214,7 @@ final class PrivateChatManager: ObservableObject { private func sendReadReceipt(for message: BitchatMessage) { guard !sentReadReceipts.contains(message.id), - let senderPeerID = message.senderPeerID else { + let senderPeerID = message.senderPeer?.id else { return } diff --git a/bitchat/ViewModels/ChatViewModel.swift b/bitchat/ViewModels/ChatViewModel.swift index 6d1fdc93..b58d9722 100644 --- a/bitchat/ViewModels/ChatViewModel.swift +++ b/bitchat/ViewModels/ChatViewModel.swift @@ -153,7 +153,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { @MainActor private func normalizedSenderKey(for message: BitchatMessage) -> String { - if let spid = message.senderPeerID { + if let spid = message.senderPeer?.id { if spid.hasPrefix("nostr:") || spid.hasPrefix("nostr_") { let bare: String = { if spid.hasPrefix("nostr:") { return String(spid.dropFirst(6)) } @@ -1231,7 +1231,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { // If no nickname in status, try to get from private chat messages if nickname == nil, let messages = privateChats[peerID], !messages.isEmpty { // Get the nickname from the first message where this peer was the sender - nickname = messages.first { $0.senderPeerID == peerID }?.sender + nickname = messages.first { $0.senderPeer?.id == peerID }?.sender } let finalNickname = nickname ?? "Unknown" @@ -2010,7 +2010,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { if let gh = currentGeohash { if var arr = geoTimelines[gh] { arr.removeAll { msg in - if let spid = msg.senderPeerID, spid.hasPrefix("nostr") { + if let spid = msg.senderPeer?.id, msg.senderPeer?.isNostr == true { if let full = nostrKeyMapping[spid]?.lowercased() { return full == hex } } return false @@ -2021,7 +2021,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { switch activeChannel { case .location: messages.removeAll { msg in - if let spid = msg.senderPeerID, spid.hasPrefix("nostr") { + if let spid = msg.senderPeer?.id, msg.senderPeer?.isNostr == true { if let full = nostrKeyMapping[spid]?.lowercased() { return full == hex } } return false @@ -2501,7 +2501,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { originalSender: message.originalSender, isPrivate: message.isPrivate, recipientNickname: message.recipientNickname, - senderPeerID: message.senderPeerID == meshService.myPeerID ? meshService.myPeerID : peerID, // Update peer ID if it's from them + senderPeerID: message.senderPeer?.id == meshService.myPeerID ? meshService.myPeerID : peerID, // Update peer ID if it's from them mentions: message.mentions, deliveryStatus: message.deliveryStatus ) @@ -2512,7 +2512,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { // 1. Not a message we sent // 2. Message is recent (< 60s old) // Never mark old messages as unread during consolidation - if message.senderPeerID != meshService.myPeerID { + if message.senderPeer?.id != meshService.myPeerID { let messageAge = Date().timeIntervalSince(message.timestamp) if messageAge < 60 && !sentReadReceipts.contains(message.id) { hasActualUnreadMessages = true @@ -2671,7 +2671,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { // Store the Nostr pubkey if provided (for messages from unknown senders) if let nostrPubkey = notification.userInfo?["nostrPubkey"] as? String, - let senderPeerID = message.senderPeerID { + let senderPeerID = message.senderPeer?.id { // Store mapping for read receipts nostrKeyMapping[senderPeerID] = nostrPubkey } @@ -3001,7 +3001,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { case .location(let ch) = LocationChannelManager.shared.selectedChannel, let id = try? NostrIdentityBridge.deriveIdentity(forGeohash: ch.geohash) { let messages = privateChats[peerID] ?? [] - for message in messages where message.senderPeerID == peerID && !message.isRelay { + for message in messages where message.senderPeer?.id == peerID && !message.isRelay { if !sentReadReceipts.contains(message.id) { SecureLogger.debug("GeoDM: sending READ for mid=\(message.id.prefix(8))… to=\(recipientHex.prefix(8))…", category: .session) let nostrTransport = NostrTransport(keychain: keychain) @@ -3043,7 +3043,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { for message in messagesToAck { // Only send read ACKs for messages from the peer (not our own) // Check both the ephemeral peer ID and stable Noise key as sender - if (message.senderPeerID == peerID || message.senderPeerID == noiseKeyHex) && !message.isRelay { + if (message.senderPeer?.id == peerID || message.senderPeer?.id == noiseKeyHex) && !message.isRelay { // Skip if we already sent an ACK for this message if !sentReadReceipts.contains(message.id) { // Use stable Noise key hex if available; else fall back to peerID @@ -3290,7 +3290,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { func formatMessageAsText(_ message: BitchatMessage, colorScheme: ColorScheme) -> AttributedString { // Determine if this message was sent by self (mesh, geo, or DM) let isSelf: Bool = { - if let spid = message.senderPeerID { + if let spid = message.senderPeer?.id { // In geohash channels, compare against our per-geohash nostr short ID if case .location(let ch) = activeChannel, spid.hasPrefix("nostr:") { if let myGeo = try? NostrIdentityBridge.deriveIdentity(forGeohash: ch.geohash) { @@ -3325,7 +3325,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { let fontWeight: Font.Weight = isSelf ? .bold : .medium senderStyle.font = .system(size: 14, weight: fontWeight, design: .monospaced) // Make sender clickable: encode senderPeerID into a custom URL - if let spid = message.senderPeerID, let url = URL(string: "bitchat://user/\(spid.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) ?? spid)") { + if let spid = message.senderPeer?.id, let url = URL(string: "bitchat://user/\(spid.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) ?? spid)") { senderStyle.link = url } @@ -3855,7 +3855,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { @MainActor private func peerColor(for message: BitchatMessage, isDark: Bool) -> Color { - if let spid = message.senderPeerID { + if let spid = message.senderPeer?.id { if spid.hasPrefix("nostr:") || spid.hasPrefix("nostr_") { let bare: String = { if spid.hasPrefix("nostr:") { return String(spid.dropFirst(6)) } @@ -4659,7 +4659,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { originalSender: msg.originalSender, isPrivate: msg.isPrivate, recipientNickname: msg.recipientNickname, - senderPeerID: (msg.senderPeerID == meshService.myPeerID) ? meshService.myPeerID : stableKeyHex, + senderPeerID: (msg.senderPeer?.id == meshService.myPeerID) ? meshService.myPeerID : stableKeyHex, mentions: msg.mentions, deliveryStatus: msg.deliveryStatus ) @@ -4687,7 +4687,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { if let messages = privateChats[peerID] { for message in messages { // Remove read receipts for messages FROM this peer (not TO this peer) - if message.senderPeerID == peerID { + if message.senderPeer?.id == peerID { sentReadReceipts.remove(message.id) } } @@ -5548,7 +5548,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { if senderNickname == nil { for (_, messages) in privateChats { if let previousMessage = messages.first(where: { - $0.senderPeerID == tempPeerID + $0.senderPeer?.id == tempPeerID }) { finalSenderNickname = previousMessage.sender break @@ -5691,7 +5691,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { /// Check if a message should be blocked based on sender @MainActor private func isMessageBlocked(_ message: BitchatMessage) -> Bool { - if let peerID = message.senderPeerID ?? getPeerIDForNickname(message.sender) { + if let peerID = message.senderPeer?.id ?? getPeerIDForNickname(message.sender) { // Check mesh/known peers first if isPeerBlocked(peerID) { return true } // Check geohash (Nostr) blocks using mapping to full pubkey @@ -5731,7 +5731,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { originalSender: message.originalSender, isPrivate: message.isPrivate, recipientNickname: message.recipientNickname, - senderPeerID: message.senderPeerID, + senderPeerID: message.senderPeer?.id, mentions: message.mentions, deliveryStatus: message.deliveryStatus ) @@ -5840,7 +5840,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { @MainActor private func handlePrivateMessage(_ message: BitchatMessage) { SecureLogger.debug("📥 handlePrivateMessage called for message from \(message.sender)", category: .session) - let senderPeerID = message.senderPeerID ?? getPeerIDForNickname(message.sender) + let senderPeerID = message.senderPeer?.id ?? getPeerIDForNickname(message.sender) guard let peerID = senderPeerID else { SecureLogger.warning("⚠️ Could not get peer ID for sender \(message.sender)", category: .session) @@ -5951,7 +5951,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { readerNickname: nickname ) - let recipientID = message.senderPeerID ?? peerID + let recipientID = message.senderPeer?.id ?? peerID Task { @MainActor in var originalTransport: String? = nil @@ -5983,7 +5983,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate { if isMessageBlocked(finalMessage) { return } // Classify origin: geochat if senderPeerID starts with 'nostr:', else mesh (or system) - let isGeo = finalMessage.senderPeerID?.hasPrefix("nostr:") == true + let isGeo = finalMessage.senderPeer?.isNostrColon == true // Apply per-sender and per-content rate limits (drop if exceeded) if finalMessage.sender != "system" { diff --git a/bitchat/Views/ContentView.swift b/bitchat/Views/ContentView.swift index 84b0cd7e..79eaf04b 100644 --- a/bitchat/Views/ContentView.swift +++ b/bitchat/Views/ContentView.swift @@ -459,7 +459,7 @@ struct ContentView: View { if let name = viewModel.meshService.peerNickname(peerID: peerID) { selectedMessageSender = name } else { - selectedMessageSender = viewModel.messages.last(where: { $0.senderPeerID == peerID && $0.sender != "system" })?.sender + selectedMessageSender = viewModel.messages.last(where: { $0.senderPeer?.id == peerID && $0.sender != "system" })?.sender } } if viewModel.isSelfSender(peerID: selectedMessageSenderID, displayName: selectedMessageSender) { diff --git a/bitchatTests/EndToEnd/PublicChatE2ETests.swift b/bitchatTests/EndToEnd/PublicChatE2ETests.swift index 1df9ff7a..eb795a3d 100644 --- a/bitchatTests/EndToEnd/PublicChatE2ETests.swift +++ b/bitchatTests/EndToEnd/PublicChatE2ETests.swift @@ -111,7 +111,7 @@ final class PublicChatE2ETests: XCTestCase { originalSender: message.sender, isPrivate: message.isPrivate, recipientNickname: message.recipientNickname, - senderPeerID: message.senderPeerID, + senderPeerID: message.senderPeer?.id, mentions: message.mentions ) @@ -439,7 +439,7 @@ final class PublicChatE2ETests: XCTestCase { if let message = BitchatMessage(packet.payload) { // Don't relay own messages - guard message.senderPeerID != node.peerID else { return } + guard message.senderPeer?.id != node.peerID else { return } // Create relay message let relayMessage = BitchatMessage( @@ -451,7 +451,7 @@ final class PublicChatE2ETests: XCTestCase { originalSender: message.isRelay ? message.originalSender : message.sender, isPrivate: message.isPrivate, recipientNickname: message.recipientNickname, - senderPeerID: message.senderPeerID, + senderPeerID: message.senderPeer?.id, mentions: message.mentions ) diff --git a/bitchatTests/Integration/IntegrationTests.swift b/bitchatTests/Integration/IntegrationTests.swift index 076dd281..30fef2af 100644 --- a/bitchatTests/Integration/IntegrationTests.swift +++ b/bitchatTests/Integration/IntegrationTests.swift @@ -627,7 +627,7 @@ final class IntegrationTests: XCTestCase { guard packet.ttl > 1 else { return } if let message = BitchatMessage(packet.payload) { - guard message.senderPeerID != node.peerID else { return } + guard message.senderPeer?.id != node.peerID else { return } let relayMessage = BitchatMessage( id: message.id, @@ -638,7 +638,7 @@ final class IntegrationTests: XCTestCase { originalSender: message.isRelay ? message.originalSender : message.sender, isPrivate: message.isPrivate, recipientNickname: message.recipientNickname, - senderPeerID: message.senderPeerID, + senderPeerID: message.senderPeer?.id, mentions: message.mentions ) diff --git a/bitchatTests/Mocks/MockBLEService.swift b/bitchatTests/Mocks/MockBLEService.swift index ce2730b0..01258c97 100644 --- a/bitchatTests/Mocks/MockBLEService.swift +++ b/bitchatTests/Mocks/MockBLEService.swift @@ -331,7 +331,7 @@ final class MockBLEService: NSObject { let nextTTL = packet.ttl > 0 ? packet.ttl - 1 : 0 for neighbor in neighbors() { // Avoid immediate echo loopback to sender if known - if let sender = message.senderPeerID, sender == neighbor.peerID { continue } + if let sender = message.senderPeer?.id, sender == neighbor.peerID { continue } var relay = packet relay.ttl = nextTTL neighbor.simulateIncomingPacket(relay) diff --git a/bitchatTests/Protocol/BinaryProtocolTests.swift b/bitchatTests/Protocol/BinaryProtocolTests.swift index 3ea6d163..9b981df8 100644 --- a/bitchatTests/Protocol/BinaryProtocolTests.swift +++ b/bitchatTests/Protocol/BinaryProtocolTests.swift @@ -204,7 +204,7 @@ final class BinaryProtocolTests: XCTestCase { XCTAssertEqual(decodedMessage.content, message.content) XCTAssertEqual(decodedMessage.sender, message.sender) - XCTAssertEqual(decodedMessage.senderPeerID, message.senderPeerID) + XCTAssertEqual(decodedMessage.senderPeerID, message.senderPeer?.id) XCTAssertEqual(decodedMessage.isPrivate, message.isPrivate) // Timestamp should be close (within 1 second due to conversion)