[geary/mjog/rfc822-cleanup: 21/21] Geary.RFC822.Message: Update API for SMTP formatting



commit bee1ec4c897ad543b3e38352a99cd2c9800dcd01
Author: Michael Gratton <mike vee net>
Date:   Wed May 6 14:44:44 2020 +1000

    Geary.RFC822.Message: Update API for SMTP formatting
    
    Remove `Message.without_bcc` ctor since we can now get GMime to exclude
    BCC headers when serialising, avoiding the overhead of taking a complete
    copy of the message just to strip BCCs.
    
    Rename `get_network_buffer` to `get_rfc822_buffer` to be a bit more
    explicit in that's the way to get an actual RFC822 formatted message.
    Replace book args with flags, and take a protocol-specific approach
    rather than feature-specific, because in the end you're either going to
    want all the SMTP formatting quirks or none of them.
    
    Remove custom dot-stuffing support from Smtp.ClientConnection, since it
    is redundant.

 src/engine/imap/api/imap-folder-session.vala |  2 +-
 src/engine/outbox/outbox-folder.vala         |  2 +-
 src/engine/rfc822/rfc822-message.vala        | 90 ++++++++++++++++++----------
 src/engine/smtp/smtp-client-connection.vala  | 49 +++++----------
 src/engine/smtp/smtp-client-session.vala     |  6 +-
 test/engine/rfc822/rfc822-message-test.vala  | 50 +++++++++++++---
 6 files changed, 118 insertions(+), 81 deletions(-)
---
diff --git a/src/engine/imap/api/imap-folder-session.vala b/src/engine/imap/api/imap-folder-session.vala
index e0e376870..b7c42a862 100644
--- a/src/engine/imap/api/imap-folder-session.vala
+++ b/src/engine/imap/api/imap-folder-session.vala
@@ -1069,7 +1069,7 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
 
         MailboxSpecifier mailbox = session.get_mailbox_for_path(this.folder.path);
         AppendCommand cmd = new AppendCommand(
-            mailbox, msg_flags, internaldate, message.get_network_buffer(false)
+            mailbox, msg_flags, internaldate, message.get_rfc822_buffer()
         );
 
         Gee.Map<Command, StatusResponse> responses = yield exec_commands_async(
diff --git a/src/engine/outbox/outbox-folder.vala b/src/engine/outbox/outbox-folder.vala
index 6b5217b18..8a0835562 100644
--- a/src/engine/outbox/outbox-folder.vala
+++ b/src/engine/outbox/outbox-folder.vala
@@ -122,7 +122,7 @@ public class Geary.Outbox.Folder :
             // save in database ready for SMTP, but without dot-stuffing
             Db.Statement stmt = cx.prepare(
                 "INSERT INTO SmtpOutboxTable (message, ordering) VALUES (?, ?)");
-            stmt.bind_string_buffer(0, rfc822.get_network_buffer(false));
+            stmt.bind_string_buffer(0, rfc822.get_rfc822_buffer());
             stmt.bind_int64(1, ordering);
 
             int64 new_id = stmt.exec_insert(cancellable);
diff --git a/src/engine/rfc822/rfc822-message.vala b/src/engine/rfc822/rfc822-message.vala
index fae87da24..1c780e062 100644
--- a/src/engine/rfc822/rfc822-message.vala
+++ b/src/engine/rfc822/rfc822-message.vala
@@ -16,6 +16,7 @@
  */
 public class Geary.RFC822.Message : BaseObject, EmailHeaderSet {
 
+
     /**
      * Callback for including non-text MIME entities in message bodies.
      *
@@ -30,11 +31,36 @@ public class Geary.RFC822.Message : BaseObject, EmailHeaderSet {
      */
     public delegate string? InlinePartReplacer(Part part);
 
+
     private const string HEADER_IN_REPLY_TO = "In-Reply-To";
     private const string HEADER_REFERENCES = "References";
     private const string HEADER_MAILER = "X-Mailer";
     private const string HEADER_BCC = "Bcc";
 
+    /** Options to use when serialising a message in RFC 822 format. */
+    [Flags]
+    public enum RFC822FormatOptions {
+
+        /** Format for RFC 822 in general. */
+        NONE,
+
+        /**
+         * The message should be serialised for transmission via SMTP.
+         *
+         * SMTP imposes both operational and data-format requirements
+         * on RFC 822 style messages. In particular, BCC headers
+         * should not be included since they will expose BCC
+         * recipients, and lines must be dot-stuffed so as to avoid
+         * terminating the message early if a line starting with a `.`
+         * is encountered.
+         *
+         * See [[http://tools.ietf.org/html/rfc5321#section-4.5.2]]
+         */
+        SMTP_FORMAT;
+
+    }
+
+
     // Internal note: If a header field is added here, it *must* be
     // set in Message.from_gmime_message(), below.
 
@@ -440,22 +466,6 @@ public class Geary.RFC822.Message : BaseObject, EmailHeaderSet {
         this.message.set_mime_part(main_part);
     }
 
-    // Makes a copy of the given message without the BCC fields. This is used for sending the email
-    // without sending the BCC headers to all recipients.
-    public Message.without_bcc(Message email) throws GLib.Error {
-        // GMime doesn't make it easy to get a copy of the body of a message.  It's easy to
-        // make a new message and add in all the headers, but calling set_mime_part() with
-        // the existing one's get_mime_part() result yields a double Content-Type header in
-        // the *original* message.  Clearly the objects aren't meant to be used like that.
-        // Barring any better way to clone a message, which I couldn't find by looking at
-        // the docs, we just dump out the old message to a buffer and read it back in to
-        // create the new object.  Kinda sucks, but our hands are tied.
-        this.from_buffer(email.message_to_memory_buffer(false, false));
-
-        this.message.remove_header(HEADER_BCC);
-        this.bcc = null;
-    }
-
     private GMime.Object? coalesce_related(Gee.List<GMime.Object> parts,
                                            string type) {
         GMime.Object? part = coalesce_parts(parts, "related");
@@ -649,22 +659,21 @@ public class Geary.RFC822.Message : BaseObject, EmailHeaderSet {
     }
 
     /**
-     * Returns the {@link Message} as a {@link Memory.Buffer} suitable for in-memory use (i.e.
-     * with native linefeed characters).
+     * Serialises the message using native (i.e. LF) line endings.
      */
     public Memory.Buffer get_native_buffer() throws Error {
-        return message_to_memory_buffer(false, false);
+        return message_to_memory_buffer(false, NONE);
     }
 
     /**
-     * Returns the {@link Message} as a {@link Memory.Buffer} suitable for transmission or
-     * storage (i.e. using protocol-specific linefeeds).
+     * Serialises the message using RFC 822 (i.e. CRLF) line endings.
      *
-     * The buffer can also be dot-stuffed if required.  See
-     * [[http://tools.ietf.org/html/rfc2821#section-4.5.2]]
+     * Returns the message as a memory buffer suitable for network
+     * transmission and interoperability with other RFC 822 consumers.
      */
-    public Memory.Buffer get_network_buffer(bool dotstuffed) throws Error {
-        return message_to_memory_buffer(true, dotstuffed);
+    public Memory.Buffer get_rfc822_buffer(RFC822FormatOptions options = NONE)
+        throws Error {
+        return message_to_memory_buffer(true, options);
     }
 
     /**
@@ -1084,7 +1093,7 @@ public class Geary.RFC822.Message : BaseObject, EmailHeaderSet {
     }
 
     private Memory.Buffer message_to_memory_buffer(bool encode_lf,
-                                                   bool stuff_smtp)
+                                                   RFC822FormatOptions options)
         throws Error {
         ByteArray byte_array = new ByteArray();
         GMime.StreamMem stream = new GMime.StreamMem.with_byte_array(byte_array);
@@ -1096,18 +1105,33 @@ public class Geary.RFC822.Message : BaseObject, EmailHeaderSet {
         } else {
             stream_filter.add(new GMime.FilterDos2Unix(false));
         }
-        if (stuff_smtp) {
+        if (RFC822FormatOptions.SMTP_FORMAT in options) {
             stream_filter.add(new GMime.FilterSmtpData());
         }
 
-        if (message.write_to_stream(Geary.RFC822.get_format_options(), stream_filter) < 0)
-            throw new Error.FAILED("Unable to write RFC822 message to filter stream");
+        var format = Geary.RFC822.get_format_options();
+        if (RFC822FormatOptions.SMTP_FORMAT in options) {
+            format = format.clone();
+            format.add_hidden_header("Bcc");
+        }
+
+        if (message.write_to_stream(format, stream_filter) < 0) {
+            throw new Error.FAILED(
+                "Unable to write RFC822 message to filter stream"
+            );
+        }
 
-        if (stream_filter.flush() != 0)
-            throw new Error.FAILED("Unable to flush RFC822 message to memory stream");
+        if (stream_filter.flush() != 0) {
+            throw new Error.FAILED(
+                "Unable to flush RFC822 message to memory stream"
+            );
+        }
 
-        if (stream.flush() != 0)
-            throw new Error.FAILED("Unable to flush RFC822 message to memory buffer");
+        if (stream.flush() != 0) {
+            throw new Error.FAILED(
+                "Unable to flush RFC822 message to memory buffer"
+            );
+        }
 
         return new Memory.ByteBuffer.from_byte_array(byte_array);
     }
diff --git a/src/engine/smtp/smtp-client-connection.vala b/src/engine/smtp/smtp-client-connection.vala
index bc0e93106..5851aff73 100644
--- a/src/engine/smtp/smtp-client-connection.vala
+++ b/src/engine/smtp/smtp-client-connection.vala
@@ -119,47 +119,28 @@ internal class Geary.Smtp.ClientConnection : BaseObject, Logging.Source {
      * Returns the final Response of the transaction.  If the ResponseCode is not a successful
      * completion, the message should not be considered sent.
      */
-    public async Response send_data_async(Memory.Buffer data, bool already_dotstuffed,
-        Cancellable? cancellable = null) throws Error {
+    public async Response send_data_async(Memory.Buffer data,
+                                          GLib.Cancellable? cancellable = null)
+        throws GLib.Error {
         check_connected();
 
         // In the case of DATA, want to receive an intermediate response code, specifically 354
         Response response = yield transaction_async(new Request(Command.DATA), cancellable);
-        if (!response.code.is_start_data())
-            return response;
-
-        debug("SMTP Data: <%z>", data.size);
-
-        if (!already_dotstuffed) {
-            // By using DataStreamNewlineType.ANY, we're assured to get each line and convert to
-            // a proper line terminator for SMTP
-            DataInputStream dins = new DataInputStream(data.get_input_stream());
-            dins.set_newline_type(DataStreamNewlineType.ANY);
-
-            // Read each line and dot-stuff if necessary
-            for (;;) {
-                size_t length;
-                string? line = yield dins.read_line_async(Priority.DEFAULT, cancellable, out length);
-                if (line == null)
-                    break;
-
-                // stuffing
-                if (line[0] == '.')
-                    yield Stream.write_string_async(douts, ".", cancellable);
-
-                yield Stream.write_string_async(douts, line, cancellable);
-                yield Stream.write_string_async(douts, DataFormat.LINE_TERMINATOR, cancellable);
-            }
-        } else {
+        if (response.code.is_start_data()) {
+            debug("SMTP Data: <%z>", data.size);
+
             // ready to go, send and commit
-            yield Stream.write_all_async(douts, data, cancellable);
-        }
+            yield Stream.write_all_async(this.douts, data, cancellable);
 
-        // terminate buffer and flush to server
-        yield Stream.write_string_async(douts, DataFormat.DATA_TERMINATOR, cancellable);
-        yield douts.flush_async(Priority.DEFAULT, cancellable);
+            // terminate buffer and flush to server
+            yield Stream.write_string_async(
+                this.douts, DataFormat.DATA_TERMINATOR, cancellable
+            );
+            yield this.douts.flush_async(Priority.DEFAULT, cancellable);
 
-        return yield recv_response_async(cancellable);
+            response = yield recv_response_async(cancellable);
+        }
+        return response;
     }
 
     public async void send_request_async(Request request, Cancellable? cancellable = null) throws Error {
diff --git a/src/engine/smtp/smtp-client-session.vala b/src/engine/smtp/smtp-client-session.vala
index 9e60a624c..33069afde 100644
--- a/src/engine/smtp/smtp-client-session.vala
+++ b/src/engine/smtp/smtp-client-session.vala
@@ -214,9 +214,9 @@ public class Geary.Smtp.ClientSession : BaseObject, Logging.Source {
         yield send_rcpts_async(addrlist, cancellable);
 
         // DATA
-        Geary.RFC822.Message email_copy = new Geary.RFC822.Message.without_bcc(email);
-        response = yield cx.send_data_async(email_copy.get_network_buffer(true), true,
-            cancellable);
+        response = yield cx.send_data_async(
+            email.get_rfc822_buffer(), cancellable
+        );
         if (!response.code.is_success_completed())
             response.throw_error("Unable to send message");
 
diff --git a/test/engine/rfc822/rfc822-message-test.vala b/test/engine/rfc822/rfc822-message-test.vala
index cb4bde342..478d56339 100644
--- a/test/engine/rfc822/rfc822-message-test.vala
+++ b/test/engine/rfc822/rfc822-message-test.vala
@@ -59,9 +59,10 @@ This is the second line.
         add_test("get_searchable_recipients", get_searchable_recipients);
         add_test("from_composed_email", from_composed_email);
         add_test("from_composed_email_inline_attachments", from_composed_email_inline_attachments);
-        add_test("get_network_buffer", get_network_buffer);
-        add_test("get_network_buffer_dot_stuff", get_network_buffer_dot_stuff);
-        add_test("get_network_buffer_long_ascii_line", get_network_buffer_long_ascii_line);
+        add_test("get_rfc822_buffer", get_rfc822_buffer);
+        add_test("get_rfc822_buffer_dot_stuff", get_rfc822_buffer_dot_stuff);
+        add_test("get_rfc822_buffer_no_bcc", get_rfc822_buffer_no_bcc);
+        add_test("get_rfc822_buffer_long_ascii_line", get_rfc822_buffer_long_ascii_line);
     }
 
     public void basic_message_from_buffer() throws GLib.Error {
@@ -210,13 +211,13 @@ This is the second line.
         assert_true(searchable.contains("Jane Doe BCC <jdoe_bcc somewhere tld>"), "Expected bcc address");
     }
 
-    public void get_network_buffer() throws GLib.Error {
+    public void get_rfc822_buffer() throws GLib.Error {
         Message test = resource_to_message(BASIC_TEXT_PLAIN);
-        Memory.Buffer buffer = test.get_network_buffer(true);
+        Memory.Buffer buffer = test.get_rfc822_buffer(NONE);
         assert_true(buffer.to_string() == NETWORK_BUFFER_EXPECTED, "Network buffer differs");
     }
 
-    public void get_network_buffer_dot_stuff() throws GLib.Error {
+    public void get_rfc822_buffer_dot_stuff() throws GLib.Error {
         RFC822.MailboxAddress to = new RFC822.MailboxAddress(
             "Test", "test example com"
         );
@@ -235,11 +236,42 @@ This is the second line.
         );
         Geary.RFC822.Message message = message_from_composed_email.end(async_result());
 
-        string message_data = message.get_network_buffer(true).to_string();
+        string message_data = message.get_rfc822_buffer(SMTP_FORMAT).to_string();
         assert_true(message_data.has_suffix("..newline\r\n..\r\n"));
     }
 
-    public void get_network_buffer_long_ascii_line() throws GLib.Error {
+    public void get_rfc822_buffer_no_bcc() throws GLib.Error {
+        RFC822.MailboxAddress to = new RFC822.MailboxAddress(
+            "Test", "test example com"
+        );
+        RFC822.MailboxAddress bcc = new RFC822.MailboxAddress(
+            "BCC", "bcc example com"
+        );
+        RFC822.MailboxAddress from = new RFC822.MailboxAddress(
+            "Sender", "sender example com"
+        );
+        Geary.ComposedEmail composed = new Geary.ComposedEmail(
+            new GLib.DateTime.now_local(),
+            new Geary.RFC822.MailboxAddresses.single(from)
+        ).set_to(
+            new Geary.RFC822.MailboxAddresses.single(to)
+        ).set_bcc(
+            new Geary.RFC822.MailboxAddresses.single(bcc)
+        );
+        composed.body_text = "\nbody\n";
+
+        this.message_from_composed_email.begin(
+            composed,
+            this.async_completion
+        );
+        Geary.RFC822.Message message = message_from_composed_email.end(async_result());
+
+        string message_data = message.get_rfc822_buffer(SMTP_FORMAT).to_string();
+        assert_true("To: Test <test example com>\r\n" in message_data);
+        assert_false("bcc" in message_data.down());
+    }
+
+    public void get_rfc822_buffer_long_ascii_line() throws GLib.Error {
         RFC822.MailboxAddress to = new RFC822.MailboxAddress(
             "Test", "test example com"
         );
@@ -265,7 +297,7 @@ This is the second line.
         );
         Geary.RFC822.Message message = message_from_composed_email.end(async_result());
 
-        string message_data = message.get_network_buffer(true).to_string();
+        string message_data = message.get_rfc822_buffer(SMTP_FORMAT).to_string();
         foreach (var line in message_data.split("\n")) {
             assert_true(line.length < 1000, line);
         }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]