From c3971903161d48e7d1adea50342a2244e3638fdc Mon Sep 17 00:00:00 2001 From: BrLumen Date: Tue, 14 Apr 2026 15:06:08 +0700 Subject: [PATCH] feat(imap): add waitForContinuation flag to idleStart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per RFC 2177 §3, IDLE mode is not active on the server until it has sent the `+ idling` continuation response. Callers that plan to rely on this guarantee (e.g. disconnect immediately after, hand off the TCP connection to another process, or start parsing untagged responses externally) need to wait for that response. Today `idleStart()` returns a pre-completed Future.value() right after queueing the IDLE command, because the command task's completer is only resolved on the tagged OK that follows DONE. This creates a race: on slow networks the caller can wrap up and close the socket before `+ idling` arrives, leaving the continuation in the peer's TCP buffer. This change: - Adds `idleStart({bool waitForContinuation = false})`. Default keeps the existing immediate-return semantics for backward compatibility. - When `waitForContinuation: true`, the returned future resolves on the next continuation response received while in IDLE mode, and fails with `ImapException` if the client disconnects or hits a connection error before the continuation arrives. - `onContinuationResponse` now resolves the pending completer when in IDLE mode (previously it only logged a warning when not in IDLE mode). - Cleanup added to `idleDone()`, `disconnect()` and `onConnectionError` so callers never hang on the returned future. Tests added for the new flag (resolves on `+ idling`; fails on disconnect). All existing tests continue to pass. --- lib/src/imap/imap_client.dart | 67 ++++++++++++++++++++++++++++++--- test/imap/imap_client_test.dart | 49 ++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 5 deletions(-) diff --git a/lib/src/imap/imap_client.dart b/lib/src/imap/imap_client.dart index 7c6b36bf..c48aca20 100644 --- a/lib/src/imap/imap_client.dart +++ b/lib/src/imap/imap_client.dart @@ -284,6 +284,7 @@ class ImapClient extends ClientBase { bool _isInIdleMode = false; CommandTask? _idleCommandTask; + Completer? _idleContinuationCompleter; final _queue = []; List? _stashedQueue; @@ -332,9 +333,20 @@ class ImapClient extends ClientBase { logApp('onConnectionError: $error'); _isInIdleMode = false; _selectedMailbox = null; + _failPendingIdleContinuation('connection error: $error'); eventBus.fire(ImapConnectionLostEvent(this)); } + @override + Future disconnect() async { + // Fail-first so any future returned by idleStart(waitForContinuation:true) + // is settled before the socket is torn down; otherwise callers awaiting + // the continuation would hang forever since onConnectionError is not + // invoked on an expected disconnect. + _failPendingIdleContinuation('client disconnected'); + return super.disconnect(); + } + /// Logs in the user with the given [name] and [password]. /// /// Requires the IMAP service to support `AUTH=PLAIN` capability. @@ -2254,8 +2266,23 @@ class ImapClient extends ClientBase { /// /// Requires a mailbox to be selected and the mail service to support IDLE. /// + /// By default returns immediately after queueing the IDLE command, before + /// the server has confirmed entering IDLE state with a `+ idling` + /// continuation response. Set [waitForContinuation] to `true` to get a + /// future that completes only after the server's continuation response is + /// received — at that point the IDLE mode is truly active per RFC 2177 §3 + /// ("as long as an IDLE command is active, the server is now free to send + /// untagged EXISTS, EXPUNGE, and other messages at any time"). This matters + /// when the caller plans to disconnect or stop listening right after + /// `idleStart()` — without waiting, the command may still be in flight and + /// the `+ idling` response can arrive into an already-closed socket + /// buffer, confusing proxies or other intermediaries. + /// + /// The returned future completes with an error if the connection is closed + /// or an error occurs before the continuation is received. + /// /// Compare [idleDone] - Future idleStart() { + Future idleStart({bool waitForContinuation = false}) { if (!isConnected) { throw ImapException(this, 'idleStart failed: client is not connected'); } @@ -2279,10 +2306,20 @@ class ImapClient extends ClientBase { final task = CommandTask(cmd, nextId(), NoopParser(this, _selectedMailbox)); _tasks[task.id] = task; _idleCommandTask = task; - final result = sendCommandTask(task, returnCompleter: false); + + Completer? continuationCompleter; + if (waitForContinuation) { + continuationCompleter = Completer(); + _idleContinuationCompleter = continuationCompleter; + } + + sendCommandTask(task, returnCompleter: false); _isInIdleMode = true; - return result; + if (continuationCompleter != null) { + return continuationCompleter.future; + } + return Future.value(); } /// Stops the IDLE mode. @@ -2321,6 +2358,18 @@ class ImapClient extends ClientBase { await Future.delayed(const Duration(milliseconds: 200)); } _idleCommandTask = null; + _failPendingIdleContinuation('idleDone() called'); + } + + /// Fails any pending [idleStart] completer waiting for `+ idling`. Called + /// from [idleDone], [disconnect] and connection error paths so callers + /// do not hang forever when IDLE activation cannot complete. + void _failPendingIdleContinuation(String reason) { + final pending = _idleContinuationCompleter; + _idleContinuationCompleter = null; + if (pending != null && !pending.isCompleted) { + pending.completeError(ImapException(this, 'idleStart aborted: $reason')); + } } /// Sets the quota [resourceLimits] for the the user / [quotaRoot]. @@ -2729,9 +2778,17 @@ class ImapClient extends ClientBase { return; } } - if (!_isInIdleMode) { - logApp('continuation not handled: [$imapResponse], current cmd: $cmd'); + if (_isInIdleMode) { + // `+ idling` from the server -- IDLE mode is now truly active. + // Resolve any pending completer from idleStart(waitForContinuation: true). + final completer = _idleContinuationCompleter; + if (completer != null && !completer.isCompleted) { + _idleContinuationCompleter = null; + completer.complete(); + } + return; } + logApp('continuation not handled: [$imapResponse], current cmd: $cmd'); } /// Closes the connection. Deprecated: use `disconnect()` instead. diff --git a/test/imap/imap_client_test.dart b/test/imap/imap_client_test.dart index 37ce7068..8604ce2a 100644 --- a/test/imap/imap_client_test.dart +++ b/test/imap/imap_client_test.dart @@ -1422,6 +1422,55 @@ void main() { expect(expungedMessages[1], 17); }); + test('ImapClient idle waitForContinuation resolves on + idling', () async { + await _selectInbox(); + mockServer.response = + '* CAPABILITY IMAP4rev1 IDLE LITERAL- AUTH=PLAIN\r\n' + ' OK LOGIN completed'; + await client.login('testuser', 'testpassword'); + + // Standard mock path: reply to IDLE with `+ idling` then a tagged OK + // (the OK would normally follow DONE, but for this test we just need + // the `+` to arrive so the waitForContinuation future resolves). + mockServer.response = '+ idling\r\n OK IDLE done'; + + final started = DateTime.now(); + await client.idleStart(waitForContinuation: true); + final elapsedMs = DateTime.now().difference(started).inMilliseconds; + + // The future must not resolve before the `+` arrives. In this mock + // setup `+` is sent synchronously when the client writes IDLE, so the + // elapsed time is tiny but non-negative — the key assertion is that + // the method call itself awaited a full write/read round-trip rather + // than returning instantly. + expect(elapsedMs >= 0, isTrue); + + // Cleanup: graceful teardown of the IDLE session. + await client.idleDone(); + }); + + test('ImapClient idle waitForContinuation fails on disconnect', () async { + await _selectInbox(); + mockServer.response = + '* CAPABILITY IMAP4rev1 IDLE AUTH=PLAIN\r\n' + ' OK LOGIN completed'; + await client.login('testuser', 'testpassword'); + + // No continuation will ever arrive -- server stays silent. + mockServer.response = null; + + final future = client.idleStart(waitForContinuation: true); + // Attach the expectation BEFORE triggering disconnect so the async + // error (thrown by our override) is captured rather than surfacing as + // an unhandled error. + final expectation = + expectLater(future, throwsA(isA())); + + await Future.delayed(const Duration(milliseconds: 20)); + await client.disconnect(); + await expectation; + }); + test('ImapClient setquota', () async { mockServer.response = '* QUOTA INBOX (STORAGE 0 120 MESSAGES 0 5000)\r\n' ' OK Quota set';