From feb001e17fb012f8008b8cc488b75e9f5b75b4f8 Mon Sep 17 00:00:00 2001 From: David Hobley Date: Wed, 23 Sep 2026 08:36:14 +1000 Subject: [PATCH] fix(imap): handle mailbox names containing parentheses Two independent failures for a mailbox such as INBOX.Financial.Audit(s): ListParser, for an extended LIST response (LIST-STATUS / LIST-EXTENDED return options), found the extended-data block with indexOf('(') which matched the '(' inside the quoted mailbox name. The name was truncated (taking the closing quote with it), so the later quote-strip ate the final character and the mailbox surfaced as 'Aud' at a path that did not exist on the server. It now looks for the first '(' outside a double-quoted string. _encodeMailboxPath only quoted a path containing a space. RFC 3501 section 9 lists '(' ')' '{' among the atom-specials, so an unquoted `COPY 1:3 INBOX.Audit(s)` (or SELECT, MOVE, CREATE, ...) is rejected by servers with "BAD Invalid characters in atom". Those characters now force quoting as a space does. MockImapServer records what the client sends so tests can assert on the command text actually written. --- lib/src/imap/imap_client.dart | 6 +++ lib/src/private/imap/list_parser.dart | 24 +++++++++- test/imap/imap_client_test.dart | 65 +++++++++++++++++++++++++++ test/imap/mock_imap_server.dart | 5 +++ 4 files changed, 98 insertions(+), 2 deletions(-) diff --git a/lib/src/imap/imap_client.dart b/lib/src/imap/imap_client.dart index 2d90f3ec..12320c41 100644 --- a/lib/src/imap/imap_client.dart +++ b/lib/src/imap/imap_client.dart @@ -1302,7 +1302,13 @@ class ImapClient extends ClientBase { } final pathSeparator = serverInfo.pathSeparator ?? '/'; var encodedPath = Mailbox.encode(path, pathSeparator); + // RFC 3501: '(' ')' '{' are atom-specials and may not appear in an + // unquoted atom, so a mailbox such as "Audit(s)" has to be quoted or + // the server rejects the command with "BAD Invalid characters in atom". if (encodedPath.contains(' ') || + encodedPath.contains('(') || + encodedPath.contains(')') || + encodedPath.contains('{') || (alwaysQuote && !encodedPath.startsWith('"'))) { encodedPath = '"$encodedPath"'; } diff --git a/lib/src/private/imap/list_parser.dart b/lib/src/private/imap/list_parser.dart index 414cad73..a98c75af 100644 --- a/lib/src/private/imap/list_parser.dart +++ b/lib/src/private/imap/list_parser.dart @@ -81,9 +81,14 @@ class ListParser extends ResponseParser> { // Parses extended data final boxExtendedData = >{}; if (isExtended) { - final extraInfoStartIndex = listDetails.indexOf('('); + // Only a '(' outside a double-quoted string opens the extended data. + // Mailbox names may contain parentheses (e.g. "INBOX.Audit(s)") and + // those must not be mistaken for the extended-data delimiter. + final extraInfoStartIndex = _firstUnquotedOpenParen(listDetails); final extraInfoEndIndex = listDetails.lastIndexOf(')'); - if (extraInfoEndIndex != -1 && extraInfoStartIndex < extraInfoEndIndex) { + if (extraInfoStartIndex != -1 && + extraInfoEndIndex != -1 && + extraInfoStartIndex < extraInfoEndIndex) { final extraInfo = listDetails.substring( extraInfoStartIndex + 1, extraInfoEndIndex, @@ -143,6 +148,21 @@ class ListParser extends ResponseParser> { boxes.add(box); } + /// Returns the index of the first '(' that is not inside a double-quoted + /// string, or -1 if there is none. + static int _firstUnquotedOpenParen(String s) { + var inQuote = false; + for (var i = 0; i < s.length; i++) { + if (s[i] == '"') { + inQuote = !inQuote; + } else if (s[i] == '(' && !inQuote) { + return i; + } + } + + return -1; + } + void _addFlags( int flagsStartIndex, int flagsEndIndex, diff --git a/test/imap/imap_client_test.dart b/test/imap/imap_client_test.dart index 7c712d48..6405b363 100644 --- a/test/imap/imap_client_test.dart +++ b/test/imap/imap_client_test.dart @@ -220,6 +220,53 @@ void main() { ); }); + test('ImapClient listMailboxes via LIST-STATUS preserves parentheses in ' + 'mailbox names', () async { + // Regression: with isExtended=true (LIST-STATUS return options), the + // extended-data parser did listDetails.indexOf('(') which found the '(' + // inside "INBOX.Financial.Audit(s)" and treated 's)' as extended data. + // The path was truncated (removing the closing '"' too), so the + // subsequent quote-strip ate the 'i', leaving name='Aud' and a path + // that did not exist on the server. + mockServer.response = + '* LIST (\\HasChildren \\UnMarked) "." "INBOX.Financial.Audit(s)"\r\n' + '* STATUS "INBOX.Financial.Audit(s)" (MESSAGES 0 UNSEEN 0)\r\n' + '* LIST (\\HasNoChildren \\UnMarked) "." "INBOX.Financial.Home Hardware"\r\n' + '* STATUS "INBOX.Financial.Home Hardware" (MESSAGES 40 UNSEEN 0)\r\n' + ' OK List completed (0.001 + 0.000 secs).'; + final listResponse = await client.listMailboxes( + path: '"INBOX.Financial."', + recursive: false, + returnOptions: [ + ReturnOption.status(['MESSAGES', 'UNSEEN']), + ReturnOption.children(), + ], + ); + expect(listResponse, hasLength(2)); + + final audit = listResponse[0]; + expect( + audit.name, + equals('Audit(s)'), + reason: 'name was truncated at "(" inside mailbox name', + ); + expect( + audit.path, + equals('INBOX.Financial.Audit(s)'), + reason: 'path was truncated at "(" inside mailbox name', + ); + expect(audit.hasChildren, isTrue); + + final homeHardware = listResponse[1]; + expect( + homeHardware.name, + equals('Home Hardware'), + reason: 'quoted name with space should be preserved', + ); + expect(homeHardware.path, equals('INBOX.Financial.Home Hardware')); + expect(homeHardware.messagesExists, equals(40)); + }); + test('ImapClient LSUB', () async { mockServer.response = '* LSUB (\\HasChildren \\Marked) "/" INBOX\r\n' @@ -1172,6 +1219,24 @@ void main() { ); }); + test('ImapClient copy quotes a target path containing parentheses', () async { + // '(' and ')' are atom-specials (RFC 3501 section 9), so an unquoted + // `COPY 1:3 INBOX.Financial.Audit(s)` is rejected by servers with + // "BAD Invalid characters in atom". _encodeMailboxPath used to quote a + // path only when it contained a space. + await _selectInbox(); + mockServer.response = ' OK messages copied'; + await client.copy( + MessageSequence.fromRange(1, 3), + targetMailboxPath: 'INBOX.Financial.Audit(s)', + ); + + expect( + mockServer.requests.last, + contains(' COPY 1:3 "INBOX.Financial.Audit(s)"'), + ); + }); + test('ImapClient uid copy', () async { await _selectInbox(); mockServer.response = diff --git a/test/imap/mock_imap_server.dart b/test/imap/mock_imap_server.dart index 47d56058..db3d3724 100644 --- a/test/imap/mock_imap_server.dart +++ b/test/imap/mock_imap_server.dart @@ -21,8 +21,13 @@ class MockImapServer { String? response; String? _overrideTag; + /// Everything the client has sent, one entry per received chunk, so tests + /// can assert on the command text actually written to the socket. + final requests = []; + void parseRequest(Uint8List data) { final line = String.fromCharCodes(data); + requests.add(line); // print('C: $line'); final firstSpaceIndex = line.indexOf(' '); String? tag = firstSpaceIndex == -1