Merge 02d8f1c46f into b33e8485f3
Strycher
4 days ago
committed by
GitHub
commit
a369a5af1d
@ -0,0 +1,120 @@
|
||||
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 `<key:type:name>` 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, `<key:type:name>`.
|
||||
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.
|
||||
//
|
||||
// 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<MeshCoreConnector, bool>(
|
||||
(c) => c.isKnownContact(parsed.publicKeyHex),
|
||||
);
|
||||
|
||||
final label = known
|
||||
? l10n.contacts_cardAlreadyAdded(parsed.name)
|
||||
: l10n.contacts_cardAddContact(parsed.name);
|
||||
|
||||
// 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, 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: BoxConstraints(maxWidth: maxChipWidth),
|
||||
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,
|
||||
),
|
||||
),
|
||||
),
|
||||
],
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
if (known) {
|
||||
return Tooltip(
|
||||
message: l10n.contacts_cardAlreadyAddedTooltip,
|
||||
child: chip,
|
||||
);
|
||||
}
|
||||
|
||||
return InkWell(
|
||||
borderRadius: BorderRadius.circular(8),
|
||||
onTap: () => showAddContactByKeyDialog(context, initialKeyText: card),
|
||||
child: chip,
|
||||
);
|
||||
}
|
||||
}
|
||||
@ -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<int> 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');
|
||||
});
|
||||
});
|
||||
}
|
||||
@ -0,0 +1,148 @@
|
||||
// Receiving a contact card (#610).
|
||||
//
|
||||
// Before this, an incoming <key:type:name> rendered as raw text while the stock
|
||||
// app showed a native Add Contact button for the very same payload. Sharing
|
||||
// worked outbound only.
|
||||
|
||||
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/l10n/app_localizations.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 {}});
|
||||
|
||||
/// Public key hexes already in contacts. Mirrors the connector's own
|
||||
/// `_knownContactKeys` set, which is what the chip now consults.
|
||||
final Set<String> known;
|
||||
|
||||
@override
|
||||
bool isKnownContact(String publicKeyHex) => known.contains(publicKeyHex);
|
||||
}
|
||||
|
||||
Future<void> _pump(WidgetTester tester, String text, _Conn conn) =>
|
||||
tester.pumpWidget(
|
||||
ChangeNotifierProvider<MeshCoreConnector>.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: {_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('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 {
|
||||
await _pump(tester, '<$_key:2:Roger KY4RS 🧙>', _Conn());
|
||||
|
||||
expect(find.textContaining('Roger KY4RS 🧙'), findsOneWidget);
|
||||
expect(find.byIcon(Icons.person_add_alt_1), findsOneWidget);
|
||||
});
|
||||
}
|
||||
Loading…
Reference in new issue