From 151b34ea793c95687c64d50cf7046184138bccfd Mon Sep 17 00:00:00 2001 From: link2xt Date: Fri, 10 Feb 2023 09:54:50 +0000 Subject: [PATCH] python: handle NULL value returned from dc_get_msg Returning None in this case and checked with mypy that callers can handle it. --- CHANGELOG.md | 2 ++ python/src/deltachat/account.py | 9 ++++++--- python/src/deltachat/chat.py | 12 ++++++++++-- python/src/deltachat/events.py | 27 +++++++++++++++------------ python/src/deltachat/message.py | 10 ++++++++-- 5 files changed, 41 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87b5ed507..8b97d693e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ ## Fixes - Fix Securejoin for multiple devices on a joining side #3982 +- python: handle NULL value returned from `dc_get_msg()` #4020 + Account.`get_message_by_id` may return `None` in this case. ## API-Changes diff --git a/python/src/deltachat/account.py b/python/src/deltachat/account.py index 0235fd798..e9ec5e311 100644 --- a/python/src/deltachat/account.py +++ b/python/src/deltachat/account.py @@ -20,7 +20,6 @@ from .cutil import ( from_optional_dc_charpointer, iter_array, ) -from .events import EventThread, FFIEventLogger from .message import Message from .tracker import ConfigureTracker, ImexTracker @@ -63,6 +62,8 @@ class Account(object): MissingCredentials = MissingCredentials def __init__(self, db_path, os_name=None, logging=True, closed=False) -> None: + from .events import EventThread + """initialize account object. :param db_path: a path to the account database. The database @@ -368,7 +369,7 @@ class Account(object): def get_fresh_messages(self) -> Generator[Message, None, None]: """yield all fresh messages from all chats.""" dc_array = ffi.gc(lib.dc_get_fresh_msgs(self._dc_context), lib.dc_array_unref) - yield from iter_array(dc_array, lambda x: Message.from_db(self, x)) + return (x for x in iter_array(dc_array, lambda x: Message.from_db(self, x)) if x is not None) def create_chat(self, obj) -> Chat: """Create a 1:1 chat with Account, Contact or e-mail address.""" @@ -413,7 +414,7 @@ class Account(object): def get_device_chat(self) -> Chat: return Contact(self, const.DC_CONTACT_ID_DEVICE).create_chat() - def get_message_by_id(self, msg_id: int) -> Message: + def get_message_by_id(self, msg_id: int) -> Optional[Message]: """return Message instance. :param msg_id: integer id of this message. :returns: :class:`deltachat.message.Message` instance. @@ -596,6 +597,8 @@ class Account(object): # def run_account(self, addr=None, password=None, account_plugins=None, show_ffi=False): + from .events import FFIEventLogger + """get the account running, configure it if necessary. add plugins if provided. :param addr: the email address of the account diff --git a/python/src/deltachat/chat.py b/python/src/deltachat/chat.py index 3ac47a01c..1548829ae 100644 --- a/python/src/deltachat/chat.py +++ b/python/src/deltachat/chat.py @@ -282,12 +282,20 @@ class Chat(object): if msg.is_out_preparing(): assert msg.id != 0 # get a fresh copy of dc_msg, the core needs it - msg = Message.from_db(self.account, msg.id) + maybe_msg = Message.from_db(self.account, msg.id) + if maybe_msg is not None: + msg = maybe_msg + else: + raise ValueError("message does not exist") + sent_id = lib.dc_send_msg(self.account._dc_context, self.id, msg._dc_msg) if sent_id == 0: raise ValueError("message could not be sent") # modify message in place to avoid bad state for the caller - msg._dc_msg = Message.from_db(self.account, sent_id)._dc_msg + sent_msg = Message.from_db(self.account, sent_id) + if sent_msg is None: + raise ValueError("cannot load just sent message from the database") + msg._dc_msg = sent_msg._dc_msg return msg def send_text(self, text): diff --git a/python/src/deltachat/events.py b/python/src/deltachat/events.py index 6db3de61e..b96e6987f 100644 --- a/python/src/deltachat/events.py +++ b/python/src/deltachat/events.py @@ -13,6 +13,7 @@ from .capi import ffi, lib from .cutil import from_optional_dc_charpointer from .hookspec import account_hookimpl from .message import map_system_message +from .account import Account def get_dc_event_name(integer, _DC_EVENTNAME_MAP={}): @@ -221,7 +222,7 @@ class EventThread(threading.Thread): With each Account init this callback thread is started. """ - def __init__(self, account) -> None: + def __init__(self, account: Account) -> None: self.account = account super(EventThread, self).__init__(name="events") self.daemon = True @@ -298,20 +299,22 @@ class EventThread(threading.Thread): yield "ac_configure_completed", {"success": success, "comment": comment} elif name == "DC_EVENT_INCOMING_MSG": msg = account.get_message_by_id(ffi_event.data2) - yield map_system_message(msg) or ("ac_incoming_message", {"message": msg}) + if msg is not None: + yield map_system_message(msg) or ("ac_incoming_message", {"message": msg}) elif name == "DC_EVENT_MSGS_CHANGED": if ffi_event.data2 != 0: msg = account.get_message_by_id(ffi_event.data2) - if msg.is_outgoing(): - res = map_system_message(msg) - if res and res[0].startswith("ac_member"): - yield res - yield "ac_outgoing_message", {"message": msg} - elif msg.is_in_fresh(): - yield map_system_message(msg) or ( - "ac_incoming_message", - {"message": msg}, - ) + if msg is not None: + if msg.is_outgoing(): + res = map_system_message(msg) + if res and res[0].startswith("ac_member"): + yield res + yield "ac_outgoing_message", {"message": msg} + elif msg.is_in_fresh(): + yield map_system_message(msg) or ( + "ac_incoming_message", + {"message": msg}, + ) elif name == "DC_EVENT_REACTIONS_CHANGED": assert ffi_event.data1 > 0 msg = account.get_message_by_id(ffi_event.data2) diff --git a/python/src/deltachat/message.py b/python/src/deltachat/message.py index 0970c985e..d84713bf8 100644 --- a/python/src/deltachat/message.py +++ b/python/src/deltachat/message.py @@ -42,9 +42,15 @@ class Message(object): ) @classmethod - def from_db(cls, account, id): + def from_db(cls, account, id) -> Optional["Message"]: + """Attempt to load the message from the database given its ID. + + None is returned if the message does not exist, i.e. deleted.""" assert id > 0 - return cls(account, ffi.gc(lib.dc_get_msg(account._dc_context, id), lib.dc_msg_unref)) + res = lib.dc_get_msg(account._dc_context, id) + if res == ffi.NULL: + return None + return cls(account, ffi.gc(res, lib.dc_msg_unref)) @classmethod def new_empty(cls, account, view_type):