[geary/cherry-pick-737bdba1] Merge branch 'mjog/571-imap-fetch-space-workaround-redux' into 'mainline'
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/cherry-pick-737bdba1] Merge branch 'mjog/571-imap-fetch-space-workaround-redux' into 'mainline'
- Date: Sat, 26 Oct 2019 05:24:42 +0000 (UTC)
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]
