From c54ff85ccdcc02b71a0be5e43299d41920dbdb29 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Wed, 6 May 2020 14:44:44 +1000 Subject: [PATCH] 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 e0e37687..b7c42a86 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 responses = yield exec_commands_async( diff --git a/src/engine/outbox/outbox-folder.vala b/src/engine/outbox/outbox-folder.vala index 6b5217b1..8a083556 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 fae87da2..1c780e06 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 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 (stream_filter.flush() != 0) - throw new Error.FAILED("Unable to flush RFC822 message to memory stream"); + if (message.write_to_stream(format, stream_filter) < 0) { + throw new Error.FAILED( + "Unable to write RFC822 message to filter stream" + ); + } - if (stream.flush() != 0) - throw new Error.FAILED("Unable to flush RFC822 message to memory buffer"); + 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" + ); + } 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 bc0e9310..5851aff7 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; + if (response.code.is_start_data()) { + debug("SMTP Data: <%z>", data.size); - 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 { // 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( + this.douts, DataFormat.DATA_TERMINATOR, cancellable + ); + yield this.douts.flush_async(Priority.DEFAULT, cancellable); + + response = yield recv_response_async(cancellable); } - - // terminate buffer and flush to server - yield Stream.write_string_async(douts, DataFormat.DATA_TERMINATOR, cancellable); - yield douts.flush_async(Priority.DEFAULT, cancellable); - - return 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 9e60a624..33069afd 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 cb4bde34..478d5633 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 "), "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 \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); }