Commit 29ae18b3 authored by Jim Nelson's avatar Jim Nelson

In Non-English locales, Geary displays wrong month: Closes #7354

Stricter parsing and application of INTERNALDATE.
parent 3a508576
......@@ -14,3 +14,4 @@ install(FILES version-011.sql DESTINATION ${SQL_DEST})
install(FILES version-012.sql DESTINATION ${SQL_DEST})
install(FILES version-013.sql DESTINATION ${SQL_DEST})
install(FILES version-014.sql DESTINATION ${SQL_DEST})
install(FILES version-015.sql DESTINATION ${SQL_DEST})
--
-- Dummy database upgrade to fix the INTERNALDATE of messages that were accidentally stored in
-- localized format. See src/engine/imap-db/imap-db-database.vala in post_upgrade() for the code
-- that runs the upgrade, and http://redmine.yorba.org/issues/7354 for more information.
--
......@@ -285,6 +285,7 @@ engine/util/util-single-item.vala
engine/util/util-stream.vala
engine/util/util-string.vala
engine/util/util-synchronization.vala
engine/util/util-time.vala
engine/util/util-trillian.vala
)
......
......@@ -144,7 +144,7 @@ private void on_log(string prefix, LogLevelFlags log_levels, string message) {
if (stream == null)
return;
Time tm = Time.local(time_t());
GLib.Time tm = GLib.Time.local(time_t());
stream.printf("%s %02d:%02d:%02d %lf %s\n", prefix, tm.hour, tm.minute, tm.second,
entry_timer.elapsed(), message);
......
......@@ -170,7 +170,7 @@ public class Geary.Db.Connection : Geary.Db.Context {
* in SQLite, however, includes 1 and 0, so an integer may be mistaken as a boolean.
*/
public bool get_pragma_bool(string name) throws Error {
string response = query("PRAGMA %s".printf(name)).string_at(0);
string response = query("PRAGMA %s".printf(name)).nonnull_string_at(0);
switch (response.down()) {
case "1":
case "yes":
......@@ -221,7 +221,7 @@ public class Geary.Db.Connection : Geary.Db.Context {
* Returns the result of a PRAGMA as a string. See [[http://www.sqlite.org/pragma.html]]
*/
public string get_pragma_string(string name) throws Error {
return query("PRAGMA %s".printf(name)).string_at(0);
return query("PRAGMA %s".printf(name)).nonnull_string_at(0);
}
/**
......
......@@ -126,16 +126,33 @@ public class Geary.Db.Result : Geary.Db.Context {
/**
* column is zero-based.
*
* Returns a null string if the element is NULL.
*
* @see nonnull_string_at
*/
public unowned string string_at(int column) throws DatabaseError {
public unowned string? string_at(int column) throws DatabaseError {
verify_at(column);
unowned string s = statement.stmt.column_text(column);
log("string_at(%d) -> %s", column, s);
unowned string? s = statement.stmt.column_text(column);
log("string_at(%d) -> %s", column, (s != null) ? s : "(null)");
return s;
}
/**
* column is zero-based.
*
* Returns an empty string if the element is NULL.
*
* @see string_at
*/
public unowned string nonnull_string_at(int column) throws DatabaseError {
unowned string? s = string_at(column);
return (s != null) ? s : "";
}
/**
* column is zero-based.
*/
......@@ -143,7 +160,7 @@ public class Geary.Db.Result : Geary.Db.Context {
// Memory.StringBuffer is not entirely suited for this, as it can result in extra copies
// internally ... GrowableBuffer is better for large blocks
Memory.GrowableBuffer buffer = new Memory.GrowableBuffer();
buffer.append(string_at(column).data);
buffer.append(nonnull_string_at(column).data);
return buffer;
}
......@@ -231,11 +248,27 @@ public class Geary.Db.Result : Geary.Db.Context {
/**
* name is the name of the column in the result set. See Statement.get_column_index() for name
* matching rules.
*
* Returns a null string if the element is NULL.
*
* @see nonnull_string_for
*/
public unowned string string_for(string name) throws DatabaseError {
public unowned string? string_for(string name) throws DatabaseError {
return string_at(convert_for(name));
}
/**
* name is the name of the column in the result set. See Statement.get_column_index() for name
* matching rules.
*
* Returns an empty string if the element is NULL.
*
* @see string_for
*/
public unowned string nonnull_string_for(string name) throws DatabaseError {
return nonnull_string_at(convert_for(name));
}
/**
* name is the name of the column in the result set. See Statement.get_column_index() for name
* matching rules.
......
......@@ -331,7 +331,7 @@ private class Geary.ImapDB.Account : BaseObject {
Db.Result result = statement.exec(cancellable);
while (!result.finished) {
try {
Contact contact = new Contact(result.string_at(0), result.string_at(1),
Contact contact = new Contact(result.nonnull_string_at(0), result.string_at(1),
result.int_at(2), result.string_at(3), ContactFlags.deserialize(result.string_at(4)));
contacts.add(contact);
} catch (Geary.DatabaseError err) {
......@@ -823,7 +823,7 @@ private class Geary.ImapDB.Account : BaseObject {
Db.Result result = stmt.exec(cancellable);
while (!result.finished) {
// Build a list of search offsets.
string[] offset_array = result.string_at(0).split(" ");
string[] offset_array = result.nonnull_string_at(0).split(" ");
Gee.ArrayList<SearchOffset> all_offsets = new Gee.ArrayList<SearchOffset>();
int j = 0;
while (true) {
......@@ -837,7 +837,7 @@ private class Geary.ImapDB.Account : BaseObject {
// Iterate over the offset list, scrape strings from the database, and push
// the results into our return set.
foreach(SearchOffset offset in all_offsets) {
string text = result.string_at(offset.column + 1);
string text = result.nonnull_string_at(offset.column + 1);
search_matches.add(text[offset.byte_offset : offset.byte_offset + offset.size].down());
}
......@@ -1222,7 +1222,7 @@ private class Geary.ImapDB.Account : BaseObject {
return null;
int64 parent_id = result.int64_at(0);
string name = result.string_at(1);
string name = result.nonnull_string_at(1);
// Here too, one level of loop detection is better than nothing.
if (folder_id == parent_id) {
......
......@@ -87,6 +87,10 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
case 14:
post_upgrade_expand_page_size();
break;
case 15:
post_upgrade_fix_localized_internaldates();
break;
}
}
......@@ -114,7 +118,7 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
Db.Result select = query("SELECT id, name FROM FolderTable");
while (!select.finished) {
int64 id = select.int64_at(0);
string encoded_name = select.string_at(1);
string encoded_name = select.nonnull_string_at(1);
try {
string canonical_name = Geary.ImapUtf7.imap_utf7_to_utf8(encoded_name);
......@@ -207,7 +211,7 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
try {
time_t as_time_t = (internaldate != null ?
new Geary.Imap.InternalDate(internaldate).as_time_t : -1);
Geary.Imap.InternalDate.decode(internaldate).as_time_t : -1);
Db.Statement update = cx.prepare(
"UPDATE MessageTable SET internaldate_time_t=? WHERE id=?");
......@@ -301,6 +305,65 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
}
}
// Version 15
private void post_upgrade_fix_localized_internaldates() {
try {
exec_transaction(Db.TransactionType.RW, (cx) => {
Db.Statement stmt = cx.prepare("""
SELECT id, internaldate, fields
FROM MessageTable
""");
Gee.HashMap<int64?, Geary.Email.Field> invalid_ids = new Gee.HashMap<
int64?, Geary.Email.Field>();
Db.Result results = stmt.exec();
while (!results.finished) {
string? internaldate = results.string_at(1);
try {
if (!String.is_empty(internaldate))
Imap.InternalDate.decode(internaldate);
} catch (Error err) {
int64 invalid_id = results.rowid_at(0);
debug("Invalid INTERNALDATE \"%s\" found at row %s in %s: %s",
internaldate != null ? internaldate : "(null)",
invalid_id.to_string(), db_file.get_path(), err.message);
invalid_ids.set(invalid_id, (Geary.Email.Field) results.int_at(2));
}
results.next();
}
// used prepared statement for iterating over list
stmt = cx.prepare("""
UPDATE MessageTable
SET fields=?, internaldate=?, internaldate_time_t=?, rfc822_size=?
WHERE id=?
""");
stmt.bind_null(1);
stmt.bind_null(2);
stmt.bind_null(3);
foreach (int64 invalid_id in invalid_ids.keys) {
stmt.bind_int(0, invalid_ids.get(invalid_id).clear(Geary.Email.Field.PROPERTIES));
stmt.bind_rowid(4, invalid_id);
stmt.exec();
// reuse statment, overwrite invalid_id, fields only
stmt.reset(Db.ResetScope.SAVE_BINDINGS);
}
return Db.TransactionOutcome.COMMIT;
});
} catch (Error err) {
debug("Error fixing INTERNALDATES during upgrade to schema 15 for %s: %s",
db_file.get_path(), err.message);
}
}
private void on_prepare_database_connection(Db.Connection cx) throws Error {
cx.set_busy_timeout_msec(Db.Connection.RECOMMENDED_BUSY_TIMEOUT_MSEC);
cx.set_foreign_keys(true);
......
......@@ -1854,7 +1854,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
Gee.List<Geary.Attachment> list = new Gee.ArrayList<Geary.Attachment>();
do {
list.add(new ImapDB.Attachment(cx.database.db_file.get_parent(), results.string_at(1),
results.string_at(2), results.int64_at(3), message_id, results.rowid_at(0),
results.nonnull_string_at(2), results.int64_at(3), message_id, results.rowid_at(0),
Geary.Attachment.Disposition.from_int(results.int_at(4))));
} while (results.next(cancellable));
......
......@@ -154,7 +154,7 @@ private class Geary.ImapDB.MessageRow {
Imap.InternalDate? constructed = null;
try {
constructed = new Imap.InternalDate(internaldate);
constructed = Imap.InternalDate.decode(internaldate);
} catch (Error err) {
debug("Unable to construct internaldate object from \"%s\": %s", internaldate,
err.message);
......
......@@ -7,26 +7,89 @@
/**
* A representations of IMAP's INTERNALDATE field.
*
* INTERNALDATE's format is
*
* dd-Mon-yyyy hh:mm:ss +hhmm
*
* Note that Mon is the standard ''English'' three-letter abbreviation.
*
* See [[http://tools.ietf.org/html/rfc3501#section-2.3.3]]
*/
public class Geary.Imap.InternalDate : Geary.MessageData.AbstractMessageData, Geary.Imap.MessageData,
Gee.Hashable<InternalDate>, Gee.Comparable<InternalDate> {
// see get_en_us_mon() for explanation
private const string[] EN_US_MON = {
"Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
};
private const string[] EN_US_MON_DOWN = {
"jan", "feb", "mar", "apr", "may", "jun", "jul", "aug", "sep", "oct", "nov", "dec"
};
public DateTime value { get; private set; }
public time_t as_time_t { get; private set; }
public string? original { get; private set; default = null; }
public InternalDate(string internaldate) throws ImapError {
as_time_t = GMime.utils_header_decode_date(internaldate, null);
if (as_time_t == 0) {
throw new ImapError.PARSE_ERROR("Unable to parse \"%s\": not INTERNALDATE format",
internaldate);
}
value = new DateTime.from_unix_local(as_time_t);
private InternalDate(string original, DateTime datetime) {
this.original = original;
value = datetime;
as_time_t = Time.datetime_to_time_t(datetime);
}
public InternalDate.from_date_time(DateTime datetime) throws ImapError {
value = datetime;
as_time_t = Time.datetime_to_time_t(datetime);
}
public static InternalDate decode(string internaldate) throws ImapError {
if (String.is_empty(internaldate))
throw new ImapError.PARSE_ERROR("Invalid INTERNALDATE: empty string");
if (internaldate.length > 64)
throw new ImapError.PARSE_ERROR("Invalid INTERNALDATE: too long (%d)", internaldate.length);
// Alas, GMime.utils_header_decode_date() is too forgiving for our needs, so do it manually
int day, year, hour, min, sec;
char mon[4];
char tz[6];
int count = internaldate.scanf("%d-%3s-%d %d:%d:%d %5s", out day, mon, out year, out hour,
out min, out sec, tz);
if (count != 7)
throw new ImapError.PARSE_ERROR("Invalid INTERNALDATE \"%s\": too few fields (%d)", internaldate, count);
// check numerical ranges; this does not verify this is an actual date, DateTime will do
// that (and round upward, which has to be accepted)
if (!Numeric.int_in_range_inclusive(day, 1, 31)
|| !Numeric.int_in_range_inclusive(hour, 0, 23)
|| !Numeric.int_in_range_inclusive(min, 0, 59)
|| !Numeric.int_in_range_inclusive(sec, 0, 59)
|| year < 1970) {
throw new ImapError.PARSE_ERROR("Invalid INTERNALDATE \"%s\": bad numerical range", internaldate);
}
// check month (this catches localization problems)
int month = -1;
string mon_down = ((string) mon).down();
for (int ctr = 0; ctr < EN_US_MON_DOWN.length; ctr++) {
if (mon_down == EN_US_MON_DOWN[ctr]) {
month = ctr;
break;
}
}
if (month < 0)
throw new ImapError.PARSE_ERROR("Invalid INTERNALDATE \"%s\": bad month", internaldate);
// TODO: verify timezone
// assemble into DateTime, which validates the time as well (this is why we want to keep
// original around, for other reasons) ... month is 1-based in DateTime
DateTime datetime = new DateTime(new TimeZone((string) tz), year, month + 1, day, hour, min,
sec);
return new InternalDate(internaldate, datetime);
}
/**
......@@ -51,19 +114,32 @@ public class Geary.Imap.InternalDate : Geary.MessageData.AbstractMessageData, Ge
* @see serialize_for_search
*/
public string serialize() {
return value.format("%d-%b-%Y %H:%M:%S %z");
return original ?? value.format("%d-%%s-%Y %H:%M:%S %z").printf(get_en_us_mon());
}
/**
* Returns the {@link InternalDate}'s string representation for a SEARCH function.
* Returns the {@link InternalDate}'s string representation for a SEARCH command.
*
* SEARCH does not respect time or timezone, so drop when sending it. See
* [[http://tools.ietf.org/html/rfc3501#section-6.4.4]]
*
* @see serialize
* @see SearchCommand
*/
public string serialize_for_search() {
return value.format("%d-%b-%Y");
return value.format("%d-%%s-%Y").printf(get_en_us_mon());
}
/**
* Because IMAP's INTERNALDATE strings are ''never'' localized (as best as I can gather), so
* need to use en_US appreviated month names, as that's the only value in INTERNALDATE that is
* in a language and not a numeric value.
*/
private string get_en_us_mon() {
// month is 1-based inside of DateTime
int mon = (value.get_month() - 1).clamp(0, EN_US_MON.length - 1);
return EN_US_MON[mon];
}
public uint hash() {
......
......@@ -109,7 +109,7 @@ public class Geary.Imap.InternalDateDecoder : Geary.Imap.FetchDataDecoder {
}
protected override MessageData decode_string(StringParameter stringp) throws ImapError {
return new InternalDate(stringp.value);
return InternalDate.decode(stringp.value);
}
}
......
/* Copyright 2013 Yorba Foundation
*
* This software is licensed under the GNU Lesser General Public License
* (version 2.1 or later). See the COPYING file in this distribution.
*/
namespace Geary.Time {
/**
* Converts a DateTime object into the nearest approximation of time_t.
*
* Since DateTime can store down to the microsecond and dates before UNIX epoch, there's some
* truncating going on here.
*/
public time_t datetime_to_time_t(DateTime datetime) {
GLib.Time tm = GLib.Time();
tm.second = datetime.get_second();
tm.minute = datetime.get_minute();
tm.hour = datetime.get_hour();
tm.day = datetime.get_day_of_month();
// month is 1-based in DateTime
tm.month = Numeric.int_floor(datetime.get_month() - 1, 0);
// Time's year is number of years after 1900
tm.year = Numeric.int_floor(datetime.get_year() - 1900, 1900);
tm.isdst = datetime.is_daylight_savings() ? 1 : 0;
return tm.mktime();
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment