From 767dc1164edae9b42be64911375bc3232819199c Mon Sep 17 00:00:00 2001 From: Winston Lowe Date: Sun, 22 Mar 2026 10:50:11 -0700 Subject: [PATCH] refactor: Replace string reading methods with CString equivalents and improve error handling --- lib/connector/meshcore_connector.dart | 92 ++++++++++------------ lib/connector/meshcore_protocol.dart | 71 +++++++---------- lib/models/channel_message.dart | 10 +-- lib/models/message.dart | 2 +- lib/services/ble_debug_log_service.dart | 13 --- lib/services/message_retry_service.dart | 9 +-- lib/widgets/debug_frame_viewer.dart | 8 -- test/services/retry_and_protocol_test.dart | 12 +-- 8 files changed, 82 insertions(+), 135 deletions(-) diff --git a/lib/connector/meshcore_connector.dart b/lib/connector/meshcore_connector.dart index e8555ff3..5dc660e5 100644 --- a/lib/connector/meshcore_connector.dart +++ b/lib/connector/meshcore_connector.dart @@ -2127,9 +2127,7 @@ class MeshCoreConnector extends ChangeNotifier { outboundText, selfKey, ); - final ackHashHex = ackHash - .map((b) => b.toRadixString(16).padLeft(2, '0')) - .join(); + final ackHashHex = ackHashToHex(ackHash); final messageBytes = utf8.encode(outboundText).length; _pendingRepeaterAcks[ackHashHex]?.timeout?.cancel(); _pendingRepeaterAcks[ackHashHex] = _RepeaterAckContext( @@ -2901,7 +2899,7 @@ class MeshCoreConnector extends ChangeNotifier { _currentSf = reader.readByte(); _currentCr = reader.readByte(); - _selfName = reader.readString(); + _selfName = reader.readCString(); } catch (e) { _appDebugLogService?.error( 'Error parsing SELF_INFO frame: $e', @@ -3554,7 +3552,7 @@ class MeshCoreConnector extends ChangeNotifier { reader.skipBytes(4); // Skip extra 4 bytes for signed/plain variants } - final msgText = reader.readString(); + final msgText = reader.readCString(); final flags = txtType; final shiftedType = flags >> 2; @@ -3724,10 +3722,9 @@ class MeshCoreConnector extends ChangeNotifier { final packet = _parseRawPacket(raw); if (packet == null || packet.payloadType != _payloadTypeGroupText) return; - final payload = packet.payload; - if (payload.length <= _cipherMacSize) return; - final channelHash = payload[0]; - final encrypted = Uint8List.fromList(payload.sublist(1)); + final payload = BufferReader(packet.payload); + final channelHash = payload.readByte(); + final encrypted = Uint8List.fromList(payload.readRemainingBytes()); // Use cached channels as fallback if live channels not yet loaded final channelsToSearch = _channels.isNotEmpty @@ -3741,15 +3738,14 @@ class MeshCoreConnector extends ChangeNotifier { final decryptedBytes = _decryptPayload(channel.psk, encrypted); if (decryptedBytes == null || decryptedBytes.length < 6) return; final decrypted = BufferReader(decryptedBytes); - // Skip header + SNR + reserved (2) - decrypted.skipBytes(4); + + final timestampRaw = decrypted.readUInt32LE(); final txtType = decrypted.readByte(); if ((txtType >> 2) != 0) { return; } - final timestampRaw = decrypted.readUInt32LE(); - final text = decrypted.readString(); + final text = decrypted.readCString(); final parsed = _splitSenderText(text); final decodedText = Smaz.tryDecodePrefixed(parsed.text) ?? parsed.text; @@ -3811,13 +3807,13 @@ class MeshCoreConnector extends ChangeNotifier { try { final reader = BufferReader(frame); - reader.skipBytes(2); // code + is_flood + reader.skipBytes(2); //Skip code and is_flood final ackHash = reader.readUInt32LE(); final timeoutMs = reader.readUInt32LE(); // Check if this is a CLI command ACK - if so, ignore it if (_lastSentWasCliCommand) { - final ackHashHex = ackHash.toRadixString(16).padLeft(8, '0'); + final ackHashHex = ackHashToHex(ackHash); debugPrint('Ignoring CLI command ACK (sent): $ackHashHex'); _lastSentWasCliCommand = false; return; @@ -3949,7 +3945,7 @@ class MeshCoreConnector extends ChangeNotifier { } bool _handleRepeaterCommandSent(int ackHash, int timeoutMs) { - final ackHashHex = ackHash.toRadixString(16).padLeft(8, '0'); + final ackHashHex = ackHashToHex(ackHash); final entry = _pendingRepeaterAcks[ackHashHex]; if (entry == null) return false; @@ -3968,7 +3964,7 @@ class MeshCoreConnector extends ChangeNotifier { } bool _handleRepeaterCommandAck(int ackHash, int tripTimeMs) { - final ackHashHex = ackHash.toRadixString(16).padLeft(8, '0'); + final ackHashHex = ackHashToHex(ackHash); final entry = _pendingRepeaterAcks.remove(ackHashHex); if (entry == null) return false; entry.timeout?.cancel(); @@ -4319,36 +4315,34 @@ class MeshCoreConnector extends ChangeNotifier { } _RawPacket? _parseRawPacket(Uint8List raw) { - if (raw.length < 3) return null; - var index = 0; - final header = raw[index++]; - final routeType = header & _phRouteMask; - final hasTransport = - routeType == _routeTransportFlood || routeType == _routeTransportDirect; - if (hasTransport) { - if (raw.length < index + 4) return null; - index += 4; - } - if (raw.length <= index) return null; - final pathLenRaw = raw[index++]; - final pathByteLen = _decodePathByteLen(pathLenRaw); - if (raw.length < index + pathByteLen) return null; - final pathBytes = Uint8List.fromList( - raw.sublist(index, index + pathByteLen), - ); - index += pathByteLen; - if (raw.length <= index) return null; - final payload = Uint8List.fromList(raw.sublist(index)); + try { + final reader = BufferReader(raw); + final header = reader.readByte(); + final routeType = header & _phRouteMask; + final hasTransport = + routeType == _routeTransportFlood || + routeType == _routeTransportDirect; + if (hasTransport) { + reader.skipBytes(2); // Skip reserved bytes in transport header + } + final pathLenRaw = reader.readByte(); + final pathByteLen = _decodePathByteLen(pathLenRaw); + final pathBytes = reader.readBytes(pathByteLen); + final payload = reader.readBytes(reader.remaining); - return _RawPacket( - header: header, - routeType: routeType, - payloadType: (header >> _phTypeShift) & _phTypeMask, - payloadVer: (header >> _phVerShift) & _phVerMask, - pathLenRaw: pathLenRaw, - pathBytes: pathBytes, - payload: payload, - ); + return _RawPacket( + header: header, + routeType: routeType, + payloadType: (header >> _phTypeShift) & _phTypeMask, + payloadVer: (header >> _phVerShift) & _phVerMask, + pathLenRaw: pathLenRaw, + pathBytes: pathBytes, + payload: payload, + ); + } catch (e) { + appLogger.warn('Error parsing raw packet: $e'); + return null; + } } int _computeChannelHash(Uint8List psk) { @@ -4767,7 +4761,7 @@ class MeshCoreConnector extends ChangeNotifier { void _handleCustomVars(Uint8List frame) { final buf = BufferReader(frame.sublist(1)); try { - _currentCustomVars = _parseKeyValueString(buf.readString()); + _currentCustomVars = _parseKeyValueString(buf.readCString()); } catch (e) { appLogger.warn('Malformed custom vars frame: $e', tag: 'Connector'); } @@ -4924,7 +4918,7 @@ class MeshCoreConnector extends ChangeNotifier { longitude = packet.readInt32LE() / 1e6; } if (hasName && packet.remaining > 0) { - name = packet.readString(); + name = packet.readCString(); } } catch (e) { appLogger.warn('Malformed advert frame: $e', tag: 'Connector'); @@ -4986,7 +4980,7 @@ class MeshCoreConnector extends ChangeNotifier { longitude = advert.readInt32LE() / 1e6; } if (hasName && advert.remaining > 0) { - name = advert.readString(); + name = advert.readCString(); } } catch (e) { appLogger.warn('Malformed advert frame: $e', tag: 'Connector'); diff --git a/lib/connector/meshcore_protocol.dart b/lib/connector/meshcore_protocol.dart index a2e20cdf..b3687563 100644 --- a/lib/connector/meshcore_protocol.dart +++ b/lib/connector/meshcore_protocol.dart @@ -1,6 +1,8 @@ import 'dart:convert'; import 'dart:typed_data'; +import 'package:flutter/widgets.dart'; + // Buffer Reader - sequential binary data reader with pointer tracking class BufferReader { int _pointer = 0; @@ -37,16 +39,6 @@ class BufferReader { Uint8List readRemainingBytes() => readBytes(remaining); - String readString() { - _lastPointer = _pointer; - final value = readRemainingBytes(); - try { - return utf8.decode(Uint8List.fromList(value), allowMalformed: true); - } catch (e) { - return String.fromCharCodes(value); // Latin-1 fallback - } - } - String readCStringGreedy(int maxLength) { _lastPointer = _pointer; final value = []; @@ -62,11 +54,12 @@ class BufferReader { } } - String readCString(int maxLength) { + String readCString({int maxLength = -1}) { final backupPointer = _pointer; final value = []; int counter = 0; - while (counter < maxLength) { + final maxLen = maxLength >= 0 ? maxLength : remaining; + while (counter < maxLen) { final byte = readByte(); if (byte == 0) break; value.add(byte); @@ -409,48 +402,40 @@ ParsedContactText? parseContactMessageText(Uint8List frame) { // Signed messages have a 4-byte signature after the timestamp, before the text message.skipBytes(4); } - final text = message.readString(); + final text = message.readCString(); if (text.isEmpty) return null; return ParsedContactText(senderPrefix: senderPrefix, text: text); } catch (e) { + debugPrint('Error parsing contact message text: $e'); return null; } } -// // Helper to read uint32 little-endian -// int readUint32LE(Uint8List data, int offset) { -// return data[offset] | -// (data[offset + 1] << 8) | -// (data[offset + 2] << 16) | -// (data[offset + 3] << 24); -// } +// Helper to read uint32 little-endian +int readUint32LE(Uint8List data, int offset) { + return data[offset] | + (data[offset + 1] << 8) | + (data[offset + 2] << 16) | + (data[offset + 3] << 24); +} -// // Helper to read uint16 little-endian -// int readUint16LE(Uint8List data, int offset) { -// return data[offset] | (data[offset + 1] << 8); -// } +// Helper to read uint16 little-endian +int readUint16LE(Uint8List data, int offset) { + return data[offset] | (data[offset + 1] << 8); +} -// // Helper to read int32 little-endian -// int readInt32LE(Uint8List data, int offset) { -// int val = readUint32LE(data, offset); -// if (val >= 0x80000000) val -= 0x100000000; -// return val; -// } +// Helper to read int32 little-endian +int readInt32LE(Uint8List data, int offset) { + int val = readUint32LE(data, offset); + if (val >= 0x80000000) val -= 0x100000000; + return val; +} -// // Helper to read null-terminated UTF-8 string -// String readCString(Uint8List data, int offset, int maxLen) { -// int end = offset; -// while (end < offset + maxLen && end < data.length && data[end] != 0) { -// end++; -// } -// try { -// return utf8.decode(data.sublist(offset, end), allowMalformed: true); -// } catch (e) { -// // Fallback to Latin-1 if UTF-8 decoding fails -// return String.fromCharCodes(data.sublist(offset, end)); -// } -// } +// Helper to convert uint32 to hex string +String ackHashToHex(int ackHash) { + return ackHash.toRadixString(16).padLeft(8, '0'); +} // Helper to convert public key to hex string String pubKeyToHex(Uint8List pubKey) { diff --git a/lib/models/channel_message.dart b/lib/models/channel_message.dart index ab816304..98a8f1d4 100644 --- a/lib/models/channel_message.dart +++ b/lib/models/channel_message.dart @@ -2,6 +2,7 @@ import 'dart:typed_data'; import '../connector/meshcore_protocol.dart'; import '../helpers/reaction_helper.dart'; import '../helpers/smaz.dart'; +import '../utils/app_logger.dart'; enum ChannelMessageStatus { pending, sent, failed } @@ -114,8 +115,8 @@ class ChannelMessage { // V3: [0]=code [1]=SNR [2]=rsv1 [3]=rsv2 [4]=channel_idx [5]=path_len [path... optional] [txt_type] [timestamp x4] [text...] // Non-V3: [0]=code [1]=channel_idx [2]=path_len [3]=txt_type [4-7]=timestamp [8+]=text if (frame.length < 8) return null; - final reader = BufferReader(frame); try { + final reader = BufferReader(frame); final code = reader.readByte(); if (code != respCodeChannelMsgRecv && code != respCodeChannelMsgRecvV3) { return null; @@ -133,11 +134,9 @@ class ChannelMessage { pathLen = reader.readByte(); txtType = reader.readByte(); final hasPath = (flags & 0x01) != 0; - if (hasPath) { + if (hasPath && pathLen > 0) { reader.rewind(); // Rewind to read path length again for pathBytes pathBytes = reader.readBytes(pathLen); - // Force text type to plain if path is present - txtType = txtTypePlain; } else { pathLen = 0; } @@ -152,7 +151,7 @@ class ChannelMessage { return null; } - final text = reader.readString(); + final text = reader.readCString(); // Extract sender name and actual message from "name: msg" format String senderName = 'Unknown'; @@ -185,6 +184,7 @@ class ChannelMessage { channelIndex: channelIdx, ); } catch (e) { + appLogger.error('Error parsing channel message frame: $e'); // If parsing fails, return null to avoid crashes return null; } diff --git a/lib/models/message.dart b/lib/models/message.dart index d1660dd3..6b930c08 100644 --- a/lib/models/message.dart +++ b/lib/models/message.dart @@ -105,7 +105,7 @@ class Message { if ((flags >> 2) != txtTypePlain) { return null; } - final text = reader.readString(); + final text = reader.readCString(); return Message( senderKey: senderKey, diff --git a/lib/services/ble_debug_log_service.dart b/lib/services/ble_debug_log_service.dart index 745b243a..df2822bd 100644 --- a/lib/services/ble_debug_log_service.dart +++ b/lib/services/ble_debug_log_service.dart @@ -245,19 +245,6 @@ class BleDebugLogService extends ChangeNotifier { } } - // Helper to read uint32 little-endian - int readUint32LE(Uint8List data, int offset) { - return data[offset] | - (data[offset + 1] << 8) | - (data[offset + 2] << 16) | - (data[offset + 3] << 24); - } - - // // Helper to read uint16 little-endian - int readUint16LE(Uint8List data, int offset) { - return data[offset] | (data[offset + 1] << 8); - } - String _frameDetail(int code, Uint8List frame) { switch (code) { case respCodeSent: diff --git a/lib/services/message_retry_service.dart b/lib/services/message_retry_service.dart index 19204188..94f3caf4 100644 --- a/lib/services/message_retry_service.dart +++ b/lib/services/message_retry_service.dart @@ -98,7 +98,7 @@ class MessageRetryService extends ChangeNotifier { /// Compute expected ACK hash using same algorithm as firmware: /// SHA256([timestamp(4)][attempt(1)][text][sender_pubkey(32)]) -> first 4 bytes - static Uint8List computeExpectedAckHash( + static int computeExpectedAckHash( int timestampSeconds, int attempt, String text, @@ -126,7 +126,8 @@ class MessageRetryService extends ChangeNotifier { // Compute SHA256 and return first 4 bytes final hash = sha256.convert(buffer); - return Uint8List.fromList(hash.bytes.sublist(0, 4)); + final bytes = Uint8List.fromList(hash.bytes.sublist(0, 4)); + return (bytes[3] << 24) | (bytes[2] << 16) | (bytes[1] << 8) | bytes[0]; } Future sendMessageWithRetry({ @@ -324,9 +325,7 @@ class MessageRetryService extends ChangeNotifier { outboundText, selfPubKey, ); - final expectedHashHex = expectedHash - .map((b) => b.toRadixString(16).padLeft(2, '0')) - .join(); + final expectedHashHex = expectedHash.toRadixString(16).padLeft(8, '0'); _expectedHashToMessageId[expectedHashHex] = messageId; final shortText = message.text.length > 20 diff --git a/lib/widgets/debug_frame_viewer.dart b/lib/widgets/debug_frame_viewer.dart index 05e312be..c8dc3712 100644 --- a/lib/widgets/debug_frame_viewer.dart +++ b/lib/widgets/debug_frame_viewer.dart @@ -10,14 +10,6 @@ class DebugFrameViewer { Uint8List frame, String title, ) { - // Helper to read uint32 little-endian - int readUint32LE(Uint8List data, int offset) { - return data[offset] | - (data[offset + 1] << 8) | - (data[offset + 2] << 16) | - (data[offset + 3] << 24); - } - final hexString = frame .map((b) => b.toRadixString(16).padLeft(2, '0')) .join(' '); diff --git a/test/services/retry_and_protocol_test.dart b/test/services/retry_and_protocol_test.dart index b58da450..48d4cfba 100644 --- a/test/services/retry_and_protocol_test.dart +++ b/test/services/retry_and_protocol_test.dart @@ -169,16 +169,6 @@ void main() { expect(first, equals(second)); }); - test('hash is exactly 4 bytes long', () { - final hash = MessageRetryService.computeExpectedAckHash( - fixedTs, - 0, - fixedText, - fixedKey, - ); - expect(hash.length, equals(4)); - }); - test('hash matches manual SHA-256 computation', () { for (int attempt = 0; attempt < 4; attempt++) { final actual = MessageRetryService.computeExpectedAckHash( @@ -509,7 +499,7 @@ void main() { fixedText, fixedKey, ); - final hex = hash.map((b) => b.toRadixString(16).padLeft(2, '0')).join(); + final hex = hash.toRadixString(16).padLeft(8, '0'); expect( hashes.containsKey(hex), isFalse,