Skip to content

Commit 667965f

Browse files
Conversation threading, and the reply chain that could have taken the server down
THREAD and UID THREAD (RFC 5256), both algorithms, advertised as THREAD=ORDEREDSUBJECT THREAD=REFERENCES and riding the same bounded search machinery as SORT. REFERENCES is the actual algorithm rather than subject grouping wearing its name. Containers are linked by Message-ID / References / In-Reply-To, with In-Reply-To consulted only when References is absent; a message's own References line authoritatively breaks a parent that somebody else's line asserted; cycles are refused rather than linked; dummies are pruned except at the root, where a dummy with two or more children is what holds two replies together when the message they both answer never arrived. Threads are then grouped by RFC 2.1 base subject - the whole step machine, so "[fwd: Re: [list] x]" reduces the way the RFC says rather than the way one regular expression can - and sorted by sent date. ORDEREDSUBJECT is the flat approximation the RFC requires of anyone advertising THREAD at all. Not gated behind a setting, unlike SORT, and the asymmetry is deliberate: the work is bounded by the same per-command ceilings as SEARCH, and a gate would have cost a seeded settings row on every backend plus a schema bump - a missing row reports HM5015 on every read rather than defaulting quietly - to guard something already bounded. An adversarial pass over 11 passing tests found three defects, and the one that mattered was measured rather than argued. Prune_ and SortChildrenRecursively_ recurse once per tree level on a 1MB default worker stack; a probe compiled against those exact frames survived 6000 levels and terminated with STATUS_STACK_OVERFLOW at 8000, while the old cycle guard permitted chains near 10000. A stack overflow is not reliably catchable, so that was a crash of the whole service, reachable by any authenticated user willing to mail themselves a long enough reply chain. Links are now refused past depth 512, which bounds all three recursive walks at their source. The header pass was uninterruptible: the ceiling was checked after BuildThreads returned, which can only disown a result the connection thread has already spent minutes producing. The predicate is handed in now and stops before the next file is opened. And a Date header that was present but unparseable fell through to the 1899 OLE epoch, which the comparator reads as a raw value without consulting the validity flag - so the most malformed message in a conversation sorted to the front of it. RFC 5256 2.2's INTERNALDATE fallback now covers unparseable as well as absent. Two pre-existing weaknesses of exactly these shapes are recorded in the roadmap rather than silently inherited: SORT checks its ceiling only after its own uninterruptible header pass, and IMAPSort::CacheHeaderFields_ tests the Date header for emptiness rather than validity. 13 tests assert exact trees - chains, forks, dummy roots, subject grouping, UID mode against UIDs read back rather than assumed, BAD for an unadvertised algorithm, a same-connection stale-state control, a 600-message chain that must not crash, and the INTERNALDATE fallback. The fixture earned itself twice: it caught a stray space between root threads, which a threading client would parse as an error, and its own first date assertion was wrong rather than the code - crafted headers dated today at 10:xx against a suite running at 09:xx. Full suite: 1490/1490.
1 parent 5a07e90 commit 667965f

15 files changed

Lines changed: 1651 additions & 32 deletions

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ Mail protocols
2929
--------------
3030

3131
* **SMTP** with PIPELINING, ENHANCEDSTATUSCODES, 8BITMIME, SIZE, CHUNKING/BDAT (RFC 3030), DSN delivery status notifications (RFC 3461/3464) and SMTPUTF8/EAI for internationalised addresses.
32-
* **IMAP4rev1**, plus **IMAP4rev2** (RFC 9051) advertised with its behavioural deltas implemented — a few of the extensions rev2 folds in (LIST-STATUS, non-synchronising literals, BINARY) are still missing — with IDLE, MOVE (RFC 6851), UIDPLUS (RFC 4315), CONDSTORE/QRESYNC (RFC 7162), SEARCHRES (RFC 5182), ESEARCH (RFC 4731), SORT, ACL, NAMESPACE, ID (RFC 2971), SPECIAL-USE (RFC 6154, including explicit designation via `CREATE ... (USE (\Sent))`) and QUOTA.
32+
* **IMAP4rev1**, plus **IMAP4rev2** (RFC 9051) advertised with its behavioural deltas implemented — a few of the extensions rev2 folds in (LIST-STATUS, non-synchronising literals, BINARY) are still missing — with IDLE, MOVE (RFC 6851), UIDPLUS (RFC 4315), CONDSTORE/QRESYNC (RFC 7162), SEARCHRES (RFC 5182), ESEARCH (RFC 4731), SORT and THREAD (RFC 5256, both ORDEREDSUBJECT and REFERENCES), ACL, NAMESPACE, ID (RFC 2971), SPECIAL-USE (RFC 6154, including explicit designation via `CREATE ... (USE (\Sent))`) and QUOTA.
3333
* **POP3**, including retrieval from external POP3 accounts on a schedule.
3434
* **Public folders**, shared across accounts with per-user ACLs.
3535

Roadmap.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ strong and where it is thin far more honestly than any prose summary.
5555
| **The capability matrix** | | | | |
5656
| [SMTP and ESMTP](#smtp-and-esmtp) | 24 || 4 ||
5757
| [Transport security and deliverability](#transport-security-and-deliverability) | 43 || 4 | 1 |
58-
| [IMAP](#imap) | 57 || 18 | 3 |
58+
| [IMAP](#imap) | 58 || 17 | 3 |
5959
| [POP3](#pop3) | 21 || 6 ||
6060
| [Sieve, ManageSieve and rules](#sieve-managesieve-and-rules) | 48 | 0 | 16 ||
6161
| [Authentication and cryptography](#authentication-and-cryptography) | 57 | 0 | 18 ||
@@ -72,7 +72,7 @@ strong and where it is thin far more honestly than any prose summary.
7272
| [Future-proofing: standards and protocols](#future-proofing-standards-and-protocols) | 2 || 4 | 2 |
7373
| [Future-proofing: platform and supply chain](#future-proofing-platform-and-supply-chain) | 5 | 1 | 2 | 2 |
7474
| [Future-proofing: deployment and operations](#future-proofing-deployment-and-operations) | 4 | 1 | 4 ||
75-
| **Total** | **621** | **12** | **147** | **14** |
75+
| **Total** | **622** | **12** | **146** | **14** |
7676

7777
Three things stand out and are worth naming rather than leaving to be inferred.
7878
**Storage and the administration surface are the best-covered areas**, and the
@@ -329,7 +329,7 @@ the source, not from documentation.
329329

330330
### IMAP
331331

332-
57 shipped · 0 underway · 18 not started · 3 deferred
332+
58 shipped · 0 underway · 17 not started · 3 deferred
333333

334334
| | Capability | Detail |
335335
|:-:|---|---|
@@ -407,7 +407,7 @@ the source, not from documentation.
407407
|| REPLACE (RFC 8508) | Not implemented and not advertised; there is no REPLACE or UID REPLACE command, so clients must emulate draft updates with APPEND + STORE \Deleted + EXPUNGE. |
408408
|| RFC 9208 QUOTA (QUOTA=RES-STORAGE / RES-MESSAGE, SETQUOTA) | Not implemented. The bare "QUOTA" capability atom is advertised rather than the RFC 9208 QUOTA=RES-* form, there is no SETQUOTA, no per-mailbox quota roots and no OVERQUOTA response code. Listed in the roadmap's IMAP-extension backlog. |
409409
|| SAVEDATE (RFC 8514) | Not implemented. No SAVEDATE FETCH item and no SAVEDATE/SAVEDBEFORE/SAVEDSINCE/SAVEDATESUPPORTED search keys. Listed in the roadmap's IMAP-extension backlog. |
410-
|| THREAD (RFC 5256) | Not implemented and not advertised. No THREAD command in the dispatch table and no reference anywhere in the source. SORT ships from the same RFC but THREAD does not… |
410+
| ✅ | THREAD (RFC 5256) | **Shipped 15 August 2026, both algorithms.** `THREAD` and `UID THREAD` advertise `THREAD=ORDEREDSUBJECT THREAD=REFERENCES` and ride the same bounded search machinery as SORT. REFERENCES is the full RFC algorithm rather than a subject approximation wearing its name: containers are linked by `Message-ID`/`References`/`In-Reply-To` (falling back to In-Reply-To only when References is absent), a message's own References line authoritatively breaks a parent another message asserted, cycles are refused, dummies are pruned except at the root where they hold orphaned siblings together, threads are then grouped by RFC 2.1 base subject - the full step machine, so `[fwd: Re: [list] x]` reduces the way the RFC says - and sorted by sent date with the INTERNALDATE fallback. **An adversarial pass found three defects the 11 passing tests did not.** A deep reply chain overflowed the 1MB worker stack: `Prune_` and `SortChildrenRecursively_` recurse per level, and a probe compiled against those exact frames survived 6000 levels and died with `STATUS_STACK_OVERFLOW` at 8000 - a whole-service crash any authenticated user could reach by mailing themselves a long enough chain, now bounded by a 512 depth cap at link time. The header pass was uninterruptible, so `IMAPSearchTimeout` could only disown a result the connection thread had already spent minutes producing; the ceiling is now handed in and stops the pass before the next file opens. And a Date header that was present but unparseable fell through to the 1899 OLE epoch and sorted to the FRONT of its thread, because the comparator reads the raw value and not the validity flag - RFC 5256 2.2's INTERNALDATE fallback now covers unparseable as well as absent. 13 tests assert exact trees. **Two pre-existing weaknesses of the same shape are recorded, not silently inherited:** SORT checks its ceiling only after its own uninterruptible header pass, and `IMAPSort::CacheHeaderFields_` tests the Date header for emptiness rather than validity. |
411411
|| UNAUTHENTICATE (RFC 8437) | Not implemented and not advertised; there is no way to return an authenticated session to the not-authenticated state for connection reuse. |
412412
|| URLAUTH (RFC 4467) / BURL | Not implemented and not advertised; no GENURLAUTH, URLFETCH or RESETKEY commands and no IMAP URL parser. |
413413
|| WITHIN (RFC 5032) OLDER / YOUNGER search keys | Implemented and advertised. Both compare the message's internal date against (now - n seconds) using the whole timestamp rather than the calendar date, which is the point of the extension and what makes "YOUNGER 3600" answerable at all; relative-age searches no longer have to be rewritten as absolute BEFORE/SINCE. The interval is validated as an nz-number and clamped at a century, so a zero, negative or non-numeric interval is BAD instead of silently becoming "0 seconds ago". |

hmailserver/source/Server/IMAP/IMAPCommandCapability.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,13 @@ namespace HM
3232
if (pConfig->GetUseIMAPSort())
3333
sResponse += " SORT";
3434

35+
// RFC 5256. Unconditional where SORT is gated, and deliberately so: the work
36+
// is bounded by the same per-command ceilings as SEARCH and SORT, and a gate
37+
// would cost a seeded settings row on every backend plus a schema bump - see
38+
// the reasoning on IMAPThread. ORDEREDSUBJECT is required of any server that
39+
// advertises THREAD at all; REFERENCES is the one clients actually want.
40+
sResponse += " THREAD=ORDEREDSUBJECT THREAD=REFERENCES";
41+
3542
if (pConfig->GetUseIMAPACL())
3643
sResponse += " ACL";
3744

hmailserver/source/Server/IMAP/IMAPCommandSearch.cpp

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include "IMAPCommandSEARCH.h"
88
#include "IMAPConnection.h"
99
#include "IMAPSort.h"
10+
#include "IMAPThread.h"
1011
#include "IMAPConfiguration.h"
1112
#include "IMAPListLookup.h"
1213

@@ -27,8 +28,9 @@
2728

2829
namespace HM
2930
{
30-
IMAPCommandSEARCH::IMAPCommandSEARCH(bool bIsSort) :
31-
is_sort_(bIsSort),
31+
IMAPCommandSEARCH::IMAPCommandSEARCH(IMAPSearchCommandMode mode) :
32+
is_sort_(mode == IMAPSearchModeSort),
33+
is_thread_(mode == IMAPSearchModeThread),
3234
is_uid_(false),
3335
is_esearch_(false),
3436
esearch_min_(false),
@@ -93,8 +95,9 @@ namespace HM
9395

9496
// RFC 4731 (ESEARCH): an optional "RETURN (...)" result-options clause may
9597
// follow the SEARCH keyword. Detect and consume it before criteria parsing.
96-
// (RETURN with SORT is ESORT/RFC 5267 and is intentionally not handled here.)
97-
if (!is_sort_)
98+
// (RETURN with SORT is ESORT/RFC 5267 and is intentionally not handled here,
99+
// and RFC 5267's THREAD variant likewise.)
100+
if (!is_sort_ && !is_thread_)
98101
{
99102
String sTrimmed = pArgument->Command();
100103
sTrimmed.TrimLeft();
@@ -132,7 +135,7 @@ namespace HM
132135
// RFC 9051 (IMAP4rev2): once the client has enabled IMAP4rev2, a SEARCH (or UID
133136
// SEARCH) without an explicit RETURN clause still returns its results in an
134137
// ESEARCH response (with the ALL result option), not the legacy "* SEARCH" line.
135-
if (!is_sort_ && !is_esearch_ && pConnection->GetImap4Rev2Enabled())
138+
if (!is_sort_ && !is_thread_ && !is_esearch_ && pConnection->GetImap4Rev2Enabled())
136139
{
137140
is_esearch_ = true;
138141
esearch_all_ = true;
@@ -143,13 +146,23 @@ namespace HM
143146
// RFC 5182 (SEARCHRES): the parser resolves a "$" search key against this.
144147
pParser->SetSavedSearchResult(pConnection->GetSavedSearchResult());
145148

146-
IMAPResult result = pParser->ParseCommand(pArgument, is_sort_);
149+
IMAPResult result = pParser->ParseCommand(pArgument,
150+
is_thread_ ? IMAPSearchModeThread : (is_sort_ ? IMAPSearchModeSort : IMAPSearchModeSearch));
147151
if (result.GetResult() != IMAPResult::ResultOK)
148152
return result;
149153

150154
if (is_sort_ && !pParser->GetSortParser())
151155
return IMAPResult(IMAPResult::ResultBad, "Incorrect search commands.");
152156

157+
// RFC 5256: naming an algorithm the server did not advertise is a protocol
158+
// error. BAD rather than a silent fallback, because a client that asked for
159+
// REFERENCES and silently got ORDEREDSUBJECT would render wrong conversation
160+
// trees with no way to know it.
161+
IMAPThread::Algorithm threadAlgorithm = IMAPThread::AlgorithmReferences;
162+
163+
if (is_thread_ && !IMAPThread::ParseAlgorithm(pParser->GetThreadAlgorithm(), threadAlgorithm))
164+
return IMAPResult(IMAPResult::ResultBad, "Unsupported threading algorithm.");
165+
153166
// Mails in current box
154167
std::shared_ptr<IMAPFolder> pCurFolder = pConnection->GetCurrentFolder();
155168

@@ -215,6 +228,26 @@ namespace HM
215228
return AbortSearch_(pConnection, message_count_);
216229
}
217230

231+
if (is_thread_)
232+
{
233+
// Threading reads a header per matching message, and the ceiling is
234+
// handed IN rather than checked afterwards. SORT above does the latter,
235+
// and the difference matters: a post-hoc check can only disown a result
236+
// the connection thread has already spent minutes producing, whereas
237+
// this stops before the next file is opened. On a folder large enough
238+
// for the timeout to bite, that is the difference between a bounded
239+
// command and a bounded-looking one.
240+
IMAPThread oThreader;
241+
242+
const bool completed = oThreader.BuildThreads(pConnection, vecMatchingMessages, is_uid_,
243+
threadAlgorithm,
244+
[this]() { return BoundExceeded_(); },
245+
thread_response_body_);
246+
247+
if (!completed || BoundExceeded_())
248+
return AbortSearch_(pConnection, message_count_);
249+
}
250+
218251
typedef std::pair<int, std::shared_ptr<Message> > MessagePair;
219252
for(MessagePair messagePair : vecMatchingMessages)
220253
{
@@ -284,6 +317,12 @@ namespace HM
284317
}
285318
else if (is_sort_)
286319
sResponse = "* SORT" + sMatching + "\r\n";
320+
else if (is_thread_)
321+
{
322+
// The body carries its own leading space when there is one - an empty
323+
// result is a bare "* THREAD", not "* THREAD ".
324+
sResponse = "* THREAD" + thread_response_body_ + "\r\n";
325+
}
287326
else
288327
{
289328
sResponse = "* SEARCH" + sMatching;
@@ -678,6 +717,10 @@ namespace HM
678717
bytes_examined_ = 0;
679718
bound_reason_.Empty();
680719

720+
// The handler is reused per connection like everything else here, so a THREAD
721+
// that matched nothing must not answer with the PREVIOUS thread's tree.
722+
thread_response_body_.Empty();
723+
681724
// Read once, here. If they were read per message a configuration reload
682725
// during a long search could raise the ceiling out from under it.
683726
search_timeout_seconds_ = IniFileSettings::Instance()->GetIMAPSearchTimeout();

hmailserver/source/Server/IMAP/IMAPCommandSearch.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ namespace HM
1818
class IMAPCommandSEARCH : public IMAPCommand
1919
{
2020
public:
21-
IMAPCommandSEARCH(bool bIsSort);
21+
IMAPCommandSEARCH(IMAPSearchCommandMode mode);
2222
virtual ~IMAPCommandSEARCH();
2323

2424
void SetIsUID() { is_uid_ = true; }
@@ -57,6 +57,7 @@ namespace HM
5757
std::shared_ptr<MimeHeader> mime_header_;
5858

5959
bool is_sort_;
60+
bool is_thread_;
6061
bool is_uid_;
6162

6263
// RFC 4731 (ESEARCH) result-option state, parsed from a "RETURN (...)" clause.
@@ -91,5 +92,10 @@ namespace HM
9192

9293
// Which ceiling was passed, and by how much, for the application log entry.
9394
String bound_reason_;
95+
96+
// The rendered RFC 5256 tree for a THREAD command, built where SORT sorts and
97+
// consumed where the response is assembled. Cleared by ResetSearchState_,
98+
// because the handler is reused across commands on a connection.
99+
String thread_response_body_;
94100
};
95101
}

hmailserver/source/Server/IMAP/IMAPCommandUID.cpp

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ namespace HM
192192
}
193193
else if (sTypeOfUID.CompareNoCase(_T("SEARCH")) == 0)
194194
{
195-
std::shared_ptr<IMAPCommandSEARCH> pCommand = std::shared_ptr<IMAPCommandSEARCH> (new IMAPCommandSEARCH(false));
195+
std::shared_ptr<IMAPCommandSEARCH> pCommand = std::shared_ptr<IMAPCommandSEARCH> (new IMAPCommandSEARCH(IMAPSearchModeSearch));
196196
pCommand->SetIsUID();
197197
IMAPResult result = pCommand->ExecuteCommand(pConnection, pArgument);
198198

@@ -203,10 +203,23 @@ namespace HM
203203
}
204204
else if (sTypeOfUID.CompareNoCase(_T("SORT")) == 0)
205205
{
206-
std::shared_ptr<IMAPCommandSEARCH> pCommand = std::shared_ptr<IMAPCommandSEARCH> (new IMAPCommandSEARCH(true));
206+
std::shared_ptr<IMAPCommandSEARCH> pCommand = std::shared_ptr<IMAPCommandSEARCH> (new IMAPCommandSEARCH(IMAPSearchModeSort));
207207
pCommand->SetIsUID();
208208
IMAPResult result = pCommand->ExecuteCommand(pConnection, pArgument);
209-
209+
210+
if (result.GetResult() == IMAPResult::ResultOK)
211+
pConnection->SendAsciiData(sTag + " OK UID completed\r\n");
212+
213+
return result;
214+
}
215+
else if (sTypeOfUID.CompareNoCase(_T("THREAD")) == 0)
216+
{
217+
// RFC 5256: UID THREAD is THREAD with UIDs in the tree instead of
218+
// sequence numbers. Same shape as UID SORT above.
219+
std::shared_ptr<IMAPCommandSEARCH> pCommand = std::shared_ptr<IMAPCommandSEARCH> (new IMAPCommandSEARCH(IMAPSearchModeThread));
220+
pCommand->SetIsUID();
221+
IMAPResult result = pCommand->ExecuteCommand(pConnection, pArgument);
222+
210223
if (result.GetResult() == IMAPResult::ResultOK)
211224
pConnection->SendAsciiData(sTag + " OK UID completed\r\n");
212225

hmailserver/source/Server/IMAP/IMAPConnection.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,9 @@ namespace HM
168168
{
169169
mapCommandHandlers[IMAP_UID] = std::shared_ptr<IMAPCommandUID>(new IMAPCommandUID());
170170
mapCommandHandlers[IMAP_APPEND] = std::shared_ptr<IMAPCommandAppend>(new IMAPCommandAppend());
171-
mapCommandHandlers[IMAP_SEARCH] = std::shared_ptr<IMAPCommandSEARCH>(new IMAPCommandSEARCH(false));
172-
mapCommandHandlers[IMAP_SORT] = std::shared_ptr<IMAPCommandSEARCH>(new IMAPCommandSEARCH(true));
171+
mapCommandHandlers[IMAP_SEARCH] = std::shared_ptr<IMAPCommandSEARCH>(new IMAPCommandSEARCH(IMAPSearchModeSearch));
172+
mapCommandHandlers[IMAP_SORT] = std::shared_ptr<IMAPCommandSEARCH>(new IMAPCommandSEARCH(IMAPSearchModeSort));
173+
mapCommandHandlers[IMAP_THREAD] = std::shared_ptr<IMAPCommandSEARCH>(new IMAPCommandSEARCH(IMAPSearchModeThread));
173174
mapCommandHandlers[IMAP_IDLE] = std::shared_ptr<IMAPCommandIdle>(new IMAPCommandIdle(std::dynamic_pointer_cast<IMAPConnection>(shared_from_this())));
174175

175176
mapStaticHandlers = StaticIMAPCommandHandlers::Instance()->GetStaticHandlers();
@@ -685,6 +686,8 @@ namespace HM
685686
return IMAP_IDLE;
686687
else if (sCommand == _T("SORT"))
687688
return IMAP_SORT;
689+
else if (sCommand == _T("THREAD"))
690+
return IMAP_THREAD;
688691
else if (sCommand == _T("NAMESPACE"))
689692
return IMAP_NAMESPACE;
690693
else if (sCommand == _T("MYRIGHTS"))
@@ -868,10 +871,15 @@ namespace HM
868871
command.
869872
*/
870873

874+
// RFC 5256 carries SORT's rule over verbatim: untagged EXPUNGE responses are
875+
// not permitted while responding to THREAD, but are permitted during a UID
876+
// THREAD - and active_command is IMAP_UID in the UID case, so the exclusion
877+
// below covers exactly the non-UID form, which is the required behaviour.
871878
bool send_expunge = active_command != IMAP_FETCH &&
872879
active_command != IMAP_STORE &&
873880
active_command != IMAP_SEARCH &&
874-
active_command != IMAP_SORT;
881+
active_command != IMAP_SORT &&
882+
active_command != IMAP_THREAD;
875883

876884
notification_client_->SendCachedNotifications(send_expunge);
877885
}

hmailserver/source/Server/IMAP/IMAPConnection.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ namespace HM
8484
IMAP_MOVE = 136,
8585
IMAP_ID = 137,
8686
IMAP_UNSELECT = 138,
87-
IMAP_ENABLE = 139
87+
IMAP_ENABLE = 139,
88+
IMAP_THREAD = 140
8889
};
8990

9091
void ParseData(const AnsiString &Request);

0 commit comments

Comments
 (0)