Allow IMAP atoms to be terminated by an atom-special w/o needing a space.

Fixes Bug 781488.

* src/engine/imap/transport/imap-deserializer.vala (Deserializer): Update
  char handling in START_PARAM state to be more explict and to handle all
  possible chars, not just a subset. Then, simply defer to START_PARAM to
  determine what to do when an invalid atom char is encountered when in
  the ATOM state. Rename SYSTEM_FLAG state to just FLAG since it also
  handles extension flags, and only use it for handling the first char in
  the flag so "\*" can be treated appropriately, then defer to the ATOM
  state, so flag keywords are treated in the same way as regular
  atoms. Add unit tests.
This commit is contained in:
Michael James Gratton 2017-10-30 13:08:22 +11:00
parent 8adddddad3
commit 25e700d22a
2 changed files with 159 additions and 78 deletions

View file

@ -37,7 +37,7 @@ public class Geary.Imap.Deserializer : BaseObject {
TAG,
START_PARAM,
ATOM,
SYSTEM_FLAG,
FLAG,
QUOTED,
QUOTED_ESCAPE,
PARTIAL_BODY_ATOM,
@ -84,7 +84,6 @@ public class Geary.Imap.Deserializer : BaseObject {
private Geary.Memory.GrowableBuffer? block_buffer = null;
private unowned uint8[]? current_buffer = null;
private int ins_priority = Priority.DEFAULT;
private char[] atom_specials_exceptions = { ' ', ' ', '\0' };
/**
@ -163,10 +162,10 @@ public class Geary.Imap.Deserializer : BaseObject {
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.SYSTEM_FLAG, Event.CHAR, on_system_flag_char),
new Geary.State.Mapping(State.SYSTEM_FLAG, Event.EOL, on_atom_eol),
new Geary.State.Mapping(State.SYSTEM_FLAG, Event.EOS, on_eos),
new Geary.State.Mapping(State.SYSTEM_FLAG, Event.ERROR, on_error),
new Geary.State.Mapping(State.FLAG, Event.CHAR, on_first_flag_char),
new Geary.State.Mapping(State.FLAG, Event.EOL, on_atom_eol),
new Geary.State.Mapping(State.FLAG, Event.EOS, on_eos),
new Geary.State.Mapping(State.FLAG, Event.ERROR, on_error),
new Geary.State.Mapping(State.QUOTED, Event.CHAR, on_quoted_char),
new Geary.State.Mapping(State.QUOTED, Event.EOS, on_eos),
@ -540,29 +539,52 @@ public class Geary.Imap.Deserializer : BaseObject {
// open response code
ResponseCode response_code = new ResponseCode();
push(response_code);
return State.START_PARAM;
case ']':
if (ch == get_current_context_terminator()) {
return pop();
}
// Received an unexpected closing brace
return State.FAILED;
case '{':
return State.LITERAL;
case '\"':
return State.QUOTED;
case '(':
// open list
ListParameter list = new ListParameter();
push(list);
return State.START_PARAM;
default:
// if current context's terminator, close the context, otherwise deserializer is
// now "in" an Atom
if (ch == get_current_context_terminator())
case ')':
if (ch == get_current_context_terminator()) {
return pop();
else
return on_atom_char(state, event, user);
}
// Received an unexpected closing parens
return State.FAILED;
case '\\':
// Start of a flag
append_to_string(ch);
return State.FLAG;
case ' ':
// just consume the space
return State.START_PARAM;
default:
if (!DataFormat.is_atom_special(ch)) {
append_to_string(ch);
return State.ATOM;
}
// Received an invalid atom-char
return State.FAILED;
}
}
@ -585,86 +607,70 @@ public class Geary.Imap.Deserializer : BaseObject {
return State.TAG;
}
private uint on_atom_char(uint state, uint event, void *user) {
char ch = *((char *) user);
// The partial body fetch results ("BODY[section]" or "BODY[section]<partial>" and their
// .peek variants) offer so many exceptions to the decoding process they're given their own
// state
if (ch == '[' && (is_current_string_ci("body") || is_current_string_ci("body.peek"))) {
append_to_string(ch);
return State.PARTIAL_BODY_ATOM;
}
// get the terminator for this context and re-use the atom_special_exceptions array to
// pass to DataFormat.is_atom_special() (this means not allocating a new array on the heap
// for each call here, which isn't a problem because the FSM is non-reentrant)
char terminator = get_current_context_terminator();
atom_specials_exceptions[1] = terminator;
// drop if not allowed for atoms, barring specials which indicate special state changes
if (DataFormat.is_atom_special(ch, (string) atom_specials_exceptions))
return State.ATOM;
// message flag indicator is only legal at start of atom
if (ch == '\\' && is_current_string_empty()) {
append_to_string(ch);
return State.SYSTEM_FLAG;
}
// space indicates end-of-atom
// space indicates end-of-atom, consume it and return to go.
if (ch == ' ') {
save_string_parameter(false);
return State.START_PARAM;
}
if (ch == get_current_context_terminator()) {
save_string_parameter(false);
return pop();
}
append_to_string(ch);
return State.ATOM;
}
private uint on_system_flag_char(uint state, uint event, void *user) {
char ch = *((char *) user);
// see note in on_atom_char for why/how this works
// as does an end-of-list delim
char terminator = get_current_context_terminator();
atom_specials_exceptions[1] = terminator;
// drop if not allowed for atoms, barring specials which indicate state changes
// note that asterisk is allowed for flags
if (ch != '*' && DataFormat.is_atom_special(ch, (string) atom_specials_exceptions))
return State.SYSTEM_FLAG;
// space indicates end-of-system-flag
if (ch == ' ') {
save_string_parameter(false);
return State.START_PARAM;
}
// close-parens/close-square-bracket after a system flag indicates end-of-list/end-of-response
// code
if (ch == terminator) {
save_string_parameter(false);
return pop();
}
// Any of the atom specials indicate end of atom, so save and
// work out where to go next
if (DataFormat.is_atom_special(ch)) {
save_string_parameter(false);
return on_first_param_char(state, event, user);
}
append_to_string(ch);
return State.SYSTEM_FLAG;
return State.ATOM;
}
// Although flags basically have the form "\atom", the special
// flag "\*" isn't a valid atom, and hence we need a special case
// for flag parsing. This is only used for the first char after
// the '\', since all following chars must be valid for atoms
private uint on_first_flag_char(uint state, uint event, void *user) {
char ch = *((char *) user);
// handle special flag "\*", this also indicates end-of-flag
if (ch == '*') {
append_to_string(ch);
save_string_parameter(false);
return State.START_PARAM;
}
// Any of the atom specials indicate end of atom, but we are
// at the first char after a "\" and a flag has to have at
// least one atom char to be valid, so if we get an
// atom-special here bail out.
if (DataFormat.is_atom_special(ch)) {
return State.FAILED;
}
// Append the char, but then switch to the ATOM state, since
// '*' is no longer valid and the remainder must be an atom
append_to_string(ch);
return State.ATOM;
}
private uint on_eol(uint state, uint event, void *user) {
return flush_params();
}

View file

@ -19,6 +19,14 @@ class Geary.Imap.DeserializerTest : Gee.TestCase {
public DeserializerTest() {
base("Geary.Imap.DeserializerTest");
add_test("test_gmail_greeting", test_gmail_greeting);
add_test("test_cyrus_2_4_greeting", test_cyrus_2_4_greeting);
add_test("test_aliyun_greeting", test_aliyun_greeting);
add_test("test_gmail_flags", test_gmail_flags);
add_test("test_gmail_permanent_flags", test_gmail_permanent_flags);
add_test("test_cyrus_flags", test_cyrus_flags);
}
public override void set_up() {
@ -31,6 +39,73 @@ class Geary.Imap.DeserializerTest : Gee.TestCase {
async_result();
}
public void test_gmail_greeting() {
string greeting = "* OK Gimap ready for requests from 115.187.245.46 c194mb399904375ivc";
this.stream.add_data(greeting.data);
this.stream.add_data(EOL.data);
this.process.begin(Expect.MESSAGE, (obj, ret) => { async_complete(ret); });
RootParameters? message = this.process.end(async_result());
assert(message.to_string() == greeting);
}
public void test_cyrus_2_4_greeting() {
string greeting = "* OK [CAPABILITY IMAP4rev1 LITERAL+ ID ENABLE AUTH=PLAIN SASL-IR] mogul Cyrus IMAP v2.4.12-Debian-2.4.12-2 server ready";
this.stream.add_data(greeting.data);
this.stream.add_data(EOL.data);
this.process.begin(Expect.MESSAGE, (obj, ret) => { async_complete(ret); });
RootParameters? message = this.process.end(async_result());
assert(message.to_string() == greeting);
}
public void test_aliyun_greeting() {
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, (obj, ret) => { async_complete(ret); });
RootParameters? message = this.process.end(async_result());
assert(message.to_string() == parsed);
}
public void test_gmail_flags() {
string flags = """* FLAGS (\Answered \Flagged \Draft \Deleted \Seen $NotPhishing $Phishing)""";
this.stream.add_data(flags.data);
this.stream.add_data(EOL.data);
this.process.begin(Expect.MESSAGE, (obj, ret) => { async_complete(ret); });
RootParameters? message = this.process.end(async_result());
assert(message.to_string() == flags);
}
public void test_gmail_permanent_flags() {
string flags = """* OK [PERMANENTFLAGS (\Answered \Flagged \Draft \Deleted \Seen $NotPhishing $Phishing \*)] Flags permitted.""";
this.stream.add_data(flags.data);
this.stream.add_data(EOL.data);
this.process.begin(Expect.MESSAGE, (obj, ret) => { async_complete(ret); });
RootParameters? message = this.process.end(async_result());
assert(message.to_string() == flags);
}
public void test_cyrus_flags() {
string flags = """* 2934 FETCH (FLAGS (\Answered \Seen $Quuxo::Spam::Trained) UID 3041)""";
this.stream.add_data(flags.data);
this.stream.add_data(EOL.data);
this.process.begin(Expect.MESSAGE, (obj, ret) => { async_complete(ret); });
RootParameters? message = this.process.end(async_result());
assert(message.to_string() == flags);
}
protected async RootParameters? process(Expect expected) {
RootParameters? message = null;
bool eos = false;