Skip to content

Commit

Permalink
net_imap: Avoid desynchronization from unsolicited UID during MOVE/COPY.
Browse files Browse the repository at this point in the history
When doing a transparent cross-server MOVE or COPY operation, we would
originally request only three items, not including the UID, since it
doesn't matter. However, some servers will send the UID anyways, and
in particular, if this was sent after all the requested items, our
parsing logic would have moved onto the next message and this would
result in reading unexpected data that didn't correspond to the next
message. This could be handled by having logic to detect receiving
the UID and ignore it, but a cleaner way is to simply always request
the UID and discard it, to avoid getting out of sync like this to
begin with.
  • Loading branch information
InterLinked1 committed Apr 26, 2024
1 parent 89cecc8 commit 4b4c4db
Showing 1 changed file with 12 additions and 3 deletions.
15 changes: 12 additions & 3 deletions nets/net_imap.c
Original file line number Diff line number Diff line change
Expand Up @@ -2719,7 +2719,13 @@ static int handle_remote_move(struct imap_session *imap, char *dest, const char
if (imap->client) { /* Source is remote, destination is either local or another remote */
/* RFC 4549 4.2.2.2: Moving a message from a remote mailbox to a local one */
struct bbs_tcp_client *tcpclient = &imap->client->client;
res = imap_client_send_log(imap->client, "%s %sFETCH %s (INTERNALDATE FLAGS BODY.PEEK[])\r\n", imap->tag, usinguid ? "UID " : "", sequences);
/* We don't actually need the UID and don't care what it is;
* however, requesting it makes the parsing logic easier
* since some servers will send the UID to us even if the client doesn't ask for it.
* Therefore, this ensures that we always process the UID as part of the current message,
* to avoid the scenario where the server sends an unsolicited UID after sending everything
* we anticipated, and then needing to have logic to ignore that. */
res = imap_client_send_log(imap->client, "%s %sFETCH %s (UID INTERNALDATE FLAGS BODY.PEEK[])\r\n", imap->tag, usinguid ? "UID " : "", sequences);
if (res < 0) {
goto cleanup;
}
Expand Down Expand Up @@ -2768,14 +2774,17 @@ static int handle_remote_move(struct imap_session *imap, char *dest, const char
goto cleanup;
}
s++;
while (!strlen_zero(s) && items_received < 3) {
while (!strlen_zero(s) && items_received < 4) {
tmp = strsep(&s, " ");
if (!strcasecmp(tmp, "FLAGS")) {
char *flagstr = parensep(&s);
if (!strlen_zero(flagstr)) {
REPLACE(flags, flagstr);
}
items_received++;
} else if (!strcasecmp(tmp, "UID")) {
/* Don't care, ignore and discard */
items_received++;
} else if (!strcasecmp(tmp, "INTERNALDATE")) {
char *datestr = quotesep(&s);
if (!strlen_zero(datestr)) {
Expand Down Expand Up @@ -2839,7 +2848,7 @@ static int handle_remote_move(struct imap_session *imap, char *dest, const char
break;
}
}
/* Actual copy the message from source to dest */
/* Actually copy the message from source to dest */
res = bbs_readline_getn(tcpclient->rfd, destfd, &tcpclient->rldata, SEC_MS(10), (size_t) size);
if (res != size) {
bbs_warning("Wanted to copy message of size %lu, but only got %lu bytes?\n", size, res);
Expand Down

0 comments on commit 4b4c4db

Please sign in to comment.