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

Merge branch 'mjog/server-quirks' into 'mainline'

Browse source 
Server quirks

See merge request GNOME/geary!508
This commit is contained in:
Michael Gratton 2020-06-30 07:50:05 +00:00
commit 3550c69723
9 changed files with 121 additions and 46 deletions
Download patch file Download diff file Expand all files Collapse all files

8
src/engine/api/geary-endpoint.vala
View file

@ -71,14 +71,6 @@ public class Geary.Endpoint : BaseObject {
get; set; default = TlsCertificateFlags.VALIDATE_ALL;
}
/**
* The maximum number of commands that will be pipelined at once.
*
* If 0 (the default), there is no limit on the number of
* pipelined commands sent to this endpoint.
*/
public uint max_pipeline_batch_size = 0;
/**
* When set, TLS has reported certificate issues.
*

15
src/engine/api/geary-engine.vala
View file

@ -426,22 +426,7 @@ public class Geary.Engine : BaseObject {
uint timeout = service.protocol == Protocol.IMAP
? Imap.ClientConnection.RECOMMENDED_TIMEOUT_SEC
: Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC;
shared = new_endpoint(provider, service, timeout);
// XXX this is pretty hacky, move this back into the
// OutlookAccount somehow
if (provider == ServiceProvider.OUTLOOK) {
// As of June 2016, outlook.com's IMAP servers have a bug
// where a large number (~50) of pipelined STATUS commands on
// mailboxes with many messages will eventually cause it to
// break command parsing and return a BAD response, causing us
// to drop the connection. Limit the number of pipelined
// commands per batch to work around this. See b.g.o Bug
// 766552
shared.max_pipeline_batch_size = 25;
}
this.shared_endpoints.set(key, new EndpointWeakRef(shared));
}

12
src/engine/imap/api/imap-client-service.vala
View file

@ -49,6 +49,18 @@ public class Geary.Imap.ClientService : Geary.ClientService {
// around the former. See #746
quirks.flag_atom_exceptions = "]";
break;
case ServiceProvider.OUTLOOK:
// As of June 2016, outlook.com's IMAP servers have a bug
// where a large number (~50) of pipelined STATUS commands
// on mailboxes with many messages will eventually cause
// it to break command parsing and return a BAD response,
// causing us to drop the connection. Limit the number of
// pipelined commands per batch to work around this. See
// b.g.o Bug 766552
quirks.max_pipeline_batch_size = 25;
break;
default:
// noop
break;

31
src/engine/imap/api/imap-folder-session.vala
View file

@ -38,13 +38,7 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
/** Determines if this folder accepts custom IMAP flags. */
public Trillian accepts_user_flags { get; private set; default = Trillian.UNKNOWN; }
/**
* Set to true when it's detected that the server doesn't allow a
* space between "header.fields" and the list of email headers to
* be requested via FETCH; see:
* [[https://bugzilla.gnome.org/show_bug.cgi?id=714902|Bug * 714902]]
*/
public bool imap_header_fields_hack { get; private set; default = false; }
private Quirks quirks;
private Nonblocking.Mutex cmd_mutex = new Nonblocking.Mutex();
private Gee.HashMap<SequenceNumber, FetchedData>? fetch_accumulator = null;
@ -93,6 +87,7 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
throws Error {
base(session);
this.folder = folder;
this.quirks = session.quirks;
if (folder.properties.attrs.is_no_select) {
throw new ImapError.NOT_SUPPORTED(
@ -253,7 +248,20 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
break;
case ResponseCodeType.UIDNEXT:
this.folder.properties.uid_next = response_code.get_uid_next();
try {
this.folder.properties.uid_next = response_code.get_uid_next();
} catch (ImapError.INVALID err) {
// Some mail servers e.g hMailServer and
// whatever is used by home.pl (dovecot?)
// sends UIDNEXT 0. Just ignore these since
// there nothing else that can be done. See
// GNOME/geary#711
if (response_code.get_as_string(1).as_int64() == 0) {
warning("Ignoring bad UIDNEXT 0 from server");
} else {
throw err;
}
}
break;
case ResponseCodeType.UIDVALIDITY:
@ -388,7 +396,8 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
//
// See <https://gitlab.gnome.org/GNOME/geary/issues/571>
if (!header_fields.is_empty) {
if (!this.imap_header_fields_hack || header_fields.size == 1) {
if (!this.quirks.fetch_header_part_no_space ||
header_fields.size == 1) {
header_specifiers = new FetchBodyDataSpecifier[1];
header_specifiers[0] = new FetchBodyDataSpecifier.peek(
FetchBodyDataSpecifier.SectionPart.HEADER_FIELDS,
@ -412,7 +421,7 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
}
foreach (FetchBodyDataSpecifier header in header_specifiers) {
if (this.imap_header_fields_hack) {
if (this.quirks.fetch_header_part_no_space) {
header.omit_request_header_fields_space();
}
cmds.add(new FetchCommand.body_data_type(msg_set, header));
@ -1100,7 +1109,7 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
// property was enabled after sending command but
// before response returned
if (specifier.request_header_fields_space) {
this.imap_header_fields_hack = true;
this.quirks.fetch_header_part_no_space = true;
return true;
}
}

26
src/engine/imap/api/imap-quirks.vala
View file

@ -1,5 +1,5 @@
/*
* Copyright © 2019-2020 Michael Gratton <mike@vee.net>
* Copyright © 2020 Michael Gratton <mike@vee.net>
*
* This software is licensed under the GNU Lesser General Public License
* (version 2.1 or later). See the COPYING file in this distribution.
@ -11,8 +11,32 @@
public class Geary.Imap.Quirks : BaseObject {
/**
* Whether spaces are disallowed in header parts of fetch commands.
*
* If true, HEADER parts of a BODY section may not contain any
* spaces.
*
* E.g. this conformant form is not supported:
*
* a008 UID FETCH * BODY.PEEK[HEADER.FIELDS (REFERENCES)]
*
* Whereas this non-conformant form is supported:
*
* a008 UID FETCH * BODY.PEEK[HEADER.FIELDS(REFERENCES)]
*/
public bool fetch_header_part_no_space { get; set; default = false; }
/** The set of additional characters allowed in an IMAP flag. */
public string? flag_atom_exceptions { get; set; }
/**
* The maximum number of commands that will be pipelined at once.
*
* If 0 (the default), there is no limit on the number of
* pipelined commands sent to this endpoint.
*/
public uint max_pipeline_batch_size { get; set; default = 0; }
}

21
src/engine/imap/response/imap-status-data.vala
View file

@ -101,7 +101,20 @@ public class Geary.Imap.StatusData : Object {
break;
case StatusDataType.UIDNEXT:
uid_next = new UID.checked(valuep.as_int64());
try {
uid_next = new UID.checked(valuep.as_int64());
} catch (ImapError.INVALID err) {
// Some mail servers e.g hMailServer and
// whatever is used by home.pl (dovecot?)
// sends UIDNEXT 0. Just ignore these
// since there nothing else that can be
// done. See GNOME/geary#711
if (valuep.as_int64() == 0) {
warning("Ignoring bad UIDNEXT 0 from server");
} else {
throw err;
}
}
break;
case StatusDataType.UIDVALIDITY:
@ -118,8 +131,10 @@ public class Geary.Imap.StatusData : Object {
break;
}
} catch (ImapError ierr) {
message("Bad value at %d/%d in STATUS response \"%s\": %s", ctr, ctr + 1,
server_data.to_string(), ierr.message);
warning(
"Bad value at %d/%d in STATUS response \"%s\": %s",
ctr, ctr + 1, server_data.to_string(), ierr.message
);
}
}

6
src/engine/imap/transport/imap-client-session.vala
View file

@ -286,6 +286,9 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
/** Records the actual name and delimiter used for the inbox */
internal MailboxInformation? inbox { get; private set; default = null; }
/** The quirks being used by this session. */
internal Quirks quirks { get; set; }
// Locations personal mailboxes for this session
private Gee.List<Namespace> personal_namespaces = new Gee.ArrayList<Namespace>();
@ -296,7 +299,6 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
private Gee.List<Namespace> shared_namespaces = new Gee.ArrayList<Namespace>();
private Endpoint imap_endpoint;
private Quirks quirks;
private Geary.State.Machine fsm;
private ClientConnection? cx = null;
@ -1290,7 +1292,7 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
// the endpoint's max pipeline size is positive, if so use
// multiple batches with a maximum size of that.
uint max_batch_size = this.imap_endpoint.max_pipeline_batch_size;
uint max_batch_size = this.quirks.max_pipeline_batch_size;
if (max_batch_size < 1) {
max_batch_size = cmds.size;
}

30
src/engine/imap/transport/imap-deserializer.vala
View file

@ -30,6 +30,10 @@ public class Geary.Imap.Deserializer : BaseObject, Logging.Source {
private const size_t MAX_BLOCK_READ_SIZE = 4096;
private const string[] RESPONSE_ATOMS = {
"OK", "NO", "BAD", "BYE", "PREAUTH"
};
private enum Mode {
LINE,
BLOCK,
@ -49,6 +53,7 @@ public class Geary.Imap.Deserializer : BaseObject, Logging.Source {
LITERAL,
LITERAL_DATA_BEGIN,
LITERAL_DATA,
RESPONSE_TEXT,
FAILED,
CLOSED,
COUNT
@ -102,6 +107,7 @@ public class Geary.Imap.Deserializer : BaseObject, Logging.Source {
private Geary.Memory.GrowableBuffer? block_buffer = null;
private unowned uint8[]? current_buffer = null;
private int ins_priority = Priority.DEFAULT;
private bool is_parsing_flags = false;
@ -179,12 +185,12 @@ public class Geary.Imap.Deserializer : BaseObject, Logging.Source {
new Geary.State.Mapping(State.START_PARAM, Event.ERROR, on_error),
new Geary.State.Mapping(State.ATOM, Event.CHAR, on_atom_char),
new Geary.State.Mapping(State.ATOM, Event.EOL, on_atom_eol),
new Geary.State.Mapping(State.ATOM, Event.EOL, on_param_eol),
new Geary.State.Mapping(State.ATOM, Event.EOS, on_eos),
new Geary.State.Mapping(State.ATOM, Event.ERROR, on_error),
new Geary.State.Mapping(State.FLAG, Event.CHAR, on_flag_char),
new Geary.State.Mapping(State.FLAG, Event.EOL, on_atom_eol),
new Geary.State.Mapping(State.FLAG, Event.EOL, on_param_eol),
new Geary.State.Mapping(State.FLAG, Event.EOS, on_eos),
new Geary.State.Mapping(State.FLAG, Event.ERROR, on_error),
@ -217,6 +223,11 @@ public class Geary.Imap.Deserializer : BaseObject, Logging.Source {
new Geary.State.Mapping(State.LITERAL_DATA, Event.EOS, on_eos),
new Geary.State.Mapping(State.LITERAL_DATA, Event.ERROR, on_error),
new Geary.State.Mapping(State.RESPONSE_TEXT, Event.CHAR, on_response_text_char),
new Geary.State.Mapping(State.RESPONSE_TEXT, Event.EOL, on_param_eol),
new Geary.State.Mapping(State.RESPONSE_TEXT, Event.EOS, on_eos),
new Geary.State.Mapping(State.RESPONSE_TEXT, Event.ERROR, on_error),
new Geary.State.Mapping(State.FAILED, Event.EOL, on_failed_eol),
new Geary.State.Mapping(State.FAILED, Event.EOS, Geary.State.nop),
new Geary.State.Mapping(State.FAILED, Event.ERROR, Geary.State.nop),
@ -594,7 +605,12 @@ public class Geary.Imap.Deserializer : BaseObject, Logging.Source {
return State.START_PARAM;
default:
if (!this.is_parsing_flags) {
if (this.context_stack.size == 1 &&
this.context.size >= 2 &&
this.context.get(1).to_string().ascii_up() in RESPONSE_ATOMS) {
append_to_string(ch);
return State.RESPONSE_TEXT;
} else if (!this.is_parsing_flags) {
if (DataFormat.is_atom_special(ch)) {
warning("Received an invalid atom-char: %c", ch);
return State.FAILED;
@ -701,7 +717,7 @@ public class Geary.Imap.Deserializer : BaseObject, Logging.Source {
return State.TAG;
}
private uint on_atom_eol(uint state, uint event, void *user) {
private uint on_param_eol(uint state, uint event, void *user) {
// clean up final atom
save_string_parameter(false);
flush_params();
@ -841,6 +857,12 @@ public class Geary.Imap.Deserializer : BaseObject, Logging.Source {
return State.START_PARAM;
}
private uint on_response_text_char(uint state, uint event, void *user) {
char ch = *((char *) user);
append_to_string(ch);
return State.RESPONSE_TEXT;
}
private uint on_eos() {
debug("EOS");

18
test/engine/imap/transport/imap-deserializer-test.vala
View file

@ -47,6 +47,9 @@ class Geary.Imap.DeserializerTest : TestCase {
// fail, disable for the moment
add_test("invalid_flag_prefix", invalid_flag_prefix);
add_test("reserved_in_response_text", reserved_in_response_text);
add_test("instant_eos", instant_eos);
add_test("bye_eos", bye_eos);
}
@ -220,14 +223,13 @@ class Geary.Imap.DeserializerTest : TestCase {
public void aliyun_greeting() throws Error {
string greeting = "* OK AliYun IMAP Server Ready(10.147.40.164)";
string parsed = "* OK AliYun IMAP Server Ready (10.147.40.164)";
this.stream.add_data(greeting.data);
this.stream.add_data(EOL.data);
this.process.begin(Expect.MESSAGE, this.async_completion);
RootParameters? message = this.process.end(async_result());
assert(message.to_string() == parsed);
assert(message.to_string() == greeting);
}
public void invalid_atom_prefix() throws Error {
@ -320,6 +322,18 @@ class Geary.Imap.DeserializerTest : TestCase {
//this.process.end(async_result());
}
public void reserved_in_response_text() throws Error {
// As seen in #711
string line = """a008 BAD Missing ] in: header.fields""";
this.stream.add_data(line.data);
this.stream.add_data(EOL.data);
this.process.begin(Expect.MESSAGE, this.async_completion);
RootParameters? message = this.process.end(async_result());
assert(message.to_string() == line);
}
public void instant_eos() throws Error {
this.process.begin(Expect.EOS, this.async_completion);
this.process.end(async_result());