From edc1d23ef520871b9585ed5d8fdced9ee8b98594 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Mon, 23 Mar 2015 12:41:49 -0400 Subject: [bug] add extra CRLF to avoid bad mime parsing in Thunderbird Thunderbird (as of 37.0b1) will display a blank body (with no attachments) if some conditions are met: * disk synchronization is disabled * mime_part_on_demand = true * msg size is bigger than the parts_on_demand threshold (30000 by default). Comparing the logs with a well behaved imap server (dovecot, on this case), it's easy to see that twisted implementation is lacking an extra line separator at the end of each group of headers that is rendered in response to each of the `BODY.PEEK[X.MIME]` command that the mime_parts_on_demand will issue after getting the BODYSTRUCTURE. This change patches the spew_body command on the body server. We still would have to see if this is a bad behaviour in the thunderbird side. The most similar bug I've found is: https://bugzilla.mozilla.org/show_bug.cgi?id=149771 Which apparently was happening with exchange server. We should send the patch to upstream twisted as well. Note that this fix is not enough: the following commit, about fixing the case of the boundary passed in the BODYSTRUCTURE response is also needed to fix the bug (since a bad parsing happens all the same). Resolves: #6773, #5010 Documentation: #6773 Releases: 0.4.0 --- mail/src/leap/mail/imap/server.py | 61 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/mail/src/leap/mail/imap/server.py b/mail/src/leap/mail/imap/server.py index 3e10171..26d2001 100644 --- a/mail/src/leap/mail/imap/server.py +++ b/mail/src/leap/mail/imap/server.py @@ -60,6 +60,67 @@ class LEAPIMAPServer(imap4.IMAP4Server): # populate the test account properly (and only once # per session) + ############################################################# + # + # Twisted imap4 patch to workaround bad mime rendering in TB. + # See https://leap.se/code/issues/6773 + # and https://bugzilla.mozilla.org/show_bug.cgi?id=149771 + # Still unclear if this is a thunderbird bug. + # TODO send this patch upstream + # + ############################################################# + + def spew_body(self, part, id, msg, _w=None, _f=None): + if _w is None: + _w = self.transport.write + for p in part.part: + if msg.isMultipart(): + msg = msg.getSubPart(p) + elif p > 0: + # Non-multipart messages have an implicit first part but no + # other parts - reject any request for any other part. + raise TypeError("Requested subpart of non-multipart message") + + if part.header: + hdrs = msg.getHeaders(part.header.negate, *part.header.fields) + hdrs = imap4._formatHeaders(hdrs) + _w(str(part) + ' ' + imap4._literal(hdrs)) + elif part.text: + _w(str(part) + ' ') + _f() + return imap4.FileProducer(msg.getBodyFile() + ).beginProducing(self.transport + ) + elif part.mime: + hdrs = imap4._formatHeaders(msg.getHeaders(True)) + + ###### PATCHED ##################################### + _w(str(part) + ' ' + imap4._literal(hdrs + "\r\n")) + ###### END PATCHED ################################# + + elif part.empty: + _w(str(part) + ' ') + _f() + if part.part: + return imap4.FileProducer(msg.getBodyFile() + ).beginProducing(self.transport + ) + else: + mf = imap4.IMessageFile(msg, None) + if mf is not None: + return imap4.FileProducer(mf.open()).beginProducing(self.transport) + return imap4.MessageProducer(msg, None, self._scheduler).beginProducing(self.transport) + + else: + _w('BODY ' + imap4.collapseNestedLists([imap4.getBodyStructure(msg)])) + + ################################################################## + # + # END Twisted imap4 patch to workaround bad mime rendering in TB. + # #6773 + # + ################################################################## + def lineReceived(self, line): """ Attempt to parse a single line from the server. -- cgit v1.2.3 From 62a02b3b1e14afcbec82d3fca2f906ca6dd38715 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Mon, 23 Mar 2015 14:39:39 -0400 Subject: [bug] fix wrong case in the boundary passed in BODYSTRUCTURE By removing this call to lower(), we avoid a bug in which the BODYSTRUCTURE response returns a boundary all in lower case. Should send patch upstream to twisted. Related: #6773 --- mail/src/leap/mail/imap/server.py | 40 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/mail/src/leap/mail/imap/server.py b/mail/src/leap/mail/imap/server.py index 26d2001..3aeca54 100644 --- a/mail/src/leap/mail/imap/server.py +++ b/mail/src/leap/mail/imap/server.py @@ -36,6 +36,38 @@ from twisted.mail.imap4 import IllegalClientResponse from twisted.mail.imap4 import LiteralString, LiteralFile +def _getContentType(msg): + """ + Return a two-tuple of the main and subtype of the given message. + """ + attrs = None + mm = msg.getHeaders(False, 'content-type').get('content-type', None) + if mm: + mm = ''.join(mm.splitlines()) + mimetype = mm.split(';') + if mimetype: + type = mimetype[0].split('/', 1) + if len(type) == 1: + major = type[0] + minor = None + elif len(type) == 2: + major, minor = type + else: + major = minor = None + # XXX patched --------------------------------------------- + attrs = dict(x.strip().split('=', 1) for x in mimetype[1:]) + # XXX patched --------------------------------------------- + else: + major = minor = None + else: + major = minor = None + return major, minor, attrs + +# Monkey-patch _getContentType to avoid bug that passes lower-case boundary in +# BODYSTRUCTURE response. +imap4._getContentType = _getContentType + + class LEAPIMAPServer(imap4.IMAP4Server): """ An IMAP4 Server with a LEAP Storage Backend. @@ -84,7 +116,9 @@ class LEAPIMAPServer(imap4.IMAP4Server): if part.header: hdrs = msg.getHeaders(part.header.negate, *part.header.fields) hdrs = imap4._formatHeaders(hdrs) - _w(str(part) + ' ' + imap4._literal(hdrs)) + # PATCHED ########################################## + _w(str(part) + ' ' + imap4._literal(hdrs + "\r\n")) + # PATCHED ########################################## elif part.text: _w(str(part) + ' ') _f() @@ -94,9 +128,9 @@ class LEAPIMAPServer(imap4.IMAP4Server): elif part.mime: hdrs = imap4._formatHeaders(msg.getHeaders(True)) - ###### PATCHED ##################################### + # PATCHED ########################################## _w(str(part) + ' ' + imap4._literal(hdrs + "\r\n")) - ###### END PATCHED ################################# + # END PATCHED ###################################### elif part.empty: _w(str(part) + ' ') -- cgit v1.2.3 From 822385e16b4f0f7a7d74905984bc4b4d76d187be Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Mon, 23 Mar 2015 12:57:02 -0400 Subject: [bug] report the correct size for mime parts The MIME size is the size of the body, w/o counting the headers. Releases: 0.4.0 --- mail/src/leap/mail/mail.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/mail/src/leap/mail/mail.py b/mail/src/leap/mail/mail.py index d92ff79..fd2f39a 100644 --- a/mail/src/leap/mail/mail.py +++ b/mail/src/leap/mail/mail.py @@ -135,7 +135,15 @@ class MessagePart(object): self._index = index def get_size(self): - return self._pmap['size'] + """ + Size of the body, in octets. + """ + total = self._pmap['size'] + _h = self.get_headers() + headers = len( + '\n'.join(["%s: %s" % (k, v) for k, v in dict(_h).items()])) + # have to subtract 2 blank lines + return total - headers - 2 def get_body_file(self): payload = "" @@ -148,6 +156,7 @@ class MessagePart(object): raise NotImplementedError if payload: payload = _encode_payload(payload) + return _write_and_rewind(payload) def get_headers(self): @@ -252,9 +261,10 @@ class Message(object): def get_size(self): """ - Size, in octets. + Size of the whole message, in octets (including headers). """ - return self._wrapper.fdoc.size + total = self._wrapper.fdoc.size + return total def is_multipart(self): """ -- cgit v1.2.3 From 76c5a1b7d53fc0c5df0adbbd9382d0713494cea1 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Mon, 23 Mar 2015 12:59:12 -0400 Subject: [bug] re-add fix for multiple headers This fix stores as multi-line headers that are repeated, and that were being discarded when storing them in a regular dict. It had been removed during the last refactor. I also store headers now as a case-insensitive dict, which solves other problems with the implementation of the twisted imap. Releases: 0.4.0 --- mail/src/leap/mail/adaptors/soledad.py | 18 ++++++++---------- mail/src/leap/mail/imap/messages.py | 25 +++++++++++++++++++------ mail/src/leap/mail/mail.py | 20 +++++++++++++++++++- mail/src/leap/mail/tests/test_mail.py | 8 +++++--- 4 files changed, 51 insertions(+), 20 deletions(-) diff --git a/mail/src/leap/mail/adaptors/soledad.py b/mail/src/leap/mail/adaptors/soledad.py index 490e014..7a1a92d 100644 --- a/mail/src/leap/mail/adaptors/soledad.py +++ b/mail/src/leap/mail/adaptors/soledad.py @@ -1114,6 +1114,7 @@ def _split_into_parts(raw): msg, parts, chash, multi = _parse_msg(raw) size = len(msg.as_string()) + body_phash = walk.get_body_phash(msg) parts_map = walk.walk_msg_tree(parts, body_phash=body_phash) @@ -1161,16 +1162,13 @@ def _build_headers_doc(msg, chash, body_phash, parts_map): It takes into account possibly repeated headers. """ - headers = msg.items() - - # TODO move this manipulation to IMAP - #headers = defaultdict(list) - #for k, v in msg.items(): - #headers[k].append(v) - ## "fix" for repeated headers. - #for k, v in headers.items(): - #newline = "\n%s: " % (k,) - #headers[k] = newline.join(v) + headers = defaultdict(list) + for k, v in msg.items(): + headers[k].append(v) + # "fix" for repeated headers (as in "Received:" + for k, v in headers.items(): + newline = "\n%s: " % (k.lower(),) + headers[k] = newline.join(v) lower_headers = lowerdict(dict(headers)) msgid = first(_MSGID_RE.findall( diff --git a/mail/src/leap/mail/imap/messages.py b/mail/src/leap/mail/imap/messages.py index 02aac2e..13943b1 100644 --- a/mail/src/leap/mail/imap/messages.py +++ b/mail/src/leap/mail/imap/messages.py @@ -208,6 +208,18 @@ class IMAPMessagePart(object): return IMAPMessagePart(subpart) +class CaseInsensitiveDict(dict): + """ + A dictionary subclass that will allow case-insenstive key lookups. + """ + + def __setitem__(self, key, value): + super(CaseInsensitiveDict, self).__setitem__(key.lower(), value) + + def __getitem__(self, key): + return super(CaseInsensitiveDict, self).__getitem__(key.lower()) + + def _format_headers(headers, negate, *names): # current server impl. expects content-type to be present, so if for # some reason we do not have headers, we have to return at least that @@ -228,13 +240,13 @@ def _format_headers(headers, negate, *names): # default to most likely standard charset = find_charset(headers, "utf-8") - _headers = dict() - for key, value in headers.items(): - # twisted imap server expects *some* headers to be lowercase - # We could use a CaseInsensitiveDict here... - if key.lower() == "content-type": - key = key.lower() + # We will return a copy of the headers dictionary that + # will allow case-insensitive lookups. In some parts of the twisted imap + # server code the keys are expected to be in lower case, and in this way + # we avoid having to convert them. + _headers = CaseInsensitiveDict() + for key, value in headers.items(): if not isinstance(key, str): key = key.encode(charset, 'replace') if not isinstance(value, str): @@ -247,4 +259,5 @@ def _format_headers(headers, negate, *names): # filter original dict by negate-condition if cond(key): _headers[key] = value + return _headers diff --git a/mail/src/leap/mail/mail.py b/mail/src/leap/mail/mail.py index fd2f39a..99c3873 100644 --- a/mail/src/leap/mail/mail.py +++ b/mail/src/leap/mail/mail.py @@ -17,6 +17,7 @@ """ Generic Access to Mail objects: Public LEAP Mail API. """ +import itertools import uuid import logging import StringIO @@ -98,6 +99,23 @@ def _encode_payload(payload, ctype=""): return payload +def _unpack_headers(headers_dict): + """ + Take a "packed" dict containing headers (with repeated keys represented as + line breaks inside each value, preceded by the header key) and return a + list of tuples in which each repeated key has a different tuple. + """ + headers_l = headers_dict.items() + for i, (k, v) in enumerate(headers_l): + splitted = v.split(k.lower() + ": ") + if len(splitted) != 1: + inner = zip( + itertools.cycle([k]), + map(lambda l: l.rstrip('\n'), splitted)) + headers_l = headers_l[:i] + inner + headers_l[i+1:] + return headers_l + + class MessagePart(object): # TODO This class should be better abstracted from the data model. # TODO support arbitrarily nested multiparts (right now we only support @@ -242,7 +260,7 @@ class Message(object): """ Get the raw headers document. """ - return [tuple(item) for item in self._wrapper.hdoc.headers] + return self._wrapper.hdoc.headers def get_body_file(self, store): """ diff --git a/mail/src/leap/mail/tests/test_mail.py b/mail/src/leap/mail/tests/test_mail.py index d326ca8..2c03933 100644 --- a/mail/src/leap/mail/tests/test_mail.py +++ b/mail/src/leap/mail/tests/test_mail.py @@ -26,7 +26,7 @@ from email.parser import Parser from email.Utils import formatdate from leap.mail.adaptors.soledad import SoledadMailAdaptor -from leap.mail.mail import MessageCollection, Account +from leap.mail.mail import MessageCollection, Account, _unpack_headers from leap.mail.mailbox_indexer import MailboxIndexer from leap.mail.tests.common import SoledadTestMixin @@ -144,8 +144,10 @@ class MessageTestCase(SoledadTestMixin, CollectionMixin): def _test_get_headers_cb(self, msg): self.assertTrue(msg is not None) - expected = _get_parsed_msg().items() - self.assertEqual(msg.get_headers(), expected) + expected = [ + (str(key.lower()), str(value)) + for (key, value) in _get_parsed_msg().items()] + self.assertItemsEqual(_unpack_headers(msg.get_headers()), expected) def test_get_body_file(self): d = self.get_inserted_msg(multi=True) -- cgit v1.2.3 From 491f65f854736f1f113a288c6021397de0041412 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Mon, 23 Mar 2015 13:01:54 -0400 Subject: [bug] temporary workaround to allow display on some muas Until we implement sequences, this avoids breaking with certain MUAs like mutt. Releases: 0.4.0 --- mail/src/leap/mail/imap/mailbox.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/mail/src/leap/mail/imap/mailbox.py b/mail/src/leap/mail/imap/mailbox.py index 61baca5..5d4e597 100644 --- a/mail/src/leap/mail/imap/mailbox.py +++ b/mail/src/leap/mail/imap/mailbox.py @@ -500,9 +500,9 @@ class IMAPMailbox(object): is_sequence = True if uid == 0 else False # XXX DEBUG --- if you attempt to use the `getmail` utility under - # imap/tests, it will choke until we implement sequence numbers. This - # is an easy hack meanwhile. - # is_sequence = False + # imap/tests, or muas like mutt, it will choke until we implement + # sequence numbers. This is an easy hack meanwhile. + is_sequence = False # ----------------------------------------------------------------- getmsg = self.collection.get_message_by_uid @@ -581,7 +581,14 @@ class IMAPMailbox(object): MessagePart. :rtype: tuple """ - is_sequence = True if uid == 0 else False + # is_sequence = True if uid == 0 else False + + # XXX FIXME ----------------------------------------------------- + # imap/tests, or muas like mutt, it will choke until we implement + # sequence numbers. This is an easy hack meanwhile. + is_sequence = False + # --------------------------------------------------------------- + if is_sequence: raise NotImplementedError -- cgit v1.2.3 From 373bbb7552ca6012edeae8bf3b8ee2d75cd5f58f Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Mon, 23 Mar 2015 13:03:10 -0400 Subject: [feature] make deferred list error-tolerant just in case Releases: 0.4.0 --- mail/src/leap/mail/imap/mailbox.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/mail/src/leap/mail/imap/mailbox.py b/mail/src/leap/mail/imap/mailbox.py index 5d4e597..0eff317 100644 --- a/mail/src/leap/mail/imap/mailbox.py +++ b/mail/src/leap/mail/imap/mailbox.py @@ -514,13 +514,14 @@ class IMAPMailbox(object): d_imapmsg = [] for msg in messages: d_imapmsg.append(getimapmsg(msg)) - return defer.gatherResults(d_imapmsg) + return defer.gatherResults(d_imapmsg, consumeErrors=True) def _zip_msgid(imap_messages): zipped = zip( list(msg_range), imap_messages) return (item for item in zipped) + # XXX not called?? def _unset_recent(sequence): reactor.callLater(0, self.unset_recent_flags, sequence) return sequence @@ -528,12 +529,12 @@ class IMAPMailbox(object): d_msg = [] for msgid in msg_range: # XXX We want cdocs because we "probably" are asked for the - # body. We should be smarted at do_FETCH and pass a parameter + # body. We should be smarter at do_FETCH and pass a parameter # to this method in order not to prefetch cdocs if they're not # going to be used. d_msg.append(getmsg(msgid, get_cdocs=True)) - d = defer.gatherResults(d_msg) + d = defer.gatherResults(d_msg, consumeErrors=True) d.addCallback(_get_imap_msg) d.addCallback(_zip_msgid) return d -- cgit v1.2.3 From a1d9b725e957c1ecadcbe85994c811c032c558e0 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Mon, 23 Mar 2015 13:08:24 -0400 Subject: [bug] move creation_ts to mail generic api This also fixes a bug in which INBOX wasn't being given a creation timestamp, and therefore always being identified with the same UIDVALIDITY = 1, which could be confusing MUAs since this value should be unique, and it's relied on to uniquely identifying a given message. Releases: 0.4.0 --- mail/src/leap/mail/imap/account.py | 15 ++------------- mail/src/leap/mail/mail.py | 17 ++++++++++++++++- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/mail/src/leap/mail/imap/account.py b/mail/src/leap/mail/imap/account.py index 38df845..ccb4b75 100644 --- a/mail/src/leap/mail/imap/account.py +++ b/mail/src/leap/mail/imap/account.py @@ -163,28 +163,17 @@ class IMAPAccount(object): # FIXME --- return failure instead of AssertionError # See AccountTestCase... leap_assert(name, "Need a mailbox name to create a mailbox") - if creation_ts is None: - # by default, we pass an int value - # taken from the current time - # we make sure to take enough decimals to get a unique - # mailbox-uidvalidity. - creation_ts = int(time.time() * 10E2) def check_it_does_not_exist(mailboxes): if name in mailboxes: raise imap4.MailboxCollision, repr(name) return mailboxes - def set_mbox_creation_ts(collection): - d = collection.set_mbox_attr("created", creation_ts) - d.addCallback(lambda _: collection) - return d - d = self.account.list_all_mailbox_names() d.addCallback(check_it_does_not_exist) - d.addCallback(lambda _: self.account.add_mailbox(name)) + d.addCallback(lambda _: self.account.add_mailbox( + name, creation_ts=creation_ts)) d.addCallback(lambda _: self.account.get_collection_by_mailbox(name)) - d.addCallback(set_mbox_creation_ts) d.addCallback(self._return_mailbox_from_collection) return d diff --git a/mail/src/leap/mail/mail.py b/mail/src/leap/mail/mail.py index 99c3873..89f89b0 100644 --- a/mail/src/leap/mail/mail.py +++ b/mail/src/leap/mail/mail.py @@ -21,6 +21,7 @@ import itertools import uuid import logging import StringIO +import time import weakref from twisted.internet import defer @@ -833,7 +834,20 @@ class Account(object): d = self.adaptor.get_all_mboxes(self.store) return d - def add_mailbox(self, name): + def add_mailbox(self, name, creation_ts=None): + + if creation_ts is None: + # by default, we pass an int value + # taken from the current time + # we make sure to take enough decimals to get a unique + # mailbox-uidvalidity. + creation_ts = int(time.time() * 10E2) + + def set_creation_ts(wrapper): + wrapper.created = creation_ts + d = wrapper.update(self.store) + d.addCallback(lambda _: wrapper) + return d def create_uuid(wrapper): if not wrapper.uuid: @@ -849,6 +863,7 @@ class Account(object): return d d = self.adaptor.get_or_create_mbox(self.store, name) + d.addCallback(set_creation_ts) d.addCallback(create_uuid) d.addCallback(create_uid_table_cb) return d -- cgit v1.2.3 From e51ede2702f1899570f5b8d0915f146fa8fddf38 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Thu, 26 Mar 2015 15:59:33 -0400 Subject: [bug] fix early append notification There's a workaround for "slow" APPENDS to an inbox, and it is that we have a flag to allow returning early when JUST the mdoc (the meta-document) has been written. However, this was givin a problem when doing a FETCH right after an APPEND (with notify_just_mdoc=True) has been done. This commit fixes it by making the FETCH command first check if there's an ongoing pending write, and queueing itself right after the write queue has been completed. This fixes the testFullAppend regression. Releases: 0.4.0 --- mail/src/leap/mail/adaptors/soledad.py | 34 +++++++++++++++++++++++------- mail/src/leap/mail/imap/mailbox.py | 12 ++--------- mail/src/leap/mail/imap/messages.py | 14 +----------- mail/src/leap/mail/imap/tests/test_imap.py | 13 ++++++------ mail/src/leap/mail/mail.py | 32 ++++++++++++++++++++++------ mail/src/leap/mail/utils.py | 21 ++++++++++++++++++ 6 files changed, 83 insertions(+), 43 deletions(-) diff --git a/mail/src/leap/mail/adaptors/soledad.py b/mail/src/leap/mail/adaptors/soledad.py index 7a1a92d..b8e5fd4 100644 --- a/mail/src/leap/mail/adaptors/soledad.py +++ b/mail/src/leap/mail/adaptors/soledad.py @@ -491,7 +491,7 @@ class MessageWrapper(object): for doc_id, cdoc in zip(self.mdoc.cdocs, self.cdocs.values()): cdoc.set_future_doc_id(doc_id) - def create(self, store, notify_just_mdoc=False): + def create(self, store, notify_just_mdoc=False, pending_inserts_dict=None): """ Create all the parts for this message in the store. @@ -503,7 +503,7 @@ class MessageWrapper(object): Be warned that in that case there will be no record of failures when creating the other part-documents. - Other-wise, this method will return a deferred that will wait for + Otherwise, this method will return a deferred that will wait for the creation of all the part documents. Setting this flag to True is mostly a convenient workaround for the @@ -513,6 +513,9 @@ class MessageWrapper(object): times will be enough to have all the queued insert operations finished. :type notify_just_mdoc: bool + :param pending_inserts_dict: + a dictionary with the pending inserts ids. + :type pending_inserts_dict: dict :return: a deferred whose callback will be called when either all the part documents have been written, or just the metamsg-doc, @@ -527,26 +530,41 @@ class MessageWrapper(object): leap_assert(self.fdoc.doc_id is None, "Cannot create: fdoc has a doc_id") + def unblock_pending_insert(result): + msgid = self.hdoc.headers.get('Message-Id', None) + try: + d = pending_inserts_dict[msgid] + d.callback(msgid) + except KeyError: + pass + return result + # TODO check that the doc_ids in the mdoc are coherent - d = [] + self.d = [] + mdoc_created = self.mdoc.create(store) - d.append(mdoc_created) - d.append(self.fdoc.create(store)) + fdoc_created = self.fdoc.create(store) + + self.d.append(mdoc_created) + self.d.append(fdoc_created) if not self._is_copy: if self.hdoc.doc_id is None: - d.append(self.hdoc.create(store)) + self.d.append(self.hdoc.create(store)) for cdoc in self.cdocs.values(): if cdoc.doc_id is not None: # we could be just linking to an existing # content-doc. continue - d.append(cdoc.create(store)) + self.d.append(cdoc.create(store)) + + self.all_inserted_d = defer.gatherResults(self.d) if notify_just_mdoc: + self.all_inserted_d.addCallback(unblock_pending_insert) return mdoc_created else: - return defer.gatherResults(d) + return self.all_inserted_d def update(self, store): """ diff --git a/mail/src/leap/mail/imap/mailbox.py b/mail/src/leap/mail/imap/mailbox.py index 0eff317..1412344 100644 --- a/mail/src/leap/mail/imap/mailbox.py +++ b/mail/src/leap/mail/imap/mailbox.py @@ -492,13 +492,8 @@ class IMAPMailbox(object): :rtype: deferred with a generator that yields... """ - # For the moment our UID is sequential, so we - # can treat them all the same. - # Change this to the flag that twisted expects when we - # switch to content-hash based index + local UID table. - - is_sequence = True if uid == 0 else False - + # TODO implement sequence + # is_sequence = True if uid == 0 else False # XXX DEBUG --- if you attempt to use the `getmail` utility under # imap/tests, or muas like mutt, it will choke until we implement # sequence numbers. This is an easy hack meanwhile. @@ -583,7 +578,6 @@ class IMAPMailbox(object): :rtype: tuple """ # is_sequence = True if uid == 0 else False - # XXX FIXME ----------------------------------------------------- # imap/tests, or muas like mutt, it will choke until we implement # sequence numbers. This is an easy hack meanwhile. @@ -672,7 +666,6 @@ class IMAPMailbox(object): :rtype: tuple """ # TODO implement sequences - # TODO how often is thunderbird doing this? is_sequence = True if uid == 0 else False if is_sequence: raise NotImplementedError @@ -730,7 +723,6 @@ class IMAPMailbox(object): read-write. """ # TODO implement sequences - # TODO how often is thunderbird doing this? is_sequence = True if uid == 0 else False if is_sequence: raise NotImplementedError diff --git a/mail/src/leap/mail/imap/messages.py b/mail/src/leap/mail/imap/messages.py index 13943b1..4c6f10d 100644 --- a/mail/src/leap/mail/imap/messages.py +++ b/mail/src/leap/mail/imap/messages.py @@ -22,7 +22,7 @@ from twisted.mail import imap4 from twisted.internet import defer from zope.interface import implements -from leap.mail.utils import find_charset +from leap.mail.utils import find_charset, CaseInsensitiveDict logger = logging.getLogger(__name__) @@ -208,18 +208,6 @@ class IMAPMessagePart(object): return IMAPMessagePart(subpart) -class CaseInsensitiveDict(dict): - """ - A dictionary subclass that will allow case-insenstive key lookups. - """ - - def __setitem__(self, key, value): - super(CaseInsensitiveDict, self).__setitem__(key.lower(), value) - - def __getitem__(self, key): - return super(CaseInsensitiveDict, self).__getitem__(key.lower()) - - def _format_headers(headers, negate, *names): # current server impl. expects content-type to be present, so if for # some reason we do not have headers, we have to return at least that diff --git a/mail/src/leap/mail/imap/tests/test_imap.py b/mail/src/leap/mail/imap/tests/test_imap.py index c4f752b..af1bd69 100644 --- a/mail/src/leap/mail/imap/tests/test_imap.py +++ b/mail/src/leap/mail/imap/tests/test_imap.py @@ -25,8 +25,8 @@ XXX add authors from the original twisted tests. @license: GPLv3, see included LICENSE file """ # XXX review license of the original tests!!! - import os +import string import types @@ -38,6 +38,7 @@ from twisted.python import failure from twisted import cred from leap.mail.imap.mailbox import IMAPMailbox +from leap.mail.imap.messages import CaseInsensitiveDict from leap.mail.imap.tests.utils import IMAP4HelperMixin @@ -74,8 +75,8 @@ class TestRealm: # # DEBUG --- -#from twisted.internet.base import DelayedCall -#DelayedCall.debug = True +# from twisted.internet.base import DelayedCall +# DelayedCall.debug = True class LEAPIMAP4ServerTestCase(IMAP4HelperMixin): @@ -810,7 +811,7 @@ class LEAPIMAP4ServerTestCase(IMAP4HelperMixin): infile = util.sibpath(__file__, 'rfc822.message') message = open(infile) acc = self.server.theAccount - mailbox_name = "root/subthing" + mailbox_name = "appendmbox/subthing" def add_mailbox(): return acc.addMailbox(mailbox_name) @@ -843,7 +844,7 @@ class LEAPIMAP4ServerTestCase(IMAP4HelperMixin): uid, msg = fetched[0] parsed = self.parser.parse(open(infile)) expected_body = parsed.get_payload() - expected_headers = dict(parsed.items()) + expected_headers = CaseInsensitiveDict(parsed.items()) def assert_flags(flags): self.assertEqual( @@ -860,7 +861,7 @@ class LEAPIMAP4ServerTestCase(IMAP4HelperMixin): self.assertEqual(expected_body, gotbody) def assert_headers(headers): - self.assertItemsEqual(expected_headers, headers) + self.assertItemsEqual(map(string.lower, expected_headers), headers) d = defer.maybeDeferred(msg.getFlags) d.addCallback(assert_flags) diff --git a/mail/src/leap/mail/mail.py b/mail/src/leap/mail/mail.py index 89f89b0..4fe08a6 100644 --- a/mail/src/leap/mail/mail.py +++ b/mail/src/leap/mail/mail.py @@ -35,7 +35,7 @@ from leap.mail.adaptors.soledad import SoledadMailAdaptor from leap.mail.constants import INBOX_NAME from leap.mail.constants import MessageFlags from leap.mail.mailbox_indexer import MailboxIndexer -from leap.mail.utils import find_charset +from leap.mail.utils import find_charset, CaseInsensitiveDict logger = logging.getLogger(name=__name__) @@ -179,7 +179,7 @@ class MessagePart(object): return _write_and_rewind(payload) def get_headers(self): - return self._pmap.get("headers", []) + return CaseInsensitiveDict(self._pmap.get("headers", [])) def is_multipart(self): return self._pmap.get("multi", False) @@ -261,7 +261,7 @@ class Message(object): """ Get the raw headers document. """ - return self._wrapper.hdoc.headers + return CaseInsensitiveDict(self._wrapper.hdoc.headers) def get_body_file(self, store): """ @@ -364,6 +364,8 @@ class MessageCollection(object): store = None messageklass = Message + _pending_inserts = dict() + def __init__(self, adaptor, store, mbox_indexer=None, mbox_wrapper=None): """ Constructor for a MessageCollection. @@ -440,6 +442,8 @@ class MessageCollection(object): if not absolute: raise NotImplementedError("Does not support relative ids yet") + get_doc_fun = self.mbox_indexer.get_doc_id_from_uid + def get_msg_from_mdoc_id(doc_id): if doc_id is None: return None @@ -447,7 +451,16 @@ class MessageCollection(object): self.messageklass, self.store, doc_id, uid=uid, get_cdocs=get_cdocs) - d = self.mbox_indexer.get_doc_id_from_uid(self.mbox_uuid, uid) + def cleanup_and_get_doc_after_pending_insert(result): + for key in result: + self._pending_inserts.pop(key) + return get_doc_fun(self.mbox_uuid, uid) + + if not self._pending_inserts: + d = get_doc_fun(self.mbox_uuid, uid) + else: + d = defer.gatherResults(self._pending_inserts.values()) + d.addCallback(cleanup_and_get_doc_after_pending_insert) d.addCallback(get_msg_from_mdoc_id) return d @@ -572,13 +585,16 @@ class MessageCollection(object): # TODO watch out if the use of this method in IMAP COPY/APPEND is # passing the right date. # XXX mdoc ref is a leaky abstraction here. generalize. - leap_assert_type(flags, tuple) leap_assert_type(date, str) msg = self.adaptor.get_msg_from_string(Message, raw_msg) wrapper = msg.get_wrapper() + if notify_just_mdoc: + msgid = msg.get_headers()['message-id'] + self._pending_inserts[msgid] = defer.Deferred() + if not self.is_mailbox_collection(): raise NotImplementedError() @@ -600,10 +616,14 @@ class MessageCollection(object): (wrapper.mdoc.serialize(),)) return defer.succeed("mdoc_id not inserted") # XXX BUG ----------------------------------------- + return self.mbox_indexer.insert_doc( self.mbox_uuid, doc_id) - d = wrapper.create(self.store, notify_just_mdoc=notify_just_mdoc) + d = wrapper.create( + self.store, + notify_just_mdoc=notify_just_mdoc, + pending_inserts_dict=self._pending_inserts) d.addCallback(insert_mdoc_id, wrapper) d.addErrback(lambda f: f.printTraceback()) d.addCallback(self.cb_signal_unread_to_ui) diff --git a/mail/src/leap/mail/utils.py b/mail/src/leap/mail/utils.py index 8e51024..029e9f5 100644 --- a/mail/src/leap/mail/utils.py +++ b/mail/src/leap/mail/utils.py @@ -351,3 +351,24 @@ def json_loads(data): obj = json.loads(data, cls=json.JSONDecoder) return obj + + +class CaseInsensitiveDict(dict): + """ + A dictionary subclass that will allow case-insenstive key lookups. + """ + def __init__(self, d=None): + if d is None: + d = [] + if isinstance(d, dict): + for key, value in d.items(): + self[key] = value + else: + for key, value in d: + self[key] = value + + def __setitem__(self, key, value): + super(CaseInsensitiveDict, self).__setitem__(key.lower(), value) + + def __getitem__(self, key): + return super(CaseInsensitiveDict, self).__getitem__(key.lower()) -- cgit v1.2.3