[bug-mailutils] Bugs in movemail with IMAP folders

[Thomas Guyot-Sionnest]

Hi,

I sent two emails some time ago about bugs with IMAP folders.

For one of them I sent a patch for both the 1.2 and 2.0 versions, though
there's three other distinct bugs with IMAP on 2.0 so I'm using 1.2
right now. To the best of my knowledge these patches are fully RFC
compliant; see the original email ("Re: Bugs in mailutils with IMAP
folders" sent on Jan. 13 2009 for more details). I attached thee patches
again.


Regarding the other bugs in 2.0; (see the original email ("Bugs in
mailutils with IMAP folders" sent on Jan. 7 2009 for more details).

The first two can be debugged without access to an IMAP server (URL
parsing bugs). I experienced them with an url of the form
"imap://user:address@hidden/Public Folders/Test"

1. Parse Error in mailbox/url.c:363

I'm not sure what the exact fix is, but returning 0 instead of
MU_ERR_PARSE works around it and leads to the 2nd bug

2. Invalid Argument in mailbox/mailbox.c:56)

The return code from mu_url_aget_path is not correct. This is a macro
defined in mailbox/url.c:541 and I'm having hard time following it. My
dirty fix was to override "rc" in mailbox/mailbox.c:56

3. On the default folder (INBOX) or once the two bugs above are fixed, I
always get the following error if movemail can successfully connect to
IMAP and get the folder's content:

Default folder: Cannot append message 1: Cannot allocate memory

I haven't looked at this one yet as there are two other blocker bugs
that would need fixing anyway.

Thanks,

--
Thomas

diff --git a/libproto/imap/folder.c b/libproto/imap/folder.c
index 944e9b2..9b1a6a9 100644
--- a/libproto/imap/folder.c
+++ b/libproto/imap/folder.c
@@ -827,7 +827,7 @@ folder_imap_delete (mu_folder_t folder, const char *name)
   switch (f_imap->state)
     {
     case IMAP_NO_STATE:
-      status = imap_writeline (f_imap, "g%u DELETE %s\r\n", f_imap->seq++,
+      status = imap_writeline (f_imap, "g%u DELETE \"%s\"\r\n", f_imap->seq++,
                               name);
       CHECK_ERROR (f_imap, status);
       MU_DEBUG (folder->debug, MU_DEBUG_PROT, f_imap->buffer);
@@ -1104,7 +1104,7 @@ folder_imap_rename (mu_folder_t folder, const char 
*oldpath,
   switch (f_imap->state)
     {
     case IMAP_NO_STATE:
-      status = imap_writeline (f_imap, "g%u RENAME %s %s\r\n",
+      status = imap_writeline (f_imap, "g%u RENAME \"%s\" \"%s\"\r\n",
                               f_imap->seq++, oldpath, newpath);
       CHECK_ERROR (f_imap, status);
       MU_DEBUG (folder->debug, MU_DEBUG_PROT, f_imap->buffer);
@@ -1142,7 +1142,7 @@ folder_imap_subscribe (mu_folder_t folder, const char 
*name)
   switch (f_imap->state)
     {
     case IMAP_NO_STATE:
-      status = imap_writeline (f_imap, "g%u SUBSCRIBE %s\r\n",
+      status = imap_writeline (f_imap, "g%u SUBSCRIBE \"%s\"\r\n",
                               f_imap->seq++, name);
       CHECK_ERROR (f_imap, status);
       MU_DEBUG (folder->debug, MU_DEBUG_PROT, f_imap->buffer);
@@ -1180,7 +1180,7 @@ folder_imap_unsubscribe (mu_folder_t folder, const char 
*name)
   switch (f_imap->state)
     {
     case IMAP_NO_STATE:
-      status = imap_writeline (f_imap, "g%u UNSUBSCRIBE %s\r\n",
+      status = imap_writeline (f_imap, "g%u UNSUBSCRIBE \"%s\"\r\n",
                               f_imap->seq++, name);
       CHECK_ERROR (f_imap, status);
       MU_DEBUG (folder->debug, MU_DEBUG_PROT, f_imap->buffer);
diff --git a/libproto/imap/mbox.c b/libproto/imap/mbox.c
index 67f81a1..47f9d61 100644
--- a/libproto/imap/mbox.c
+++ b/libproto/imap/mbox.c
@@ -315,9 +315,9 @@ mailbox_imap_open (mu_mailbox_t mailbox, int flags)
              return 0;
            else if (status)
              return status;
-           status = imap_writeline (f_imap, "g%u CREATE %s\r\n",
+           status = imap_writeline (f_imap, "g%u CREATE \"%s\"\r\n",
                                     f_imap->seq, path);
-           MU_DEBUG2 (folder->debug, MU_DEBUG_PROT, "g%u CREATE %s\n",
+           MU_DEBUG2 (folder->debug, MU_DEBUG_PROT, "g%u CREATE \"%s\"\n",
                       f_imap->seq, path);
            f_imap->seq++;
            if (status != 0)
@@ -682,7 +682,7 @@ imap_messages_count (mu_mailbox_t mailbox, size_t *pnum)
   switch (f_imap->state)
     {
     case IMAP_NO_STATE:
-      status = imap_writeline (f_imap, "g%s %s %s\r\n",
+      status = imap_writeline (f_imap, "g%s %s \"%s\"\r\n",
                               mu_umaxtostr (0, f_imap->seq++), 
                                MBX_WRITABLE(mailbox) ? "SELECT" : "EXAMINE",
                                m_imap->name);
@@ -1029,7 +1029,7 @@ imap_append_message0 (mu_mailbox_t mailbox, mu_message_t 
msg)
        mu_message_size (msg, &size);
        mu_message_lines (msg, &lines);
        total = size + lines;
-       status = imap_writeline (f_imap, "g%s APPEND %s %s {%s}\r\n",
+       status = imap_writeline (f_imap, "g%s APPEND \"%s\" %s {%s}\r\n",
                                 mu_umaxtostr (0, f_imap->seq++),
                                 path,
                                 abuf,
@@ -1124,7 +1124,7 @@ imap_copy_message (mu_mailbox_t mailbox, mu_message_t msg)
        /* Check for a valid mailbox name.  */
        status = mu_url_sget_path (mailbox->url, &path);
        if (status == 0)
-         status = imap_writeline (f_imap, "g%s COPY %s %s\r\n",
+         status = imap_writeline (f_imap, "g%s COPY %s \"%s\"\r\n",
                                   mu_umaxtostr (0, f_imap->seq++),
                                   mu_umaxtostr (1, msg_imap->num),
                                   path);

diff --git a/libproto/imap/folder.c b/libproto/imap/folder.c
index cbdcf6f..1f71560 100644
--- a/libproto/imap/folder.c
+++ b/libproto/imap/folder.c
@@ -772,7 +772,7 @@ folder_imap_delete (mu_folder_t folder, const char *name)
   switch (f_imap->state)
     {
     case IMAP_NO_STATE:
-      status = imap_writeline (f_imap, "g%u DELETE %s\r\n", f_imap->seq++,
+      status = imap_writeline (f_imap, "g%u DELETE \"%s\"\r\n", f_imap->seq++,
                               name);
       CHECK_ERROR (f_imap, status);
       FOLDER_DEBUG0 (folder, MU_DEBUG_PROT, f_imap->buffer);
@@ -1035,7 +1035,7 @@ folder_imap_rename (mu_folder_t folder, const char 
*oldpath,
   switch (f_imap->state)
     {
     case IMAP_NO_STATE:
-      status = imap_writeline (f_imap, "g%u RENAME %s %s\r\n",
+      status = imap_writeline (f_imap, "g%u RENAME \"%s\" \"%s\"\r\n",
                               f_imap->seq++, oldpath, newpath);
       CHECK_ERROR (f_imap, status);
       FOLDER_DEBUG0 (folder, MU_DEBUG_PROT, f_imap->buffer);
@@ -1073,7 +1073,7 @@ folder_imap_subscribe (mu_folder_t folder, const char 
*name)
   switch (f_imap->state)
     {
     case IMAP_NO_STATE:
-      status = imap_writeline (f_imap, "g%u SUBSCRIBE %s\r\n",
+      status = imap_writeline (f_imap, "g%u SUBSCRIBE \"%s\"\r\n",
                               f_imap->seq++, name);
       CHECK_ERROR (f_imap, status);
       FOLDER_DEBUG0 (folder, MU_DEBUG_PROT, f_imap->buffer);
@@ -1111,7 +1111,7 @@ folder_imap_unsubscribe (mu_folder_t folder, const char 
*name)
   switch (f_imap->state)
     {
     case IMAP_NO_STATE:
-      status = imap_writeline (f_imap, "g%u UNSUBSCRIBE %s\r\n",
+      status = imap_writeline (f_imap, "g%u UNSUBSCRIBE \"%s\"\r\n",
                               f_imap->seq++, name);
       CHECK_ERROR (f_imap, status);
       FOLDER_DEBUG0 (folder, MU_DEBUG_PROT, f_imap->buffer);
diff --git a/libproto/imap/mbox.c b/libproto/imap/mbox.c
index b810f58..ab4c338 100644
--- a/libproto/imap/mbox.c
+++ b/libproto/imap/mbox.c
@@ -307,9 +307,9 @@ mailbox_imap_open (mu_mailbox_t mailbox, int flags)
            if (path == NULL)
              return ENOMEM;
            mu_url_get_path (folder->url, path, len + 1, NULL);
-           status = imap_writeline (f_imap, "g%u CREATE %s\r\n",
+           status = imap_writeline (f_imap, "g%u CREATE \"%s\"\r\n",
                                     f_imap->seq, path);
-           MAILBOX_DEBUG2 (folder, MU_DEBUG_PROT, "g%u CREATE %s\n",
+           MAILBOX_DEBUG2 (folder, MU_DEBUG_PROT, "g%u CREATE \"%s\"\n",
                            f_imap->seq, path);
            f_imap->seq++;
            free (path);
@@ -675,7 +675,7 @@ imap_messages_count (mu_mailbox_t mailbox, size_t *pnum)
   switch (f_imap->state)
     {
     case IMAP_NO_STATE:
-      status = imap_writeline (f_imap, "g%s %s %s\r\n",
+      status = imap_writeline (f_imap, "g%s %s \"%s\"\r\n",
                               mu_umaxtostr (0, f_imap->seq++), 
                                MBX_WRITABLE(mailbox) ? "SELECT" : "EXAMINE",
                                m_imap->name);
@@ -1040,7 +1040,7 @@ imap_append_message0 (mu_mailbox_t mailbox, mu_message_t 
msg)
        mu_message_size (msg, &size);
        mu_message_lines (msg, &lines);
        total = size + lines;
-       status = imap_writeline (f_imap, "g%s APPEND %s %s {%s}\r\n",
+       status = imap_writeline (f_imap, "g%s APPEND \"%s\" %s {%s}\r\n",
                                 mu_umaxtostr (0, f_imap->seq++),
                                 path,
                                 abuf,
@@ -1142,7 +1142,7 @@ imap_copy_message (mu_mailbox_t mailbox, mu_message_t msg)
        if (path == NULL)
          return ENOMEM;
        mu_url_get_path (mailbox->url, path, n + 1, NULL);
-       status = imap_writeline (f_imap, "g%s COPY %s %s\r\n",
+       status = imap_writeline (f_imap, "g%s COPY %s \"%s\"\r\n",
                                 mu_umaxtostr (0, f_imap->seq++),
                                 mu_umaxtostr (1, msg_imap->num),
                                 path);