Verify IMAP restart consistency

2026-07-30 02:31:21 +02:00
parent b052489d27
commit b88ce2a634
5 changed files with 530 additions and 1 deletions
@@ -102,6 +102,12 @@ one-off files in `/tmp`:
  limit with recovery, synchronized and non-synchronizing literal limits,
  malformed sequence/UID sets, ordinary command pipelining, and disposal of
  cleartext pipelined across STARTTLS.
+- `imap-reconnect-consistency-check.py` exercises parallel APPENDs and
+  simultaneous selected clients, verifies flag merging, COPY/EXPUNGE,
+  reconnect after an abrupt disconnect, and persistent STATUS, UID,
+  subscription, flags, and message bytes across a real Bongo service restart.
+  It intentionally keeps one TLS session open during restart to cover clean
+  shutdown of an idle client.
 - `upstream-imaptest-check.py` runs the generic Dovecot ImapTest project
  against live Bongo IMAPS. Scripted mode stages a selected core, search,
  `imap-01-11`, `imap-01-12`, `imap-01-13`, `imap-01-14`, or complete
@@ -0,0 +1,461 @@
+#!/usr/bin/env python3
+# This program is free software, licensed under the terms of the GNU GPL.
+# See the Bongo COPYING file for full details.
+# Copyright (c) 2026 Bongo Project contributors
+
+"""Verify concurrent IMAP sessions and Store consistency across restart."""
+
+from __future__ import annotations
+
+import os
+import re
+import socket
+import ssl
+import subprocess
+import threading
+import time
+
+
+HOST = os.environ.get("BONGO_TEST_HOST", "127.0.0.1")
+PORT = int(os.environ.get("BONGO_TEST_IMAPS_PORT", "993"))
+TIMEOUT = float(os.environ.get("BONGO_TEST_TIMEOUT", "10"))
+USERNAME = os.environ.get("BONGO_TEST_USER", "test1")
+PASSWORD = os.environ.get("BONGO_TEST_PASSWORD", "")
+SYSTEMCTL = os.environ.get("BONGO_TEST_SYSTEMCTL", "/usr/bin/systemctl")
+PARALLEL_APPENDS = int(os.environ.get("BONGO_TEST_PARALLEL_APPENDS", "6"))
+TAGGED = re.compile(rb"^([A-Za-z0-9]+) (OK|NO|BAD)(?: .*)?\r\n$")
+LITERAL = re.compile(rb"(?:~)?\{([0-9]+)\}\r\n$")
+APPENDUID = re.compile(rb"\[APPENDUID [0-9]+ ([0-9]+)\]", re.IGNORECASE)
+
+
+class IMAPConsistencyError(RuntimeError):
+    """Raised when live concurrent or restart behavior is inconsistent."""
+
+
+def quote(value: str) -> str:
+    return '"' + value.replace("\\", "\\\\").replace('"', '\\"') + '"'
+
+
+class IMAPConnection:
+    def __init__(self) -> None:
+        raw = socket.create_connection((HOST, PORT), timeout=TIMEOUT)
+        context = ssl.create_default_context()
+        context.check_hostname = False
+        context.verify_mode = ssl.CERT_NONE
+        context.minimum_version = ssl.TLSVersion.TLSv1_2
+        self.socket = context.wrap_socket(raw, server_hostname=HOST)
+        self.socket.settimeout(TIMEOUT)
+        self.reader = self.socket.makefile("rb")
+        self.counter = 0
+        greeting = self.line()
+        if not greeting.startswith(b"* OK "):
+            raise IMAPConsistencyError(
+                f"unexpected IMAP greeting: {greeting!r}"
+            )
+
+    def close(self) -> None:
+        try:
+            self.reader.close()
+        finally:
+            self.socket.close()
+
+    def line(self) -> bytes:
+        line = self.reader.readline(65538)
+        if not line:
+            raise IMAPConsistencyError(
+                "connection closed while waiting for response"
+            )
+        if len(line) > 65536 or not line.endswith(b"\r\n"):
+            raise IMAPConsistencyError(f"invalid IMAP response line: {line!r}")
+        return line
+
+    def next_tag(self) -> str:
+        self.counter += 1
+        return f"C{self.counter:04d}"
+
+    def response(
+        self, tag: str, expected: tuple[str, ...] = ("OK",)
+    ) -> bytes:
+        result = bytearray()
+        while True:
+            line = self.line()
+            result.extend(line)
+            literal = LITERAL.search(line)
+            if literal is not None:
+                size = int(literal.group(1))
+                data = self.reader.read(size)
+                if data is None or len(data) != size:
+                    raise IMAPConsistencyError("short IMAP response literal")
+                result.extend(data)
+                continue
+            match = TAGGED.match(line)
+            if match is None or match.group(1).decode("ascii") != tag:
+                continue
+            status = match.group(2).decode("ascii")
+            if status not in expected:
+                raise IMAPConsistencyError(
+                    f"{tag} expected {expected}, received {line!r}"
+                )
+            return bytes(result)
+
+    def command(
+        self, command: str, expected: tuple[str, ...] = ("OK",)
+    ) -> bytes:
+        tag = self.next_tag()
+        self.socket.sendall(f"{tag} {command}\r\n".encode("ascii"))
+        return self.response(tag, expected)
+
+    def append(self, mailbox: str, message: bytes) -> int:
+        tag = self.next_tag()
+        self.socket.sendall(
+            f"{tag} APPEND {quote(mailbox)} {{{len(message)}}}\r\n".encode(
+                "ascii"
+            )
+        )
+        continuation = self.line()
+        if not continuation.startswith(b"+"):
+            raise IMAPConsistencyError(
+                f"APPEND continuation missing: {continuation!r}"
+            )
+        self.socket.sendall(message + b"\r\n")
+        response = self.response(tag)
+        match = APPENDUID.search(response)
+        if match is None:
+            raise IMAPConsistencyError(f"APPENDUID missing: {response!r}")
+        return int(match.group(1))
+
+
+def login(client: IMAPConnection) -> None:
+    client.command(f"LOGIN {quote(USERNAME)} {quote(PASSWORD)}")
+
+
+def message(subject: str, identifier: str) -> bytes:
+    return (
+        b"From: imap20@bongo.test\r\n"
+        b"To: test1@bongo.test\r\n"
+        + f"Subject: {subject}\r\n".encode("ascii")
+        + f"Message-ID: <{identifier}@bongo.test>\r\n".encode("ascii")
+        + b"\r\n"
+        + f"IMAP-20 persistent body {identifier}\r\n".encode("ascii")
+    )
+
+
+def parse_status(response: bytes) -> dict[str, int]:
+    match = re.search(rb"^\* STATUS .+ \(([^)]*)\)\r$", response, re.MULTILINE)
+    if match is None:
+        raise IMAPConsistencyError(f"STATUS data missing: {response!r}")
+    fields = match.group(1).decode("ascii").split()
+    if len(fields) % 2:
+        raise IMAPConsistencyError(f"odd STATUS field list: {fields!r}")
+    return {
+        fields[index].upper(): int(fields[index + 1])
+        for index in range(0, len(fields), 2)
+    }
+
+
+def parse_search(response: bytes) -> set[int]:
+    match = re.search(rb"^\* SEARCH(?: ([0-9 ]+))?\r$", response, re.MULTILINE)
+    if match is None:
+        raise IMAPConsistencyError(f"SEARCH response missing: {response!r}")
+    if match.group(1) is None:
+        return set()
+    return {int(value) for value in match.group(1).split()}
+
+
+def restart_bongo() -> None:
+    completed = subprocess.run(
+        ["sudo", "-n", SYSTEMCTL, "restart", "bongo.service"],
+        check=False,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+        text=True,
+        timeout=60,
+    )
+    if completed.returncode:
+        raise IMAPConsistencyError(
+            f"bongo restart failed ({completed.returncode}): "
+            f"{completed.stdout.strip()}"
+        )
+
+    deadline = time.monotonic() + 30
+    while time.monotonic() < deadline:
+        try:
+            probe = IMAPConnection()
+        except (OSError, IMAPConsistencyError):
+            time.sleep(0.25)
+            continue
+        probe.close()
+        return
+    raise IMAPConsistencyError("Bongo IMAPS did not return after restart")
+
+
+def append_parallel(mailbox: str, token: str) -> dict[int, int]:
+    barrier = threading.Barrier(PARALLEL_APPENDS)
+    results: dict[int, int] = {}
+    failures: list[BaseException] = []
+    lock = threading.Lock()
+
+    def worker(index: int) -> None:
+        client: IMAPConnection | None = None
+        try:
+            client = IMAPConnection()
+            login(client)
+            barrier.wait(timeout=TIMEOUT)
+            uid = client.append(
+                mailbox,
+                message(f"parallel {index}", f"{token}-parallel-{index}"),
+            )
+            with lock:
+                results[index] = uid
+            client.command("LOGOUT")
+        except BaseException as error:
+            with lock:
+                failures.append(error)
+            try:
+                barrier.abort()
+            except threading.BrokenBarrierError:
+                pass
+        finally:
+            if client is not None:
+                try:
+                    client.close()
+                except OSError:
+                    pass
+
+    threads = [
+        threading.Thread(target=worker, args=(index,), daemon=True)
+        for index in range(PARALLEL_APPENDS)
+    ]
+    for thread in threads:
+        thread.start()
+    for thread in threads:
+        thread.join(TIMEOUT * 2)
+    if any(thread.is_alive() for thread in threads):
+        raise IMAPConsistencyError("parallel APPEND workers did not finish")
+    if failures:
+        raise IMAPConsistencyError(f"parallel APPEND failed: {failures[0]}")
+    if len(results) != PARALLEL_APPENDS:
+        raise IMAPConsistencyError(
+            f"parallel APPEND count mismatch: {results!r}"
+        )
+    if len(set(results.values())) != PARALLEL_APPENDS:
+        raise IMAPConsistencyError(
+            f"parallel APPEND allocated duplicate UIDs: {results!r}"
+        )
+    return results
+
+
+def empty_and_delete(client: IMAPConnection, mailbox: str) -> None:
+    client.command(f"SELECT {quote(mailbox)}", ("OK", "NO"))
+    client.command("STORE 1:* +FLAGS.SILENT (\\Deleted)", ("OK", "NO"))
+    client.command("EXPUNGE", ("OK", "NO", "BAD"))
+    client.command("CLOSE", ("OK", "BAD"))
+    client.command(f"DELETE {quote(mailbox)}", ("OK", "NO"))
+
+
+def main() -> int:
+    if not PASSWORD:
+        raise IMAPConsistencyError("BONGO_TEST_PASSWORD must be set")
+    if PARALLEL_APPENDS < 2:
+        raise IMAPConsistencyError("BONGO_TEST_PARALLEL_APPENDS must be >= 2")
+
+    token = f"{os.getpid():x}{time.time_ns():x}"
+    mailbox = f"BongoConsistency{token}"
+    copybox = f"{mailbox}Copy"
+    owner = IMAPConnection()
+    open_clients: list[IMAPConnection] = [owner]
+    created: set[str] = set()
+    subscribed = False
+
+    try:
+        login(owner)
+        owner.command(f"CREATE {mailbox}")
+        created.add(mailbox)
+        owner.command(f"CREATE {copybox}")
+        created.add(copybox)
+        owner.command(f"SUBSCRIBE {mailbox}")
+        subscribed = True
+        seed_uid = owner.append(
+            mailbox, message("persistent seed", f"{token}-seed")
+        )
+        appended = append_parallel(mailbox, token)
+        all_uids = {seed_uid, *appended.values()}
+
+        first = IMAPConnection()
+        second = IMAPConnection()
+        third = IMAPConnection()
+        open_clients.extend((first, second, third))
+        for client in (first, second, third):
+            login(client)
+            selected = client.command(f"SELECT {mailbox}")
+            if not re.search(
+                fr"^\* {len(all_uids)} EXISTS\r$".encode(),
+                selected,
+                re.MULTILINE,
+            ):
+                raise IMAPConsistencyError(
+                    f"simultaneous SELECT count mismatch: {selected!r}"
+                )
+
+        first.command(f"UID STORE {seed_uid} +FLAGS.SILENT (\\Flagged)")
+        update = second.command("NOOP")
+        if b"\\Flagged" not in update:
+            raise IMAPConsistencyError(
+                f"concurrent Flagged update not visible: {update!r}"
+            )
+        second.command(f"UID STORE {seed_uid} +FLAGS.SILENT (\\Answered)")
+        update = third.command("NOOP")
+        if b"\\Flagged" not in update or b"\\Answered" not in update:
+            raise IMAPConsistencyError(
+                f"concurrent flag union not visible: {update!r}"
+            )
+
+        copy = first.command(f"UID COPY {seed_uid} {quote(copybox)}")
+        if b"COPYUID" not in copy:
+            raise IMAPConsistencyError(f"UID COPY lacked COPYUID: {copy!r}")
+
+        removed_uid = max(appended.values())
+        first.command(
+            f"UID STORE {removed_uid} +FLAGS.SILENT (\\Deleted)"
+        )
+        first.command(f"UID EXPUNGE {removed_uid}")
+        all_uids.remove(removed_uid)
+        third.command("NOOP")
+
+        # Abruptly close a selected session, then prove a fresh session sees
+        # the committed Store state rather than per-process cached state.
+        third.close()
+        open_clients.remove(third)
+        reconnect = IMAPConnection()
+        open_clients.append(reconnect)
+        login(reconnect)
+        reconnect.command(f"SELECT {mailbox}")
+        if parse_search(reconnect.command("UID SEARCH ALL")) != all_uids:
+            raise IMAPConsistencyError("reconnect returned a stale UID set")
+        reconnect.command("CLOSE")
+
+        before = parse_status(
+            owner.command(
+                f"STATUS {quote(mailbox)} "
+                "(MESSAGES UIDNEXT UIDVALIDITY SIZE)"
+            )
+        )
+        if before["MESSAGES"] != len(all_uids):
+            raise IMAPConsistencyError(
+                f"pre-restart message count mismatch: {before!r}"
+            )
+
+        lingering = IMAPConnection()
+        open_clients.append(lingering)
+        login(lingering)
+        lingering.command(f"SELECT {mailbox}")
+        restart_bongo()
+        try:
+            lingering.command("NOOP")
+        except (IMAPConsistencyError, OSError, ssl.SSLError):
+            pass
+        else:
+            raise IMAPConsistencyError(
+                "pre-restart IMAP connection survived daemon restart"
+            )
+
+        for client in open_clients:
+            try:
+                client.close()
+            except OSError:
+                pass
+        open_clients.clear()
+
+        owner = IMAPConnection()
+        open_clients.append(owner)
+        login(owner)
+        after = parse_status(
+            owner.command(
+                f"STATUS {quote(mailbox)} "
+                "(MESSAGES UIDNEXT UIDVALIDITY SIZE)"
+            )
+        )
+        if after != before:
+            raise IMAPConsistencyError(
+                f"STATUS changed across restart: before={before} after={after}"
+            )
+
+        owner.command(f"SELECT {mailbox}")
+        if parse_search(owner.command("UID SEARCH ALL")) != all_uids:
+            raise IMAPConsistencyError("UID set changed across restart")
+        flags = owner.command(f"UID FETCH {seed_uid} FLAGS")
+        if b"\\Flagged" not in flags or b"\\Answered" not in flags:
+            raise IMAPConsistencyError(
+                f"concurrent flags did not persist: {flags!r}"
+            )
+        content = owner.command(f"UID FETCH {seed_uid} BODY.PEEK[]")
+        if f"Message-ID: <{token}-seed@bongo.test>".encode() not in content:
+            raise IMAPConsistencyError("seed body changed across restart")
+        owner.command("CLOSE")
+
+        subscriptions = owner.command(f'LSUB "" {quote(mailbox)}')
+        if mailbox.encode() not in subscriptions:
+            raise IMAPConsistencyError("subscription changed across restart")
+
+        copied = owner.command(f"SELECT {quote(copybox)}")
+        if not re.search(rb"^\* 1 EXISTS\r$", copied, re.MULTILINE):
+            raise IMAPConsistencyError(
+                f"copied mailbox count changed: {copied!r}"
+            )
+        copied_content = owner.command("FETCH 1 BODY.PEEK[]")
+        if f"Message-ID: <{token}-seed@bongo.test>".encode() not in copied_content:
+            raise IMAPConsistencyError("copied message changed across restart")
+        owner.command("CLOSE")
+
+        owner.command(f"UNSUBSCRIBE {mailbox}")
+        subscribed = False
+        for current in (copybox, mailbox):
+            empty_and_delete(owner, current)
+            created.remove(current)
+        owner.command("LOGOUT")
+
+        print(
+            "IMAP-20 PASS "
+            f"parallel-appends={PARALLEL_APPENDS}/uids-unique "
+            "sessions=flags-unioned/copy/expunge "
+            "disconnect=reconnect-consistent "
+            "restart=status/uidvalidity/uidnext/uids/flags/body/subscription "
+            "cleanup=yes"
+        )
+        return 0
+    finally:
+        cleanup: IMAPConnection | None = None
+        if created or subscribed:
+            try:
+                cleanup = IMAPConnection()
+                login(cleanup)
+            except (IMAPConsistencyError, OSError):
+                pass
+        if cleanup is not None:
+            if subscribed:
+                try:
+                    cleanup.command(
+                        f"UNSUBSCRIBE {mailbox}", ("OK", "NO", "BAD")
+                    )
+                except (IMAPConsistencyError, OSError):
+                    pass
+            for current in (copybox, mailbox):
+                if current not in created:
+                    continue
+                try:
+                    empty_and_delete(cleanup, current)
+                except (IMAPConsistencyError, OSError):
+                    pass
+            try:
+                cleanup.close()
+            except OSError:
+                pass
+        for client in open_clients:
+            try:
+                client.close()
+            except OSError:
+                pass
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())
@@ -102,6 +102,15 @@ Oversized synchronizing literals are rejected before continuation;
 non-synchronizing literals are consumed without storing them so the stream
 can be resynchronized safely.

+Mailbox state remains authoritative in the Store rather than in an IMAP
+process. Committed messages receive unique, monotonically increasing UIDs;
+UIDVALIDITY, UIDNEXT, flags, subscriptions, and message data survive client
+disconnects and complete Bongo service restarts. Simultaneous selected
+clients refresh their views through Store notifications or NOOP. On service
+shutdown Bongo closes active client and Store transports before waiting for
+the IMAP workers, so an idle TLS peer cannot delay restart until the command
+timeout.
+
 AUTH capabilities are generated from the GNU GSASL mechanisms which Bongo
 has actually enabled. Password mechanisms are advertised only on a TLS
 connection; unencrypted connections offer STARTTLS and keep LOGIN disabled.
@@ -187,7 +187,7 @@ review does not mark an open live test as passed.
 | IMAP-17 | SPECIAL-USE and CREATE-SPECIAL-USE folders | [PASS](test-evidence/0.7-r1.md#imap-17) |  |  |
 | IMAP-18 | Old-client plain IMAP4 subset without rev1/rev2 assumptions | [PASS](test-evidence/0.7-r1.md#imap-18) |  |  |
 | IMAP-19 | Invalid states, malformed sets/literals, oversized input, and STARTTLS plaintext-pipeline injection resistance | [PASS](test-evidence/0.7-r1.md#imap-19) |  |  |
-| IMAP-20 | Reconnect, simultaneous clients, restart, mailbox consistency |  |  |  |
+| IMAP-20 | Reconnect, simultaneous clients, restart, mailbox consistency | [PASS](test-evidence/0.7-r1.md#imap-20) |  |  |
 | IMAP-21 | RFC 9208 capability, GETQUOTA/GETQUOTAROOT, storage units and unknown mailbox handling |  |  |  |
 | IMAP-22 | DELETED-STORAGE and [OVERQUOTA] for APPEND/COPY/MOVE; no unadvertised QUOTASET |  |  |  |
 | IMAP-23 | Dovecot ImapTest scripted core/search suites and deterministic state-tracked multi-client stress |  |  |  |
@@ -4122,6 +4122,59 @@ The normal and strict `-Wall -Wextra -Werror` builds each passed all 108 CTest
 cases. The service remained active and its journal contained no new IMAP or
 Store error after the malformed-input run.

+## IMAP-20
+
+Result: **PASS**
+
+Commits `fe7c12a4` and `b052489d` were installed as the Gentoo GCC 15.3.0
+debug build and tested on 2026-07-30. The reusable live test opens six
+parallel authenticated TLS sessions and proves that concurrent APPENDs
+receive distinct UIDs. Two simultaneous selected clients then merge
+independent `\Flagged` and `\Answered` changes, observe the result after
+NOOP, and exercise UID COPY plus selective UID EXPUNGE.
+
+After an abrupt selected-client disconnect, a new connection observes the
+same UID set. The test records MESSAGES, UIDNEXT, UIDVALIDITY, and SIZE,
+keeps another selected TLS session idle, and restarts the complete Bongo
+service. The stale transport is closed and a new authenticated session sees
+the exact prior STATUS values, UID set, flags, message body and Message-ID,
+subscription, and copied mailbox.
+
+The design was compared directly with Dovecot 2.4.4
+`src/lib-index/mail-index.h` and `mail-index-transaction.c`, where UIDNEXT is
+monotonic and committed index transactions refresh readers, and Cyrus IMAP
+3.12.3 `imap/mailbox.c`, where mailbox index locks serialize writers and
+LAST_UID/UIDVALIDITY are committed in mailbox metadata. Bongo follows the
+same externally visible principles through Store-assigned persistent
+`nmap.mail.imapuid`, Store collection locks, and mailbox watches.
+
+The first restart run exposed a shutdown defect rather than a data defect:
+an idle TLS client left `bongoimap` blocked until the manager escalated.
+Bongo now closes all active IMAP transports before waiting for session
+workers, matching the existing POP3 shutdown ordering. ConnIO's force-close
+path shuts down the transport before attempting TLS close notification, so a
+peer which does not answer `close_notify` cannot block the agent. The
+post-fix restart completed cleanly in about two seconds with no stubborn
+IMAP or Store process in the journal.
+
+```text
+IMAP-20 PASS parallel-appends=6/uids-unique sessions=flags-unioned/copy/expunge disconnect=reconnect-consistent restart=status/uidvalidity/uidnext/uids/flags/body/subscription cleanup=yes
+```
+
+The IMAP COPY/MOVE, EXPUNGE, IDLE, and Store-watch reuse live regressions
+passed. The complete Dovecot ImapTest corpus remained green:
+
+```text
+35 test groups: 0 failed, 0 skipped due to missing capabilities
+base protocol: 0/439 individual commands failed
+extensions: 0/88 individual commands failed
+PASS: Dovecot ImapTest imap-01-14 suite (35 scripted test groups, combined)
+```
+
+The normal and strict `-Wall -Wextra -Werror` builds each passed all 108 CTest
+cases after both shutdown changes. The final live run completed in 7.27
+seconds including service restart, reconnection, verification, and cleanup.
+
 ## IMAP retrospective source audit

 Result: **PASS for IMAP-01 through IMAP-08**