From 08416aa3cb9b17b1ea927ddd1a8c6358f1a5fef2 Mon Sep 17 00:00:00 2001 From: Strycher Date: Wed, 9 Sep 2026 03:05:23 -0400 Subject: [PATCH 1/4] fix(#636): stop truncating names mid-codepoint on the wire Three writers copied raw UTF-8 bytes up to a 31-byte boundary with no codepoint check, so any name crossing it was cut mid-character and invalid UTF-8 reached the radio. It was then stored that way and handed back to every client that later read the contact or channel. ASCII names were unaffected, which is why it went unnoticed. An 11-character Chinese or Japanese name is 33 bytes against a 31-byte budget, so it corrupted every time. Adds utf8TruncateToBytes, which stops on a GRAPHEME CLUSTER boundary. Codepoint-safe is not sufficient: a ZWJ emoji sequence is several codepoints joined by U+200D plus a variation selector, so a codepoint-safe cut can still leave a bare glyph, a dangling joiner or an orphaned selector. The characters package was already a dependency, and flutter/widgets re-exports it, so this costs nothing new. Fixed at all three sites: - writeCString, used for contact names and channel names - buildSetAdvertNameFrame, which sets THIS device's own advert name That last one mattered most: it is the name the whole mesh sees, the name embedded in the contact cards this device emits, and what other clients match against. Corrupt it once and it propagates outward. The strongest test sweeps every budget from 0 upward across a string mixing 1, 3 and 4-byte characters and asserts the output always decodes strictly as UTF-8, which is precisely the property that was broken. ASCII output is unchanged. Found by the standards#145 Gemini review on the #619 branch, which was told not to re-report writeCString and located the third site instead. Co-Authored-By: Claude Opus 5 --- lib/connector/meshcore_protocol.dart | 46 ++++++- test/connector/utf8_name_truncation_test.dart | 124 ++++++++++++++++++ 2 files changed, 163 insertions(+), 7 deletions(-) create mode 100644 test/connector/utf8_name_truncation_test.dart diff --git a/lib/connector/meshcore_protocol.dart b/lib/connector/meshcore_protocol.dart index 41c0722..449be0c 100644 --- a/lib/connector/meshcore_protocol.dart +++ b/lib/connector/meshcore_protocol.dart @@ -2,8 +2,37 @@ import 'dart:convert'; import 'dart:math'; import 'dart:typed_data'; +// `flutter/widgets.dart` re-exports package:characters, which supplies the +// grapheme-cluster iteration used by utf8TruncateToBytes below. import 'package:flutter/widgets.dart'; +/// Encodes [s] as UTF-8, truncated to at most [maxBytes], never splitting a +/// character. (#636) +/// +/// Truncation stops on a **grapheme cluster** boundary, not a byte boundary and +/// not merely a codepoint boundary. Cutting on bytes puts invalid UTF-8 on the +/// wire, which every reader then renders as replacement characters. Cutting +/// between codepoints is valid UTF-8 but still wrong: a ZWJ emoji sequence is +/// several codepoints joined by U+200D plus a variation selector, so a +/// codepoint-safe cut can still leave a bare glyph, a dangling joiner, or an +/// orphaned selector. +/// +/// The on-wire name field is 32 bytes including a null terminator, so callers +/// pass 31. That holds roughly 10 CJK characters, or two ZWJ emoji. +Uint8List utf8TruncateToBytes(String s, int maxBytes) { + if (maxBytes <= 0) return Uint8List(0); + final whole = utf8.encode(s); + if (whole.length <= maxBytes) return Uint8List.fromList(whole); + + final out = []; + for (final cluster in s.characters) { + final encoded = utf8.encode(cluster); + if (out.length + encoded.length > maxBytes) break; + out.addAll(encoded); + } + return Uint8List.fromList(out); +} + // Buffer Reader - sequential binary data reader with pointer tracking class BufferReader { int _pointer = 0; @@ -132,8 +161,10 @@ class BufferWriter { void writeCString(String string, int maxLength) { final bytes = Uint8List(maxLength); - final encoded = utf8.encode(string); - for (var i = 0; i < maxLength - 1 && i < encoded.length; i++) { + // Grapheme-safe: a raw byte copy would cut a multi-byte character in half + // and put invalid UTF-8 on the wire. (#636) + final encoded = utf8TruncateToBytes(string, maxLength - 1); + for (var i = 0; i < encoded.length; i++) { bytes[i] = encoded[i]; } writeBytes(bytes); @@ -1065,13 +1096,14 @@ Uint8List buildSendSelfAdvertFrame({bool flood = false}) { // Build CMD_SET_ADVERT_NAME frame // Format: [cmd][name...] Uint8List buildSetAdvertNameFrame(String name) { - final nameBytes = utf8.encode(name); - final nameLen = nameBytes.length < maxNameSize - ? nameBytes.length - : maxNameSize - 1; + // Grapheme-safe. This one matters most of the three: it sets THIS device's + // own advert name, which is what every other node on the mesh sees, what is + // embedded in the contact cards this device emits, and what other clients + // match against. Corrupt it once and the corruption propagates outward. + // (#636) final writer = BufferWriter(); writer.writeByte(cmdSetAdvertName); - writer.writeBytes(Uint8List.fromList(nameBytes.sublist(0, nameLen))); + writer.writeBytes(utf8TruncateToBytes(name, maxNameSize - 1)); return writer.toBytes(); } diff --git a/test/connector/utf8_name_truncation_test.dart b/test/connector/utf8_name_truncation_test.dart new file mode 100644 index 0000000..346d8a5 --- /dev/null +++ b/test/connector/utf8_name_truncation_test.dart @@ -0,0 +1,124 @@ +// Grapheme-safe name truncation (#636). +// +// The on-wire name field is 32 bytes including a null terminator, so 31 are +// usable. Before this fix the three writers copied raw bytes up to that +// boundary, cutting multi-byte characters in half and putting invalid UTF-8 on +// the wire. ASCII names were unaffected, which is why it went unnoticed. + +import 'dart:convert'; +import 'dart:typed_data'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:meshcore_open/connector/meshcore_protocol.dart'; + +/// Decodes strictly. Throws if the bytes are not valid UTF-8, which is exactly +/// the failure this fix prevents. +String strictDecode(List bytes) => + utf8.decode(bytes, allowMalformed: false); + +void main() { + group('utf8TruncateToBytes (#636)', () { + test('leaves anything that already fits completely alone', () { + for (final s in ['', 'Bob', 'Roger KY4RS', 'a' * 31]) { + expect(utf8TruncateToBytes(s, 31), utf8.encode(s), reason: s); + } + }); + + test('never emits invalid UTF-8, whatever the cut point', () { + // The core property. Sweep every budget across a string whose characters + // are 1, 3 and 4 bytes, so a byte-wise cut would land mid-character at + // many of these lengths. + const s = 'ab中文🧙cd漢字'; + for (var budget = 0; budget <= utf8.encode(s).length + 2; budget++) { + final out = utf8TruncateToBytes(s, budget); + expect(out.length, lessThanOrEqualTo(budget)); + // Would throw on a split character. + expect( + () => strictDecode(out), + returnsNormally, + reason: 'budget $budget', + ); + expect( + s.startsWith(strictDecode(out)), + isTrue, + reason: 'budget $budget', + ); + } + }); + + test('CJK truncates on a character boundary, not a byte one', () { + // 11 CJK characters is 33 bytes against a 31-byte budget. + const name = '中文节点名称测试一二三'; + expect(utf8.encode(name).length, 33); + final out = utf8TruncateToBytes(name, 31); + // 10 characters at 3 bytes each fit; the 11th does not. + expect(out.length, 30); + expect(strictDecode(out), '中文节点名称测试一二'); + }); + + test('a ZWJ emoji sequence is kept whole or dropped whole', () { + // The mage is 4 codepoints joined by ZWJ plus a variation selector, 13 + // bytes. A codepoint-safe cut would still be wrong here: it could leave a + // bare mage, a dangling joiner, or an orphaned selector. + const mage = '\u{1F9D9}‍♂️'; + expect(utf8.encode(mage).length, 13); + + // One byte short of fitting: the whole cluster must go. + expect(utf8TruncateToBytes(mage, 12), isEmpty); + // Exactly fitting: kept intact. + expect(strictDecode(utf8TruncateToBytes(mage, 13)), mage); + + // And no partial cluster survives at any budget below 13. + for (var b = 0; b < 13; b++) { + expect(utf8TruncateToBytes(mage, b), isEmpty, reason: 'budget $b'); + } + }); + + test('a zero or negative budget yields nothing', () { + expect(utf8TruncateToBytes('anything', 0), isEmpty); + expect(utf8TruncateToBytes('anything', -5), isEmpty); + }); + }); + + group('the three writers are grapheme-safe (#636)', () { + // Reads a fixed-width, null-padded name field back out. + String nameFrom(Uint8List frame, int offset, int width) { + final slice = frame.sublist(offset, offset + width); + final end = slice.indexOf(0); + return strictDecode(slice.sublist(0, end < 0 ? slice.length : end)); + } + + const cjk = '中文节点名称测试一二三'; + + test('buildSetAdvertNameFrame: own advert name survives intact', () { + // The most externally visible of the three: this is the name the whole + // mesh sees and the name embedded in emitted contact cards. + final frame = buildSetAdvertNameFrame(cjk); + final decoded = strictDecode(frame.sublist(1)); + expect(decoded, '中文节点名称测试一二'); + expect(frame.length - 1, lessThanOrEqualTo(maxNameSize - 1)); + }); + + test('buildUpdateContactPathFrame: contact name survives intact', () { + final frame = buildUpdateContactPathFrame( + Uint8List(pubKeySize), + Uint8List(0), + -1, + name: cjk, + ); + expect(nameFrom(frame, contactNameOffset, maxNameSize), '中文节点名称测试一二'); + }); + + test('buildSetChannelFrame: channel name survives intact', () { + final frame = buildSetChannelFrame(0, cjk, Uint8List(16)); + // [cmd][idx][name x32][psk x16] + expect(nameFrom(frame, 2, maxNameSize), '中文节点名称测试一二'); + }); + + test('ASCII names are byte-identical to the old behaviour', () { + // No regression for the overwhelmingly common case. + final frame = buildSetAdvertNameFrame('Roger KY4RS'); + expect(strictDecode(frame.sublist(1)), 'Roger KY4RS'); + }); + }); +} From 2716afda50750b7f4807a6e042a695b846ebdf19 Mon Sep 17 00:00:00 2001 From: Strycher Date: Wed, 9 Sep 2026 03:05:42 -0400 Subject: [PATCH 2/4] feat(#610): render a received contact card as an Add Contact chip The receive half of contact sharing. Until now an incoming rendered as raw text while the stock app showed a native Add Contact button for the identical payload, so sharing worked outbound only: a stock user could add an Offband user from a card, but not the reverse. Confirmed by the owner on hardware. Reuses the proven mention-chip mechanism in TranslatedMessageContent rather than adding new machinery. Both patterns are now collected and sorted by position, so a message carrying a mention AND a card renders both in the right order; they cannot overlap, since a mention is @[...] and a card is <...>. Tapping opens the add dialog seeded with the card rather than adding silently. A contact is an identity, so adding one stays a deliberate act with the key, name and type visible first. Two deliberate refusals: - A card whose key is already a contact renders inert, mirroring stock, which warned the owner rather than silently re-adding. - A card matching the shape but failing the parser, such as an out-of-range type, falls back to plain text. Showing an Add chip there would promise something the parser will refuse. Parsing already existed from the #611 Gemini review, so this change is rendering and dedupe only. Epic #610 under Feature #609. Co-Authored-By: Claude Opus 5 --- lib/l10n/app_en.arb | 17 +++ lib/l10n/app_localizations.dart | 18 +++ lib/l10n/app_localizations_bg.dart | 13 ++ lib/l10n/app_localizations_de.dart | 13 ++ lib/l10n/app_localizations_en.dart | 13 ++ lib/l10n/app_localizations_es.dart | 13 ++ lib/l10n/app_localizations_fr.dart | 13 ++ lib/l10n/app_localizations_hu.dart | 13 ++ lib/l10n/app_localizations_it.dart | 13 ++ lib/l10n/app_localizations_ja.dart | 13 ++ lib/l10n/app_localizations_ko.dart | 13 ++ lib/l10n/app_localizations_nl.dart | 13 ++ lib/l10n/app_localizations_pl.dart | 13 ++ lib/l10n/app_localizations_pt.dart | 13 ++ lib/l10n/app_localizations_ru.dart | 13 ++ lib/l10n/app_localizations_sk.dart | 13 ++ lib/l10n/app_localizations_sl.dart | 13 ++ lib/l10n/app_localizations_sv.dart | 13 ++ lib/l10n/app_localizations_uk.dart | 13 ++ lib/l10n/app_localizations_zh.dart | 13 ++ lib/widgets/contact_card_chip.dart | 93 +++++++++++++++ lib/widgets/translated_message_content.dart | 39 ++++-- test/widgets/contact_card_chip_test.dart | 124 ++++++++++++++++++++ untranslated.json | 51 ++++++++ 24 files changed, 569 insertions(+), 7 deletions(-) create mode 100644 lib/widgets/contact_card_chip.dart create mode 100644 test/widgets/contact_card_chip_test.dart diff --git a/lib/l10n/app_en.arb b/lib/l10n/app_en.arb index 248b112..d7a9dfd 100644 --- a/lib/l10n/app_en.arb +++ b/lib/l10n/app_en.arb @@ -2521,6 +2521,23 @@ "contacts_verifiedByMessage": "Key confirmed. A message with this contact went through, which only works with the matching key.", "contacts_verifiedKeyOnly": "Added from a key. Nothing has confirmed it on air yet.", "contacts_lastSeenNever": "Not heard yet", + "contacts_cardAddContact": "Add {name}", + "@contacts_cardAddContact": { + "placeholders": { + "name": { + "type": "String" + } + } + }, + "contacts_cardAlreadyAdded": "{name}", + "@contacts_cardAlreadyAdded": { + "placeholders": { + "name": { + "type": "String" + } + } + }, + "contacts_cardAlreadyAddedTooltip": "Already in your contacts", "chat_attachTooltip": "Add to message", "chat_attachGif": "GIF", "chat_attachMyContact": "My contact card", diff --git a/lib/l10n/app_localizations.dart b/lib/l10n/app_localizations.dart index c886449..0518899 100644 --- a/lib/l10n/app_localizations.dart +++ b/lib/l10n/app_localizations.dart @@ -7576,6 +7576,24 @@ abstract class AppLocalizations { /// **'Not heard yet'** String get contacts_lastSeenNever; + /// No description provided for @contacts_cardAddContact. + /// + /// In en, this message translates to: + /// **'Add {name}'** + String contacts_cardAddContact(String name); + + /// No description provided for @contacts_cardAlreadyAdded. + /// + /// In en, this message translates to: + /// **'{name}'** + String contacts_cardAlreadyAdded(String name); + + /// No description provided for @contacts_cardAlreadyAddedTooltip. + /// + /// In en, this message translates to: + /// **'Already in your contacts'** + String get contacts_cardAlreadyAddedTooltip; + /// No description provided for @chat_attachTooltip. /// /// In en, this message translates to: diff --git a/lib/l10n/app_localizations_bg.dart b/lib/l10n/app_localizations_bg.dart index b0cf93b..cba48a2 100644 --- a/lib/l10n/app_localizations_bg.dart +++ b/lib/l10n/app_localizations_bg.dart @@ -4425,6 +4425,19 @@ class AppLocalizationsBg extends AppLocalizations { @override String get contacts_lastSeenNever => 'Not heard yet'; + @override + String contacts_cardAddContact(String name) { + return 'Add $name'; + } + + @override + String contacts_cardAlreadyAdded(String name) { + return '$name'; + } + + @override + String get contacts_cardAlreadyAddedTooltip => 'Already in your contacts'; + @override String get chat_attachTooltip => 'Add to message'; diff --git a/lib/l10n/app_localizations_de.dart b/lib/l10n/app_localizations_de.dart index a55a6ac..39ddff6 100644 --- a/lib/l10n/app_localizations_de.dart +++ b/lib/l10n/app_localizations_de.dart @@ -4437,6 +4437,19 @@ class AppLocalizationsDe extends AppLocalizations { @override String get contacts_lastSeenNever => 'Not heard yet'; + @override + String contacts_cardAddContact(String name) { + return 'Add $name'; + } + + @override + String contacts_cardAlreadyAdded(String name) { + return '$name'; + } + + @override + String get contacts_cardAlreadyAddedTooltip => 'Already in your contacts'; + @override String get chat_attachTooltip => 'Add to message'; diff --git a/lib/l10n/app_localizations_en.dart b/lib/l10n/app_localizations_en.dart index 0415d17..a308c43 100644 --- a/lib/l10n/app_localizations_en.dart +++ b/lib/l10n/app_localizations_en.dart @@ -4359,6 +4359,19 @@ class AppLocalizationsEn extends AppLocalizations { @override String get contacts_lastSeenNever => 'Not heard yet'; + @override + String contacts_cardAddContact(String name) { + return 'Add $name'; + } + + @override + String contacts_cardAlreadyAdded(String name) { + return '$name'; + } + + @override + String get contacts_cardAlreadyAddedTooltip => 'Already in your contacts'; + @override String get chat_attachTooltip => 'Add to message'; diff --git a/lib/l10n/app_localizations_es.dart b/lib/l10n/app_localizations_es.dart index 7bd6e11..f6523dc 100644 --- a/lib/l10n/app_localizations_es.dart +++ b/lib/l10n/app_localizations_es.dart @@ -4426,6 +4426,19 @@ class AppLocalizationsEs extends AppLocalizations { @override String get contacts_lastSeenNever => 'Not heard yet'; + @override + String contacts_cardAddContact(String name) { + return 'Add $name'; + } + + @override + String contacts_cardAlreadyAdded(String name) { + return '$name'; + } + + @override + String get contacts_cardAlreadyAddedTooltip => 'Already in your contacts'; + @override String get chat_attachTooltip => 'Add to message'; diff --git a/lib/l10n/app_localizations_fr.dart b/lib/l10n/app_localizations_fr.dart index 4d514b2..a1097d3 100644 --- a/lib/l10n/app_localizations_fr.dart +++ b/lib/l10n/app_localizations_fr.dart @@ -4448,6 +4448,19 @@ class AppLocalizationsFr extends AppLocalizations { @override String get contacts_lastSeenNever => 'Not heard yet'; + @override + String contacts_cardAddContact(String name) { + return 'Add $name'; + } + + @override + String contacts_cardAlreadyAdded(String name) { + return '$name'; + } + + @override + String get contacts_cardAlreadyAddedTooltip => 'Already in your contacts'; + @override String get chat_attachTooltip => 'Add to message'; diff --git a/lib/l10n/app_localizations_hu.dart b/lib/l10n/app_localizations_hu.dart index 676dafb..c236146 100644 --- a/lib/l10n/app_localizations_hu.dart +++ b/lib/l10n/app_localizations_hu.dart @@ -4443,6 +4443,19 @@ class AppLocalizationsHu extends AppLocalizations { @override String get contacts_lastSeenNever => 'Not heard yet'; + @override + String contacts_cardAddContact(String name) { + return 'Add $name'; + } + + @override + String contacts_cardAlreadyAdded(String name) { + return '$name'; + } + + @override + String get contacts_cardAlreadyAddedTooltip => 'Already in your contacts'; + @override String get chat_attachTooltip => 'Add to message'; diff --git a/lib/l10n/app_localizations_it.dart b/lib/l10n/app_localizations_it.dart index 5e9bb80..a00a17b 100644 --- a/lib/l10n/app_localizations_it.dart +++ b/lib/l10n/app_localizations_it.dart @@ -4431,6 +4431,19 @@ class AppLocalizationsIt extends AppLocalizations { @override String get contacts_lastSeenNever => 'Not heard yet'; + @override + String contacts_cardAddContact(String name) { + return 'Add $name'; + } + + @override + String contacts_cardAlreadyAdded(String name) { + return '$name'; + } + + @override + String get contacts_cardAlreadyAddedTooltip => 'Already in your contacts'; + @override String get chat_attachTooltip => 'Add to message'; diff --git a/lib/l10n/app_localizations_ja.dart b/lib/l10n/app_localizations_ja.dart index 566fa32..b1e0e9b 100644 --- a/lib/l10n/app_localizations_ja.dart +++ b/lib/l10n/app_localizations_ja.dart @@ -4228,6 +4228,19 @@ class AppLocalizationsJa extends AppLocalizations { @override String get contacts_lastSeenNever => 'Not heard yet'; + @override + String contacts_cardAddContact(String name) { + return 'Add $name'; + } + + @override + String contacts_cardAlreadyAdded(String name) { + return '$name'; + } + + @override + String get contacts_cardAlreadyAddedTooltip => 'Already in your contacts'; + @override String get chat_attachTooltip => 'Add to message'; diff --git a/lib/l10n/app_localizations_ko.dart b/lib/l10n/app_localizations_ko.dart index 0321b79..a7a03f1 100644 --- a/lib/l10n/app_localizations_ko.dart +++ b/lib/l10n/app_localizations_ko.dart @@ -4230,6 +4230,19 @@ class AppLocalizationsKo extends AppLocalizations { @override String get contacts_lastSeenNever => 'Not heard yet'; + @override + String contacts_cardAddContact(String name) { + return 'Add $name'; + } + + @override + String contacts_cardAlreadyAdded(String name) { + return '$name'; + } + + @override + String get contacts_cardAlreadyAddedTooltip => 'Already in your contacts'; + @override String get chat_attachTooltip => 'Add to message'; diff --git a/lib/l10n/app_localizations_nl.dart b/lib/l10n/app_localizations_nl.dart index f6dd647..7aa8ff0 100644 --- a/lib/l10n/app_localizations_nl.dart +++ b/lib/l10n/app_localizations_nl.dart @@ -4411,6 +4411,19 @@ class AppLocalizationsNl extends AppLocalizations { @override String get contacts_lastSeenNever => 'Not heard yet'; + @override + String contacts_cardAddContact(String name) { + return 'Add $name'; + } + + @override + String contacts_cardAlreadyAdded(String name) { + return '$name'; + } + + @override + String get contacts_cardAlreadyAddedTooltip => 'Already in your contacts'; + @override String get chat_attachTooltip => 'Add to message'; diff --git a/lib/l10n/app_localizations_pl.dart b/lib/l10n/app_localizations_pl.dart index 08c0a80..77fd257 100644 --- a/lib/l10n/app_localizations_pl.dart +++ b/lib/l10n/app_localizations_pl.dart @@ -4441,6 +4441,19 @@ class AppLocalizationsPl extends AppLocalizations { @override String get contacts_lastSeenNever => 'Not heard yet'; + @override + String contacts_cardAddContact(String name) { + return 'Add $name'; + } + + @override + String contacts_cardAlreadyAdded(String name) { + return '$name'; + } + + @override + String get contacts_cardAlreadyAddedTooltip => 'Already in your contacts'; + @override String get chat_attachTooltip => 'Add to message'; diff --git a/lib/l10n/app_localizations_pt.dart b/lib/l10n/app_localizations_pt.dart index e091edf..a20e565 100644 --- a/lib/l10n/app_localizations_pt.dart +++ b/lib/l10n/app_localizations_pt.dart @@ -4423,6 +4423,19 @@ class AppLocalizationsPt extends AppLocalizations { @override String get contacts_lastSeenNever => 'Not heard yet'; + @override + String contacts_cardAddContact(String name) { + return 'Add $name'; + } + + @override + String contacts_cardAlreadyAdded(String name) { + return '$name'; + } + + @override + String get contacts_cardAlreadyAddedTooltip => 'Already in your contacts'; + @override String get chat_attachTooltip => 'Add to message'; diff --git a/lib/l10n/app_localizations_ru.dart b/lib/l10n/app_localizations_ru.dart index 3a543c3..500f4f9 100644 --- a/lib/l10n/app_localizations_ru.dart +++ b/lib/l10n/app_localizations_ru.dart @@ -4433,6 +4433,19 @@ class AppLocalizationsRu extends AppLocalizations { @override String get contacts_lastSeenNever => 'Not heard yet'; + @override + String contacts_cardAddContact(String name) { + return 'Add $name'; + } + + @override + String contacts_cardAlreadyAdded(String name) { + return '$name'; + } + + @override + String get contacts_cardAlreadyAddedTooltip => 'Already in your contacts'; + @override String get chat_attachTooltip => 'Add to message'; diff --git a/lib/l10n/app_localizations_sk.dart b/lib/l10n/app_localizations_sk.dart index 0cba92f..5e2bfac 100644 --- a/lib/l10n/app_localizations_sk.dart +++ b/lib/l10n/app_localizations_sk.dart @@ -4407,6 +4407,19 @@ class AppLocalizationsSk extends AppLocalizations { @override String get contacts_lastSeenNever => 'Not heard yet'; + @override + String contacts_cardAddContact(String name) { + return 'Add $name'; + } + + @override + String contacts_cardAlreadyAdded(String name) { + return '$name'; + } + + @override + String get contacts_cardAlreadyAddedTooltip => 'Already in your contacts'; + @override String get chat_attachTooltip => 'Add to message'; diff --git a/lib/l10n/app_localizations_sl.dart b/lib/l10n/app_localizations_sl.dart index b7bbd5a..68874f3 100644 --- a/lib/l10n/app_localizations_sl.dart +++ b/lib/l10n/app_localizations_sl.dart @@ -4401,6 +4401,19 @@ class AppLocalizationsSl extends AppLocalizations { @override String get contacts_lastSeenNever => 'Not heard yet'; + @override + String contacts_cardAddContact(String name) { + return 'Add $name'; + } + + @override + String contacts_cardAlreadyAdded(String name) { + return '$name'; + } + + @override + String get contacts_cardAlreadyAddedTooltip => 'Already in your contacts'; + @override String get chat_attachTooltip => 'Add to message'; diff --git a/lib/l10n/app_localizations_sv.dart b/lib/l10n/app_localizations_sv.dart index b8e4588..8d1031a 100644 --- a/lib/l10n/app_localizations_sv.dart +++ b/lib/l10n/app_localizations_sv.dart @@ -4380,6 +4380,19 @@ class AppLocalizationsSv extends AppLocalizations { @override String get contacts_lastSeenNever => 'Not heard yet'; + @override + String contacts_cardAddContact(String name) { + return 'Add $name'; + } + + @override + String contacts_cardAlreadyAdded(String name) { + return '$name'; + } + + @override + String get contacts_cardAlreadyAddedTooltip => 'Already in your contacts'; + @override String get chat_attachTooltip => 'Add to message'; diff --git a/lib/l10n/app_localizations_uk.dart b/lib/l10n/app_localizations_uk.dart index 4c6569b..ac8e0fc 100644 --- a/lib/l10n/app_localizations_uk.dart +++ b/lib/l10n/app_localizations_uk.dart @@ -4432,6 +4432,19 @@ class AppLocalizationsUk extends AppLocalizations { @override String get contacts_lastSeenNever => 'Not heard yet'; + @override + String contacts_cardAddContact(String name) { + return 'Add $name'; + } + + @override + String contacts_cardAlreadyAdded(String name) { + return '$name'; + } + + @override + String get contacts_cardAlreadyAddedTooltip => 'Already in your contacts'; + @override String get chat_attachTooltip => 'Add to message'; diff --git a/lib/l10n/app_localizations_zh.dart b/lib/l10n/app_localizations_zh.dart index 39fff5d..d0a2d12 100644 --- a/lib/l10n/app_localizations_zh.dart +++ b/lib/l10n/app_localizations_zh.dart @@ -4131,6 +4131,19 @@ class AppLocalizationsZh extends AppLocalizations { @override String get contacts_lastSeenNever => 'Not heard yet'; + @override + String contacts_cardAddContact(String name) { + return 'Add $name'; + } + + @override + String contacts_cardAlreadyAdded(String name) { + return '$name'; + } + + @override + String get contacts_cardAlreadyAddedTooltip => 'Already in your contacts'; + @override String get chat_attachTooltip => 'Add to message'; diff --git a/lib/widgets/contact_card_chip.dart b/lib/widgets/contact_card_chip.dart new file mode 100644 index 0000000..051f882 --- /dev/null +++ b/lib/widgets/contact_card_chip.dart @@ -0,0 +1,93 @@ +import 'package:flutter/material.dart'; +import 'package:provider/provider.dart'; + +import '../connector/meshcore_connector.dart'; +import '../l10n/l10n.dart'; +import '../models/contact.dart'; +import 'add_contact_by_key_dialog.dart'; + +/// Renders a received contact share card as a tappable Add Contact affordance +/// instead of the raw `` text. (#610) +/// +/// This is the receive half of the exchange. The send half (#611) already +/// emits this format, and the stock app already renders it as a native Add +/// Contact button, so until now sharing worked outbound only: a stock user +/// could add an Offband user from a card, but not the reverse. +/// +/// Tapping opens the add dialog seeded with the card, so the user sees the key, +/// name and type before anything is written to the radio. It does NOT add +/// silently: a contact is an identity, and adding one should be a deliberate +/// act with the details visible. +class ContactCardChip extends StatelessWidget { + const ContactCardChip({super.key, required this.card, required this.style}); + + /// The raw matched card text, ``. + final String card; + + final TextStyle style; + + @override + Widget build(BuildContext context) { + final parsed = Contact.fromChannelShare(card); + final scheme = Theme.of(context).colorScheme; + final l10n = context.l10n; + + // The regex matched the shape but the parser rejected the contents, e.g. a + // type outside the documented range. Show the original text rather than a + // chip that would lie about being addable. + if (parsed == null) return Text(card, style: style); + + // Mirrors what stock does: it warned the owner when the contact was + // already held rather than silently re-adding. + final known = context.select( + (c) => c.contacts.any((x) => x.publicKeyHex == parsed.publicKeyHex), + ); + + final label = known + ? l10n.contacts_cardAlreadyAdded(parsed.name) + : l10n.contacts_cardAddContact(parsed.name); + + final chip = Container( + padding: const EdgeInsets.symmetric(horizontal: 8, vertical: 3), + decoration: BoxDecoration( + color: known + ? scheme.onSurface.withValues(alpha: 0.08) + : scheme.primaryContainer, + borderRadius: BorderRadius.circular(8), + ), + child: Row( + mainAxisSize: MainAxisSize.min, + children: [ + Icon( + known ? Icons.how_to_reg : Icons.person_add_alt_1, + size: 15, + color: known ? scheme.onSurfaceVariant : scheme.onPrimaryContainer, + ), + const SizedBox(width: 5), + Text( + label, + style: style.copyWith( + fontWeight: FontWeight.w500, + color: known + ? scheme.onSurfaceVariant + : scheme.onPrimaryContainer, + ), + ), + ], + ), + ); + + if (known) { + return Tooltip( + message: l10n.contacts_cardAlreadyAddedTooltip, + child: chip, + ); + } + + return InkWell( + borderRadius: BorderRadius.circular(8), + onTap: () => showAddContactByKeyDialog(context, initialKeyText: card), + child: chip, + ); + } +} diff --git a/lib/widgets/translated_message_content.dart b/lib/widgets/translated_message_content.dart index 96b8917..97b8623 100644 --- a/lib/widgets/translated_message_content.dart +++ b/lib/widgets/translated_message_content.dart @@ -1,6 +1,7 @@ import 'package:flutter/material.dart'; import '../helpers/link_handler.dart'; +import 'contact_card_chip.dart'; class TranslatedMessageContent extends StatelessWidget { final String displayText; @@ -25,6 +26,15 @@ class TranslatedMessageContent extends StatelessWidget { // A leading `@[Name] ` reply prefix. static final RegExp _replyPrefix = RegExp(r'^@\[([^\]]+)\]\s+'); + /// A contact share card, `<64-hex key:type:name>`, rendered as a tappable + /// Add Contact chip rather than the raw text it used to show. (#610) + /// + /// This is the format real clients put on the air, confirmed because the + /// stock app renders it as a native Add Contact button. The name is the final + /// field and may contain colons and spaces, so it is matched greedily to the + /// closing bracket; brackets themselves are stripped by the emitter. + static final RegExp _contactCard = RegExp(r'<[0-9a-fA-F]{64}:\d+:[^>]*>'); + /// The name of a leading `@[Name]` reply mention, or null. Lets callers show /// a reply chip above content that isn't rendered as text (e.g. a reply-gif, /// #232). @@ -50,27 +60,42 @@ class TranslatedMessageContent extends StatelessWidget { } Widget _buildText(BuildContext context, String text, TextStyle textStyle) { - if (!_mention.hasMatch(text)) { + final hasMention = _mention.hasMatch(text); + final hasCard = _contactCard.hasMatch(text); + if (!hasMention && !hasCard) { return LinkHandler.buildLinkifyText( context: context, text: text, style: textStyle, ); } - // Mentions can't be interleaved with the Linkify widget, so a message that - // contains a mention renders as rich text with chip spans (links inside a - // mention message are not tappable, same as the prior leading-mention - // path). Messages without a mention keep full link support above. + // Neither chip can be interleaved with the Linkify widget, so a message + // containing one renders as rich text with chip spans (links inside such a + // message are not tappable, same as the prior leading-mention path). + // Messages with neither keep full link support above. + // + // Both patterns are collected and sorted by position, so a message + // carrying a mention AND a contact card renders both in the right order. + // They cannot overlap: a mention is `@[...]`, a card is `<...>`. + final matches = [ + ..._mention.allMatches(text), + ..._contactCard.allMatches(text), + ]..sort((a, b) => a.start.compareTo(b.start)); + final spans = []; var last = 0; - for (final m in _mention.allMatches(text)) { + for (final m in matches) { + if (m.start < last) continue; if (m.start > last) { spans.add(TextSpan(text: text.substring(last, m.start))); } + final isCard = text[m.start] == '<'; spans.add( WidgetSpan( alignment: PlaceholderAlignment.middle, - child: mentionChip(context, m.group(1)!, textStyle), + child: isCard + ? ContactCardChip(card: m.group(0)!, style: textStyle) + : mentionChip(context, m.group(1)!, textStyle), ), ); last = m.end; diff --git a/test/widgets/contact_card_chip_test.dart b/test/widgets/contact_card_chip_test.dart new file mode 100644 index 0000000..01bfe63 --- /dev/null +++ b/test/widgets/contact_card_chip_test.dart @@ -0,0 +1,124 @@ +// Receiving a contact card (#610). +// +// Before this, an incoming rendered as raw text while the stock +// app showed a native Add Contact button for the very same payload. Sharing +// worked outbound only. + +import 'dart:typed_data'; + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:provider/provider.dart'; + +import 'package:meshcore_open/connector/meshcore_connector.dart'; +import 'package:meshcore_open/connector/meshcore_protocol.dart'; +import 'package:meshcore_open/l10n/app_localizations.dart'; +import 'package:meshcore_open/models/contact.dart'; +import 'package:meshcore_open/widgets/translated_message_content.dart'; + +const _key = '00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff'; +const _card = '<$_key:1:Ka8sbi>'; + +class _Conn extends MeshCoreConnector { + _Conn({this.known = const []}); + final List known; + + @override + List get contacts => known; +} + +Contact _contact(String keyHex) => Contact( + publicKey: hex2Uint8List(keyHex), + name: 'Ka8sbi', + type: advTypeChat, + pathLength: -1, + path: Uint8List(0), + lastSeen: DateTime(2026, 9, 9), +); + +Future _pump(WidgetTester tester, String text, _Conn conn) => + tester.pumpWidget( + ChangeNotifierProvider.value( + value: conn, + child: MaterialApp( + localizationsDelegates: AppLocalizations.localizationsDelegates, + supportedLocales: AppLocalizations.supportedLocales, + home: Scaffold( + body: TranslatedMessageContent( + displayText: text, + style: const TextStyle(fontSize: 14), + ), + ), + ), + ), + ); + +void main() { + testWidgets('a received card renders as an actionable chip, not raw text', ( + tester, + ) async { + await _pump(tester, 'here is mine $_card', _Conn()); + + // The raw payload must not be shown. + expect(find.textContaining(_key), findsNothing); + // An actionable affordance is offered instead. + expect(find.byIcon(Icons.person_add_alt_1), findsOneWidget); + expect(find.byType(InkWell), findsOneWidget); + // The caption survives alongside it. + expect(find.textContaining('here is mine'), findsOneWidget); + }); + + testWidgets('a card for a known contact does not offer to re-add', ( + tester, + ) async { + // Mirrors stock, which warned rather than silently re-adding. + await _pump(tester, _card, _Conn(known: [_contact(_key)])); + + expect(find.byIcon(Icons.how_to_reg), findsOneWidget); + expect(find.byIcon(Icons.person_add_alt_1), findsNothing); + // Not tappable, so it cannot be double-added. + expect(find.byType(InkWell), findsNothing); + }); + + testWidgets('a malformed card falls back to plain text rather than lying', ( + tester, + ) async { + // Right shape, impossible type. Rendering an Add chip here would promise + // something the parser will refuse. + const bad = '<$_key:9:Bob>'; + await _pump(tester, bad, _Conn()); + + expect(find.byIcon(Icons.person_add_alt_1), findsNothing); + expect(find.textContaining(bad), findsOneWidget); + }); + + testWidgets('a mention and a card in one message both render', ( + tester, + ) async { + // The two patterns are collected and sorted by position, so this is the + // case that would break a naive single-regex implementation. + await _pump(tester, '@[Bob] add this $_card', _Conn()); + + expect(find.textContaining('@Bob'), findsOneWidget); + expect(find.byIcon(Icons.person_add_alt_1), findsOneWidget); + expect(find.textContaining(_key), findsNothing); + }); + + testWidgets('a plain message is untouched and keeps link support', ( + tester, + ) async { + await _pump(tester, 'just a normal message', _Conn()); + + expect(find.byIcon(Icons.person_add_alt_1), findsNothing); + expect(find.textContaining('just a normal message'), findsOneWidget); + }); + + testWidgets('names with spaces and emoji survive into the chip label', ( + tester, + ) async { + await _pump(tester, '<$_key:2:Roger KY4RS 🧙>', _Conn()); + + expect(find.textContaining('Roger KY4RS 🧙'), findsOneWidget); + expect(find.byIcon(Icons.person_add_alt_1), findsOneWidget); + }); +} diff --git a/untranslated.json b/untranslated.json index 2f8061a..8a88df9 100644 --- a/untranslated.json +++ b/untranslated.json @@ -115,6 +115,9 @@ "contacts_verifiedByMessage", "contacts_verifiedKeyOnly", "contacts_lastSeenNever", + "contacts_cardAddContact", + "contacts_cardAlreadyAdded", + "contacts_cardAlreadyAddedTooltip", "chat_attachTooltip", "chat_attachGif", "chat_attachMyContact", @@ -261,6 +264,9 @@ "contacts_verifiedByMessage", "contacts_verifiedKeyOnly", "contacts_lastSeenNever", + "contacts_cardAddContact", + "contacts_cardAlreadyAdded", + "contacts_cardAlreadyAddedTooltip", "chat_attachTooltip", "chat_attachGif", "chat_attachMyContact", @@ -407,6 +413,9 @@ "contacts_verifiedByMessage", "contacts_verifiedKeyOnly", "contacts_lastSeenNever", + "contacts_cardAddContact", + "contacts_cardAlreadyAdded", + "contacts_cardAlreadyAddedTooltip", "chat_attachTooltip", "chat_attachGif", "chat_attachMyContact", @@ -553,6 +562,9 @@ "contacts_verifiedByMessage", "contacts_verifiedKeyOnly", "contacts_lastSeenNever", + "contacts_cardAddContact", + "contacts_cardAlreadyAdded", + "contacts_cardAlreadyAddedTooltip", "chat_attachTooltip", "chat_attachGif", "chat_attachMyContact", @@ -699,6 +711,9 @@ "contacts_verifiedByMessage", "contacts_verifiedKeyOnly", "contacts_lastSeenNever", + "contacts_cardAddContact", + "contacts_cardAlreadyAdded", + "contacts_cardAlreadyAddedTooltip", "chat_attachTooltip", "chat_attachGif", "chat_attachMyContact", @@ -845,6 +860,9 @@ "contacts_verifiedByMessage", "contacts_verifiedKeyOnly", "contacts_lastSeenNever", + "contacts_cardAddContact", + "contacts_cardAlreadyAdded", + "contacts_cardAlreadyAddedTooltip", "chat_attachTooltip", "chat_attachGif", "chat_attachMyContact", @@ -991,6 +1009,9 @@ "contacts_verifiedByMessage", "contacts_verifiedKeyOnly", "contacts_lastSeenNever", + "contacts_cardAddContact", + "contacts_cardAlreadyAdded", + "contacts_cardAlreadyAddedTooltip", "chat_attachTooltip", "chat_attachGif", "chat_attachMyContact", @@ -1137,6 +1158,9 @@ "contacts_verifiedByMessage", "contacts_verifiedKeyOnly", "contacts_lastSeenNever", + "contacts_cardAddContact", + "contacts_cardAlreadyAdded", + "contacts_cardAlreadyAddedTooltip", "chat_attachTooltip", "chat_attachGif", "chat_attachMyContact", @@ -1283,6 +1307,9 @@ "contacts_verifiedByMessage", "contacts_verifiedKeyOnly", "contacts_lastSeenNever", + "contacts_cardAddContact", + "contacts_cardAlreadyAdded", + "contacts_cardAlreadyAddedTooltip", "chat_attachTooltip", "chat_attachGif", "chat_attachMyContact", @@ -1429,6 +1456,9 @@ "contacts_verifiedByMessage", "contacts_verifiedKeyOnly", "contacts_lastSeenNever", + "contacts_cardAddContact", + "contacts_cardAlreadyAdded", + "contacts_cardAlreadyAddedTooltip", "chat_attachTooltip", "chat_attachGif", "chat_attachMyContact", @@ -1575,6 +1605,9 @@ "contacts_verifiedByMessage", "contacts_verifiedKeyOnly", "contacts_lastSeenNever", + "contacts_cardAddContact", + "contacts_cardAlreadyAdded", + "contacts_cardAlreadyAddedTooltip", "chat_attachTooltip", "chat_attachGif", "chat_attachMyContact", @@ -1721,6 +1754,9 @@ "contacts_verifiedByMessage", "contacts_verifiedKeyOnly", "contacts_lastSeenNever", + "contacts_cardAddContact", + "contacts_cardAlreadyAdded", + "contacts_cardAlreadyAddedTooltip", "chat_attachTooltip", "chat_attachGif", "chat_attachMyContact", @@ -1867,6 +1903,9 @@ "contacts_verifiedByMessage", "contacts_verifiedKeyOnly", "contacts_lastSeenNever", + "contacts_cardAddContact", + "contacts_cardAlreadyAdded", + "contacts_cardAlreadyAddedTooltip", "chat_attachTooltip", "chat_attachGif", "chat_attachMyContact", @@ -2013,6 +2052,9 @@ "contacts_verifiedByMessage", "contacts_verifiedKeyOnly", "contacts_lastSeenNever", + "contacts_cardAddContact", + "contacts_cardAlreadyAdded", + "contacts_cardAlreadyAddedTooltip", "chat_attachTooltip", "chat_attachGif", "chat_attachMyContact", @@ -2159,6 +2201,9 @@ "contacts_verifiedByMessage", "contacts_verifiedKeyOnly", "contacts_lastSeenNever", + "contacts_cardAddContact", + "contacts_cardAlreadyAdded", + "contacts_cardAlreadyAddedTooltip", "chat_attachTooltip", "chat_attachGif", "chat_attachMyContact", @@ -2305,6 +2350,9 @@ "contacts_verifiedByMessage", "contacts_verifiedKeyOnly", "contacts_lastSeenNever", + "contacts_cardAddContact", + "contacts_cardAlreadyAdded", + "contacts_cardAlreadyAddedTooltip", "chat_attachTooltip", "chat_attachGif", "chat_attachMyContact", @@ -2451,6 +2499,9 @@ "contacts_verifiedByMessage", "contacts_verifiedKeyOnly", "contacts_lastSeenNever", + "contacts_cardAddContact", + "contacts_cardAlreadyAdded", + "contacts_cardAlreadyAddedTooltip", "chat_attachTooltip", "chat_attachGif", "chat_attachMyContact", From a2097f1a31667d4bc8802b63262d385050fe80b2 Mon Sep 17 00:00:00 2001 From: Strycher Date: Wed, 9 Sep 2026 04:25:58 -0400 Subject: [PATCH 3/4] fix(#610): apply Gemini review, harden the card parser and chip Three accepted findings, plus a real defect the adversarial test cases flushed out. A long name overflowed the chip by 405 pixels. The name comes off a public radio channel and is attacker-controlled, so anyone could break a message list by posting a card with a long name. The label is now bounded and ellipsized, with an explicit width ceiling because a WidgetSpan hands its child unbounded width. fromChannelShare no longer scans from the first < to the last >. Two cards in one message were read as a single span running from the first key to the last name. That was reachable: the add dialog passes raw pasted text straight to it. It now takes the first well-formed card via a regex whose name class cannot cross a closing bracket. The known-contact check is O(1) against the connector's existing _knownContactKeys set, through a new isKnownContact. Scanning contacts ran per chip on every notification, and the public knownContactKeys getter copies the whole set, so neither belonged in a build method. Card-versus-mention is now carried on the match instead of inferred from text[m.start] == '<', which would mis-route the day a third bracketed pattern is added. Two findings were rejected on evidence and are justified in the PR body: a claimed missing null terminator (firmware sets it from the frame length itself) and a claimed greedy-regex failure (disproven by test). Epic #610. Co-Authored-By: Claude Opus 5 --- lib/connector/meshcore_connector.dart | 9 +++ lib/models/contact.dart | 24 +++++-- lib/widgets/contact_card_chip.dart | 70 ++++++++++++++------- lib/widgets/translated_message_content.dart | 14 +++-- test/widgets/contact_card_chip_test.dart | 58 ++++++++++++----- 5 files changed, 122 insertions(+), 53 deletions(-) diff --git a/lib/connector/meshcore_connector.dart b/lib/connector/meshcore_connector.dart index 192adb3..4b8bf6f 100644 --- a/lib/connector/meshcore_connector.dart +++ b/lib/connector/meshcore_connector.dart @@ -943,6 +943,15 @@ class MeshCoreConnector extends ChangeNotifier { int get maxContacts => _maxContacts; int get maxChannels => _maxChannels; Set get knownContactKeys => Set.unmodifiable(_knownContactKeys); + + /// O(1) membership test for a contact by public key hex. + /// + /// Use this from a widget `build`, never [knownContactKeys], which copies the + /// whole set on every call, nor a scan of [contacts]. A received contact card + /// asks this question once per rendered chip on every connector + /// notification. (#610) + bool isKnownContact(String publicKeyHex) => + _knownContactKeys.contains(publicKeyHex); double? get contactSyncProgress { final total = _contactSyncTotal; if (!_isLoadingContacts || total == null || total <= 0) return null; diff --git a/lib/models/contact.dart b/lib/models/contact.dart index df10384..f4d1dc6 100644 --- a/lib/models/contact.dart +++ b/lib/models/contact.dart @@ -296,6 +296,13 @@ class Contact { '&public_key=$publicKeyHex' '&type=$type'; + /// The body of one compact channel share, ``, captured + /// without its delimiters. `[^>]*` cannot cross a closing bracket, so each + /// card matches individually even when several sit in one message. + static final RegExp _channelShareBody = RegExp( + r'<([0-9a-fA-F]{64}:\d+:[^>]*)>', + ); + /// Compact contact share for a CHANNEL message, ``. (#611) /// /// This is a second, different format from [toShareUri], and deliberately so. @@ -338,12 +345,17 @@ class Contact { /// Rendering a received card as a tappable Add Contact affordance is #610 and /// is separate; this only handles text pasted or scanned into the add flow. static Contact? fromChannelShare(String text) { - final trimmed = text.trim(); - // Tolerate a card embedded in a longer message, which is how it arrives. - final start = trimmed.indexOf('<'); - final end = trimmed.lastIndexOf('>'); - if (start < 0 || end <= start) return null; - final body = trimmed.substring(start + 1, end); + // Tolerate a card embedded in a longer message, which is how it arrives, + // and take the FIRST well-formed one. + // + // Deliberately not a scan from the first `<` to the last `>`: a message + // carrying two cards would then be read as a single span running from the + // first key to the last name and parse as garbage. That is reachable, + // because the add dialog passes raw pasted text straight to here. + // (Gemini review, #610) + final match = _channelShareBody.firstMatch(text); + if (match == null) return null; + final body = match.group(1)!; final firstColon = body.indexOf(':'); if (firstColon < 0) return null; diff --git a/lib/widgets/contact_card_chip.dart b/lib/widgets/contact_card_chip.dart index 051f882..97a7ae4 100644 --- a/lib/widgets/contact_card_chip.dart +++ b/lib/widgets/contact_card_chip.dart @@ -39,41 +39,63 @@ class ContactCardChip extends StatelessWidget { // Mirrors what stock does: it warned the owner when the contact was // already held rather than silently re-adding. + // + // O(1) against the connector's maintained key set. Scanning `contacts` + // here would run per rendered chip on every notification, which is + // hundreds of comparisons in a message list on a device with a large + // contact book. (Gemini review, #610) final known = context.select( - (c) => c.contacts.any((x) => x.publicKeyHex == parsed.publicKeyHex), + (c) => c.isKnownContact(parsed.publicKeyHex), ); final label = known ? l10n.contacts_cardAlreadyAdded(parsed.name) : l10n.contacts_cardAddContact(parsed.name); - final chip = Container( - padding: const EdgeInsets.symmetric(horizontal: 8, vertical: 3), - decoration: BoxDecoration( - color: known - ? scheme.onSurface.withValues(alpha: 0.08) - : scheme.primaryContainer, - borderRadius: BorderRadius.circular(8), - ), - child: Row( - mainAxisSize: MainAxisSize.min, - children: [ - Icon( - known ? Icons.how_to_reg : Icons.person_add_alt_1, - size: 15, - color: known ? scheme.onSurfaceVariant : scheme.onPrimaryContainer, - ), - const SizedBox(width: 5), - Text( - label, - style: style.copyWith( - fontWeight: FontWeight.w500, + // A WidgetSpan gives its child unbounded width, so Flexible needs a real + // ceiling to ellipsize against. Without this the chip grows to whatever + // name the sender chose. + final chip = ConstrainedBox( + constraints: const BoxConstraints(maxWidth: 260), + child: Container( + padding: const EdgeInsets.symmetric(horizontal: 8, vertical: 3), + decoration: BoxDecoration( + color: known + ? scheme.onSurface.withValues(alpha: 0.08) + : scheme.primaryContainer, + borderRadius: BorderRadius.circular(8), + ), + child: Row( + mainAxisSize: MainAxisSize.min, + children: [ + Icon( + known ? Icons.how_to_reg : Icons.person_add_alt_1, + size: 15, color: known ? scheme.onSurfaceVariant : scheme.onPrimaryContainer, ), - ), - ], + const SizedBox(width: 5), + // Bounded and ellipsized. The name comes off a public radio channel + // and is attacker-controlled, so an unbounded label lets anyone + // overflow the message layout by posting a card with a long name. + // Reproduced before this: a 405 pixel RenderFlex overflow. + // (Gemini review prompted the adversarial case, #610) + Flexible( + child: Text( + label, + maxLines: 1, + overflow: TextOverflow.ellipsis, + style: style.copyWith( + fontWeight: FontWeight.w500, + color: known + ? scheme.onSurfaceVariant + : scheme.onPrimaryContainer, + ), + ), + ), + ], + ), ), ); diff --git a/lib/widgets/translated_message_content.dart b/lib/widgets/translated_message_content.dart index 97b8623..d61f6cf 100644 --- a/lib/widgets/translated_message_content.dart +++ b/lib/widgets/translated_message_content.dart @@ -77,19 +77,21 @@ class TranslatedMessageContent extends StatelessWidget { // Both patterns are collected and sorted by position, so a message // carrying a mention AND a contact card renders both in the right order. // They cannot overlap: a mention is `@[...]`, a card is `<...>`. - final matches = [ - ..._mention.allMatches(text), - ..._contactCard.allMatches(text), - ]..sort((a, b) => a.start.compareTo(b.start)); + // Each match carries which pattern produced it. Inferring the kind from + // the text, e.g. `text[m.start] == '<'`, would silently mis-route the day + // a third bracketed pattern is added. (Gemini review, #610) + final matches = <(Match, bool isCard)>[ + ..._mention.allMatches(text).map((m) => (m, false)), + ..._contactCard.allMatches(text).map((m) => (m, true)), + ]..sort((a, b) => a.$1.start.compareTo(b.$1.start)); final spans = []; var last = 0; - for (final m in matches) { + for (final (m, isCard) in matches) { if (m.start < last) continue; if (m.start > last) { spans.add(TextSpan(text: text.substring(last, m.start))); } - final isCard = text[m.start] == '<'; spans.add( WidgetSpan( alignment: PlaceholderAlignment.middle, diff --git a/test/widgets/contact_card_chip_test.dart b/test/widgets/contact_card_chip_test.dart index 01bfe63..4693fbe 100644 --- a/test/widgets/contact_card_chip_test.dart +++ b/test/widgets/contact_card_chip_test.dart @@ -4,38 +4,28 @@ // app showed a native Add Contact button for the very same payload. Sharing // worked outbound only. -import 'dart:typed_data'; - import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:provider/provider.dart'; import 'package:meshcore_open/connector/meshcore_connector.dart'; -import 'package:meshcore_open/connector/meshcore_protocol.dart'; import 'package:meshcore_open/l10n/app_localizations.dart'; -import 'package:meshcore_open/models/contact.dart'; import 'package:meshcore_open/widgets/translated_message_content.dart'; const _key = '00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff'; const _card = '<$_key:1:Ka8sbi>'; class _Conn extends MeshCoreConnector { - _Conn({this.known = const []}); - final List known; + _Conn({this.known = const {}}); + + /// Public key hexes already in contacts. Mirrors the connector's own + /// `_knownContactKeys` set, which is what the chip now consults. + final Set known; @override - List get contacts => known; + bool isKnownContact(String publicKeyHex) => known.contains(publicKeyHex); } -Contact _contact(String keyHex) => Contact( - publicKey: hex2Uint8List(keyHex), - name: 'Ka8sbi', - type: advTypeChat, - pathLength: -1, - path: Uint8List(0), - lastSeen: DateTime(2026, 9, 9), -); - Future _pump(WidgetTester tester, String text, _Conn conn) => tester.pumpWidget( ChangeNotifierProvider.value( @@ -72,7 +62,7 @@ void main() { tester, ) async { // Mirrors stock, which warned rather than silently re-adding. - await _pump(tester, _card, _Conn(known: [_contact(_key)])); + await _pump(tester, _card, _Conn(known: {_key})); expect(find.byIcon(Icons.how_to_reg), findsOneWidget); expect(find.byIcon(Icons.person_add_alt_1), findsNothing); @@ -113,6 +103,40 @@ void main() { expect(find.textContaining('just a normal message'), findsOneWidget); }); + testWidgets('two cards in one message render as two separate chips', ( + tester, + ) async { + // Regression for the Gemini review: parsing from the first `<` to the last + // `>` would have read this as one span running from the first key to the + // second name, and produced garbage. + const other = + 'ffeeddccbbaa99887766554433221100ffeeddccbbaa99887766554433221100'; + await _pump(tester, '$_card and <$other:2:Bob>', _Conn()); + + expect(find.byIcon(Icons.person_add_alt_1), findsNWidgets(2)); + expect(find.textContaining('Ka8sbi'), findsOneWidget); + expect(find.textContaining('Bob'), findsOneWidget); + expect(find.textContaining(_key), findsNothing); + }); + + testWidgets('a nested bracket mess yields one card, and it is the outer key', ( + tester, + ) async { + // Adversarial input from a public channel. `[^>]*` cannot cross a `>`, so + // the match ends at the inner closing bracket and the nested text becomes + // part of the OUTER card's name. Exactly one add is offered, for the outer + // key, and the tap opens the dialog where the key is visible before + // anything is written to the radio. No second key is silently smuggled in. + const other = + 'ffeeddccbbaa99887766554433221100ffeeddccbbaa99887766554433221100'; + await _pump(tester, '<$_key:1:Name <$other:2:Inner>>', _Conn()); + + expect(find.byIcon(Icons.person_add_alt_1), findsOneWidget); + // The parser took the outer card, so the nested text is shown as the name + // rather than being treated as a second identity. + expect(find.textContaining('Name <'), findsOneWidget); + }); + testWidgets('names with spaces and emoji survive into the chip label', ( tester, ) async { From 02d8f1c46f2c42823de1619121f683a2cc4fa646 Mon Sep 17 00:00:00 2001 From: Strycher Date: Wed, 9 Sep 2026 04:29:54 -0400 Subject: [PATCH 4/4] fix(#610): scale the card chip width off the viewport Gemini recheck: a hardcoded 260px ceiling ellipsizes prematurely when a large accessibility font is set, even on a wide screen, and looks oddly constrained on desktop. Now 70% of viewport width, which keeps the overflow bound on a narrow phone and gives room where there is room. Co-Authored-By: Claude Opus 5 --- lib/widgets/contact_card_chip.dart | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/widgets/contact_card_chip.dart b/lib/widgets/contact_card_chip.dart index 97a7ae4..19d41e3 100644 --- a/lib/widgets/contact_card_chip.dart +++ b/lib/widgets/contact_card_chip.dart @@ -54,9 +54,14 @@ class ContactCardChip extends StatelessWidget { // A WidgetSpan gives its child unbounded width, so Flexible needs a real // ceiling to ellipsize against. Without this the chip grows to whatever - // name the sender chose. + // name the sender chose, which overflowed the message layout. + // + // Scaled off the viewport rather than a fixed pixel count, so a large + // accessibility font on a wide screen is not ellipsized while space + // remains, and a narrow phone still gets a sane bound. (Gemini recheck) + final maxChipWidth = MediaQuery.sizeOf(context).width * 0.7; final chip = ConstrainedBox( - constraints: const BoxConstraints(maxWidth: 260), + constraints: BoxConstraints(maxWidth: maxChipWidth), child: Container( padding: const EdgeInsets.symmetric(horizontal: 8, vertical: 3), decoration: BoxDecoration(