[bug-mailutils] Re: Bugs in mailutils with IMAP folders

[Thomas Guyot-Sionnest]

On 07/01/09 03:50 PM, Thomas Guyot-Sionnest wrote:
> Hy there,
> 
> I'm trying to move mails from some IMAP folders to an MBOX. None of the
> release version of mailutils worked so I started looking at the problem...
> 
> v. 1.x:
>   I can download the default folder (INBOX).
>   It fails if I specify an IMAP folder.
> 
> The problem is that it doesn't quote the folder in the SELECT/EXAMINE
> command.
> 
> imap-1.patch should apply fine on it (though I diff'ed using 2.0). With
> that change v. 1.2 works successfully.

Hi,
I just checked for rfc-compliance (rfc3501) of the patch. here's how
they define valid values for mailbox names (i.e. SELECT, EXAMINE, etc):

mailbox         = "INBOX" / astring
                    ; INBOX is case-insensitive.  All case variants of
                    ; INBOX (e.g., "iNbOx") MUST be interpreted as INBOX
                    ; not as an astring.  An astring which consists of
                    ; the case-insensitive sequence "I" "N" "B" "O" "X"
                    ; is considered to be INBOX and not an astring.
                    ;  Refer to section 5.1 for further
                    ; semantic details of mailbox names.


While the correct way for INBOX would be without the quotes, the quoted
form is to be interpreted the same way as a literal "INBOX". This is
also how the LIST command currently works.

In section 4, they define "astring" to be "either an atom or a string".
In 4.3, srting is defined has having two possible forms:

1. A literal is a sequence of zero or more octets (including CR and
   LF), prefix-quoted with an octet count in the form of an open
   brace ("{"), the number of octets, close brace ("}"), and CRLF.

2. A quoted string is a sequence of zero or more 7-bit characters,
   excluding CR and LF, with double quote (<">) characters at each
   end.

Note that this is also how it currently works for "LIST" so it shouldn't
break anything (i.e. broken clients wouldn't work anyway).

I noticed there was a bunch of commands that wasn't quoting strings as
well. I checked the RCF for every command. I also tested the
[UN]SUBSCRIBE commands manually as the rfc wasn't very clear.


The attached patches fix all of them... one for the HEAD revision and
one for what appears to be the 1.2 revision (there's no tag/branch):
ccb7ad7f...

Even with this patch on 2.0 I still have three other issues when using
movemail v.2.0, two related to the url parsing and one to writing the
mbox file (Cannon allocate memory). Therefore my only option is to use
teh patched v1.2 for now.

Also note that is looks like a bunch of commands could take a
glob_to_imap() first as they accept globs (I didn't want to play too
much with the code though)

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);