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
This commit is contained in:
Michael Gratton 2019-10-26 05:24:03 +00:00
commit 737bdba158
4 changed files with 168 additions and 89 deletions

View file

@ -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")]

View file

@ -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;
}
}
}

View file

@ -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;
}
}

View file

@ -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/>.";