From 44ccc52adfd527429e79b7fc5f1e48cfaf708bd2 Mon Sep 17 00:00:00 2001 From: Mario Fetka Date: Wed, 29 Jul 2026 13:29:31 +0200 Subject: [PATCH] Add isolated ImapTest coverage for IMAP 01 through 11 --- contrib/testing/imap-copy-move-check.py | 26 ++++++- contrib/testing/upstream-imaptest-check.py | 82 +++++++++++++++++++--- src/agents/imap/imapd.c | 8 +++ 3 files changed, 106 insertions(+), 10 deletions(-) diff --git a/contrib/testing/imap-copy-move-check.py b/contrib/testing/imap-copy-move-check.py index 65013a0..b83ede7 100755 --- a/contrib/testing/imap-copy-move-check.py +++ b/contrib/testing/imap-copy-move-check.py @@ -24,6 +24,7 @@ COPYUID = re.compile( rb"\[COPYUID ([1-9][0-9]*) ([0-9,:]+) ([0-9,:]+)\]", re.IGNORECASE, ) +UIDNEXT = re.compile(rb"\[UIDNEXT ([1-9][0-9]*)\]", re.IGNORECASE) class IMAPCheckError(RuntimeError): @@ -63,6 +64,16 @@ def copyuid(lines: list[bytes], *, untagged: bool) -> tuple[int, list[int], list return int(match.group(1)), source, target +def uidnext(lines: list[bytes]) -> int: + match = next( + (candidate for line in lines if (candidate := UIDNEXT.search(line))), + None, + ) + if match is None: + raise IMAPCheckError(f"UIDNEXT response missing from {lines!r}") + return int(match.group(1)) + + class IMAPConnection: def __init__(self) -> None: raw = socket.create_connection((HOST, PORT), timeout=TIMEOUT) @@ -319,12 +330,23 @@ def main() -> int: remaining = uid_search(client) if remaining != [original_uids[2]]: raise IMAPCheckError(f"unexpected remaining UID: {remaining!r}") + client.command("UNSELECT") + selected = False + source_reselected = client.command(f"SELECT {quote(source)}") + selected = True + announced_uidnext = uidnext(source_reselected) + if uid_search(client) != remaining: + raise IMAPCheckError("source UIDs changed while checking UIDNEXT") same_move = client.command( f"UID MOVE {remaining[0]} {quote(source)}" ) assert_move_order(same_move) _, sources, targets = copyuid(same_move, untagged=True) - if sources != remaining or len(targets) != 1 or targets[0] == remaining[0]: + if ( + sources != remaining + or targets != [announced_uidnext] + or targets[0] == remaining[0] + ): raise IMAPCheckError(f"same-mailbox MOVE mapping invalid: {same_move!r}") require_line(same_move, rb"^\* 1 EXISTS\r\n$", "same-mailbox EXISTS") final_uids = uid_search(client) @@ -362,7 +384,7 @@ def main() -> int: "IMAP-11 PASS " f"host={HOST}:{PORT} " "copy=sequence/uid move=sequence/uid/same-mailbox " - "uidplus=copyuid/order metadata=flags/keywords/internaldate " + "uidplus=copyuid/order/uidnext metadata=flags/keywords/internaldate " "sets=duplicate/nonexistent examine=copy/read-only-move " "syntax=trailing-rejected missing=trycreate cleanup=yes" ) diff --git a/contrib/testing/upstream-imaptest-check.py b/contrib/testing/upstream-imaptest-check.py index b0086ac..245ab1a 100755 --- a/contrib/testing/upstream-imaptest-check.py +++ b/contrib/testing/upstream-imaptest-check.py @@ -44,6 +44,35 @@ CORE_TESTS = ( "uidvalidity-rename", ) +IMAP_01_11_TESTS = ( + "atoms", + "append", + "close", + "copy", + "fetch", + "fetch-body", + "fetch-body-message-rfc822", + "fetch-body-message-rfc822-mime", + "fetch-body-message-rfc822-x2", + "fetch-body-mime", + "fetch-bodystructure", + "fetch-envelope", + "id", + "list", + "listext", + "logout", + "move", + "mutf7", + "nil", + "pipeline", + "select", + "store", + "subscribe", + "uidplus", + "uidvalidity", + "uidvalidity-rename", +) + SEARCH_TESTS = ( "esearch", "search-addresses", @@ -100,6 +129,8 @@ def selected_tests( names = explicit elif suite == "core": names = CORE_TESTS + elif suite == "imap-01-11": + names = IMAP_01_11_TESTS elif suite == "search": names = SEARCH_TESTS elif suite == "all": @@ -189,18 +220,39 @@ def scripted(args: argparse.Namespace) -> None: source = find_source(args.source) with tempfile.TemporaryDirectory(prefix="bongo-imaptest-scripted-") as raw: directory = Path(raw) - staged = directory / "tests" - staged.mkdir() explicit = tuple(args.test_group) - count = stage_tests(source, args.suite, staged, explicit) userfile = credentials_file(directory, args.username, args.password) - command = common_arguments(args, userfile) - command.extend((f"box={args.mailbox}", f"test={staged}")) - run(command, args.timeout, directory) + selected = selected_tests(source, args.suite, explicit) + count = len(selected) + if args.isolate_groups: + for index, path in enumerate(selected, 1): + staged = directory / f"tests-{index:02d}" + staged.mkdir() + stage_tests(source, args.suite, staged, (path.name,)) + command = common_arguments(args, userfile) + command.extend( + ( + f"box={args.mailbox}-{index:02x}", + f"test={staged}", + ) + ) + run(command, args.timeout, directory) + print( + f"PASS: Dovecot ImapTest group {path.name} " + f"({index}/{count})" + ) + else: + staged = directory / "tests" + staged.mkdir() + stage_tests(source, args.suite, staged, explicit) + command = common_arguments(args, userfile) + command.extend((f"box={args.mailbox}", f"test={staged}")) + run(command, args.timeout, directory) label = ", ".join(explicit) if explicit else f"{args.suite} suite" print( f"PASS: Dovecot ImapTest {label} " - f"({count} scripted test groups)" + f"({count} scripted test groups" + f"{', isolated' if args.isolate_groups else ', combined'})" ) @@ -397,7 +449,19 @@ def parser() -> argparse.ArgumentParser: ) result.add_argument("--ca-file", default=os.environ.get("BONGO_TEST_CA_FILE")) result.add_argument("--imap4rev2", action="store_true") - result.add_argument("--suite", choices=("core", "search", "all"), default="core") + result.add_argument( + "--suite", + choices=("core", "imap-01-11", "search", "all"), + default="core", + ) + result.add_argument( + "--isolate-groups", + action="store_true", + help=( + "run each scripted group in a separate ImapTest process and " + "mailbox so one failure cannot cascade into later groups" + ), + ) result.add_argument( "--test-group", action="append", @@ -435,6 +499,8 @@ def main() -> int: "the upstream scripted corpus tests IMAP4rev1 semantics; " "use --imap4rev2 with stress mode and Bongo's dedicated rev2 tests" ) + if args.mode != "scripted" and args.isolate_groups: + raise UpstreamTestError("--isolate-groups is only valid in scripted mode") if args.mode == "scripted": scripted(args) elif args.mode == "stress": diff --git a/src/agents/imap/imapd.c b/src/agents/imap/imapd.c index f868ac1..946fe27 100644 --- a/src/agents/imap/imapd.c +++ b/src/agents/imap/imapd.c @@ -572,6 +572,14 @@ ParseCollectionLine(char *buffer, FolderInformation *folder) { char *ptr; + /* + * COLLECTIONS does not carry UIDNEXT. A freshly allocated or reused + * FolderInformation must therefore start with uidNext == 0 so FolderOpen + * derives it from the Store's persistent collection counter. Leaving + * this field untouched made SELECT occasionally advertise heap data as + * UIDNEXT after mailbox reloads. + */ + memset(folder, 0, sizeof(*folder)); ptr = buffer; folder->guid = HexToUInt64(ptr, &ptr); if (*ptr == ' ') {