On Sun, Nov 18, 2018 at 06:37:47PM +0100, Marcin Niestroj wrote:
> This patch adds a new step after all messages from IMAP server are
> fetched. It sorts fetched messages in ascending UID order, so any code
> executed in later stages can rely on that. It solves issue described
> above with poczta.o2.pl server, so all 1335 (in that case) messages
> are downloaded.
>
thanks.
i reshuffled the patch somewhat, but it's actually pretty much still the
same code.
please check whether i didn't break it on the way.
>From acfa3a2bbc0f4243096843d08671521ae2ad545f Mon Sep 17 00:00:00 2001
From: Oswald Buddenhagen <o...@users.sf.net>
Date: Sat, 24 Nov 2018 14:15:53 +0100
Subject: [PATCH] sort messages from UID FETCH request
turns out that some IMAP servers (e.g., poczta.o2.pl) do not return
messages in ascending UID order in response to a UID FETCH request,
which makes the driver violate the API contract.
counter this by sorting the messages. this also addresses the
long-standing (but hypothetical) issue that parallel UID FETCH requests
could be handled out-of-order and thus also lead to mixed up results.
based on patch by Marcin Niestroj <macius19...@gmail.com>.
---
src/driver.c | 9 +++++++++
src/driver.h | 1 +
src/drv_imap.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++----
3 files changed, 58 insertions(+), 4 deletions(-)
diff --git a/src/driver.c b/src/driver.c
index e522d14..e28d817 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -27,6 +27,15 @@
driver_t *drivers[N_DRIVERS] = { &maildir_driver, &imap_driver };
+int
+count_generic_messages( message_t *msgs )
+{
+ int count = 0;
+ for (; msgs; msgs = msgs->next)
+ count++;
+ return count;
+}
+
void
free_generic_messages( message_t *msgs )
{
diff --git a/src/driver.h b/src/driver.h
index 7dc047a..18f8bfd 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -259,6 +259,7 @@ struct driver {
int (*get_fail_state)( store_conf_t *conf );
};
+int count_generic_messages( message_t * );
void free_generic_messages( message_t * );
void parse_generic_store( store_conf_t *store, conffile_t *cfg );
diff --git a/src/drv_imap.c b/src/drv_imap.c
index 06414e2..9e137cf 100644
--- a/src/drv_imap.c
+++ b/src/drv_imap.c
@@ -597,8 +597,9 @@ imap_refcounted_new_cmd( imap_cmd_refcounted_state_t *sts )
free( sts ); \
}
-#define DONE_REFCOUNTED_STATE_ARGS(sts, ...) \
+#define DONE_REFCOUNTED_STATE_ARGS(sts, finalize, ...) \
if (!--sts->gen.ref_count) { \
+ finalize \
sts->callback( sts->gen.ret_val, __VA_ARGS__, sts->callback_aux ); \
free( sts ); \
}
@@ -1102,7 +1103,6 @@ parse_fetch_rsp( imap_store_t *ctx, list_t *list, char *s ATTR_UNUSED )
if (status & M_FLAGS)
msgdata->flags = mask;
} else {
- /* XXX this will need sorting for out-of-order (multiple queries) */
cur = nfcalloc( sizeof(*cur) );
*ctx->msgapp = &cur->gen;
ctx->msgapp = &cur->gen.next;
@@ -2587,6 +2587,47 @@ imap_load_box( store_t *gctx, uint minuid, uint maxuid, uint newuid, uint seenui
}
}
+static int
+imap_sort_msgs_comp( const void *a_, const void *b_ )
+{
+ const message_t *a = *(const message_t * const *)a_;
+ const message_t *b = *(const message_t * const *)b_;
+
+ if (a->uid < b->uid)
+ return -1;
+ if (a->uid > b->uid)
+ return 1;
+ return 0;
+}
+
+static void
+imap_sort_msgs( imap_store_t *ctx )
+{
+ int count = count_generic_messages( ctx->msgs );
+ if (count <= 1)
+ return;
+
+ message_t **t = nfmalloc( sizeof(*t) * count );
+
+ message_t *m = ctx->msgs;
+ for (int i = 0; i < count; i++) {
+ t[i] = m;
+ m = m->next;
+ }
+
+ qsort( t, count, sizeof(*t), imap_sort_msgs_comp );
+
+ ctx->msgs = t[0];
+
+ int j;
+ for (j = 0; j < count - 1; j++)
+ t[j]->next = t[j + 1];
+ ctx->msgapp = &t[j]->next;
+ *ctx->msgapp = NULL;
+
+ free( t );
+}
+
static void imap_submit_load_p2( imap_store_t *, imap_cmd_t *, int );
static void
@@ -2615,7 +2656,10 @@ imap_submit_load_p2( imap_store_t *ctx, imap_cmd_t *cmd, int response )
static void
imap_submit_load_p3( imap_store_t *ctx, imap_load_box_state_t *sts )
{
- DONE_REFCOUNTED_STATE_ARGS(sts, ctx->msgs, ctx->total_msgs, ctx->recent_msgs)
+ DONE_REFCOUNTED_STATE_ARGS(sts, {
+ if (sts->gen.ret_val == DRV_OK)
+ imap_sort_msgs( ctx );
+ }, ctx->msgs, ctx->total_msgs, ctx->recent_msgs)
}
/******************* imap_fetch_msg *******************/
@@ -3013,7 +3057,7 @@ imap_list_store_p2( imap_store_t *ctx, imap_cmd_t *cmd, int response )
static void
imap_list_store_p3( imap_store_t *ctx, imap_list_store_state_t *sts )
{
- DONE_REFCOUNTED_STATE_ARGS(sts, ctx->boxes)
+ DONE_REFCOUNTED_STATE_ARGS(sts, , ctx->boxes)
}
/******************* imap_cancel_cmds *******************/
--
2.16.1.1.g7edac48446
_______________________________________________
isync-devel mailing list
isync-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/isync-devel
