[geary/cherry-pick-737bdba1] Merge branch 'mjog/571-imap-fetch-space-workaround-redux' into 'mainline'



commit ede9d92f99b1b08da7a64f2e66f2afbed719a3d4
Author: Michael Gratton <mike vee net>
Date:   Sat Oct 26 05:24:03 2019 +0000

    Merge branch 'mjog/571-imap-fetch-space-workaround-redux' into 'mainline'
    
    IMAP body part fetch space workaround redux
    
    Closes #571
    
    See merge request GNOME/geary!342
    
    (cherry picked from commit 737bdba15866c88153bdc2138ead50568f0fc80b)
    
    8cd310be Extend the IMAP FETCH HEADER.FIELDS hack for multiple fields
    ab77067d Fix RFC822.Header.get_header_names returning null names

 bindings/vapi/gmime-2.6.vapi                 |   2 +-
 src/engine/imap/api/imap-folder-session.vala | 207 +++++++++++++++++----------
 src/engine/rfc822/rfc822-message-data.vala   |  26 ++--
 test/engine/rfc822-message-data-test.vala    |  22 +++
 4 files changed, 168 insertions(+), 89 deletions(-)
---
diff --git a/bindings/vapi/gmime-2.6.vapi b/bindings/vapi/gmime-2.6.vapi
index 2c4ad52a..7665a784 100644
--- a/bindings/vapi/gmime-2.6.vapi
+++ b/bindings/vapi/gmime-2.6.vapi
@@ -505,7 +505,7 @@ namespace GMime {
                [CCode (cname = "g_mime_header_list_get")]
                public unowned string @get (string name);
                [CCode (cname = "g_mime_header_list_get_iter")]
-               public bool get_iter (out unowned GMime.HeaderIter iter);
+               public bool get_iter (GMime.HeaderIter iter);
                [CCode (cname = "g_mime_header_list_get_stream")]
                public unowned GMime.Stream get_stream ();
                [CCode (cname = "g_mime_header_list_prepend")]
diff --git a/src/engine/imap/api/imap-folder-session.vala b/src/engine/imap/api/imap-folder-session.vala
index 2769a76b..828fc1da 100644
--- a/src/engine/imap/api/imap-folder-session.vala
+++ b/src/engine/imap/api/imap-folder-session.vala
@@ -346,10 +346,14 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
         return (search_results.size > 0) ? search_results : null;
     }
 
-    private Gee.Collection<FetchCommand> assemble_list_commands(Imap.MessageSet msg_set,
-        Geary.Email.Field fields, out FetchBodyDataSpecifier? header_specifier,
-        out FetchBodyDataSpecifier? body_specifier, out FetchBodyDataSpecifier? preview_specifier,
-        out FetchBodyDataSpecifier? preview_charset_specifier) {
+    private Gee.Collection<FetchCommand> assemble_list_commands(
+        Imap.MessageSet msg_set,
+        Geary.Email.Field fields,
+        out FetchBodyDataSpecifier[]? header_specifiers,
+        out FetchBodyDataSpecifier? body_specifier,
+        out FetchBodyDataSpecifier? preview_specifier,
+        out FetchBodyDataSpecifier? preview_charset_specifier
+    ) {
         // getting all the fields can require multiple FETCH commands (some servers don't handle
         // well putting every required data item into single command), so aggregate FetchCommands
         Gee.Collection<FetchCommand> cmds = new Gee.ArrayList<FetchCommand>();
@@ -365,19 +369,57 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
         // convert bulk of the "basic" fields into a one or two FETCH commands (some servers have
         // exhibited bugs or return NO when too many FETCH data types are combined on a single
         // command)
+        header_specifiers = null;
         if (fields.requires_any(BASIC_FETCH_FIELDS)) {
-            Gee.List<FetchDataSpecifier> data_types = new Gee.ArrayList<FetchDataSpecifier>();
-            fields_to_fetch_data_types(fields, data_types, out header_specifier);
+            Gee.List<FetchDataSpecifier> basic_types =
+                new Gee.LinkedList<FetchDataSpecifier>();
+            Gee.List<string> header_fields = new Gee.LinkedList<string>();
+
+            fields_to_fetch_data_types(fields, basic_types, header_fields);
 
             // Add all simple data types as one FETCH command
-            if (data_types.size > 0)
-                cmds.add(new FetchCommand(msg_set, data_types, null));
+            if (!basic_types.is_empty) {
+                cmds.add(new FetchCommand(msg_set, basic_types, null));
+            }
 
-            // Add all body data types as separate FETCH command
-            if (header_specifier != null)
-                cmds.add(new FetchCommand.body_data_type(msg_set, header_specifier));
-        } else {
-            header_specifier = null;
+            // Add all header field requests as separate FETCH
+            // command(s). If the HEADER.FIELDS hack is enabled and
+            // there is more than one header that needs fetching, we
+            // need to send multiple commands since we can't separate
+            // them by spaces in the same command.
+            //
+            // See <https://gitlab.gnome.org/GNOME/geary/issues/571>
+            if (!header_fields.is_empty) {
+                if (!this.imap_header_fields_hack || header_fields.size == 1) {
+                    header_specifiers = new FetchBodyDataSpecifier[1];
+                    header_specifiers[0] = new FetchBodyDataSpecifier.peek(
+                        FetchBodyDataSpecifier.SectionPart.HEADER_FIELDS,
+                        null,
+                        -1,
+                        -1,
+                        header_fields.to_array()
+                    );
+                } else {
+                    header_specifiers = new FetchBodyDataSpecifier[header_fields.size];
+                    int i = 0;
+                    foreach (string name in header_fields) {
+                        header_specifiers[i++] = new FetchBodyDataSpecifier.peek(
+                            FetchBodyDataSpecifier.SectionPart.HEADER_FIELDS,
+                            null,
+                            -1,
+                            -1,
+                            new string[] { name }
+                        );
+                    }
+                }
+
+                foreach (FetchBodyDataSpecifier header in header_specifiers) {
+                    if (this.imap_header_fields_hack) {
+                        header.omit_request_header_fields_space();
+                    }
+                    cmds.add(new FetchCommand.body_data_type(msg_set, header));
+                }
+            }
         }
 
         // RFC822 BODY is a separate command
@@ -440,15 +482,20 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
         throws Error {
         Gee.HashMap<SequenceNumber, FetchedData> fetched =
             new Gee.HashMap<SequenceNumber, FetchedData>();
-        FetchBodyDataSpecifier? header_specifier = null;
+        FetchBodyDataSpecifier[]? header_specifiers = null;
         FetchBodyDataSpecifier? body_specifier = null;
         FetchBodyDataSpecifier? preview_specifier = null;
         FetchBodyDataSpecifier? preview_charset_specifier = null;
         bool success = false;
         while (!success) {
-            Gee.Collection<FetchCommand> cmds = assemble_list_commands(msg_set, fields,
-                out header_specifier, out body_specifier, out preview_specifier,
-                out preview_charset_specifier);
+            Gee.Collection<FetchCommand> cmds = assemble_list_commands(
+                msg_set,
+                fields,
+                out header_specifiers,
+                out body_specifier,
+                out preview_specifier,
+                out preview_charset_specifier
+            );
             if (cmds.size == 0) {
                 throw new ImapError.INVALID("No FETCH commands generate for list request %s %s",
                     msg_set.to_string(), fields.to_list_string());
@@ -491,8 +538,16 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
                 }
 
                 try {
-                    Geary.Email email = fetched_data_to_email(to_string(), uid, fetched_data, fields,
-                        header_specifier, body_specifier, preview_specifier, preview_charset_specifier);
+                    Geary.Email email = fetched_data_to_email(
+                        to_string(),
+                        uid,
+                        fetched_data,
+                        fields,
+                        header_specifiers,
+                        body_specifier,
+                        preview_specifier,
+                        preview_charset_specifier
+                    );
                     if (!email.fields.fulfills(fields)) {
                         message("%s: %s missing=%s fetched=%s", to_string(), email.id.to_string(),
                             fields.clear(email.fields).to_list_string(), fetched_data.to_string());
@@ -669,18 +724,16 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
     // NOTE: If fields are added or removed from this method, BASIC_FETCH_FIELDS *must* be updated
     // as well
     private void fields_to_fetch_data_types(Geary.Email.Field fields,
-        Gee.List<FetchDataSpecifier> data_types_list, out FetchBodyDataSpecifier? header_specifier) {
-        // pack all the needed headers into a single FetchBodyDataType
-        string[] field_names = new string[0];
-
+                                            Gee.List<FetchDataSpecifier> basic_types,
+                                            Gee.List<string> header_fields) {
         // The assumption here is that because ENVELOPE is such a common fetch command, the
         // server will have optimizations for it, whereas if we called for each header in the
         // envelope separately, the server has to chunk harder parsing the RFC822 header ... have
         // to add References because IMAP ENVELOPE doesn't return them for some reason (but does
         // return Message-ID and In-Reply-To)
         if (fields.is_all_set(Geary.Email.Field.ENVELOPE)) {
-            data_types_list.add(FetchDataSpecifier.ENVELOPE);
-            field_names += "References";
+            basic_types.add(FetchDataSpecifier.ENVELOPE);
+            header_fields.add("References");
 
             // remove those flags and process any remaining
             fields = fields.clear(Geary.Email.Field.ENVELOPE);
@@ -689,35 +742,35 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
         foreach (Geary.Email.Field field in Geary.Email.Field.all()) {
             switch (fields & field) {
                 case Geary.Email.Field.DATE:
-                    field_names += "Date";
+                    header_fields.add("Date");
                 break;
 
                 case Geary.Email.Field.ORIGINATORS:
-                    field_names += "From";
-                    field_names += "Sender";
-                    field_names += "Reply-To";
+                    header_fields.add("From");
+                    header_fields.add("Sender");
+                    header_fields.add("Reply-To");
                 break;
 
                 case Geary.Email.Field.RECEIVERS:
-                    field_names += "To";
-                    field_names += "Cc";
-                    field_names += "Bcc";
+                    header_fields.add("To");
+                    header_fields.add("Cc");
+                    header_fields.add("Bcc");
                 break;
 
                 case Geary.Email.Field.REFERENCES:
-                    field_names += "References";
-                    field_names += "Message-ID";
-                    field_names += "In-Reply-To";
+                    header_fields.add("References");
+                    header_fields.add("Message-ID");
+                    header_fields.add("In-Reply-To");
                 break;
 
                 case Geary.Email.Field.SUBJECT:
-                    field_names += "Subject";
+                    header_fields.add("Subject");
                 break;
 
                 case Geary.Email.Field.HEADER:
                     // TODO: If the entire header is being pulled, then no need to pull down partial
                     // headers; simply get them all and decode what is needed directly
-                    data_types_list.add(FetchDataSpecifier.RFC822_HEADER);
+                    basic_types.add(FetchDataSpecifier.RFC822_HEADER);
                 break;
 
                 case Geary.Email.Field.NONE:
@@ -732,22 +785,18 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
                     assert_not_reached();
             }
         }
-
-        // convert field names into single FetchBodyDataType object
-        if (field_names.length > 0) {
-            header_specifier = new FetchBodyDataSpecifier.peek(
-                FetchBodyDataSpecifier.SectionPart.HEADER_FIELDS, null, -1, -1, field_names);
-            if (imap_header_fields_hack)
-                header_specifier.omit_request_header_fields_space();
-        } else {
-            header_specifier = null;
-        }
     }
 
-    private static Geary.Email fetched_data_to_email(string folder_name, UID uid,
-        FetchedData fetched_data, Geary.Email.Field required_fields,
-        FetchBodyDataSpecifier? header_specifier, FetchBodyDataSpecifier? body_specifier,
-        FetchBodyDataSpecifier? preview_specifier, FetchBodyDataSpecifier? preview_charset_specifier) throws 
Error {
+    private static Geary.Email fetched_data_to_email(
+        string folder_name,
+        UID uid,
+        FetchedData fetched_data,
+        Geary.Email.Field required_fields,
+        FetchBodyDataSpecifier[]? header_specifiers,
+        FetchBodyDataSpecifier? body_specifier,
+        FetchBodyDataSpecifier? preview_specifier,
+        FetchBodyDataSpecifier? preview_charset_specifier
+    ) throws GLib.Error {
         // note the use of INVALID_ROWID, as the rowid for this email (if one is present in the
         // database) is unknown at this time; this means ImapDB *must* create a new EmailIdentifier
         // for this email after create/merge is completed
@@ -816,20 +865,32 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
         if (internaldate != null && rfc822_size != null)
             email.set_email_properties(new Geary.Imap.EmailProperties(internaldate, rfc822_size));
 
-        // if the header was requested, convert its fields now
-        bool has_header_specifier = fetched_data.body_data_map.has_key(header_specifier);
-        if (header_specifier != null && !has_header_specifier) {
-            message("[%s] No header specifier \"%s\" found:", folder_name,
-                header_specifier.to_string());
-            foreach (FetchBodyDataSpecifier specifier in fetched_data.body_data_map.keys)
-                message("[%s] has %s", folder_name, specifier.to_string());
-        } else if (header_specifier != null && has_header_specifier) {
-            RFC822.Header headers = new RFC822.Header(
-                fetched_data.body_data_map.get(header_specifier));
+        // if any headers were requested, convert its fields now
+        if (header_specifiers != null) {
+            Gee.Map<string,string> headers = new Gee.HashMap<string,string>();
+            foreach (FetchBodyDataSpecifier header_specifier in header_specifiers) {
+                Memory.Buffer fetched_headers =
+                    fetched_data.body_data_map.get(header_specifier);
+                if (fetched_headers != null) {
+                    RFC822.Header parsed_headers = new RFC822.Header(fetched_headers);
+                    foreach (string name in parsed_headers.get_header_names()) {
+                        headers.set(name, parsed_headers.get_header(name));
+                    }
+                } else {
+                    warning(
+                        "[%s] No header specifier \"%s\" found in response:",
+                        folder_name,
+                        header_specifier.to_string()
+                    );
+                    foreach (FetchBodyDataSpecifier specifier in fetched_data.body_data_map.keys) {
+                        message("[%s] - has %s", folder_name, specifier.to_string());
+                    }
+                }
+            }
 
             // DATE
             if (required_but_not_set(Geary.Email.Field.DATE, required_fields, email)) {
-                string? value = headers.get_header("Date");
+                string? value = headers.get("Date");
                 RFC822.Date? date = null;
                 if (!String.is_empty(value)) {
                     try {
@@ -847,17 +908,17 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
             // ORIGINATORS
             if (required_but_not_set(Geary.Email.Field.ORIGINATORS, required_fields, email)) {
                 RFC822.MailboxAddresses? from = null;
-                string? value = headers.get_header("From");
+                string? value = headers.get("From");
                 if (!String.is_empty(value))
                     from = new RFC822.MailboxAddresses.from_rfc822_string(value);
 
                 RFC822.MailboxAddress? sender = null;
-                value = headers.get_header("Sender");
+                value = headers.get("Sender");
                 if (!String.is_empty(value))
                     sender = new RFC822.MailboxAddress.from_rfc822_string(value);
 
                 RFC822.MailboxAddresses? reply_to = null;
-                value = headers.get_header("Reply-To");
+                value = headers.get("Reply-To");
                 if (!String.is_empty(value))
                     reply_to = new RFC822.MailboxAddresses.from_rfc822_string(value);
 
@@ -867,17 +928,17 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
             // RECEIVERS
             if (required_but_not_set(Geary.Email.Field.RECEIVERS, required_fields, email)) {
                 RFC822.MailboxAddresses? to = null;
-                string? value = headers.get_header("To");
+                string? value = headers.get("To");
                 if (!String.is_empty(value))
                     to = new RFC822.MailboxAddresses.from_rfc822_string(value);
 
                 RFC822.MailboxAddresses? cc = null;
-                value = headers.get_header("Cc");
+                value = headers.get("Cc");
                 if (!String.is_empty(value))
                     cc = new RFC822.MailboxAddresses.from_rfc822_string(value);
 
                 RFC822.MailboxAddresses? bcc = null;
-                value = headers.get_header("Bcc");
+                value = headers.get("Bcc");
                 if (!String.is_empty(value))
                     bcc = new RFC822.MailboxAddresses.from_rfc822_string(value);
 
@@ -889,19 +950,19 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
             // References header will be present if REFERENCES were required, which is why
             // REFERENCES is set at the bottom of the method, when all information has been gathered
             if (message_id == null) {
-                string? value = headers.get_header("Message-ID");
+                string? value = headers.get("Message-ID");
                 if (!String.is_empty(value))
                     message_id = new RFC822.MessageID(value);
             }
 
             if (in_reply_to == null) {
-                string? value = headers.get_header("In-Reply-To");
+                string? value = headers.get("In-Reply-To");
                 if (!String.is_empty(value))
                     in_reply_to = new RFC822.MessageIDList.from_rfc822_string(value);
             }
 
             if (references == null) {
-                string? value = headers.get_header("References");
+                string? value = headers.get("References");
                 if (!String.is_empty(value))
                     references = new RFC822.MessageIDList.from_rfc822_string(value);
             }
@@ -909,7 +970,7 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
             // SUBJECT
             // Unlike DATE, allow for empty subjects
             if (required_but_not_set(Geary.Email.Field.SUBJECT, required_fields, email)) {
-                string? value = headers.get_header("Subject");
+                string? value = headers.get("Subject");
                 if (value != null)
                     email.set_message_subject(new RFC822.Subject.decode(value));
                 else
@@ -1038,7 +1099,7 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
                         // before response returned
                         if (specifier.request_header_fields_space) {
                             this.imap_header_fields_hack = true;
-                        return true;
+                            return true;
                         }
                     }
                 }
diff --git a/src/engine/rfc822/rfc822-message-data.vala b/src/engine/rfc822/rfc822-message-data.vala
index 0c8d9e17..2d399986 100644
--- a/src/engine/rfc822/rfc822-message-data.vala
+++ b/src/engine/rfc822/rfc822-message-data.vala
@@ -328,7 +328,7 @@ public class Geary.RFC822.Header : Geary.MessageData.BlockMessageData, Geary.RFC
     private string[]? names = null;
 
     public Header(Memory.Buffer buffer) {
-        base ("RFC822.Header", buffer);
+        base("RFC822.Header", buffer);
     }
 
     private unowned GMime.HeaderList get_headers() throws RFC822Error {
@@ -351,20 +351,16 @@ public class Geary.RFC822.Header : Geary.MessageData.BlockMessageData, Geary.RFC
     }
 
     public string[] get_header_names() throws RFC822Error {
-        if (names != null)
-            return names;
-
-        names = new string[0];
-
-        unowned GMime.HeaderIter iter;
-        if (!get_headers().get_iter(out iter))
-            return names;
-
-        do {
-            names += iter.get_name();
-        } while (iter.next());
-
-        return names;
+        if (this.names == null) {
+            this.names = new string[0];
+            GMime.HeaderIter iter = new GMime.HeaderIter();
+            if (get_headers().get_iter(iter) && iter.first()) {
+                do {
+                    names += iter.get_name();
+                } while (iter.next());
+            }
+        }
+        return this.names;
     }
 }
 
diff --git a/test/engine/rfc822-message-data-test.vala b/test/engine/rfc822-message-data-test.vala
index 2b849edb..504efaa0 100644
--- a/test/engine/rfc822-message-data-test.vala
+++ b/test/engine/rfc822-message-data-test.vala
@@ -10,7 +10,10 @@ class Geary.RFC822.MessageDataTest : TestCase {
     public MessageDataTest() {
         base("Geary.RFC822.MessageDataTest");
         add_test("date_from_rfc822", date_from_rfc822);
+        add_test("date_from_rfc822", date_from_rfc822);
         add_test("date_to_rfc822", date_to_rfc822);
+        add_test("header_from_rfc822", header_from_rfc822);
+        add_test("header_names_from_rfc822", header_names_from_rfc822);
         add_test("PreviewText.with_header", preview_text_with_header);
     }
 
@@ -42,6 +45,20 @@ class Geary.RFC822.MessageDataTest : TestCase {
         assert_string(HTML_BODY2_EXPECTED, html_preview2.buffer.to_string());
     }
 
+    public void header_from_rfc822() throws GLib.Error {
+        Header test_article = new Header(new Memory.StringBuffer(HEADER_FIXTURE));
+        assert_string("Test <test example com>", test_article.get_header("From"));
+        assert_string("test", test_article.get_header("Subject"));
+        assert_null_string(test_article.get_header("Blah"));
+    }
+
+    public void header_names_from_rfc822() throws GLib.Error {
+        Header test_article = new Header(new Memory.StringBuffer(HEADER_FIXTURE));
+        assert_int(2, test_article.get_header_names().length);
+        assert_string("From", test_article.get_header_names()[0]);
+        assert_string("Subject", test_article.get_header_names()[1]);
+    }
+
     public void date_from_rfc822() throws GLib.Error {
         const string FULL_HOUR_TZ = "Thu, 28 Feb 2019 00:00:00 -0100";
         Date full_hour_tz = new Date(FULL_HOUR_TZ);
@@ -92,6 +109,11 @@ class Geary.RFC822.MessageDataTest : TestCase {
     }
 
 
+    private const string HEADER_FIXTURE = """From: Test <test example com>
+Subject: test
+
+""";
+
     public static string PLAIN_BODY1_HEADERS = "Content-Type: text/plain; 
charset=\"us-ascii\"\r\nContent-Transfer-Encoding: 7bit\r\n";
     public static string PLAIN_BODY1_ENCODED = "-----BEGIN PGP SIGNED MESSAGE-----\r\nHash: 
SHA512\r\n\r\n=============================================================================\r\nFreeBSD-EN-16:11.vmbus
                                          Errata Notice\r\n                                                   
       The FreeBSD Project\r\n\r\nTopic:          Avoid using spin locks for channel message 
locks\r\n\r\nCategory:       core\r\nModule:         vmbus\r\nAnnounced:      2016-08-12\r\nCredits:        
Microsoft OSTC\r\nAffects:        FreeBSD 10.3\r\nCorrected:      2016-06-15 09:52:01 UTC (stable/10, 
10.3-STABLE)\r\n                2016-08-12 04:01:16 UTC (releng/10.3, 10.3-RELEASE-p7)\r\n\r\nFor general 
information regarding FreeBSD Errata Notices and Security\r\nAdvisories, including descriptions of the fields 
above, security\r\nbranches, and the following sections, please 
visit\r\n<URL:https://security.FreeBSD.org/>.\r\n";
     public static string PLAIN_BODY1_EXPECTED = "FreeBSD-EN-16:11.vmbus Errata Notice The FreeBSD Project 
Topic: Avoid using spin locks for channel message locks Category: core Module: vmbus Announced: 2016-08-12 
Credits: Microsoft OSTC Affects: FreeBSD 10.3 Corrected: 2016-06-15 09:52:01 UTC (stable/10, 10.3-STABLE) 
2016-08-12 04:01:16 UTC (releng/10.3, 10.3-RELEASE-p7) For general information regarding FreeBSD Errata 
Notices and Security Advisories, including descriptions of the fields above, security branches, and the 
following sections, please visit <URL:https://security.FreeBSD.org/>.";


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