Merge branch 'wip/543-append-failure' into 'mainline'

Fix IMAP APPEND command timeout on large email upload

Closes #543

See merge request GNOME/geary!296

src/engine/imap/command/imap-authenticate-command.vala

@@ -64,7 +64,9 @@ public class Geary.Imap.AuthenticateCommand : Command {
         // Wait to either get a response or a continuation request
         yield this.error_lock.wait_async(cancellable);
         if (this.response_literal != null) {
-            yield this.response_literal.serialize_data(ser, cancellable);
+            yield ser.push_literal_data(
+                this.response_literal.value.get_uint8_array(), cancellable
+            );
             ser.push_eol(cancellable);
             yield ser.flush_stream(cancellable);
         }

src/engine/imap/command/imap-command.vala

@@ -178,7 +178,45 @@ public class Geary.Imap.Command : BaseObject {
                     // Will get notified via continuation_requested
                     // when server indicated the literal can be sent.
                     yield this.literal_spinlock.wait_async(cancellable);
-                    yield literal.serialize_data(ser, cancellable);
+
+                    // Buffer size is dependent on timeout, since we
+                    // need to ensure we can send a full buffer before
+                    // the timeout is up. v.92 56k baud modems have
+                    // theoretical max upload of 48kbit/s and GSM 2G
+                    // 40kbit/s, but typical is usually well below
+                    // that, so assume a low end of 1kbyte/s. Hence
+                    // buffer size needs to be less than or equal to
+                    // (response_timeout * 1)k, rounded down to the
+                    // nearest power of two.
+                    uint buf_size = 1;
+                    while (buf_size <= this.response_timeout) {
+                        buf_size <<= 1;
+                    }
+                    buf_size >>= 1;
+
+                    uint8[] buf = new uint8[buf_size * 1024];
+                    GLib.InputStream data = literal.value.get_input_stream();
+                    try {
+                        while (true) {
+                            size_t read;
+                            yield data.read_all_async(
+                                buf, Priority.DEFAULT, cancellable, out read
+                            );
+                            if (read <= 0) {
+                                break;
+                            }
+
+                            buf.length = (int) read;
+                            yield ser.push_literal_data(buf, cancellable);
+                            this.response_timer.start();
+                        }
+                    } finally {
+                        try {
+                            yield data.close_async();
+                        } catch (GLib.Error err) {
+                            // Oh well
+                        }
+                    }
                 }
             }
         }

src/engine/imap/parameter/imap-list-parameter.vala

@@ -217,7 +217,7 @@ public class Geary.Imap.ListParameter : Geary.Imap.Parameter {
             return stringp;
 
         LiteralParameter? literalp = param as LiteralParameter;
-        if (literalp != null && literalp.get_size() <= MAX_STRING_LITERAL_LENGTH)
+        if (literalp != null && literalp.value.size <= MAX_STRING_LITERAL_LENGTH)
             return literalp.coerce_to_string_parameter();
 
         throw new ImapError.TYPE_ERROR("Parameter %d not of type string or literal (is %s)", index,
@@ -246,7 +246,7 @@ public class Geary.Imap.ListParameter : Geary.Imap.Parameter {
             return stringp;
 
         LiteralParameter? literalp = param as LiteralParameter;
-        if (literalp != null && literalp.get_size() <= MAX_STRING_LITERAL_LENGTH)
+        if (literalp != null && literalp.value.size <= MAX_STRING_LITERAL_LENGTH)
             return literalp.coerce_to_string_parameter();
 
         throw new ImapError.TYPE_ERROR("Parameter %d not of type string or literal (is %s)", index,
@@ -390,7 +390,7 @@ public class Geary.Imap.ListParameter : Geary.Imap.Parameter {
     public Memory.Buffer? get_as_nullable_buffer(int index) throws ImapError {
         LiteralParameter? literalp = get_if_literal(index);
         if (literalp != null)
-            return literalp.get_buffer();
+            return literalp.value;
 
         StringParameter? stringp = get_if_string(index);
         if (stringp != null)

src/engine/imap/parameter/imap-literal-parameter.vala

@@ -1,38 +1,30 @@
-/* Copyright 2016 Software Freedom Conservancy Inc.
+/*
+ * Copyright 2016 Software Freedom Conservancy Inc.
+ * Copyright 2019 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.
+ * (version 2.1 or later). See the COPYING file in this distribution.
  */
 
 /**
  * A representation of an IMAP literal parameter.
  *
- * Because a literal parameter can hold 8-bit data, this is not a descendent of
+ * Because a literal parameter can hold 8-bit data, this is not a
- * {@link StringParameter}, although some times literal data is used to store 8-bit text (for
+ * descendent of {@link StringParameter}, although some times literal
- * example, UTF-8).
+ * data is used to store 8-bit text (for example, UTF-8).
  *
  * See [[http://tools.ietf.org/html/rfc3501#section-4.3]]
  */
 
 public class Geary.Imap.LiteralParameter : Geary.Imap.Parameter {
-    private Memory.Buffer buffer;
 
-    public LiteralParameter(Memory.Buffer buffer) {
-        this.buffer = buffer;
-    }
 
-    /**
+    /** The value of the literal parameter. */
-     * Returns the number of bytes in the literal parameter's buffer.
+    public Memory.Buffer value { get; private set; }
-     */
-    public size_t get_size() {
-        return buffer.size;
-    }
 
-    /**
+
-     * Returns the literal paremeter's buffer.
+    public LiteralParameter(Memory.Buffer value) {
-     */
+        this.value = value;
-    public Memory.Buffer get_buffer() {
-        return buffer;
     }
 
     /**
@@ -45,14 +37,14 @@ public class Geary.Imap.LiteralParameter : Geary.Imap.Parameter {
      * for transmitting on the wire.
      */
     public StringParameter coerce_to_string_parameter() {
-        return new UnquotedStringParameter(buffer.get_valid_utf8());
+        return new UnquotedStringParameter(this.value.get_valid_utf8());
     }
 
     /**
      * {@inheritDoc}
      */
     public override string to_string() {
-        return "{literal/%lub}".printf(get_size());
+        return "{literal/%lub}".printf(this.value.size);
     }
 
     /**
@@ -60,17 +52,8 @@ public class Geary.Imap.LiteralParameter : Geary.Imap.Parameter {
      */
     public override void serialize(Serializer ser, GLib.Cancellable cancellable)
         throws GLib.Error {
-        ser.push_unquoted_string("{%lu}".printf(get_size()), cancellable);
+        ser.push_unquoted_string("{%lu}".printf(this.value.size), cancellable);
         ser.push_eol(cancellable);
     }
 
-    /**
-     * Serialises the literal parameter data.
-     */
-    public async void serialize_data(Serializer ser,
-                                     GLib.Cancellable cancellable)
-        throws GLib.Error {
-        yield ser.push_literal_data(buffer, cancellable);
-    }
-
 }

src/engine/imap/response/imap-fetch-data-decoder.vala

@@ -42,7 +42,7 @@ public abstract class Geary.Imap.FetchDataDecoder : BaseObject {
             // because this method is called without the help of get_as_string() (which converts
             // reasonably-length literals into StringParameters), do so here manually
             try {
-                if (literalp.get_size() <= ListParameter.MAX_STRING_LITERAL_LENGTH)
+                if (literalp.value.size <= ListParameter.MAX_STRING_LITERAL_LENGTH)
                     return decode_string(literalp.coerce_to_string_parameter());
             } catch (ImapError imap_err) {
                 // if decode_string() throws a TYPE_ERROR, retry as a LiteralParameter, otherwise
@@ -197,7 +197,7 @@ public class Geary.Imap.RFC822HeaderDecoder : Geary.Imap.FetchDataDecoder {
     }
 
     protected override MessageData decode_literal(LiteralParameter literalp) throws ImapError {
-        return new Geary.Imap.RFC822Header(literalp.get_buffer());
+        return new Geary.Imap.RFC822Header(literalp.value);
     }
 }
 
@@ -207,7 +207,7 @@ public class Geary.Imap.RFC822TextDecoder : Geary.Imap.FetchDataDecoder {
     }
 
     protected override MessageData decode_literal(LiteralParameter literalp) throws ImapError {
-        return new Geary.Imap.RFC822Text(literalp.get_buffer());
+        return new Geary.Imap.RFC822Text(literalp.value);
     }
 
     protected override MessageData decode_nil(NilParameter nilp) throws ImapError {
@@ -221,7 +221,6 @@ public class Geary.Imap.RFC822FullDecoder : Geary.Imap.FetchDataDecoder {
     }
 
     protected override MessageData decode_literal(LiteralParameter literalp) throws ImapError {
-        return new Geary.Imap.RFC822Full(literalp.get_buffer());
+        return new Geary.Imap.RFC822Full(literalp.value);
     }
 }
-

src/engine/imap/transport/imap-serializer.vala

@@ -103,14 +103,14 @@ public class Geary.Imap.Serializer : BaseObject {
     /**
      * Writes literal data to the output stream.
      */
-    public async void push_literal_data(Memory.Buffer buffer,
+    public async void push_literal_data(uint8[] buffer,
                                         GLib.Cancellable? cancellable = null)
         throws GLib.Error {
-        yield this.output.splice_async(
+        yield this.output.write_all_async(
-            buffer.get_input_stream(),
+            buffer,
-            OutputStreamSpliceFlags.NONE,
             Priority.DEFAULT,
-            cancellable
+            cancellable,
+            null
         );
     }