From 31f10e2787ea08e2544606cd8f3a0b39c154b6f3 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Tue, 18 Aug 2020 11:40:14 +1000 Subject: [PATCH] Geary.Imap: Add quirk for Envelope address structure placeholders Some servers (e.g. Dovecot) use placeholder strings instead of the empty string (e.g. "MISSING_DOMAIN") in FETCH Envelope responses when a required address part (e.g. local part, domain) are empty. This adds a quirk that can be enabled for such servers to check for the placeholders and if found, replace them with empty strings. --- src/engine/imap/api/imap-quirks.vala | 20 +++ .../message/imap-fetch-data-specifier.vala | 4 +- .../imap/parameter/imap-list-parameter.vala | 11 ++ .../response/imap-fetch-data-decoder.vala | 28 +++- .../imap/response/imap-fetched-data.vala | 2 +- .../imap-fetch-data-decoder-test.vala | 135 ++++++++++++++++++ test/meson.build | 1 + test/test-engine.vala | 1 + 8 files changed, 192 insertions(+), 10 deletions(-) create mode 100644 test/engine/imap/response/imap-fetch-data-decoder-test.vala diff --git a/src/engine/imap/api/imap-quirks.vala b/src/engine/imap/api/imap-quirks.vala index 61fe9cd7..b4443a95 100644 --- a/src/engine/imap/api/imap-quirks.vala +++ b/src/engine/imap/api/imap-quirks.vala @@ -38,6 +38,26 @@ public class Geary.Imap.Quirks : BaseObject { */ public uint max_pipeline_batch_size { get; set; default = 0; } + /** + * The value sent by the server for missing envelope mailbox local parts. + * + * IMAP FETCH ENVELOPE structures use NIL for the "mailbox name" + * part (in addition to a NIL "host name" part) as an end-of-list + * marker for RFC822 group syntax. To indicate a missing + * local-part in a non-group mailbox some mail servers use a + * string such as "MISSING_MAILBOX" rather than the empty string. + */ + public string empty_envelope_mailbox_name { get; set; default = ""; } + + /** + * The value sent by the server for missing envelope mailbox domains. + * + * IMAP FETCH ENVELOPE structures use NIL for the "host name" + * argument to indicate RFC822 group syntax. To indicate a missing + * some mail servers use a string such as "MISSING_DOMAIN" rather + * than the empty string. + */ + public string empty_envelope_host_name { get; set; default = ""; } public void update_for_server(ClientSession session) { diff --git a/src/engine/imap/message/imap-fetch-data-specifier.vala b/src/engine/imap/message/imap-fetch-data-specifier.vala index 5d05c674..7f60acc7 100644 --- a/src/engine/imap/message/imap-fetch-data-specifier.vala +++ b/src/engine/imap/message/imap-fetch-data-specifier.vala @@ -142,7 +142,7 @@ public enum Geary.Imap.FetchDataSpecifier { * * @return null if no FetchDataDecoder is associated with this value, or an invalid value. */ - public FetchDataDecoder? get_decoder() { + public FetchDataDecoder? get_decoder(Quirks quirks) { switch (this) { case UID: return new UIDDecoder(); @@ -151,7 +151,7 @@ public enum Geary.Imap.FetchDataSpecifier { return new MessageFlagsDecoder(); case ENVELOPE: - return new EnvelopeDecoder(); + return new EnvelopeDecoder(quirks); case INTERNALDATE: return new InternalDateDecoder(); diff --git a/src/engine/imap/parameter/imap-list-parameter.vala b/src/engine/imap/parameter/imap-list-parameter.vala index d580830a..777d42fc 100644 --- a/src/engine/imap/parameter/imap-list-parameter.vala +++ b/src/engine/imap/parameter/imap-list-parameter.vala @@ -29,6 +29,17 @@ public class Geary.Imap.ListParameter : Geary.Imap.Parameter { private Gee.List list = new Gee.ArrayList(); + /** Constructs a new, empty list. */ + public ListParameter() { + // noop + } + + /** Constructs a new list wit a single parameter. */ + public ListParameter.single(Parameter param) { + base(); + add(param); + } + /** * Appends a parameter to the end of this list. * diff --git a/src/engine/imap/response/imap-fetch-data-decoder.vala b/src/engine/imap/response/imap-fetch-data-decoder.vala index 1c35d99a..4701a1da 100644 --- a/src/engine/imap/response/imap-fetch-data-decoder.vala +++ b/src/engine/imap/response/imap-fetch-data-decoder.vala @@ -124,8 +124,14 @@ public class Geary.Imap.RFC822SizeDecoder : Geary.Imap.FetchDataDecoder { } public class Geary.Imap.EnvelopeDecoder : Geary.Imap.FetchDataDecoder { - public EnvelopeDecoder() { - base (FetchDataSpecifier.ENVELOPE); + + + private Quirks quirks; + + + public EnvelopeDecoder(Quirks quirks) { + base(FetchDataSpecifier.ENVELOPE); + this.quirks = quirks; } protected override MessageData decode_list(ListParameter listp) throws ImapError { @@ -150,7 +156,7 @@ public class Geary.Imap.EnvelopeDecoder : Geary.Imap.FetchDataDecoder { try { sent_date = new RFC822.Date.from_rfc822_string(sent.ascii); } catch (GLib.Error err) { - debug( + warning( "Error parsing sent date from FETCH envelope: %s", err.message ); @@ -179,14 +185,22 @@ public class Geary.Imap.EnvelopeDecoder : Geary.Imap.FetchDataDecoder { ListParameter fields = listp.get_as_empty_list(ctr); StringParameter? name = fields.get_as_nullable_string(0); StringParameter? source_route = fields.get_as_nullable_string(1); - StringParameter mailbox = fields.get_as_empty_string(2); - StringParameter domain = fields.get_as_empty_string(3); + StringParameter? mailbox = fields.get_as_empty_string(2); + StringParameter? domain = fields.get_as_empty_string(3); + + if (mailbox.ascii == this.quirks.empty_envelope_mailbox_name) { + mailbox = null; + } + if (domain.ascii == this.quirks.empty_envelope_host_name) { + domain = null; + } Geary.RFC822.MailboxAddress addr = new Geary.RFC822.MailboxAddress.imap( (name != null) ? name.nullable_ascii : null, (source_route != null) ? source_route.nullable_ascii : null, - mailbox.ascii, - domain.ascii); + mailbox != null ? mailbox.ascii : "", + domain != null ? domain.ascii : "" + ); list.add(addr); } diff --git a/src/engine/imap/response/imap-fetched-data.vala b/src/engine/imap/response/imap-fetched-data.vala index 7bc47d57..7f377396 100644 --- a/src/engine/imap/response/imap-fetched-data.vala +++ b/src/engine/imap/response/imap-fetched-data.vala @@ -73,7 +73,7 @@ public class Geary.Imap.FetchedData : Object { fetched_data.body_data_map.set(specifier, Memory.EmptyBuffer.instance); } else { FetchDataSpecifier data_item = FetchDataSpecifier.from_parameter(data_item_param); - FetchDataDecoder? decoder = data_item.get_decoder(); + FetchDataDecoder? decoder = data_item.get_decoder(server_data.quirks); if (decoder == null) { debug("Unable to decode fetch response for \"%s\": No decoder available", data_item.to_string()); diff --git a/test/engine/imap/response/imap-fetch-data-decoder-test.vala b/test/engine/imap/response/imap-fetch-data-decoder-test.vala new file mode 100644 index 00000000..c80c91ee --- /dev/null +++ b/test/engine/imap/response/imap-fetch-data-decoder-test.vala @@ -0,0 +1,135 @@ +/* + * Copyright 2019 Michael Gratton + * + * 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.FetchDataDecoderTest : TestCase { + + + public FetchDataDecoderTest() { + base("Geary.Imap.FetchDataDecoderTest"); + add_test("envelope_basic", envelope_basic); + add_test( + "envelope_mailbox_missing_mailbox_name_quirk", + envelope_mailbox_missing_mailbox_name_quirk + ); + add_test( + "envelope_mailbox_missing_host_name_quirk", + envelope_mailbox_missing_host_name_quirk + ); + } + + public void envelope_basic() throws GLib.Error { + ListParameter env = new ListParameter(); + env.add(new QuotedStringParameter("Wed, 17 Jul 1996 02:23:25 -0700 (PDT)")); + env.add(new QuotedStringParameter("Test subject")); + + // From + env.add(new ListParameter.single(new_mailbox_structure("From", "from", "example.com"))); + + // Sender + env.add(new ListParameter.single(new_mailbox_structure("From", "from", "example.com"))); + + // Reply-To + env.add(new ListParameter.single(new_mailbox_structure("From", "from", "example.com"))); + + env.add(new ListParameter.single(new_mailbox_structure("To", "to", "example.com"))); + env.add(new ListParameter.single(new_mailbox_structure("Cc", "cc", "example.com"))); + env.add(new ListParameter.single(new_mailbox_structure("Bcc", "bcc", "example.com"))); + + // In-Reply-To + env.add(new QuotedStringParameter("<1234@example.com>")); + + // Message-Id + env.add(new QuotedStringParameter("<5678@example.com>")); + + var test_article = new EnvelopeDecoder(new Quirks()); + var decoded_generic = test_article.decode(env); + var decoded = decoded_generic as Envelope; + + assert_non_null(decoded, "decoded type"); + assert_non_null(decoded.sent, "decoded sent"); + assert_equal(decoded.subject.value, "Test subject"); + assert_equal(decoded.from.to_rfc822_string(), "From "); + assert_equal(decoded.sender.to_rfc822_string(), "From "); + assert_equal(decoded.reply_to.to_rfc822_string(), "From "); + + assert_non_null(decoded.to, "to"); + assert_equal(decoded.to.to_rfc822_string(), "To "); + + assert_non_null(decoded.cc, "cc"); + assert_equal(decoded.cc.to_rfc822_string(), "Cc "); + + assert_non_null(decoded.bcc, "bcc"); + assert_equal(decoded.bcc.to_rfc822_string(), "Bcc "); + + assert_non_null(decoded.in_reply_to, "in_reply_to"); + assert_equal(decoded.in_reply_to.to_rfc822_string(), "<1234@example.com>"); + + assert_non_null(decoded.message_id, "message_id"); + assert_equal(decoded.message_id.to_rfc822_string(), "<5678@example.com>"); + } + + public void envelope_mailbox_missing_mailbox_name_quirk() throws GLib.Error { + ListParameter env = new ListParameter(); + env.add(new QuotedStringParameter("Wed, 17 Jul 1996 02:23:25 -0700 (PDT)")); + env.add(new QuotedStringParameter("Test subject")); + env.add(new ListParameter.single(new_mailbox_structure("From", "from", "example.com"))); + env.add(new ListParameter.single(new_mailbox_structure("From", "from", "example.com"))); + env.add(new ListParameter.single(new_mailbox_structure("From", "from", "example.com"))); + + env.add(new ListParameter.single(new_mailbox_structure("To", "BOGUS", "example.com"))); + env.add(new ListParameter.single(new_mailbox_structure("Cc", "cc", "example.com"))); + env.add(NilParameter.instance); + env.add(NilParameter.instance); + env.add(NilParameter.instance); + + var quirks = new Quirks(); + quirks.empty_envelope_mailbox_name = "BOGUS"; + + var test_article = new EnvelopeDecoder(quirks); + var decoded = test_article.decode(env) as Envelope; + + assert_non_null(decoded.to, "to"); + assert_equal(decoded.to.to_rfc822_string(), "To <@example.com>"); + assert_non_null(decoded.cc, "cc"); + assert_equal(decoded.cc.to_rfc822_string(), "Cc "); + } + + public void envelope_mailbox_missing_host_name_quirk() throws GLib.Error { + ListParameter env = new ListParameter(); + env.add(new QuotedStringParameter("Wed, 17 Jul 1996 02:23:25 -0700 (PDT)")); + env.add(new QuotedStringParameter("Test subject")); + env.add(new ListParameter.single(new_mailbox_structure("From", "from", "example.com"))); + env.add(new ListParameter.single(new_mailbox_structure("From", "from", "example.com"))); + env.add(new ListParameter.single(new_mailbox_structure("From", "from", "example.com"))); + + env.add(new ListParameter.single(new_mailbox_structure("To name", "to", "BOGUS"))); + env.add(new ListParameter.single(new_mailbox_structure("Cc", "cc", "example.com"))); + env.add(NilParameter.instance); + env.add(NilParameter.instance); + env.add(NilParameter.instance); + + var quirks = new Quirks(); + quirks.empty_envelope_host_name = "BOGUS"; + + var test_article = new EnvelopeDecoder(quirks); + var decoded = test_article.decode(env) as Envelope; + + assert_non_null(decoded.to, "to"); + assert_equal(decoded.to.to_rfc822_string(), "To name "); + assert_non_null(decoded.cc, "cc"); + assert_equal(decoded.cc.to_rfc822_string(), "Cc "); + } + + private ListParameter new_mailbox_structure(string name, string local, string domain) { + ListParameter mailbox = new ListParameter(); + mailbox.add(new QuotedStringParameter(name)); + mailbox.add(NilParameter.instance); + mailbox.add(new QuotedStringParameter(local)); + mailbox.add(new QuotedStringParameter(domain)); + return mailbox; + } +} diff --git a/test/meson.build b/test/meson.build index a2fc8a62..6ea5e27a 100644 --- a/test/meson.build +++ b/test/meson.build @@ -43,6 +43,7 @@ test_engine_sources = [ 'engine/imap/message/imap-data-format-test.vala', 'engine/imap/message/imap-mailbox-specifier-test.vala', 'engine/imap/parameter/imap-list-parameter-test.vala', + 'engine/imap/response/imap-fetch-data-decoder-test.vala', 'engine/imap/response/imap-namespace-response-test.vala', 'engine/imap/transport/imap-client-connection-test.vala', 'engine/imap/transport/imap-client-session-test.vala', diff --git a/test/test-engine.vala b/test/test-engine.vala index 483163de..0260ef6a 100644 --- a/test/test-engine.vala +++ b/test/test-engine.vala @@ -56,6 +56,7 @@ int main(string[] args) { engine.add_suite(new Geary.Imap.CreateCommandTest().suite); engine.add_suite(new Geary.Imap.FetchCommandTest().suite); + engine.add_suite(new Geary.Imap.FetchDataDecoderTest().suite); engine.add_suite(new Geary.Imap.ListParameterTest().suite); engine.add_suite(new Geary.Imap.MailboxSpecifierTest().suite); engine.add_suite(new Geary.Imap.NamespaceResponseTest().suite);