From ba861b8ed1a5a87995abb6721e6f401e3ef888a1 Mon Sep 17 00:00:00 2001 From: Michael James Gratton Date: Thu, 17 May 2018 15:41:01 +1000 Subject: [PATCH] Fix failing RFC822.Mailbox test. Reverting commit 1d8c4aea broke the mailbox class a bit. This fixes the issue, introduces Ascii.last_index_of which is needed by Mailbox anyway, and adds some test for it. Also 'optimises' Ascii.index_of bit as well. --- src/engine/rfc822/rfc822-mailbox-address.vala | 8 ++-- src/engine/util/util-ascii.vala | 39 +++++++++++----- test/CMakeLists.txt | 1 + test/engine/util-ascii-test.vala | 44 +++++++++++++++++++ test/meson.build | 1 + test/test-engine.vala | 1 + 6 files changed, 79 insertions(+), 15 deletions(-) create mode 100644 test/engine/util-ascii-test.vala diff --git a/src/engine/rfc822/rfc822-mailbox-address.vala b/src/engine/rfc822/rfc822-mailbox-address.vala index dcc55b9f..ff5174a5 100644 --- a/src/engine/rfc822/rfc822-mailbox-address.vala +++ b/src/engine/rfc822/rfc822-mailbox-address.vala @@ -127,7 +127,7 @@ public class Geary.RFC822.MailboxAddress : this.source_route = null; this.address = address; - int atsign = Ascii.index_of(address, '@'); + int atsign = Ascii.last_index_of(address, '@'); if (atsign > 0) { this.mailbox = address[0:atsign]; this.domain = address[atsign + 1:address.length]; @@ -136,7 +136,7 @@ public class Geary.RFC822.MailboxAddress : this.domain = ""; } } - + public MailboxAddress.imap(string? name, string? source_route, string mailbox, string domain) { this.name = (name != null) ? decode_name(name) : null; this.source_route = source_route; @@ -173,12 +173,12 @@ public class Geary.RFC822.MailboxAddress : } string address = mailbox.get_addr(); - int atsign = address.last_index_of_char('@'); + int atsign = Ascii.last_index_of(address, '@'); if (atsign == -1) { // No @ detected, try decoding in case a mailer (wrongly) // encoded the whole thing and re-try address = decode_address_part(address); - atsign = address.last_index_of_char('@'); + atsign = Ascii.last_index_of(address, '@'); } if (atsign >= 0) { diff --git a/src/engine/util/util-ascii.vala b/src/engine/util/util-ascii.vala index 4d915f68..2457e9d5 100644 --- a/src/engine/util/util-ascii.vala +++ b/src/engine/util/util-ascii.vala @@ -12,19 +12,36 @@ extern string g_ascii_strdown(string str, ssize_t len = -1); namespace Geary.Ascii { public int index_of(string str, char ch) { + // Use a pointer and explicit null check, since testing against + // the length of the string as in a traditional for loop will mean + // a call to strlen(), making the loop O(n^2) + int ret = -1; char *strptr = str; - int index = 0; - for (;;) { - char strch = *strptr++; - - if (strch == String.EOS) - return -1; - - if (strch == ch) - return index; - - index++; + int i = 0; + while (*strptr != String.EOS) { + if (*strptr++ == ch) { + ret = i; + break; + } + i++; } + return ret; +} + +public int last_index_of(string str, char ch) { + // Use a pointer and explicit null check, since testing against + // the length of the string as in a traditional for loop will mean + // a call to strlen(), making the loop O(n^2) + int ret = -1; + char *strptr = str; + int i = 0; + while (*strptr != String.EOS) { + if (*strptr++ == ch) { + ret = i; + } + i++; + } + return ret; } public bool get_next_char(string str, ref int index, out char ch) { diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index f1ad286e..381c87c7 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -36,6 +36,7 @@ set(TEST_ENGINE_SRC engine/rfc822-message-test.vala engine/rfc822-message-data-test.vala engine/rfc822-utils-test.vala + engine/util-ascii-test.vala engine/util-html-test.vala engine/util-idle-manager-test.vala engine/util-inet-test.vala diff --git a/test/engine/util-ascii-test.vala b/test/engine/util-ascii-test.vala new file mode 100644 index 00000000..483e030a --- /dev/null +++ b/test/engine/util-ascii-test.vala @@ -0,0 +1,44 @@ +/* + * Copyright 2018 Michael Gratton + * + * This software is licensed under the GNU Lesser General Public License + * (version 2.1 or later). See the COPYING file in this distribution. + */ + +class Geary.Ascii.Test : TestCase { + + public Test() { + base("Geary.Ascii.Test"); + add_test("index_of", index_of); + add_test("last_index_of", last_index_of); + } + + public void index_of() throws Error { + assert_int(-1, Ascii.index_of("", 'a')); + assert_int(0, Ascii.index_of("a", 'a')); + assert_int(0, Ascii.index_of("aa", 'a')); + + assert_int(0, Ascii.index_of("abcabc", 'a')); + assert_int(1, Ascii.index_of("abcabc", 'b')); + assert_int(2, Ascii.index_of("abcabc", 'c')); + + assert_int(0, Ascii.index_of("@", '@')); + + assert_int(-1, Ascii.index_of("abc", 'd')); + } + + public void last_index_of() throws Error { + assert_int(-1, Ascii.last_index_of("", 'a')); + assert_int(0, Ascii.last_index_of("a", 'a')); + assert_int(1, Ascii.last_index_of("aa", 'a')); + + assert_int(3, Ascii.last_index_of("abcabc", 'a')); + assert_int(4, Ascii.last_index_of("abcabc", 'b')); + assert_int(5, Ascii.last_index_of("abcabc", 'c')); + + assert_int(0, Ascii.last_index_of("@", '@')); + + assert_int(-1, Ascii.last_index_of("abc", 'd')); + } + +} diff --git a/test/meson.build b/test/meson.build index b844d0de..e74841c4 100644 --- a/test/meson.build +++ b/test/meson.build @@ -32,6 +32,7 @@ geary_test_engine_sources = [ 'engine/rfc822-message-test.vala', 'engine/rfc822-message-data-test.vala', 'engine/rfc822-utils-test.vala', + 'engine/util-ascii-test.vala', 'engine/util-html-test.vala', 'engine/util-idle-manager-test.vala', 'engine/util-inet-test.vala', diff --git a/test/test-engine.vala b/test/test-engine.vala index ab4a0efa..06349aef 100644 --- a/test/test-engine.vala +++ b/test/test-engine.vala @@ -30,6 +30,7 @@ int main(string[] args) { engine.add_suite(new Geary.App.ConversationSetTest().get_suite()); // Depends on ConversationTest and ConversationSetTest passing engine.add_suite(new Geary.App.ConversationMonitorTest().get_suite()); + engine.add_suite(new Geary.Ascii.Test().get_suite()); engine.add_suite(new Geary.HTML.UtilTest().get_suite()); engine.add_suite(new Geary.Imap.DeserializerTest().get_suite()); engine.add_suite(new Geary.Imap.CreateCommandTest().get_suite());