IMAP: check each pipelined command when it runs

jcoffey-dev commented

2026-10-29 18:49:15 +00:00

Owner

Every command in a read buffer was checked against the session state before any of them ran. Two consequences:

SELECT pipelined with FETCH or STORE got BAD No mailbox is selected for the commands after it, though RFC 9051 allows pipelining them. (The pipelining test even sent SELECT on its own to work around it.)
The other way round crashed the connection. With a mailbox selected, UNSELECT (or CLOSE, or UNAUTHENTICATE) followed in the same write by FETCH let the FETCH through. Its handler then found no mailbox and panicked on unreachable!(). With panic = 'unwind' only that client's session dies, so it isn't a security issue, but any signed-in client could cause it.

Each command is now checked just before it runs, in the state the commands before it left:

Commands batched with the one before them (FETCH, STATUS, CREATE, DELETE) are each checked, and counted against the rate limit, on their own. One that isn't allowed is answered and left out, and a refusal that ends the session (too many requests) still ends it.
STARTTLS still returns before anything pipelined after it runs.
group_requests is replaced by group_allowed.
The replica-read dispatch is boxed, because clippy's large_futures lint flagged it once the per-command check made the future bigger.

Tested:

The pipelining test now sends SELECT, STORE, FETCH, UNSELECT and DELETE in one write, and expects OK, OK, BAD for SELECT, UNSELECT and FETCH in one write.
On the old code it fails: the session panicked at client.rs:575 and r3 got no answer.
With this change, imap::imap_tests passes (45 s).
cargo fmt --all --check, clippy (nothing new in imap) and the fork checks are clean.

From OPPORTUNITIES (found by batch 3 group J).

Every command in a read buffer was checked against the session state before any of them ran. Two consequences: - **SELECT pipelined with FETCH or STORE** got `BAD No mailbox is selected` for the commands after it, though RFC 9051 allows pipelining them. (The pipelining test even sent SELECT on its own to work around it.) - **The other way round crashed the connection.** With a mailbox selected, `UNSELECT` (or `CLOSE`, or `UNAUTHENTICATE`) followed in the same write by `FETCH` let the FETCH through. Its handler then found no mailbox and panicked on `unreachable!()`. With `panic = 'unwind'` only that client's session dies, so it isn't a security issue, but any signed-in client could cause it. Each command is now checked just before it runs, in the state the commands before it left: - Commands batched with the one before them (FETCH, STATUS, CREATE, DELETE) are each checked, and counted against the rate limit, on their own. One that isn't allowed is answered and left out, and a refusal that ends the session (too many requests) still ends it. - STARTTLS still returns before anything pipelined after it runs. - `group_requests` is replaced by `group_allowed`. - The replica-read dispatch is boxed, because clippy's large_futures lint flagged it once the per-command check made the future bigger. Tested: - The pipelining test now sends SELECT, STORE, FETCH, UNSELECT and DELETE in one write, and expects `OK, OK, BAD` for SELECT, UNSELECT and FETCH in one write. - On the old code it fails: the session panicked at client.rs:575 and r3 got no answer. - With this change, `imap::imap_tests` passes (45 s). - `cargo fmt --all --check`, clippy (nothing new in imap) and the fork checks are clean. From OPPORTUNITIES (found by batch 3 group J).

jcoffey-dev added 1 commit 2026-10-29 18:49:15 +00:00

IMAP: check each pipelined command when it runs

ci / fork-checks (pull_request) Skipped

Details

ci / build (pull_request) Skipped

Details

github/ci (branch) GitHub Actions

Details

ci / github (pull_request) Successful in 10m6s

Details

6bfee4638d

Every command in a read buffer was checked against the session state
before any of them ran. A SELECT pipelined with the FETCH or STORE after
it got "BAD No mailbox is selected" for those, though RFC 9051 allows
pipelining them. The other way round was worse: with a mailbox selected,
UNSELECT (or CLOSE, or UNAUTHENTICATE) followed in the same write by
FETCH let the FETCH through, and its handler then found no mailbox and
panicked on unreachable!(), dropping the connection.

Each command is now checked just before it runs, in the state the
commands before it left. Commands batched with the one before them
(FETCH, STATUS, CREATE, DELETE) are each checked, and counted against
the rate limit, on their own; one that isn't allowed is answered and
left out, and a refusal that ends the session still ends it. STARTTLS
still returns before anything pipelined after it runs.

The pipelining test now sends SELECT with STORE and FETCH in one write,
and expects BAD for a FETCH pipelined after UNSELECT. On the old code it
fails (the session panicked); with this change the imap suite passes.

jcoffey-dev merged commit 6c69384f0a into main

2026-10-29 19:02:49 +00:00

jcoffey-dev deleted branch fix/imap-pipelined-select

2026-10-29 19:02:49 +00:00

jcoffey-dev referenced this pull request

2026-10-29 19:06:16 +00:00

Version 27.1.0-rc.4 #241

IMAP: check each pipelined command when it runs #237