Update Geary.Imap.Capabilities handling

Move Capabilities to the api directory and make immutable.

Don't pass around out params to simply increment the revision and use a
field in ClientSession, just use the last capability instance. Ensure
after starting a TLS session capabilities are cleared. Add unit tests
for getting both implicit and explicity capaibilities when initiating
a client session.
This commit is contained in:
Michael Gratton 2019-12-29 23:15:34 +10:30 committed by Michael James Gratton
parent 53ce727c03
commit b46838f100
9 changed files with 285 additions and 108 deletions

View file

@ -208,6 +208,7 @@ src/engine/db/db-versioned-database.vala
src/engine/imap/imap.vala
src/engine/imap/imap-error.vala
src/engine/imap/api/imap-account-session.vala
src/engine/imap/api/imap-capabilities.vala
src/engine/imap/api/imap-client-service.vala
src/engine/imap/api/imap-email-flags.vala
src/engine/imap/api/imap-email-properties.vala
@ -319,7 +320,6 @@ src/engine/imap/parameter/imap-quoted-string-parameter.vala
src/engine/imap/parameter/imap-root-parameters.vala
src/engine/imap/parameter/imap-string-parameter.vala
src/engine/imap/parameter/imap-unquoted-string-parameter.vala
src/engine/imap/response/imap-capabilities.vala
src/engine/imap/response/imap-continuation-response.vala
src/engine/imap/response/imap-fetch-data-decoder.vala
src/engine/imap/response/imap-fetched-data.vala

View file

@ -1,7 +1,9 @@
/* Copyright 2016 Software Freedom Conservancy Inc.
/*
* Copyright 2016 Software Freedom Conservancy Inc.
* Copyright 2019 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.
* (version 2.1 or later). See the COPYING file in this distribution.
*/
public class Geary.Imap.Capabilities : Geary.GenericCapabilities {
@ -13,6 +15,7 @@ public class Geary.Imap.Capabilities : Geary.GenericCapabilities {
public const string COMPRESS = "COMPRESS";
public const string DEFLATE_SETTING = "DEFLATE";
public const string IDLE = "IDLE";
public const string IMAP4REV1 = "IMAP4rev1";
public const string NAMESPACE = "NAMESPACE";
public const string SPECIAL_USE = "SPECIAL-USE";
public const string STARTTLS = "STARTTLS";
@ -22,29 +25,49 @@ public class Geary.Imap.Capabilities : Geary.GenericCapabilities {
public const string NAME_SEPARATOR = "=";
public const string? VALUE_SEPARATOR = null;
/**
* The version of this set of capabilities for an IMAP session.
*
* The capabilities that an IMAP session offers changes over time,
* for example after login or STARTTLS. This property supports
* detecting these changes.
*
* @see ClientSession.capabilities
*/
public int revision { get; private set; }
/**
* Creates an empty set of capabilities. revision represents the different variations of
* capabilities that an IMAP session might offer (i.e. changes after login or STARTTLS, for
* example).
* Creates an empty set of capabilities.
*/
public Capabilities(int revision) {
base (NAME_SEPARATOR, VALUE_SEPARATOR);
this.revision = revision;
public Capabilities(StringParameter[] capabilities, int revision) {
this.empty(revision);
foreach (var cap in capabilities) {
parse_and_add_capability(cap.ascii);
}
}
public bool add_parameter(StringParameter stringp) {
return parse_and_add_capability(stringp.ascii);
/**
* Creates an empty set of capabilities.
*/
public Capabilities.empty(int revision) {
base(NAME_SEPARATOR, VALUE_SEPARATOR);
this.revision = revision;
}
public override string to_string() {
return "#%d: %s".printf(revision, base.to_string());
}
/**
* Indicates an IMAP session reported support for IMAP 4rev1.
*
* See [[https://tools.ietf.org/html/rfc2177]]
*/
public bool supports_imap4rev1() {
return has_capability(IMAP4REV1);
}
/**
* Indicates the {@link ClientSession} reported support for IDLE.
*

View file

@ -69,24 +69,22 @@ public class Geary.Imap.ResponseCode : Geary.Imap.ListParameter {
/**
* Parses the {@link ResponseCode} into {@link Capabilities}, if possible.
*
* Since Capabilities are revised with various {@link ClientSession} states, this method accepts
* a ref to an int that will be incremented after handed to the Capabilities constructor. This
* can be used to track the revision of capabilities seen on the connection.
*
* @throws ImapError.INVALID if Capability was not specified.
*/
public Capabilities get_capabilities(ref int next_revision) throws ImapError {
public Capabilities get_capabilities(int revision) throws ImapError {
if (!get_response_code_type().is_value(ResponseCodeType.CAPABILITY))
throw new ImapError.INVALID("Not CAPABILITY response code: %s", to_string());
Capabilities capabilities = new Capabilities(next_revision++);
var params = new StringParameter[this.size];
int count = 0;
for (int ctr = 1; ctr < size; ctr++) {
StringParameter? param = get_if_string(ctr);
if (param != null)
capabilities.add_parameter(param);
if (param != null) {
params[count++] = param;
}
}
return capabilities;
return new Capabilities(params[0:count], revision);
}
/**

View file

@ -50,24 +50,22 @@ public class Geary.Imap.ServerData : ServerResponse {
/**
* Parses the {@link ServerData} into {@link Capabilities}, if possible.
*
* Since Capabilities are revised with various {@link ClientSession} states, this method accepts
* a ref to an int that will be incremented after handed to the Capabilities constructor. This
* can be used to track the revision of capabilities seen on the connection.
*
* @throws ImapError.INVALID if not a Capability.
*/
public Capabilities get_capabilities(ref int next_revision) throws ImapError {
if (server_data_type != ServerDataType.CAPABILITY)
public Capabilities get_capabilities(int revision) throws ImapError {
if (this.server_data_type != ServerDataType.CAPABILITY)
throw new ImapError.INVALID("Not CAPABILITY data: %s", to_string());
Capabilities capabilities = new Capabilities(next_revision++);
for (int ctr = 2; ctr < size; ctr++) {
var params = new StringParameter[this.size];
int count = 0;
for (int ctr = 1; ctr < size; ctr++) {
StringParameter? param = get_if_string(ctr);
if (param != null)
capabilities.add_parameter(param);
if (param != null) {
params[count++] = param;
}
}
return capabilities;
return new Capabilities(params[0:count], revision);
}
/**

View file

@ -228,13 +228,17 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
state_to_string, event_to_string);
/**
* {@link ClientSession} tracks server extensions reported via the CAPABILITY server data
* response.
* Set of IMAP extensions reported as being supported by the server.
*
* ClientSession stores the last seen list as a service for users and uses it internally
* (specifically for IDLE support).
* The capabilities that an IMAP session offers changes over time,
* for example after login or STARTTLS. The instance assigned to
* this property will change as these change and the instance's
* {@link Capabilities.revision} property will also monotonically
* increase over the lifetime of a single session.
*/
public Capabilities capabilities { get; private set; default = new Capabilities(0); }
public Capabilities capabilities {
get; private set; default = new Capabilities.empty(0);
}
/** Determines if this session supports the IMAP IDLE extension. */
public bool is_idle_supported {
@ -278,7 +282,6 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
/** The locations of shared mailboxes on this connection. */
internal Gee.List<Namespace> shared_namespaces = new Gee.ArrayList<Namespace>();
private Endpoint imap_endpoint;
private Geary.State.Machine fsm;
private ClientConnection? cx = null;
@ -295,7 +298,6 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
private Nonblocking.Semaphore? connect_waiter = null;
private Error? connect_err = null;
private int next_capabilities_revision = 1;
private Gee.Map<string,Namespace> namespaces = new Gee.HashMap<string,Namespace>();
@ -316,8 +318,6 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
*/
public signal void server_data_received(ServerData server_data);
public signal void capability(Capabilities capabilities);
public signal void exists(int count);
public signal void expunge(SequenceNumber seq_num);
@ -929,14 +929,14 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
GLib.Cancellable? cancellable)
throws GLib.Error {
// If no capabilities available, get them now
if (capabilities.is_empty())
if (this.capabilities.is_empty()) {
yield send_command_async(new CapabilityCommand(), cancellable);
}
// store them for comparison later
Imap.Capabilities caps = capabilities;
var last_capabilities = this.capabilities.revision;
if (imap_endpoint.tls_method == TlsNegotiationMethod.START_TLS) {
if (!caps.has_capability(Capabilities.STARTTLS)) {
if (!this.capabilities.has_capability(Capabilities.STARTTLS)) {
throw new ImapError.NOT_SUPPORTED(
"STARTTLS unavailable for %s", to_string());
}
@ -957,19 +957,26 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
resp.status.to_string()
);
}
if (last_capabilities == capabilities.revision) {
// Per RFC3501 §6.2.1, after TLS is established, all
// capabilities must be cleared and re-acquired to
// mitigate main-in-the-middle attacks. If the TLS
// command response did not update capabilities,
// explicitly do so now.
yield send_command_async(new CapabilityCommand(), cancellable);
last_capabilities = this.capabilities.revision;
}
}
// Login after STARTTLS
yield login_async(credentials, cancellable);
// if new capabilities not offered after login, get them now
if (caps.revision == capabilities.revision) {
if (last_capabilities == capabilities.revision) {
yield send_command_async(new CapabilityCommand(), cancellable);
}
// either way, new capabilities should be available
caps = capabilities;
Gee.List<ServerData> server_data = new Gee.ArrayList<ServerData>();
ulong data_id = this.server_data_received.connect((data) => { server_data.add(data); });
try {
@ -987,7 +994,7 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
// Try to determine what the connection's namespaces are
server_data.clear();
if (caps.has_capability(Capabilities.NAMESPACE)) {
if (this.capabilities.has_capability(Capabilities.NAMESPACE)) {
response = yield send_command_async(
new NamespaceCommand(),
cancellable
@ -1796,12 +1803,14 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
if (response_code != null) {
try {
if (response_code.get_response_code_type().is_value(ResponseCodeType.CAPABILITY)) {
capabilities = response_code.get_capabilities(ref next_capabilities_revision);
debug("%s %s",
status_response.status.to_string(),
capabilities.to_string());
capability(capabilities);
this.capabilities = response_code.get_capabilities(
this.capabilities.revision + 1
);
debug(
"%s set capabilities to: %s",
status_response.status.to_string(),
this.capabilities.to_string()
);
}
} catch (GLib.Error err) {
warning(
@ -1828,12 +1837,14 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
case ServerDataType.CAPABILITY:
// update ClientSession capabilities before firing signal, so external signal
// handlers that refer back to property aren't surprised
capabilities = server_data.get_capabilities(ref next_capabilities_revision);
debug("%s %s",
server_data.server_data_type.to_string(),
capabilities.to_string());
capability(capabilities);
this.capabilities = server_data.get_capabilities(
this.capabilities.revision + 1
);
debug(
"%s set capabilities to: %s",
server_data.server_data_type.to_string(),
this.capabilities.to_string()
);
break;
case ServerDataType.EXISTS:

View file

@ -87,6 +87,7 @@ geary_engine_vala_sources = files(
'imap/imap.vala',
'imap/imap-error.vala',
'imap/api/imap-account-session.vala',
'imap/api/imap-capabilities.vala',
'imap/api/imap-client-service.vala',
'imap/api/imap-email-flags.vala',
'imap/api/imap-email-properties.vala',
@ -150,7 +151,6 @@ geary_engine_vala_sources = files(
'imap/parameter/imap-root-parameters.vala',
'imap/parameter/imap-string-parameter.vala',
'imap/parameter/imap-unquoted-string-parameter.vala',
'imap/response/imap-capabilities.vala',
'imap/response/imap-continuation-response.vala',
'imap/response/imap-fetch-data-decoder.vala',
'imap/response/imap-fetched-data.vala',

View file

@ -32,39 +32,6 @@ public class Geary.GenericCapabilities : BaseObject {
return (map.size == 0);
}
public bool parse_and_add_capability(string text) {
string[] name_values = text.split(name_separator, 2);
switch (name_values.length) {
case 1:
add_capability(name_values[0]);
break;
case 2:
if (value_separator == null) {
add_capability(name_values[0], name_values[1]);
} else {
// break up second token for multiple values
string[] values = name_values[1].split(value_separator);
if (values.length <= 1) {
add_capability(name_values[0], name_values[1]);
} else {
foreach (string value in values)
add_capability(name_values[0], value);
}
}
break;
default:
return false;
}
return true;
}
public void add_capability(string name, string? setting = null) {
map.set(name, String.is_empty(setting) ? null : setting);
}
/**
* Returns true only if the capability was named as available by the server.
*/
@ -103,13 +70,6 @@ public class Geary.GenericCapabilities : BaseObject {
return (names.size > 0) ? names : null;
}
private void append(StringBuilder builder, string text) {
if (!String.is_empty(builder.str))
builder.append(String.is_empty(value_separator) ? " " : value_separator);
builder.append(text);
}
public virtual string to_string() {
Gee.Set<string>? names = get_all_names();
if (names == null || names.size == 0)
@ -132,5 +92,45 @@ public class Geary.GenericCapabilities : BaseObject {
return builder.str;
}
}
private inline void append(StringBuilder builder, string text) {
if (!String.is_empty(builder.str))
builder.append(String.is_empty(value_separator) ? " " : value_separator);
builder.append(text);
}
protected bool parse_and_add_capability(string text) {
string[] name_values = text.split(name_separator, 2);
switch (name_values.length) {
case 1:
add_capability(name_values[0]);
break;
case 2:
if (value_separator == null) {
add_capability(name_values[0], name_values[1]);
} else {
// break up second token for multiple values
string[] values = name_values[1].split(value_separator);
if (values.length <= 1) {
add_capability(name_values[0], name_values[1]);
} else {
foreach (string value in values)
add_capability(name_values[0], value);
}
}
break;
default:
return false;
}
return true;
}
private inline void add_capability(string name, string? setting = null) {
this.map.set(name, String.is_empty(setting) ? null : setting);
}
}

View file

@ -84,7 +84,7 @@ class Geary.ImapDB.AccountTest : TestCase {
new Imap.UIDValidity(7),
6 //unseen
),
new Imap.Capabilities(1)
new Imap.Capabilities.empty(0)
)
);
@ -123,7 +123,7 @@ class Geary.ImapDB.AccountTest : TestCase {
new Imap.UIDValidity(7),
6 //unseen
),
new Imap.Capabilities(1)
new Imap.Capabilities.empty(0)
)
);

View file

@ -15,12 +15,16 @@ class Geary.Imap.ClientSessionTest : TestCase {
public ClientSessionTest() {
base("Geary.Imap.ClientSessionTest");
add_test("connect_disconnect", connect_disconnect);
add_test("connect_with_capabilities", connect_with_capabilities);
if (GLib.Test.slow()) {
add_test("connect_timeout", connect_timeout);
}
add_test("login", login);
add_test("login_with_capabilities", login_with_capabilities);
add_test("logout", logout);
add_test("login_logout", login_logout);
add_test("initiate_request_capabilities", initiate_request_capabilities);
add_test("initiate_implicit_capabilities", initiate_implicit_capabilities);
}
protected override void set_up() throws GLib.Error {
@ -58,6 +62,27 @@ class Geary.Imap.ClientSessionTest : TestCase {
);
}
public void connect_with_capabilities() throws GLib.Error {
this.server.add_script_line(
SEND_LINE, "* OK [CAPABILITY IMAP4rev1] localhost test server ready"
);
this.server.add_script_line(WAIT_FOR_DISCONNECT, "");
var test_article = new ClientSession(new_endpoint());
test_article.connect_async.begin(
CONNECT_TIMEOUT, null, this.async_complete_full
);
test_article.connect_async.end(async_result());
assert_true(test_article.capabilities.supports_imap4rev1());
test_article.disconnect_async.begin(null, this.async_complete_full);
test_article.disconnect_async.end(async_result());
TestServer.Result result = this.server.wait_for_script(this.main_loop);
assert_true(result.succeeded);
}
public void connect_timeout() throws GLib.Error {
this.server.add_script_line(WAIT_FOR_DISCONNECT, "");
@ -72,13 +97,47 @@ class Geary.Imap.ClientSessionTest : TestCase {
test_article.connect_async.end(async_result());
assert_not_reached();
} catch (GLib.IOError.TIMED_OUT err) {
assert_double(timer.elapsed(), CONNECT_TIMEOUT, 0.5);
assert_double(timer.elapsed(), CONNECT_TIMEOUT, CONNECT_TIMEOUT * 0.5);
}
TestServer.Result result = this.server.wait_for_script(this.main_loop);
assert_true(result.succeeded);
}
public void login_with_capabilities() throws GLib.Error {
this.server.add_script_line(
SEND_LINE, "* OK localhost test server ready"
);
this.server.add_script_line(RECEIVE_LINE, "a001 login test password");
this.server.add_script_line(
SEND_LINE, "a001 OK [CAPABILITY IMAP4rev1] ohhai"
);
this.server.add_script_line(WAIT_FOR_DISCONNECT, "");
var test_article = new ClientSession(new_endpoint());
test_article.connect_async.begin(
CONNECT_TIMEOUT, null, this.async_complete_full
);
test_article.connect_async.end(async_result());
test_article.login_async.begin(
new Credentials(PASSWORD, "test", "password"),
null,
this.async_complete_full
);
test_article.login_async.end(async_result());
assert_true(test_article.capabilities.supports_imap4rev1());
test_article.disconnect_async.begin(null, this.async_complete_full);
test_article.disconnect_async.end(async_result());
TestServer.Result result = this.server.wait_for_script(this.main_loop);
assert_true(
result.succeeded,
result.error != null ? result.error.message : "Server result failed"
);
}
public void login() throws GLib.Error {
this.server.add_script_line(
SEND_LINE, "* OK localhost test server ready"
@ -183,6 +242,94 @@ class Geary.Imap.ClientSessionTest : TestCase {
);
}
public void initiate_request_capabilities() throws GLib.Error {
this.server.add_script_line(
SEND_LINE, "* OK localhost test server ready"
);
this.server.add_script_line(RECEIVE_LINE, "a001 capability");
this.server.add_script_line(SEND_LINE, "* CAPABILITY IMAP4rev1 LOGIN");
this.server.add_script_line(SEND_LINE, "a001 OK enjoy");
this.server.add_script_line(RECEIVE_LINE, "a002 login test password");
this.server.add_script_line(SEND_LINE, "a002 OK ohhai");
this.server.add_script_line(RECEIVE_LINE, "a003 capability");
this.server.add_script_line(SEND_LINE, "* CAPABILITY IMAP4rev1");
this.server.add_script_line(SEND_LINE, "a003 OK thanks");
this.server.add_script_line(RECEIVE_LINE, "a004 LIST \"\" INBOX");
this.server.add_script_line(SEND_LINE, "* LIST (\\HasChildren) \".\" INBOX");
this.server.add_script_line(SEND_LINE, "a004 OK there");
this.server.add_script_line(WAIT_FOR_DISCONNECT, "");
var test_article = new ClientSession(new_endpoint());
assert_true(test_article.get_protocol_state(null) == NOT_CONNECTED);
test_article.connect_async.begin(
CONNECT_TIMEOUT, null, this.async_complete_full
);
test_article.connect_async.end(async_result());
assert_true(test_article.get_protocol_state(null) == UNAUTHORIZED);
test_article.initiate_session_async.begin(
new Credentials(PASSWORD, "test", "password"),
null,
this.async_complete_full
);
test_article.initiate_session_async.end(async_result());
assert_true(test_article.capabilities.supports_imap4rev1());
assert_false(test_article.capabilities.has_capability("AUTH"));
assert_int(2, test_article.capabilities.revision);
test_article.disconnect_async.begin(null, this.async_complete_full);
test_article.disconnect_async.end(async_result());
TestServer.Result result = this.server.wait_for_script(this.main_loop);
assert_true(
result.succeeded,
result.error != null ? result.error.message : "Server result failed"
);
}
public void initiate_implicit_capabilities() throws GLib.Error {
this.server.add_script_line(
SEND_LINE, "* OK [CAPABILITY IMAP4rev1 LOGIN] localhost test server ready"
);
this.server.add_script_line(RECEIVE_LINE, "a001 login test password");
this.server.add_script_line(SEND_LINE, "a001 OK [CAPABILITY IMAP4rev1] ohhai");
this.server.add_script_line(RECEIVE_LINE, "a002 LIST \"\" INBOX");
this.server.add_script_line(SEND_LINE, "* LIST (\\HasChildren) \".\" INBOX");
this.server.add_script_line(SEND_LINE, "a002 OK there");
this.server.add_script_line(WAIT_FOR_DISCONNECT, "");
var test_article = new ClientSession(new_endpoint());
assert_true(test_article.get_protocol_state(null) == NOT_CONNECTED);
test_article.connect_async.begin(
CONNECT_TIMEOUT, null, this.async_complete_full
);
test_article.connect_async.end(async_result());
assert_true(test_article.get_protocol_state(null) == UNAUTHORIZED);
test_article.initiate_session_async.begin(
new Credentials(PASSWORD, "test", "password"),
null,
this.async_complete_full
);
test_article.initiate_session_async.end(async_result());
assert_true(test_article.capabilities.supports_imap4rev1());
assert_false(test_article.capabilities.has_capability("AUTH"));
assert_int(2, test_article.capabilities.revision);
test_article.disconnect_async.begin(null, this.async_complete_full);
test_article.disconnect_async.end(async_result());
TestServer.Result result = this.server.wait_for_script(this.main_loop);
assert_true(
result.succeeded,
result.error != null ? result.error.message : "Server result failed"
);
}
protected Endpoint new_endpoint() {
return new Endpoint(this.server.get_client_address(), NONE, 10);
}