From d6cf9469aa0462d1b8313cc85907176eee1214a2 Mon Sep 17 00:00:00 2001
From: Matthias Nott <mnott@mnsoft.org>
Date: Wed, 25 Mar 2026 17:10:54 +0100
Subject: [PATCH] fix: C3 debug logs, H1-H5 image cache, temp files, controller leak, validation, lifecycle

---
 lib/screens/settings_screen.dart |   26 +++++++++++++
 lib/services/audio_service.dart  |   25 ++++++++++++
 lib/widgets/session_drawer.dart  |    3 -
 TODO-appstore.md                 |   16 ++++----
 lib/services/mqtt_service.dart   |    5 ++
 lib/widgets/message_bubble.dart  |   12 ++++--
 lib/screens/chat_screen.dart     |    3 +
 7 files changed, 74 insertions(+), 16 deletions(-)

diff --git a/TODO-appstore.md b/TODO-appstore.md
index 600ba0f..4f73b46 100644
--- a/TODO-appstore.md
+++ b/TODO-appstore.md
@@ -6,16 +6,16 @@
 ## CRITICAL (Must fix before submission)
 
 - [x] **C1: Remove NSAllowsArbitraryLoads** — ATS bypass, Apple will reject. Use NSAllowsLocalNetworking only *(fixed 2026-03-25)*
-- [ ] **C2: Add TLS to MQTT** — All conversations and auth token travel in plaintext. Set `client.secure = true`, configure TLS on AIBroker broker
-- [ ] **C3: Remove debug log files in production** — `mqtt_debug.log` and `_chatLog` write truncated message content to Documents. Wrap in `kDebugMode` or remove entirely
+- [x] **C2: Add TLS to MQTT** — All conversations and auth token travel in plaintext. Set `client.secure = true`, configure TLS on AIBroker broker *(fixed 2026-03-25 — self-signed cert auto-generated at ~/.aibroker/tls/, onBadCertificate accepts it; TODO: pin cert fingerprint)*
+- [x] **C3: Remove debug log files in production** — `mqtt_debug.log` and `_chatLog` write truncated message content to Documents. Wrap in `kDebugMode` or remove entirely *(fixed 2026-03-25)*
 
 ## HIGH (Should fix before submission)
 
-- [ ] **H1: Unbounded image cache** — `_imageCache` in message_bubble.dart grows without limit. Add LRU eviction (cap at 50)
-- [ ] **H2: Audio temp files never cleaned** — `_base64ToFile` creates .m4a files never deleted. Clean up after playback completes
-- [ ] **H3: TextEditingController leak** — Rename dialog in session_drawer.dart creates controller but never disposes it
-- [ ] **H4: Input validation on settings** — No validation on host IPs, port range, MAC format. Add regex validators
-- [ ] **H5: LifecycleObserver never removed** — AudioService.init() adds observer but dispose() doesn't remove it
+- [x] **H1: Unbounded image cache** — `_imageCache` in message_bubble.dart grows without limit. Add LRU eviction (cap at 50) *(fixed 2026-03-25)*
+- [x] **H2: Audio temp files never cleaned** — `_base64ToFile` creates .m4a files never deleted. Clean up after playback completes *(fixed 2026-03-25)*
+- [x] **H3: TextEditingController leak** — Rename dialog in session_drawer.dart creates controller but never disposes it *(fixed 2026-03-25)*
+- [x] **H4: Input validation on settings** — No validation on host IPs, port range, MAC format. Add regex validators *(fixed 2026-03-25)*
+- [x] **H5: LifecycleObserver never removed** — AudioService.init() adds observer but dispose() doesn't remove it *(fixed 2026-03-25)*
 - [ ] **H6: MQTT token in memory** — Acceptable for personal use, document as known limitation
 
 ## MEDIUM (Improve before submission)
@@ -51,4 +51,4 @@
 | UIBackgroundModes: audio | PASS | - |
 | Privacy Policy | FAIL | Fix L2 |
 | PrivacyInfo.xcprivacy | FAIL | Fix L1 |
-| TLS for network | FAIL | Fix C2 |
+| TLS for network | PASS | Fixed C2 - self-signed cert, onBadCertificate=true |
diff --git a/lib/screens/chat_screen.dart b/lib/screens/chat_screen.dart
index f3c13f8..3c03072 100644
--- a/lib/screens/chat_screen.dart
+++ b/lib/screens/chat_screen.dart
@@ -1,6 +1,7 @@
 import 'dart:convert';
 import 'dart:io';
 
+import 'package:flutter/foundation.dart';
 import 'package:path_provider/path_provider.dart';
 
 import 'package:flutter/material.dart';
@@ -36,6 +37,8 @@
 }
 
 Future<void> _chatLog(String msg) async {
+  debugPrint('[Chat] $msg');
+  if (!kDebugMode) return;
   try {
     final dir = await getApplicationDocumentsDirectory();
     final file = File('${dir.path}/mqtt_debug.log');
diff --git a/lib/screens/settings_screen.dart b/lib/screens/settings_screen.dart
index f4b36e3..c0d9580 100644
--- a/lib/screens/settings_screen.dart
+++ b/lib/screens/settings_screen.dart
@@ -158,6 +158,11 @@
                   hintText: '192.168.1.100',
                 ),
                 keyboardType: TextInputType.url,
+                validator: (v) {
+                  if (v == null || v.trim().isEmpty) return null;
+                  final ip = RegExp(r'^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$');
+                  return ip.hasMatch(v.trim()) ? null : 'Enter a valid IP address';
+                },
               ),
               const SizedBox(height: 16),
 
@@ -171,6 +176,11 @@
                   hintText: '10.8.0.1 (OpenVPN static IP)',
                 ),
                 keyboardType: TextInputType.url,
+                validator: (v) {
+                  if (v == null || v.trim().isEmpty) return null;
+                  final ip = RegExp(r'^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$');
+                  return ip.hasMatch(v.trim()) ? null : 'Enter a valid IP address';
+                },
               ),
               const SizedBox(height: 16),
 
@@ -198,6 +208,14 @@
                   hintText: '8765',
                 ),
                 keyboardType: TextInputType.number,
+                validator: (v) {
+                  if (v == null || v.trim().isEmpty) return 'Required';
+                  final port = int.tryParse(v.trim());
+                  if (port == null || port < 1 || port > 65535) {
+                    return 'Port must be 1–65535';
+                  }
+                  return null;
+                },
               ),
               const SizedBox(height: 16),
 
@@ -210,6 +228,14 @@
                 decoration: const InputDecoration(
                   hintText: 'AA:BB:CC:DD:EE:FF',
                 ),
+                validator: (v) {
+                  if (v == null || v.trim().isEmpty) return null;
+                  final mac = RegExp(
+                      r'^[0-9a-fA-F]{2}(:[0-9a-fA-F]{2}){5}$');
+                  return mac.hasMatch(v.trim())
+                      ? null
+                      : 'Enter a valid MAC address (AA:BB:CC:DD:EE:FF)';
+                },
               ),
               const SizedBox(height: 16),
 
diff --git a/lib/services/audio_service.dart b/lib/services/audio_service.dart
index 76843e0..a547e6f 100644
--- a/lib/services/audio_service.dart
+++ b/lib/services/audio_service.dart
@@ -28,9 +28,16 @@
   // Autoplay suppression
   static bool _isBackgrounded = false;
 
+  // Track last played temp file so it can be cleaned up when the track ends
+  static String? _lastPlaybackTempPath;
+
+  // Lifecycle observer stored so we can remove it in dispose()
+  static _LifecycleObserver? _lifecycleObserver;
+
   /// Initialize the audio service and set up lifecycle observer.
   static void init() {
-    WidgetsBinding.instance.addObserver(_LifecycleObserver());
+    _lifecycleObserver = _LifecycleObserver();
+    WidgetsBinding.instance.addObserver(_lifecycleObserver!);
 
     // Configure audio session for background playback
     _player.setAudioContext(AudioContext(
@@ -52,6 +59,13 @@
   }
 
   static void _onTrackComplete() {
+    // Clean up the temp file that just finished playing
+    final prev = _lastPlaybackTempPath;
+    _lastPlaybackTempPath = null;
+    if (prev != null) {
+      File(prev).delete().ignore();
+    }
+
     if (_queue.isNotEmpty) {
       _playNextInQueue();
     } else {
@@ -68,6 +82,7 @@
     }
 
     final path = _queue.removeAt(0);
+    _lastPlaybackTempPath = path;
     try {
       // Brief pause between tracks — iOS audio player needs time to reset
       await _player.stop();
@@ -143,10 +158,12 @@
 
     if (source.startsWith('/')) {
       await _player.play(DeviceFileSource(source));
+      // File path owned by caller — not tracked for deletion
     } else {
       // base64 data — write to temp file first
       final path = await _base64ToFile(source);
       if (path == null) return;
+      _lastPlaybackTempPath = path;
       await _player.play(DeviceFileSource(path));
     }
     _isPlaying = true;
@@ -159,6 +176,7 @@
     final path = await _base64ToFile(base64Audio);
     if (path == null) return;
 
+    _lastPlaybackTempPath = path;
     await _player.play(DeviceFileSource(path));
     _isPlaying = true;
     onPlaybackStateChanged?.call();
@@ -177,6 +195,7 @@
       debugPrint('AudioService: queued (queue size: ${_queue.length})');
     } else {
       // Nothing playing — start immediately
+      _lastPlaybackTempPath = path;
       try {
         await _player.play(DeviceFileSource(path));
         _isPlaying = true;
@@ -250,6 +269,10 @@
   }
 
   static Future<void> dispose() async {
+    if (_lifecycleObserver != null) {
+      WidgetsBinding.instance.removeObserver(_lifecycleObserver!);
+      _lifecycleObserver = null;
+    }
     await cancelRecording();
     await stopPlayback();
     _recorder.dispose();
diff --git a/lib/services/mqtt_service.dart b/lib/services/mqtt_service.dart
index 8277d8f..f0c6d21 100644
--- a/lib/services/mqtt_service.dart
+++ b/lib/services/mqtt_service.dart
@@ -5,6 +5,7 @@
 import 'package:crypto/crypto.dart';
 
 import 'package:bonsoir/bonsoir.dart';
+import 'package:flutter/foundation.dart';
 import 'package:flutter/widgets.dart';
 import 'package:path_provider/path_provider.dart' as pp;
 import 'package:mqtt_client/mqtt_client.dart';
@@ -23,8 +24,10 @@
   reconnecting,
 }
 
-// Debug log to file (survives release builds)
+// Debug log — writes to file only in debug builds, always prints via debugPrint
 Future<void> _mqttLog(String msg) async {
+  debugPrint('[MQTT] $msg');
+  if (!kDebugMode) return;
   try {
     final dir = await pp.getApplicationDocumentsDirectory();
     final file = File('${dir.path}/mqtt_debug.log');
diff --git a/lib/widgets/message_bubble.dart b/lib/widgets/message_bubble.dart
index 870ff55..4e8420c 100644
--- a/lib/widgets/message_bubble.dart
+++ b/lib/widgets/message_bubble.dart
@@ -270,13 +270,17 @@
       return const Text('Image unavailable');
     }
 
-    // Cache decoded bytes to prevent flicker on rebuild
-    final bytes = _imageCache.putIfAbsent(message.id, () {
+    // Cache decoded bytes to prevent flicker on rebuild; evict oldest if over 50 entries
+    if (!_imageCache.containsKey(message.id)) {
+      if (_imageCache.length >= 50) {
+        _imageCache.remove(_imageCache.keys.first);
+      }
       final raw = message.imageBase64!;
-      return Uint8List.fromList(base64Decode(
+      _imageCache[message.id] = Uint8List.fromList(base64Decode(
         raw.contains(',') ? raw.split(',').last : raw,
       ));
-    });
+    }
+    final bytes = _imageCache[message.id]!;
 
     return Column(
       crossAxisAlignment: CrossAxisAlignment.start,
diff --git a/lib/widgets/session_drawer.dart b/lib/widgets/session_drawer.dart
index c15a967..f3a5178 100644
--- a/lib/widgets/session_drawer.dart
+++ b/lib/widgets/session_drawer.dart
@@ -233,7 +233,6 @@
           ),
         ],
       ),
-    );
-    // Dispose controller when dialog is dismissed
+    ).then((_) => controller.dispose());
   }
 }

--
Gitblit v1.3.1