Compare commits

...
Author SHA1 Message Date
Thomas SharedInboxandClaude Sonnet 4.6 c4634936ae feat(S2): validate IMAP/SMTP hostnames against injection characters
Add validateHostname / validateOptionalHostname helpers to host_utils.dart
that reject values containing @, /, \, or control characters. Wire them
into AddAccountScreen and EditAccountScreen for all host fields.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-13 23:45:37 +02:00
Bot of Thomas Güttler a0c35c647a test(R6): backoff stress tests for AccountSyncManager (#24) 2026-05-13 23:37:40 +02:00
Bot of Thomas Güttler fc592c475f feat(R4): dismissible sync error banner in email list (#23) 2026-05-13 23:14:44 +02:00
10 changed files with 455 additions and 13 deletions
@@ -65,6 +65,10 @@ abstract class SyncLogRepository {
}); });
Stream<List<SyncLogEntry>> observeSyncLogs(String accountId); Stream<List<SyncLogEntry>> observeSyncLogs(String accountId);
/// Emits the error message of the most recent sync attempt for [accountId],
/// or null when the last sync succeeded (or no syncs have run yet).
Stream<String?> observeLastError(String accountId);
} }
class NoOpSyncLogRepository implements SyncLogRepository { class NoOpSyncLogRepository implements SyncLogRepository {
@@ -90,4 +94,7 @@ class NoOpSyncLogRepository implements SyncLogRepository {
@override @override
Stream<List<SyncLogEntry>> observeSyncLogs(String accountId) => Stream<List<SyncLogEntry>> observeSyncLogs(String accountId) =>
Stream.value([]); Stream.value([]);
@override
Stream<String?> observeLastError(String accountId) => Stream.value(null);
} }
+18
View File
@@ -2,3 +2,21 @@ bool isLocalhost(String host) {
final h = host.trim().toLowerCase(); final h = host.trim().toLowerCase();
return h == 'localhost' || h == '127.0.0.1' || h == '::1'; return h == 'localhost' || h == '127.0.0.1' || h == '::1';
} }
String? validateHostname(String? value) {
if (value == null || value.trim().isEmpty) return 'Required';
return _checkHostChars(value.trim());
}
String? validateOptionalHostname(String? value) {
if (value == null || value.trim().isEmpty) return null;
return _checkHostChars(value.trim());
}
String? _checkHostChars(String h) {
if (h.contains(RegExp(r'[@/\\]')) ||
h.codeUnits.any((c) => c < 32 || c == 127)) {
return 'Invalid hostname';
}
return null;
}
@@ -99,4 +99,14 @@ class SyncLogRepositoryImpl implements SyncLogRepository {
return entries; return entries;
}); });
} }
@override
Stream<String?> observeLastError(String accountId) {
return (_db.select(_db.syncLogs)
..where((t) => t.accountId.equals(accountId))
..orderBy([(t) => OrderingTerm.desc(t.startedAt)])
..limit(1))
.watchSingleOrNull()
.map((row) => (row?.result == 'error') ? row?.errorMessage : null);
}
} }
+5
View File
@@ -85,6 +85,11 @@ final syncLogRepositoryProvider = Provider((ref) {
return SyncLogRepositoryImpl(ref.watch(dbProvider)); return SyncLogRepositoryImpl(ref.watch(dbProvider));
}); });
final syncLastErrorProvider =
StreamProvider.autoDispose.family<String?, String>((ref, accountId) {
return ref.watch(syncLogRepositoryProvider).observeLastError(accountId);
});
final reliabilityRunnerProvider = Provider<ReliabilityRunner>((ref) { final reliabilityRunnerProvider = Provider<ReliabilityRunner>((ref) {
final runner = ReliabilityRunner( final runner = ReliabilityRunner(
ref.watch(dbProvider), ref.watch(dbProvider),
+7 -5
View File
@@ -408,7 +408,7 @@ class _AddAccountScreenState extends ConsumerState<AddAccountScreen> {
_field(_passwordCtrl, 'Password', obscure: true), _field(_passwordCtrl, 'Password', obscure: true),
const Divider(height: 32), const Divider(height: 32),
Text('IMAP', style: Theme.of(context).textTheme.titleSmall), Text('IMAP', style: Theme.of(context).textTheme.titleSmall),
_field(_imapHostCtrl, 'Host'), _field(_imapHostCtrl, 'Host', validator: validateHostname),
_field(_imapPortCtrl, 'Port', keyboardType: TextInputType.number), _field(_imapPortCtrl, 'Port', keyboardType: TextInputType.number),
if (isLocalhost(_imapHostCtrl.text.trim())) if (isLocalhost(_imapHostCtrl.text.trim()))
SwitchListTile( SwitchListTile(
@@ -418,7 +418,7 @@ class _AddAccountScreenState extends ConsumerState<AddAccountScreen> {
), ),
const Divider(height: 32), const Divider(height: 32),
Text('SMTP', style: Theme.of(context).textTheme.titleSmall), Text('SMTP', style: Theme.of(context).textTheme.titleSmall),
_field(_smtpHostCtrl, 'Host'), _field(_smtpHostCtrl, 'Host', validator: validateHostname),
_field(_smtpPortCtrl, 'Port', keyboardType: TextInputType.number), _field(_smtpPortCtrl, 'Port', keyboardType: TextInputType.number),
if (isLocalhost(_smtpHostCtrl.text.trim())) if (isLocalhost(_smtpHostCtrl.text.trim()))
SwitchListTile( SwitchListTile(
@@ -475,6 +475,7 @@ class _AddAccountScreenState extends ConsumerState<AddAccountScreen> {
bool obscure = false, bool obscure = false,
bool required = true, bool required = true,
TextInputType? keyboardType, TextInputType? keyboardType,
String? Function(String?)? validator,
}) { }) {
return Padding( return Padding(
padding: const EdgeInsets.symmetric(vertical: 6), padding: const EdgeInsets.symmetric(vertical: 6),
@@ -486,9 +487,10 @@ class _AddAccountScreenState extends ConsumerState<AddAccountScreen> {
labelText: label, labelText: label,
border: const OutlineInputBorder(), border: const OutlineInputBorder(),
), ),
validator: required validator: validator ??
? (v) => (v == null || v.trim().isEmpty) ? 'Required' : null (required
: null, ? (v) => (v == null || v.trim().isEmpty) ? 'Required' : null
: null),
), ),
); );
} }
+8 -5
View File
@@ -324,11 +324,11 @@ class _EditAccountScreenState extends ConsumerState<EditAccountScreen> {
'IMAP (SSL/TLS)', 'IMAP (SSL/TLS)',
style: Theme.of(context).textTheme.titleSmall, style: Theme.of(context).textTheme.titleSmall,
), ),
_field(_imapHostCtrl, 'Host'), _field(_imapHostCtrl, 'Host', validator: validateHostname),
_field(_imapPortCtrl, 'Port', keyboardType: TextInputType.number), _field(_imapPortCtrl, 'Port', keyboardType: TextInputType.number),
const Divider(height: 32), const Divider(height: 32),
Text('SMTP', style: Theme.of(context).textTheme.titleSmall), Text('SMTP', style: Theme.of(context).textTheme.titleSmall),
_field(_smtpHostCtrl, 'Host'), _field(_smtpHostCtrl, 'Host', validator: validateHostname),
_field(_smtpPortCtrl, 'Port', keyboardType: TextInputType.number), _field(_smtpPortCtrl, 'Port', keyboardType: TextInputType.number),
if (isLocalhost(_smtpHostCtrl.text.trim())) if (isLocalhost(_smtpHostCtrl.text.trim()))
SwitchListTile( SwitchListTile(
@@ -348,6 +348,7 @@ class _EditAccountScreenState extends ConsumerState<EditAccountScreen> {
_sieveHostCtrl, _sieveHostCtrl,
'Host (leave blank to use IMAP host)', 'Host (leave blank to use IMAP host)',
required: false, required: false,
validator: validateOptionalHostname,
), ),
_field( _field(
_sievePortCtrl, _sievePortCtrl,
@@ -408,6 +409,7 @@ class _EditAccountScreenState extends ConsumerState<EditAccountScreen> {
bool obscure = false, bool obscure = false,
bool required = true, bool required = true,
TextInputType? keyboardType, TextInputType? keyboardType,
String? Function(String?)? validator,
}) { }) {
return Padding( return Padding(
padding: const EdgeInsets.symmetric(vertical: 6), padding: const EdgeInsets.symmetric(vertical: 6),
@@ -420,9 +422,10 @@ class _EditAccountScreenState extends ConsumerState<EditAccountScreen> {
labelText: label, labelText: label,
border: const OutlineInputBorder(), border: const OutlineInputBorder(),
), ),
validator: required validator: validator ??
? (v) => (v == null || v.trim().isEmpty) ? 'Required' : null (required
: null, ? (v) => (v == null || v.trim().isEmpty) ? 'Required' : null
: null),
), ),
); );
} }
+46 -3
View File
@@ -35,6 +35,9 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
bool _searchLoading = false; bool _searchLoading = false;
bool get _searching => _searchController.text.isNotEmpty; bool get _searching => _searchController.text.isNotEmpty;
// Error banner — tracks the last error message that the user dismissed.
String? _dismissedError;
// Thread-level selection (key = threadId). // Thread-level selection (key = threadId).
final Set<String> _selectedThreadIds = {}; final Set<String> _selectedThreadIds = {};
// Last-emitted thread list, used to resolve emailIds for batch operations. // Last-emitted thread list, used to resolve emailIds for batch operations.
@@ -131,9 +134,16 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
currentMailboxPath: widget.mailboxPath, currentMailboxPath: widget.mailboxPath,
), ),
bottomNavigationBar: _selecting ? _selectionBottomBar() : null, bottomNavigationBar: _selecting ? _selectionBottomBar() : null,
body: (_searchResults != null || _searchLoading) body: Column(
? _buildSearchBody() children: [
: _buildStreamBody(repo), _buildSyncErrorBanner(),
Expanded(
child: (_searchResults != null || _searchLoading)
? _buildSearchBody()
: _buildStreamBody(repo),
),
],
),
); );
} }
@@ -267,6 +277,39 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
return _buildEmailList(_searchResults!); return _buildEmailList(_searchResults!);
} }
Widget _buildSyncErrorBanner() {
final errorAsync = ref.watch(syncLastErrorProvider(widget.accountId));
final error = errorAsync.valueOrNull;
if (error == null || error == _dismissedError) {
return const SizedBox.shrink();
}
return MaterialBanner(
padding: const EdgeInsets.fromLTRB(16, 8, 8, 8),
content: Text(
error,
maxLines: 2,
overflow: TextOverflow.ellipsis,
),
leading: Icon(
Icons.sync_problem,
color: Theme.of(context).colorScheme.error,
),
backgroundColor: Theme.of(context).colorScheme.errorContainer,
actions: [
TextButton(
onPressed: () {
ref.read(syncManagerProvider).syncNow(widget.accountId);
},
child: const Text('Retry'),
),
TextButton(
onPressed: () => setState(() => _dismissedError = error),
child: const Text('Dismiss'),
),
],
);
}
Widget _buildStreamBody(EmailRepository emailRepo) { Widget _buildStreamBody(EmailRepository emailRepo) {
return RefreshIndicator( return RefreshIndicator(
onRefresh: () async { onRefresh: () async {
@@ -220,4 +220,7 @@ class _FakeLogs implements SyncLogRepository {
@override @override
Stream<List<SyncLogEntry>> observeSyncLogs(String accountId) => Stream<List<SyncLogEntry>> observeSyncLogs(String accountId) =>
Stream.value([]); Stream.value([]);
@override
Stream<String?> observeLastError(String accountId) => Stream.value(null);
} }
+3
View File
@@ -132,6 +132,9 @@ class FakeSyncLogRepository implements SyncLogRepository {
@override @override
Stream<List<SyncLogEntry>> observeSyncLogs(String accountId) => Stream<List<SyncLogEntry>> observeSyncLogs(String accountId) =>
Stream.value([]); Stream.value([]);
@override
Stream<String?> observeLastError(String accountId) => Stream.value(null);
} }
class FakeMailboxRepositoryWithInbox implements MailboxRepository { class FakeMailboxRepositoryWithInbox implements MailboxRepository {
+348
View File
@@ -0,0 +1,348 @@
import 'dart:async';
import 'package:fake_async/fake_async.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:sharedinbox/core/models/account.dart';
import 'package:sharedinbox/core/models/email.dart';
import 'package:sharedinbox/core/models/mailbox.dart';
import 'package:sharedinbox/core/repositories/account_repository.dart';
import 'package:sharedinbox/core/repositories/email_repository.dart';
import 'package:sharedinbox/core/repositories/mailbox_repository.dart';
import 'package:sharedinbox/core/repositories/sync_log_repository.dart';
import 'package:sharedinbox/core/sync/account_sync_manager.dart';
// ── helpers ───────────────────────────────────────────────────────────────────
Account _account({String id = 'a1'}) => Account(
id: id,
displayName: 'Test',
email: 'test@example.com',
imapHost: 'localhost',
);
class _FakeAccounts implements AccountRepository {
final List<Account> accounts;
_FakeAccounts([Account? account]) : accounts = [account ?? _account()];
@override
Stream<List<Account>> observeAccounts() => Stream.value(accounts);
@override
Future<Account?> getAccount(String id) async =>
accounts.cast<Account?>().firstWhere(
(a) => a?.id == id,
orElse: () => null,
);
@override
Future<void> addAccount(Account account, String password) async {}
@override
Future<void> updateAccount(Account account, {String? password}) async {}
@override
Future<void> removeAccount(String id) async {}
@override
Future<String> getPassword(String id) async => 'secret';
}
class _FakeMailboxes implements MailboxRepository {
final List<Mailbox> mailboxes;
_FakeMailboxes([this.mailboxes = const []]);
@override
Stream<List<Mailbox>> observeMailboxes(String? accountId) =>
Stream.value(mailboxes);
@override
Future<int> syncMailboxes(String accountId) async => 0;
@override
Future<Mailbox?> findMailboxByRole(String accountId, String role) async =>
null;
@override
Future<void> clearForResync(String accountId) async {}
}
class _CountingEmails implements EmailRepository {
int syncCount = 0;
int wakeUpCount = 0;
final Exception? syncError;
_CountingEmails({this.syncError});
@override
Future<SyncEmailsResult> syncEmails(String accountId, String mailbox) async {
syncCount++;
if (syncError != null) throw syncError!;
return SyncEmailsResult.zero;
}
@override
Future<int> wakeUpEmails(String accountId) async {
wakeUpCount++;
return 0;
}
@override
Future<int> flushPendingChanges(String accountId, String password) async => 0;
@override
Stream<List<Email>> observeEmails(String a, String m) => Stream.value([]);
@override
Stream<List<EmailThread>> observeThreads(String a, String m) =>
Stream.value([]);
@override
Stream<List<Email>> observeEmailsInThread(String a, String m, String t) =>
Stream.value([]);
@override
Future<Email?> getEmail(String id) async => null;
@override
Future<EmailBody> getEmailBody(String id) async =>
const EmailBody(emailId: '', attachments: []);
@override
Future<void> setFlag(String id, {bool? seen, bool? flagged}) async {}
@override
Future<void> moveEmail(String id, String dest) async {}
@override
Future<String?> deleteEmail(String id) async => null;
@override
Future<void> sendEmail(String accountId, EmailDraft draft) async {}
@override
Future<String> downloadAttachment(String id, EmailAttachment att) async => '';
@override
Future<List<Email>> searchEmails(String a, String m, String q) async => [];
@override
Future<List<Email>> searchEmailsGlobal(String? a, String q) async => [];
@override
Future<List<Email>> getEmailsByAddress(String? a, String addr) async => [];
@override
Stream<List<FailedMutation>> observeFailedMutations(String a) =>
Stream.value([]);
@override
Future<void> discardMutation(int id) async {}
@override
Future<void> retryMutation(int id) async {}
@override
Future<bool> cancelPendingChange(String id, String type) async => false;
@override
Future<void> snoozeEmail(String id, DateTime until) async {}
@override
Future<void> restoreEmails(List<Email> emails) async {}
@override
Stream<String> get onChangesQueued => const Stream.empty();
@override
Stream<void> watchJmapPush(String accountId, String password) =>
const Stream.empty();
@override
Future<ReliabilityResult> verifySyncReliability(
String accountId,
String mailboxPath,
) async =>
ReliabilityResult.healthy;
@override
Future<void> clearForResync(String accountId) async {}
}
class _FakeSyncLog implements SyncLogRepository {
final logs = <bool>[];
@override
Future<void> log({
required String accountId,
required bool success,
String? errorMessage,
required String protocol,
required int emailsFetched,
required int emailsSkipped,
required int mailboxesSynced,
required int pendingFlushed,
required int bytesTransferred,
required DateTime startedAt,
required DateTime finishedAt,
List<MailboxSyncStats> mailboxStats = const [],
String? protocolLog,
}) async {
logs.add(success);
}
@override
Stream<List<SyncLogEntry>> observeSyncLogs(String accountId) =>
Stream.value([]);
@override
Stream<String?> observeLastError(String accountId) => Stream.value(null);
}
// ── tests ─────────────────────────────────────────────────────────────────────
void main() {
group('AccountSyncManager backoff', () {
test('backoff is capped at 900 s after repeated failures', () {
fakeAsync((async) {
final emails = _CountingEmails(
syncError: Exception('connection refused'),
);
final syncLog = _FakeSyncLog();
final manager = AccountSyncManager(
_FakeAccounts(),
_FakeMailboxes([
const Mailbox(
id: 'INBOX',
accountId: 'a1',
path: 'INBOX',
name: 'Inbox',
unreadCount: 0,
totalCount: 0,
),
]),
emails,
syncLog: syncLog,
imapConnect: (_, __, ___) async =>
throw Exception('connection refused'),
);
manager.start();
// Advance 3 hours — long enough to observe many retries.
// With max backoff 900 s, we expect at least floor(3*3600/900) = 12
// attempts, and at most 3*3600/5 = 2160 (if backoff never grew).
async.elapse(const Duration(hours: 3));
final failCount = syncLog.logs.where((ok) => !ok).length;
expect(
failCount,
greaterThan(10),
reason: 'should have retried many times within 3 h',
);
expect(
failCount,
lessThan(2200),
reason: 'backoff must have kicked in — not every 5 s for 3 h',
);
manager.dispose();
async.elapse(const Duration(seconds: 1));
});
});
test('backoff resets to 5 s after a successful sync', () {
fakeAsync((async) {
int callCount = 0;
final syncLog = _FakeSyncLog();
var failsLeft = 5;
final customEmails = _OverrideEmails(
onSync: (_) async {
callCount++;
if (failsLeft > 0) {
failsLeft--;
throw Exception('transient error');
}
return SyncEmailsResult.zero;
},
);
final manager = AccountSyncManager(
_FakeAccounts(),
_FakeMailboxes([
const Mailbox(
id: 'INBOX',
accountId: 'a1',
path: 'INBOX',
name: 'Inbox',
unreadCount: 0,
totalCount: 0,
),
]),
customEmails,
syncLog: syncLog,
imapConnect: (_, __, ___) async =>
throw Exception('skip idle — force immediate loop'),
);
manager.start();
// Allow errors + backoff to build up, then a success, then more loops.
async.elapse(const Duration(seconds: 3600));
// After success, backoff should reset; failures before success should
// be exactly 5, and subsequent loops should fire frequently.
final successCount = syncLog.logs.where((ok) => ok).length;
expect(
successCount,
greaterThan(0),
reason: 'should have at least one success',
);
expect(
callCount,
greaterThan(5),
reason: 'should retry after failures and continue after success',
);
manager.dispose();
async.elapse(const Duration(seconds: 1));
});
});
test('concurrent sync errors from multiple accounts stay bounded', () {
fakeAsync((async) {
final accounts = _FakeAccounts()
..accounts.add(_account(id: 'a2'))
..accounts.add(_account(id: 'a3'));
final syncLog = _FakeSyncLog();
final manager = AccountSyncManager(
accounts,
_FakeMailboxes([
const Mailbox(
id: 'INBOX',
accountId: 'a1',
path: 'INBOX',
name: 'Inbox',
unreadCount: 0,
totalCount: 0,
),
const Mailbox(
id: 'INBOX',
accountId: 'a2',
path: 'INBOX',
name: 'Inbox',
unreadCount: 0,
totalCount: 0,
),
const Mailbox(
id: 'INBOX',
accountId: 'a3',
path: 'INBOX',
name: 'Inbox',
unreadCount: 0,
totalCount: 0,
),
]),
_CountingEmails(syncError: Exception('network error')),
syncLog: syncLog,
imapConnect: (_, __, ___) async =>
throw Exception('connection refused'),
);
manager.start();
async.elapse(const Duration(hours: 2));
// All 3 accounts retry, each bounded by the 900 s cap.
final failCount = syncLog.logs.where((ok) => !ok).length;
expect(failCount, greaterThan(5));
expect(
failCount,
lessThan(5000),
reason: 'backoff must be in effect across all accounts',
);
manager.dispose();
async.elapse(const Duration(seconds: 1));
});
});
});
}
// ── _OverrideEmails ───────────────────────────────────────────────────────────
class _OverrideEmails extends _CountingEmails {
_OverrideEmails({required Future<SyncEmailsResult> Function(String) onSync})
: _onSync = onSync;
final Future<SyncEmailsResult> Function(String) _onSync;
@override
Future<SyncEmailsResult> syncEmails(String accountId, String mailbox) =>
_onSync(mailbox);
}