nettika/geary
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

FETCH decoding fails, causing various problems: Closes #4781

Browse source 
This also closes #5079, which I believe was caused by #4781 (and
could reproduce once isolated).

This commit may also close bug #5189, but need to verify further
on reporter's machine before closing.

These problems are all related to emails lacking a Message-ID in
their header, common with spam and mass-emailers.
This commit is contained in:
Jim Nelson 2012-05-03 20:24:44 -07:00
parent 85d378701f
commit c62553007b
6 changed files with 58 additions and 142 deletions
Download patch file Download diff file Expand all files Collapse all files

7
src/engine/api/geary-conversations.vala
View file

@ -444,6 +444,13 @@ public class Geary.Conversations : Object {
Gee.MultiMap<Conversation, Geary.Email> appended_conversations = new Gee.HashMultiMap<
Conversation, Geary.Email>();
foreach (Geary.Email email in emails) {
// Skip messages already assigned to a conversation; this also deals with the problem
// of messages with no Message-ID being loaded twice (most often encountered when
// the first pass is loading messages directly from the database and the second pass
// are messages loaded from both)
if (geary_id_map.has_key(email.id))
continue;
// Right now, all threading is done with Message-IDs (no parsing of subject lines, etc.)
// If a message doesn't have a Message-ID, it's treated as its own conversation

16
src/engine/imap/decoders/imap-fetch-data-decoder.vala
View file

@ -39,6 +39,10 @@ public abstract class Geary.Imap.FetchDataDecoder {
if (literalp != null)
return decode_literal(literalp);
NilParameter? nilp = param as NilParameter;
if (nilp != null)
return decode_nil(nilp);
// bad news; this means this function isn't handling a Parameter type properly
assert_not_reached();
}
@ -54,6 +58,10 @@ public abstract class Geary.Imap.FetchDataDecoder {
protected virtual MessageData decode_literal(LiteralParameter literal) throws ImapError {
throw new ImapError.TYPE_ERROR("%s does not accept a literal parameter", data_item.to_string());
}
protected virtual MessageData decode_nil(NilParameter nil) throws ImapError {
throw new ImapError.TYPE_ERROR("%s does not accept a nil parameter", data_item.to_string());
}
}
public class Geary.Imap.UIDDecoder : Geary.Imap.FetchDataDecoder {
@ -115,11 +123,11 @@ public class Geary.Imap.EnvelopeDecoder : Geary.Imap.FetchDataDecoder {
ListParameter? cc = listp.get_as_nullable_list(6);
ListParameter? bcc = listp.get_as_nullable_list(7);
StringParameter? in_reply_to = listp.get_as_nullable_string(8);
StringParameter? message_id = listp.get_as_string(9);
StringParameter? message_id = listp.get_as_nullable_string(9);
// Although Message-ID is required to be returned by IMAP, it may be blank if the email
// does not supply it (optional according to RFC822); deal with this cognitive dissonance
if (String.is_empty(message_id.value))
if (message_id != null && String.is_empty(message_id.value))
message_id = null;
return new Envelope(new Geary.RFC822.Date(sent.value),
@ -173,6 +181,10 @@ public class Geary.Imap.RFC822TextDecoder : Geary.Imap.FetchDataDecoder {
protected override MessageData decode_literal(LiteralParameter literalp) throws ImapError {
return new Geary.Imap.RFC822Text(literalp.get_buffer());
}
protected override MessageData decode_nil(NilParameter nilp) throws ImapError {
return new Geary.Imap.RFC822Text(Memory.EmptyBuffer.instance);
}
}
public class Geary.Imap.RFC822FullDecoder : Geary.Imap.FetchDataDecoder {

16
src/engine/imap/decoders/imap-fetch-results.vala
View file

@ -42,10 +42,16 @@ public class Geary.Imap.FetchResults : Geary.Imap.CommandResults {
for (int ctr = 0; ctr < list.get_count(); ctr += 2) {
StringParameter data_item_param = list.get_as_string(ctr);
// watch for truncated lists, which indicate an empty return value
bool has_value = (ctr < (list.get_count() - 1));
if (FetchBodyDataType.is_fetch_body(data_item_param)) {
// FETCH body data items are merely a literal of all requested fields formatted
// in RFC822 header format
results.body_data.add(list.get_as_literal(ctr + 1).get_buffer());
// in RFC822 header format ... watch for empty return values
if (has_value)
results.body_data.add(list.get_as_literal(ctr + 1).get_buffer());
else
results.body_data.add(Memory.EmptyBuffer.instance);
} else {
FetchDataType data_item = FetchDataType.decode(data_item_param.value);
FetchDataDecoder? decoder = data_item.get_decoder();
@ -56,7 +62,11 @@ public class Geary.Imap.FetchResults : Geary.Imap.CommandResults {
continue;
}
results.set_data(data_item, decoder.decode(list.get_required(ctr + 1)));
// watch for empty return values
if (has_value)
results.set_data(data_item, decoder.decode(list.get_required(ctr + 1)));
else
results.set_data(data_item, decoder.decode(NilParameter.instance));
}
}

67
src/engine/sqlite/api/sqlite-folder.vala
View file

@ -8,8 +8,7 @@
// the future, to support other email services, will need to break this up.
private class Geary.Sqlite.Folder : Object, Geary.ReferenceSemantics {
public const Geary.Email.Field REQUIRED_FOR_DUPLICATE_DETECTION =
Geary.Email.Field.REFERENCES | Geary.Email.Field.PROPERTIES;
public const Geary.Email.Field REQUIRED_FOR_DUPLICATE_DETECTION = Geary.Email.Field.PROPERTIES;
public bool opened { get; private set; default = false; }
@ -113,19 +112,6 @@ private class Geary.Sqlite.Folder : Object, Geary.ReferenceSemantics {
if (!email.fields.is_all_set(REQUIRED_FOR_DUPLICATE_DETECTION))
return Sqlite.Row.INVALID_ID;
// what's more, actually need all those fields to be available, not merely attempted,
// to err on the side of safety
if (email.message_id == null)
return Sqlite.Row.INVALID_ID;
Imap.EmailProperties? imap_properties = (Imap.EmailProperties) email.properties;
string? internaldate = (imap_properties != null && imap_properties.internaldate != null)
? imap_properties.internaldate.original : null;
long rfc822_size = (imap_properties != null) ? imap_properties.rfc822_size.value : -1;
if (String.is_empty(internaldate) || rfc822_size < 0)
return Sqlite.Row.INVALID_ID;
// See if it already exists; first by UID (which is only guaranteed to be unique in a folder,
// not account-wide)
int64 message_id;
@ -134,39 +120,30 @@ private class Geary.Sqlite.Folder : Object, Geary.ReferenceSemantics {
return message_id;
}
// what's more, actually need all those fields to be available, not merely attempted,
// to err on the side of safety
Imap.EmailProperties? imap_properties = (Imap.EmailProperties) email.properties;
string? internaldate = (imap_properties != null && imap_properties.internaldate != null)
? imap_properties.internaldate.original : null;
long rfc822_size = (imap_properties != null && imap_properties.rfc822_size != null)
? imap_properties.rfc822_size.value : -1;
if (String.is_empty(internaldate) || rfc822_size < 0)
return Sqlite.Row.INVALID_ID;
// reset
message_id = Sqlite.Row.INVALID_ID;
// look for duplicate via Message-ID
Gee.List<int64?>? list = yield message_table.search_message_id_async(transaction,
email.message_id, cancellable);
// only a duplicate candidate if exactly one found, otherwise err on the side of safety
if (list != null && list.size == 1)
message_id = list[0];
// look for duplicate in IMAP message properties
Gee.List<int64?>? duplicate_ids = yield imap_message_properties_table.search_for_duplicates_async(
transaction, internaldate, rfc822_size, cancellable);
if (duplicate_ids != null && duplicate_ids.size > 0) {
// if a message_id was found via Message-ID, search for a match; else if one duplicate
// was found via IMAP properties, use that, otherwise err on the side of safety
if (message_id != Sqlite.Row.INVALID_ID) {
int64 match_id = Sqlite.Row.INVALID_ID;
foreach (int64 duplicate_id in duplicate_ids) {
if (message_id == duplicate_id) {
match_id = duplicate_id;
break;
}
}
// use the matched ID, which if not found, invalidates the discovered ID
message_id = match_id;
if (duplicate_ids.size > 1) {
debug("Warning: Multiple messages with the same internaldate (%s) and size (%lu) found in %s",
internaldate, rfc822_size, to_string());
message_id = duplicate_ids[0];
} else if (duplicate_ids.size == 1) {
message_id = duplicate_ids[0];
} else {
message_id = Sqlite.Row.INVALID_ID;
}
}
@ -177,8 +154,8 @@ private class Geary.Sqlite.Folder : Object, Geary.ReferenceSemantics {
private async bool associate_with_folder_async(Transaction transaction, int64 message_id,
Geary.Email email, Cancellable? cancellable) throws Error {
// see if an email exists at this position
MessageLocationRow? location_row = yield location_table.fetch_async(transaction,
folder_row.id, email.position, cancellable);
MessageLocationRow? location_row = yield location_table.fetch_by_ordering_async(transaction,
folder_row.id, email.id.ordering, cancellable);
if (location_row != null)
return false;
@ -208,8 +185,12 @@ private class Geary.Sqlite.Folder : Object, Geary.ReferenceSemantics {
// if already associated or a duplicate, merge and/or associate
if (message_id != Sqlite.Row.INVALID_ID) {
if (!associated)
yield associate_with_folder_async(transaction, message_id, email, cancellable);
if (!associated) {
if (!yield associate_with_folder_async(transaction, message_id, email, cancellable)) {
debug("Warning: Unable to associate %s (%lld) with %s", email.id.to_string(), message_id,
to_string());
}
}
yield merge_email_async(transaction, message_id, email, cancellable);

26
src/engine/sqlite/email/sqlite-message-location-table.vala
View file

@ -131,32 +131,6 @@ public class Geary.Sqlite.MessageLocationTable : Geary.Sqlite.Table {
return (list.size > 0) ? list : null;
}
/**
* position is one-based.
*/
public async MessageLocationRow? fetch_async(Transaction transaction, int64 folder_id,
int position, Cancellable? cancellable) throws Error {
assert(position >= 1);
Transaction locked = yield obtain_lock_async(transaction, "MessageLocationTable.fetch_async",
cancellable);
SQLHeavy.Query query = locked.prepare(
"SELECT id, message_id, ordering FROM MessageLocationTable WHERE folder_id = ? "
+ "AND remove_marker = 0 ORDER BY ordering LIMIT 1 OFFSET ?");
query.bind_int64(0, folder_id);
query.bind_int(1, position - 1);
SQLHeavy.QueryResult results = yield query.execute_async();
if (results.finished)
return null;
check_cancel(cancellable, "fetch_async");
return new MessageLocationRow(this, results.fetch_int64(0), results.fetch_int64(1), folder_id,
results.fetch_int64(2), position);
}
public async MessageLocationRow? fetch_by_ordering_async(Transaction? transaction,
int64 folder_id, int64 ordering, Cancellable? cancellable) throws Error {
Transaction locked = yield obtain_lock_async(transaction, "MessageLocationTable.fetch_by_ordering_async",

68
src/engine/sqlite/email/sqlite-message-table.vala
View file

@ -194,35 +194,6 @@ public class Geary.Sqlite.MessageTable : Geary.Sqlite.Table {
yield release_lock_async(transaction, locked, cancellable);
}
public async Gee.List<MessageRow>? list_by_message_id_async(Transaction? transaction,
Geary.RFC822.MessageID message_id, Geary.Email.Field fields, Cancellable? cancellable)
throws Error {
assert(fields != Geary.Email.Field.NONE);
Transaction locked = yield obtain_lock_async(transaction, "MessageTable.list_by_message_id_async",
cancellable);
SQLHeavy.Query query = locked.prepare(
"SELECT %s FROM MessageTable WHERE message_id=?".printf(fields_to_columns(fields)));
query.bind_string(0, message_id.value);
SQLHeavy.QueryResult results = yield query.execute_async();
if (results.finished)
return null;
check_cancel(cancellable, "list_by_message_id_async");
Gee.List<MessageRow> list = new Gee.ArrayList<MessageRow>();
do {
list.add(new MessageRow.from_query_result(this, fields, results));
yield results.next_async();
check_cancel(cancellable, "list_by_message_id_async");
} while (!results.finished);
return (list.size > 0) ? list : null;
}
public async MessageRow? fetch_async(Transaction? transaction, int64 id,
Geary.Email.Field requested_fields, Cancellable? cancellable = null) throws Error {
assert(requested_fields != Geary.Email.Field.NONE);
@ -317,44 +288,5 @@ public class Geary.Sqlite.MessageTable : Geary.Sqlite.Table {
return builder.str;
}
public async int search_message_id_count_async(Transaction? transaction,
Geary.RFC822.MessageID message_id, Cancellable? cancellable) throws Error {
Transaction locked = yield obtain_lock_async(transaction, "MessageTable.search_message_id_count",
cancellable);
SQLHeavy.Query query = locked.prepare(
"SELECT COUNT(*) FROM MessageTable WHERE message_id=?");
query.bind_string(0, message_id.value);
SQLHeavy.QueryResult result = yield query.execute_async();
check_cancel(cancellable, "search_message_id_count_async");
return (result.finished) ? 0 : result.fetch_int(0);
}
public async Gee.List<int64?>? search_message_id_async(Transaction? transaction,
Geary.RFC822.MessageID message_id, Cancellable? cancellable) throws Error {
Transaction locked = yield obtain_lock_async(transaction, "MessageTable.search_message_id_async",
cancellable);
SQLHeavy.Query query = locked.prepare(
"SELECT id FROM MessageTable WHERE message_id=?");
query.bind_string(0, message_id.value);
SQLHeavy.QueryResult result = yield query.execute_async();
check_cancel(cancellable, "search_message_id_async");
if (result.finished)
return null;
Gee.List<int64?> list = new Gee.ArrayList<int64?>();
do {
list.add(result.fetch_int64(0));
yield result.next_async();
check_cancel(cancellable, "search_message_id_async");
} while (!result.finished);
return list;
}
}