From 216ab73d20a7b508d7a229abd6a19176caa193da Mon Sep 17 00:00:00 2001 From: krishrathi1 Date: Sun, 2 Aug 2026 23:23:45 +0530 Subject: [PATCH 1/4] fix(formatting): resolve overlapping rich-text matches before rendering findAllMatches() only cross-checks each match type against mentions (and bolt11/lnurl against url+lightning); it never checks url against cashu, or any other combination the ad-hoc per-type guards didn't anticipate. An ordinary message containing a URL whose path embeds a "cashuA..."-shaped token (e.g. a mint redemption link) produces two overlapping matches for the same text -- the whole URL, and the nested cashu token inside it. formatContent's single linear rendering pass assumes matches are non-overlapping with strictly increasing ranges. Given the nested pair, it appends the URL's full text, then unconditionally re-appends the nested cashu substring a second time (styled differently), and walks its cursor backward to the cashu match's end -- which is before the URL's actual end. The trailing "remaining text" check then re-appends the tail of the URL a third time, since the cursor never advanced past it. Add a final pass that sorts matches by start position and greedily keeps only non-overlapping ones, dropping any later match whose start falls inside an already-kept match's range. This fixes the url/cashu case and generally guards any other pair of match types that isn't already handled by an explicit per-type check, without touching the existing exclusion logic. --- .../Services/MessageFormattingEngine.swift | 21 +++++++++++++-- .../MessageFormattingEngineTests.swift | 26 +++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/bitchat/Services/MessageFormattingEngine.swift b/bitchat/Services/MessageFormattingEngine.swift index 88a32e3c..024bff5d 100644 --- a/bitchat/Services/MessageFormattingEngine.swift +++ b/bitchat/Services/MessageFormattingEngine.swift @@ -368,8 +368,25 @@ final class MessageFormattingEngine { allMatches.append(ContentMatch(range: match.range(at: 0), type: .lnurl)) } - // Sort by position - return allMatches.sorted { $0.range.location < $1.range.location } + // Sort by position, then drop any match that overlaps one already + // kept. The per-type checks above only guard specific known pairs + // (e.g. bolt11/lnurl against url/lightning) -- they don't cover every + // combination, so an ordinary message can still produce overlapping + // matches of two OTHER types the checks never compared (e.g. a URL + // whose path embeds a "cashuA..."-shaped token). formatContent's + // single linear pass assumes non-overlapping, strictly-increasing + // ranges; without this pass a nested match re-renders already-shown + // text and can walk `lastEnd` backwards, duplicating a trailing + // slice of content a second time. + let sorted = allMatches.sorted { $0.range.location < $1.range.location } + var resolved: [ContentMatch] = [] + var occupiedUntil = 0 + for match in sorted { + guard match.range.location >= occupiedUntil else { continue } + resolved.append(match) + occupiedUntil = match.range.location + match.range.length + } + return resolved } private static func formatPlainContent(_ content: String, baseColor: Color, isSelf: Bool) -> AttributedString { diff --git a/bitchatTests/MessageFormattingEngineTests.swift b/bitchatTests/MessageFormattingEngineTests.swift index 5de2adea..0484c5e0 100644 --- a/bitchatTests/MessageFormattingEngineTests.swift +++ b/bitchatTests/MessageFormattingEngineTests.swift @@ -349,6 +349,32 @@ struct MessageFormattingEngineTests { #expect(String(formatted.characters) == "<@alice> \(longContent) [\(message.formattedTimestamp)]") } + + @MainActor + @Test func formatMessage_urlContainingCashuTokenIsNotDuplicated() { + // Bug: a URL match and a Cashu match are never cross-checked for + // overlap, only each independently against mentions. A URL whose + // path embeds a "cashuA..."-shaped segment (a perfectly ordinary + // mint redemption link) produces two overlapping matches; the + // single linear rendering pass then re-renders the nested cashu + // substring a second time and, because it walks `lastEnd` backwards, + // re-renders the URL's own trailing segment a third time. + let context = MockMessageFormattingContext(nickname: "carol") + let cashuToken = "cashuA" + String(repeating: "a", count: 44) + let url = "https://mint.example.com/redeem/\(cashuToken)/confirm" + let content = "redeem link \(url) now" + let message = BitchatMessage( + id: "url-cashu", + sender: "alice", + content: content, + timestamp: Date(timeIntervalSince1970: 1_700_001_111), + isRelay: false + ) + + let formatted = MessageFormattingEngine.formatMessage(message, context: context, colorScheme: .light) + + #expect(String(formatted.characters) == "<@alice> \(content) [\(message.formattedTimestamp)]") + } } @MainActor From 7ba358ce468f3133ba50da6dc25ecf6ab8505aca Mon Sep 17 00:00:00 2001 From: krishrathi1 Date: Sun, 2 Aug 2026 23:33:06 +0530 Subject: [PATCH 2/4] fix(formatting): mirror the overlap resolution in ChatMessageFormatter ChatMessageFormatter.formatMessageAsText -- the production chat-row rendering path (TextMessageView -> ChatViewModel.formatMessageAsText) -- has its own hand-copied matcher with the same gap as MessageFormattingEngine: url and cashu (and lightning/bolt11/lnurl) are never cross-checked against each other, only against mentions. Here the corruption shows differently: this formatter renders cashu and lightning-family matches as a single spacer character rather than their matched text, so a URL whose path embeds a "cashuA..."-shaped token gets an extra, spurious space injected right after the fully-rendered URL instead of duplicated text. Same fix as MessageFormattingEngine: sort by start position, then greedily keep only non-overlapping matches. I could not add an equivalent regression test here: unlike MessageFormattingEngine (formatted through a small, mockable MessageFormattingContext protocol), ChatMessageFormatter has no existing test target and is constructed against a live ChatViewModel with a large dependency graph (mesh service, identity bridge, active channel, etc.). I don't have a Swift toolchain available to build a new test harness for it with confidence, so I've left that to whoever reviews this with the ChatViewModel construction context in hand -- happy to add one if pointed at an existing lightweight construction path I've missed. --- bitchat/ViewModels/ChatMessageFormatter.swift | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/bitchat/ViewModels/ChatMessageFormatter.swift b/bitchat/ViewModels/ChatMessageFormatter.swift index 1a00315b..b08c8c7c 100644 --- a/bitchat/ViewModels/ChatMessageFormatter.swift +++ b/bitchat/ViewModels/ChatMessageFormatter.swift @@ -190,11 +190,29 @@ final class ChatMessageFormatter { } allMatches.sort { $0.range.location < $1.range.location } + // Drop any match that overlaps one already kept. The + // per-type checks above only guard specific known pairs + // (e.g. bolt11/lnurl against url+lightning); they don't + // cover every combination, so an ordinary message can still + // produce overlapping matches of two OTHER types (e.g. a URL + // whose path embeds a "cashuA..."-shaped token). The render + // loop below assumes non-overlapping, strictly-increasing + // ranges; without this pass a nested cashu/lightning match + // renders as an extra spacer character injected into + // already-shown text instead of being skipped. + var resolvedMatches: [(range: NSRange, type: String)] = [] + var occupiedUntil = 0 + for match in allMatches { + guard match.range.location >= occupiedUntil else { continue } + resolvedMatches.append(match) + occupiedUntil = match.range.location + match.range.length + } + var lastEnd = content.startIndex let myNickname = viewModel.nickname.normalizedNickname let isMentioned = message.mentions?.contains { $0.normalizedNickname == myNickname } ?? false - for (range, type) in allMatches { + for (range, type) in resolvedMatches { guard let swiftRange = Range(range, in: content) else { continue } if lastEnd < swiftRange.lowerBound { From 159f2bca8378b8a15ad4f9b3c289d3c5380076ea Mon Sep 17 00:00:00 2001 From: krishrathi1 Date: Tue, 4 Aug 2026 11:28:04 +0530 Subject: [PATCH 3/4] refactor(formatting): share the overlap-resolution pass between formatters Per review: MessageFormattingEngine and ChatMessageFormatter each grew their own copy of the sort-then-drop-overlaps pass, which risks drifting apart later. Extract it into a single generic helper, MessageFormattingEngine.resolveOverlappingMatches, keyed by an NSRange extractor closure so it works over both formatters' different match representations (a ContentMatch struct vs. a (range, type) tuple) without unifying those types. Add direct unit tests for the extracted helper (nested overlap dropped, out-of-order input sorted internally, adjacent-but-non-overlapping ranges both kept) since ChatMessageFormatter's copy wasn't independently unit-testable without a live ChatViewModel. --- .../Services/MessageFormattingEngine.swift | 36 ++++++++++++------ bitchat/ViewModels/ChatMessageFormatter.swift | 18 +++------ .../MessageFormattingEngineTests.swift | 37 +++++++++++++++++++ 3 files changed, 66 insertions(+), 25 deletions(-) diff --git a/bitchat/Services/MessageFormattingEngine.swift b/bitchat/Services/MessageFormattingEngine.swift index 024bff5d..1f5c656e 100644 --- a/bitchat/Services/MessageFormattingEngine.swift +++ b/bitchat/Services/MessageFormattingEngine.swift @@ -368,23 +368,35 @@ final class MessageFormattingEngine { allMatches.append(ContentMatch(range: match.range(at: 0), type: .lnurl)) } - // Sort by position, then drop any match that overlaps one already - // kept. The per-type checks above only guard specific known pairs - // (e.g. bolt11/lnurl against url/lightning) -- they don't cover every + // The per-type checks above only guard specific known pairs (e.g. + // bolt11/lnurl against url/lightning) -- they don't cover every // combination, so an ordinary message can still produce overlapping // matches of two OTHER types the checks never compared (e.g. a URL - // whose path embeds a "cashuA..."-shaped token). formatContent's - // single linear pass assumes non-overlapping, strictly-increasing - // ranges; without this pass a nested match re-renders already-shown - // text and can walk `lastEnd` backwards, duplicating a trailing - // slice of content a second time. - let sorted = allMatches.sorted { $0.range.location < $1.range.location } - var resolved: [ContentMatch] = [] + // whose path embeds a "cashuA..."-shaped token). Resolve any + // remaining overlap before handing matches to a rendering pass. + return resolveOverlappingMatches(allMatches) { $0.range } + } + + /// Sorts matches by start position and greedily keeps only non-overlapping + /// ones, dropping any later match whose start falls inside an + /// already-kept match's range. Rendering passes that consume the result + /// (here and in `ChatMessageFormatter`, which has its own copy of the + /// match-collection logic above) assume non-overlapping, + /// strictly-increasing ranges; without this resolution a nested match + /// re-renders already-shown text and can walk the render cursor + /// backwards, duplicating a trailing slice of content a second time. + static func resolveOverlappingMatches( + _ matches: [Match], + range: (Match) -> NSRange + ) -> [Match] { + let sorted = matches.sorted { range($0).location < range($1).location } + var resolved: [Match] = [] var occupiedUntil = 0 for match in sorted { - guard match.range.location >= occupiedUntil else { continue } + let matchRange = range(match) + guard matchRange.location >= occupiedUntil else { continue } resolved.append(match) - occupiedUntil = match.range.location + match.range.length + occupiedUntil = matchRange.location + matchRange.length } return resolved } diff --git a/bitchat/ViewModels/ChatMessageFormatter.swift b/bitchat/ViewModels/ChatMessageFormatter.swift index b08c8c7c..b6653ab6 100644 --- a/bitchat/ViewModels/ChatMessageFormatter.swift +++ b/bitchat/ViewModels/ChatMessageFormatter.swift @@ -188,25 +188,17 @@ final class ChatMessageFormatter { where !overlapsMention(match.range(at: 0)) && !overlapsOccupied(match.range(at: 0)) { allMatches.append((match.range(at: 0), "lnurl")) } - allMatches.sort { $0.range.location < $1.range.location } - // Drop any match that overlaps one already kept. The - // per-type checks above only guard specific known pairs + // The per-type checks above only guard specific known pairs // (e.g. bolt11/lnurl against url+lightning); they don't // cover every combination, so an ordinary message can still // produce overlapping matches of two OTHER types (e.g. a URL // whose path embeds a "cashuA..."-shaped token). The render // loop below assumes non-overlapping, strictly-increasing - // ranges; without this pass a nested cashu/lightning match - // renders as an extra spacer character injected into - // already-shown text instead of being skipped. - var resolvedMatches: [(range: NSRange, type: String)] = [] - var occupiedUntil = 0 - for match in allMatches { - guard match.range.location >= occupiedUntil else { continue } - resolvedMatches.append(match) - occupiedUntil = match.range.location + match.range.length - } + // ranges; without resolving those first, a nested + // cashu/lightning match renders as an extra spacer character + // injected into already-shown text instead of being skipped. + let resolvedMatches = MessageFormattingEngine.resolveOverlappingMatches(allMatches) { $0.range } var lastEnd = content.startIndex let myNickname = viewModel.nickname.normalizedNickname diff --git a/bitchatTests/MessageFormattingEngineTests.swift b/bitchatTests/MessageFormattingEngineTests.swift index 0484c5e0..f32cd69a 100644 --- a/bitchatTests/MessageFormattingEngineTests.swift +++ b/bitchatTests/MessageFormattingEngineTests.swift @@ -375,6 +375,43 @@ struct MessageFormattingEngineTests { #expect(String(formatted.characters) == "<@alice> \(content) [\(message.formattedTimestamp)]") } + + // MARK: - resolveOverlappingMatches Tests + // + // Direct coverage for the shared helper `ChatMessageFormatter` also + // calls, since its own match-collection logic (a hand-copy of + // findAllMatches above) isn't independently unit-testable without a + // live ChatViewModel. + + @Test func resolveOverlappingMatches_dropsANestedOverlap() { + let outer = (range: NSRange(location: 0, length: 10), type: "outer") + let nested = (range: NSRange(location: 3, length: 2), type: "nested") + + let resolved = MessageFormattingEngine.resolveOverlappingMatches([outer, nested]) { $0.range } + + #expect(resolved.map(\.type) == ["outer"]) + } + + @Test func resolveOverlappingMatches_keepsNonOverlappingMatchesSortedByStart() { + let first = (range: NSRange(location: 0, length: 3), type: "first") + let second = (range: NSRange(location: 5, length: 3), type: "second") + + // Passed out of order; the helper must sort internally. + let resolved = MessageFormattingEngine.resolveOverlappingMatches([second, first]) { $0.range } + + #expect(resolved.map(\.type) == ["first", "second"]) + } + + @Test func resolveOverlappingMatches_adjacentNonOverlappingMatchesAreBothKept() { + // A match starting exactly where the previous one ends is not an + // overlap (NSRange upper bound is exclusive). + let first = (range: NSRange(location: 0, length: 5), type: "first") + let adjacent = (range: NSRange(location: 5, length: 5), type: "adjacent") + + let resolved = MessageFormattingEngine.resolveOverlappingMatches([first, adjacent]) { $0.range } + + #expect(resolved.map(\.type) == ["first", "adjacent"]) + } } @MainActor From 1b9b31a245f9a545ec757f8d8e3c745b4d170a39 Mon Sep 17 00:00:00 2001 From: krishrathi1 Date: Thu, 6 Aug 2026 14:11:05 +0530 Subject: [PATCH 4/4] docs(formatting): document greedy keep-first tie-breaking in resolveOverlappingMatches --- bitchat/Services/MessageFormattingEngine.swift | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/bitchat/Services/MessageFormattingEngine.swift b/bitchat/Services/MessageFormattingEngine.swift index 1f5c656e..adb7f2c4 100644 --- a/bitchat/Services/MessageFormattingEngine.swift +++ b/bitchat/Services/MessageFormattingEngine.swift @@ -385,6 +385,13 @@ final class MessageFormattingEngine { /// strictly-increasing ranges; without this resolution a nested match /// re-renders already-shown text and can walk the render cursor /// backwards, duplicating a trailing slice of content a second time. + /// + /// Ties break by start position only, not length: whichever match starts + /// first wins outright, so an outer match (e.g. a URL) always keeps + /// priority over a shorter match nested inside it (e.g. an embedded cashu + /// token) rather than the other way around. That's intentional for + /// rendering — flip it to prefer the inner match only with a matching + /// change to how callers render the dropped outer span. static func resolveOverlappingMatches( _ matches: [Match], range: (Match) -> NSRange