Fix mailbox names not being sent to the server quoted if needed.

Since MailboxParameter inherited from StringParameter directly, it meant
that we could never send mailbox names as quoted strings. Also, the
modified-UTF-7 encoding used for mailbox names does not encode
atom-specials such as "\", so if a mailbox name contained one or more of
these, it would be sent to the mail server unquoted.

This removes the MailboxParameter class altogether, and does the
parameter conversion to/from appropriate StringParameter subclasses as
needed. Also adds unit tests for param conversion for ASCII,
atom-specials, and non-ASCII mailbox names.

Fixes issue #40

po/POTFILES.in

@@ -282,7 +282,6 @@ src/engine/imap/message/imap-fetch-data-specifier.vala
 src/engine/imap/message/imap-flag.vala
 src/engine/imap/message/imap-flags.vala
 src/engine/imap/message/imap-internal-date.vala
-src/engine/imap/message/imap-mailbox-parameter.vala
 src/engine/imap/message/imap-mailbox-specifier.vala
 src/engine/imap/message/imap-message-data.vala
 src/engine/imap/message/imap-message-flag.vala

src/engine/imap/message/imap-mailbox-parameter.vala

@@ -1,60 +0,0 @@
-/* Copyright 2016 Software Freedom Conservancy Inc.
- *
- * This software is licensed under the GNU Lesser General Public License
- * (version 2.1 or later).  See the COPYING file in this distribution.
- */
-
-/**
- * A {@link StringParameter} that holds a mailbox reference (can be wildcarded).
- *
- * Used to juggle between our internal UTF-8 representation of mailboxes and IMAP's
- * odd "modified UTF-7" representation.  The value is stored in IMAP's encoded
- * format since that's how it comes across the wire.
- */
-
-public class Geary.Imap.MailboxParameter : StringParameter {
-    public MailboxParameter(string mailbox) {
-        base (utf8_to_imap_utf7(mailbox));
-    }
-    
-    public MailboxParameter.from_string_parameter(StringParameter string_parameter) {
-        base (string_parameter.ascii);
-    }
-    
-    private static string utf8_to_imap_utf7(string utf8) {
-        try {
-            return Geary.ImapUtf7.utf8_to_imap_utf7(utf8);
-        } catch (ConvertError e) {
-            debug("Error encoding mailbox name '%s': %s", utf8, e.message);
-            return utf8;
-        }
-    }
-    
-    private static string imap_utf7_to_utf8(string imap_utf7) {
-        try {
-            return Geary.ImapUtf7.imap_utf7_to_utf8(imap_utf7);
-        } catch (ConvertError e) {
-            debug("Invalid mailbox name '%s': %s", imap_utf7, e.message);
-            return imap_utf7;
-        }
-    }
-    
-    public string decode() {
-        return imap_utf7_to_utf8(ascii);
-    }
-    
-    /**
-     * {@inheritDoc}
-     */
-    public override void serialize(Serializer ser, Tag tag) throws Error {
-        serialize_string(ser);
-    }
-    
-    /**
-     * {@inheritDoc}
-     */
-    public override string to_string() {
-        return ascii;
-    }
-}
-

src/engine/imap/message/imap-mailbox-specifier.vala

@@ -48,15 +48,38 @@ public class Geary.Imap.MailboxSpecifier : BaseObject, Gee.Hashable<MailboxSpeci
      * See [[http://tools.ietf.org/html/rfc3501#section-5.1]]
      */
     public bool is_inbox { get; private set; }
-    
+
+    /**
+     * Constructs a new specifier from a UTF-8 name.
+     */
     public MailboxSpecifier(string name) {
         init(name);
     }
-    
+
-    public MailboxSpecifier.from_parameter(MailboxParameter param) {
+    /**
-        init(param.decode());
+     * Constructs a new specifier from a IMAP modified-UTF-7 string.
+     *
+     * If a modified-UTF-7 decoding error occurs, the parameter will
+     * assumed to be UTF-8, repaired, and used instead.
+     */
+    public MailboxSpecifier.from_parameter(StringParameter param) {
+        string? name = null;
+        try {
+            name = Geary.ImapUtf7.imap_utf7_to_utf8(param.ascii);
+        } catch (ConvertError err) {
+            // Could no decode the name as IMAP modified UTF7, so per
+            // https://imapwiki.org/ClientImplementation/MailboxList
+            // assume UTF8 and repair to make sure it's valid at
+            // least.
+            debug(
+                "Error decoding mailbox name, assuming UTF-8: %s", err.message
+            );
+            name = param.ascii.make_valid();
+        }
+
+        init(name);
     }
-    
+
     /**
      * Returns true if the {@link Geary.FolderPath} points to the IMAP Inbox.
      */
@@ -187,11 +210,18 @@ public class Geary.Imap.MailboxSpecifier : BaseObject, Gee.Hashable<MailboxSpeci
         
         return !String.is_empty(basename) ? basename : name;
     }
-    
+
     public Parameter to_parameter() {
-        return new MailboxParameter(name);
+        string encoded= Geary.ImapUtf7.utf8_to_imap_utf7(this.name);
+        Parameter? param = null;
+        try {
+            param = StringParameter.get_best_for(encoded);
+        } catch (ImapError err) {
+            param = new LiteralParameter(new Geary.Memory.StringBuffer(encoded));
+        }
+        return param;
     }
-    
+
     public uint hash() {
         return is_inbox ? Ascii.stri_hash(name) : Ascii.str_hash(name);
     }

src/engine/imap/response/imap-mailbox-information.vala

@@ -71,19 +71,21 @@ public class Geary.Imap.MailboxInformation : BaseObject {
         // decode everything
         MailboxAttributes attributes = new MailboxAttributes(attrlist);
         StringParameter? delim = server_data.get_as_nullable_string(3);
-        MailboxParameter mailbox = new MailboxParameter.from_string_parameter(
+        StringParameter mailbox = server_data.get_as_string(4);
-            server_data.get_as_string(4));
+
-        
+        // If special-use flag \Inbox is set just use the canonical
-        // Set \Inbox to standard path
+        // Inbox name, otherwise decode it
-        if (canonical_inbox && Geary.Imap.MailboxAttribute.SPECIAL_FOLDER_INBOX in attributes) {
+        MailboxSpecifier? specifier =
-            return new MailboxInformation(MailboxSpecifier.inbox,
+            (canonical_inbox &&
-                (delim != null) ? delim.nullable_ascii : null, attributes);
+             Geary.Imap.MailboxAttribute.SPECIAL_FOLDER_INBOX in attributes)
-        } else {
+            ? MailboxSpecifier.inbox
-            return new MailboxInformation(new MailboxSpecifier.from_parameter(mailbox),
+            : new MailboxSpecifier.from_parameter(mailbox);
-                (delim != null) ? delim.nullable_ascii : null, attributes);
+
-        }
+        return new MailboxInformation(
+            specifier, (delim != null) ? delim.nullable_ascii : null, attributes
+        );
     }
-    
+
     /**
      * The {@link Geary.FolderPath} for the {@link mailbox}.
      *

src/engine/imap/response/imap-status-data.vala

@@ -74,10 +74,9 @@ public class Geary.Imap.StatusData : Object {
             throw new ImapError.PARSE_ERROR("Bad STATUS command name in response \"%s\"",
                 server_data.to_string());
         }
-        
+
-        MailboxParameter mailbox_param = new MailboxParameter.from_string_parameter(
+        StringParameter mailbox_param = server_data.get_as_string(2);
-            server_data.get_as_string(2));
+
-        
         int messages = UNSET;
         int recent = UNSET;
         UID? uid_next = null;

src/engine/meson.build

@@ -126,7 +126,6 @@ geary_engine_vala_sources = files(
   'imap/message/imap-flags.vala',
   'imap/message/imap-internal-date.vala',
   'imap/message/imap-mailbox-specifier.vala',
-  'imap/message/imap-mailbox-parameter.vala',
   'imap/message/imap-message-data.vala',
   'imap/message/imap-message-flag.vala',
   'imap/message/imap-message-flags.vala',

src/engine/util/util-imap-utf7.vala

@@ -94,7 +94,7 @@ private int first_encode_index(string str) {
     return -1;
 }
 
-public string utf8_to_imap_utf7(string str) throws ConvertError {
+public string utf8_to_imap_utf7(string str) {
     int p = first_encode_index(str);
     if (p < 0) {
         /* no characters that need to be encoded */

test/engine/imap/message/imap-mailbox-specifier-test.vala

@@ -0,0 +1,60 @@
+/*
+ * Copyright 2018 Michael Gratton <mike@vee.net>
+ *
+ * This software is licensed under the GNU Lesser General Public License
+ * (version 2.1 or later). See the COPYING file in this distribution.
+ */
+
+class Geary.Imap.MailboxSpecifierTest : TestCase {
+
+
+    public MailboxSpecifierTest() {
+        base("Geary.Imap.MailboxSpecifierTest");
+        add_test("to_parameter", to_parameter);
+        add_test("from_parameter", from_parameter);
+    }
+
+    public void to_parameter() throws Error {
+        assert_string(
+            "test",
+            new MailboxSpecifier("test").to_parameter().to_string()
+        );
+        assert_string(
+            "foo/bar",
+            new MailboxSpecifier("foo/bar").to_parameter().to_string()
+        );
+
+        // The param won't be quoted or escaped since
+        // QuotedStringParameter doesn't actually handle that, so just
+        // check that it is correct type
+        Parameter quoted = new MailboxSpecifier("""foo\bar""").to_parameter();
+        assert_true(quoted is QuotedStringParameter, "Backslash was not quoted");
+
+        assert_string(
+            "ol&AOk-",
+            new MailboxSpecifier("olé").to_parameter().to_string()
+        );
+    }
+
+    public void from_parameter() throws Error {
+        assert_string(
+            "test",
+            new MailboxSpecifier.from_parameter(
+                new UnquotedStringParameter("test")).name
+        );
+
+        // This won't be quoted or escaped since QuotedStringParameter
+        // doesn't actually handle that.
+        assert_string(
+            "foo\\bar",
+            new MailboxSpecifier.from_parameter(
+                new QuotedStringParameter("""foo\bar""")).name
+        );
+        assert_string(
+            "olé",
+            new MailboxSpecifier.from_parameter(
+                new UnquotedStringParameter("ol&AOk-")).name
+        );
+    }
+
+}

test/meson.build

@@ -31,6 +31,7 @@ geary_test_engine_sources = [
   'engine/db/db-versioned-database-test.vala',
   'engine/imap/command/imap-create-command-test.vala',
   'engine/imap/message/imap-data-format-test.vala',
+  'engine/imap/message/imap-mailbox-specifier-test.vala',
   'engine/imap/response/imap-namespace-response-test.vala',
   'engine/imap/transport/imap-deserializer-test.vala',
   'engine/imap-db/imap-db-attachment-test.vala',

test/test-engine.vala

@@ -40,6 +40,7 @@ int main(string[] args) {
     engine.add_suite(new Geary.Imap.DataFormatTest().get_suite());
     engine.add_suite(new Geary.Imap.CreateCommandTest().get_suite());
     engine.add_suite(new Geary.Imap.DeserializerTest().get_suite());
+    engine.add_suite(new Geary.Imap.MailboxSpecifierTest().get_suite());
     engine.add_suite(new Geary.Imap.NamespaceResponseTest().get_suite());
     engine.add_suite(new Geary.ImapDB.AttachmentTest().get_suite());
     engine.add_suite(new Geary.ImapDB.AttachmentIoTest().get_suite());