diff --git a/nets/net_imap.c b/nets/net_imap.c index 8b210bd..cf80c12 100644 --- a/nets/net_imap.c +++ b/nets/net_imap.c @@ -2622,15 +2622,12 @@ static int handle_append(struct imap_session *imap, char *s) if (res < 0) { return -1; } - /* Relay response from remote, and no need to clean up */ - res = bbs_readline(imap->client->client.rfd, &imap->client->client.rldata, "\r\n", SEC_MS(5)); - if (res < 0) { - /* No need to clean up, since for remote server, - * we're not appending seqnos/UIDs to a uintlist */ - bbs_warning("APPEND tagged response not received from remote server?\n"); + /* Relay response from remote, and no need to clean up. + * We may (should) receive an untagged EXISTS for the uploaded message, + * so use imap_client_wait_response to wait for the actual tagged response. */ + if (imap_client_wait_response(imap->client, -1, SEC_MS(5))) { return -1; } - _imap_reply(imap, "%s\r\n", imap->client->client.buf); return 0; } @@ -3035,13 +3032,16 @@ static int handle_remote_move(struct imap_session *imap, char *dest, const char if (SWRITE(destclient->client.wfd, "\r\n") != STRLEN("\r\n")) { goto cleanup; } - /* Well, hopefully that all worked! - * Use IMAP_CLIENT_EXPECT_EVENTUALLY since we may also get untagged EXISTS's for the copied messages. */ - - /*! \todo BUGBUG For any IMAP_CLIENT_EXPECT_EVENTUALLY, all the untagged responses that we "ignore - * should actually pass through to the client directly, since an offline client will need those - * in order to be synchronized. */ - IMAP_CLIENT_EXPECT_EVENTUALLY(&destclient->client, appended + 1, " OK"); /* tagged OK */ + /* Well, hopefully that all worked! Now, wait for confirmation of a successful upload. + * + * Here, we use imap_client_wait_response instead of IMAP_CLIENT_EXPECT_EVENTUALLY to wait for the tagged OK, + * because any untagged data we receive prior to that should pass through directly to the client + * (rather than being silently ignored/discarded as IMAP_CLIENT_EXPECT_EVENTUALLY does). + * This way, the client can stay synchronized with the remote server, which is important + * since APPEND can trigger untagged EXISTS's, etc. */ + if (imap_client_wait_response(destclient, -1, SEC_MS(5))) { + goto cleanup; + } } if (move) { /* Now that we copied everything to the destination mailbox, delete the source */ diff --git a/nets/net_imap/imap_client.c b/nets/net_imap/imap_client.c index 0c5f35c..aa324e9 100644 --- a/nets/net_imap/imap_client.c +++ b/nets/net_imap/imap_client.c @@ -440,6 +440,7 @@ static ssize_t client_command_passthru(struct imap_client *client, int fd, const } res = bbs_readline(tcpclient->rfd, &tcpclient->rldata, "\r\n", ms); if (res < 0) { /* Could include remote server disconnect */ + bbs_warning("Failed to receive expected response from remote server\n"); return res; } if (cb) { @@ -469,9 +470,13 @@ static ssize_t client_command_passthru(struct imap_client *client, int fd, const #endif if (!strncmp(buf, tag, (size_t) taglen)) { imap_debug(10, "<= %.*s\n", (int) res, buf); - if (STARTS_WITH(buf + taglen, "BAD")) { + if (!STARTS_WITH(buf + taglen, "OK")) { /* BAD or NO */ /* We did something we shouldn't have, oops */ - bbs_warning("Command '%.*s%.*s' failed: %s\n", taglen, tag, cmdlen > 2 ? cmdlen - 2 : cmdlen, cmd, buf); /* Don't include trailing CR LF */ + if (cmd) { + bbs_warning("Command '%.*s%.*s' failed: %s\n", taglen, tag, cmdlen > 2 ? cmdlen - 2 : cmdlen, cmd, buf); /* Don't include trailing CR LF */ + } else { + bbs_warning("Received negatory response: %s\n", buf); + } } client->lastactive = time(NULL); /* Successfully just got data from remote server */ break; /* That's all, folks! */ @@ -510,6 +515,30 @@ ssize_t __imap_client_send_log(struct imap_client *client, int log, const char * return bbs_write(client->client.wfd, buf, (size_t) len); } +int __imap_client_wait_response(struct imap_client *client, int fd, int ms, int echo, int lineno, int (*cb)(struct imap_client *client, const char *buf, size_t len, void *cbdata), void *cbdata) +{ + char tagbuf[15]; + int taglen; + const char *tag = "tag"; + + if (!client->imap) { + bbs_warning("No active IMAP client?\n"); /* Shouldn't happen... */ + bbs_soft_assert(0); + } else if (strlen_zero(client->imap->tag)) { + bbs_warning("No active IMAP tag, using generic one\n"); + bbs_soft_assert(0); + } else { + tag = client->imap->tag; + } + + taglen = snprintf(tagbuf, sizeof(tagbuf), "%s ", tag); /* Reuse the tag the client sent us, so we can just passthrough the response */ + + UNUSED(lineno); + + /* Read until we get the tagged respones */ + return client_command_passthru(client, fd, tagbuf, taglen, NULL, 0, ms, echo, cb, cbdata) <= 0; +} + int __imap_client_send_wait_response(struct imap_client *client, int fd, int ms, int echo, int lineno, int (*cb)(struct imap_client *client, const char *buf, size_t len, void *cbdata), void *cbdata, const char *fmt, ...) { char *buf; diff --git a/nets/net_imap/imap_client.h b/nets/net_imap/imap_client.h index 37f50a5..0072ade 100644 --- a/nets/net_imap/imap_client.h +++ b/nets/net_imap/imap_client.h @@ -86,11 +86,27 @@ ssize_t __attribute__ ((format (gnu_printf, 3, 4))) __imap_client_send_log(struc #define imap_client_send(client, fmt, ...) __imap_client_send_log(client, 0, fmt, ## __VA_ARGS__) #define imap_client_send_log(client, fmt, ...) __imap_client_send_log(client, 1, fmt, ## __VA_ARGS__) +#define imap_client_wait_response(client, fd, ms) __imap_client_wait_response(client, fd, ms, 1, __LINE__, NULL, NULL) + #define imap_client_send_wait_response(client, fd, ms, fmt, ...) __imap_client_send_wait_response(client, fd, ms, 1, __LINE__, NULL, NULL, fmt, ## __VA_ARGS__) #define imap_client_send_wait_response_cb(client, fd, ms, cb, cbdata, fmt, ...) __imap_client_send_wait_response(client, fd, ms, 1, __LINE__, cb, cbdata, fmt, ## __VA_ARGS__) #define imap_client_send_wait_response_noecho(client, fd, ms, fmt, ...) __imap_client_send_wait_response(client, fd, ms, 0, __LINE__, NULL, NULL, fmt, ## __VA_ARGS__) #define imap_client_send_wait_response_cb_noecho(client, fd, ms, cb, cbdata, fmt, ...) __imap_client_send_wait_response(client, fd, ms, 0, __LINE__, cb, cbdata, fmt, ## __VA_ARGS__) +/*! \brief Same as __imap_client_send_wait_response, but don't send a command initially, just wait for the tagged response */ +int __imap_client_wait_response(struct imap_client *client, int fd, int ms, int echo, int lineno, int (*cb)(struct imap_client *client, const char *buf, size_t len, void *cbdata), void *cbdata); + +/*! + * \brief Send a command to remote IMAP server and wait for a response, passing through any receive untagged responses inbetween directly back to the client + * \param client + * \param fd Additional file descriptor on which to poll(used only for IDLE, -1 otherwise) + * \param ms Max time to wait for poll / bbs_readline + * \param echo Whether to relay output from remote server to our local client + * \param lineno + * \param cb Custom callback to run for each line received from remote server. Returning -1 from callback will terminate wait loop + * \param cbdata Callback data for callback function + * \param fmt printf-style fmt string + */ int __attribute__ ((format (gnu_printf, 8, 9))) __imap_client_send_wait_response(struct imap_client *client, int fd, int ms, int echo, int lineno, int (*cb)(struct imap_client *client, const char *buf, size_t len, void *cbdata), void *cbdata, const char *fmt, ...); /*! \retval Number of replacements made */