Rewrite mailcap handling.

Add a class that encapsulates the handler and is responsible for
managing the temporary file, if one is needed.

Use this class for both rendering inline content and displaying
attachments externally.

External attachments are now wrapped in an asyncio task that is added to
a pool of tasks managed by ui.

5 files changed, 188 insertions, 106 deletions

diff --git a/alot/commands/thread.py b/alot/commands/thread.py
index 84b1c611..ff70289b 100644
--- a/alot/commands/thread.py
+++ b/alot/commands/thread.py
@@ -3,6 +3,7 @@
 # This file is released under the GNU GPL, version 3 or a later revision.
 # For further details see the COPYING file
 import argparse
+import asyncio
 import logging
 import mailcap
 import os
@@ -14,10 +15,11 @@ from email.utils import getaddresses, parseaddr
 from email.message import Message
 
 import urwid
+from urwid.util import detected_encoding
+
 from io import BytesIO
 
 from . import Command, registerCommand
-from .globals import ExternalCommand
 from .globals import ComposeCommand
 from .globals import MoveCommand
 from .globals import CommandCanceled
@@ -30,9 +32,9 @@ from ..db.attachment import Attachment
 from ..db.errors import DatabaseROError
 from ..settings.const import settings
 from ..helper import formataddr
-from ..helper import parse_mailcap_nametemplate
 from ..helper import split_commandstring
 from ..utils import argparse as cargparse
+from ..utils.mailcap import MailcapHandler
 
 MODE = 'thread'
 
@@ -908,60 +910,49 @@ class OpenAttachmentCommand(Command):
 
     async def apply(self, ui):
         logging.info('open attachment')
+        data     = self.attachment.get_data()
         mimetype = self.attachment.get_content_type()
+        part     = self.attachment.get_mime_representation()
+        fname    = self.attachment.get_filename()
 
-        # returns pair of preliminary command string and entry dict containing
-        # more info. We only use the dict and construct the command ourselves
-        _, entry = settings.mailcap_find_match(mimetype)
-        if entry:
-            afterwards = None  # callback, will rm tempfile if used
-            handler_stdin = None
-            tempfile_name = None
-            handler_raw_commandstring = entry['view']
-            # read parameter
-            part = self.attachment.get_mime_representation()
-            parms = tuple('='.join(p) for p in part.get_params())
-
-            # in case the mailcap defined command contains no '%s',
-            # we pipe the files content to the handling command via stdin
-            if '%s' in handler_raw_commandstring:
-                nametemplate = entry.get('nametemplate', '%s')
-                prefix, suffix = parse_mailcap_nametemplate(nametemplate)
-
-                fn_hook = settings.get_hook('sanitize_attachment_filename')
-                if fn_hook:
-                    # get filename
-                    filename = self.attachment.get_filename()
-                    prefix, suffix = fn_hook(filename, prefix, suffix)
-
-                with tempfile.NamedTemporaryFile(delete=False, prefix=prefix,
-                                                 suffix=suffix) as tmpfile:
-                    tempfile_name = tmpfile.name
-                    self.attachment.write(tmpfile)
-
-                def afterwards():
-                    os.unlink(tempfile_name)
-            else:
-                handler_stdin = BytesIO()
-                self.attachment.write(handler_stdin)
+        h = MailcapHandler(data, mimetype, part.get_params(), fname, 'view')
+        if not h:
+            ui.notify('No handler for: %s' % mimetype)
+            return
 
-            # create handler command list
-            handler_cmd = mailcap.subst(handler_raw_commandstring, mimetype,
-                                        filename=tempfile_name, plist=parms)
+        # TODO: page copiousoutput
+        # TODO: hook for processing the command
+        if h.needs_terminal:
+            with ui.paused(), h:
+                logging.debug('Displaying part %s on terminal: %s',
+                              mimetype, h.cmd)
+
+                try:
+                    result = subprocess.run(h.cmd, shell = True, check = True,
+                                            input = h.stdin, stderr = subprocess.PIPE)
+                except subprocess.CalledProcessError as e:
+                    logging.error('Calling mailcap handler "%s" failed with code %d: %s',
+                                  h.cmd, e.returncode,
+                                  e.stderr.decode(detected_encoding, errors = 'backslashreplace'))
+            return
 
-            handler_cmdlist = split_commandstring(handler_cmd)
+        # does not need terminal - launch asynchronously
+        async def view_attachment_task(h):
+            with h:
+                logging.debug('Displaying part %s asynchronously: %s',
+                              mimetype, h.cmd)
 
-            # 'needsterminal' makes handler overtake the terminal
-            # XXX: could this be repalced with "'needsterminal' not in entry"?
-            overtakes = entry.get('needsterminal') is None
+                stdin  = subprocess.PIPE if h.stdin else subprocess.DEVNULL
+                stdout = subprocess.DEVNULL
+                stderr = subprocess.DEVNULL
+                child = await asyncio.create_subprocess_shell(h.cmd, stdin, stdout, stderr)
+                await child.communicate(h.stdin)
 
-            await ui.apply_command(ExternalCommand(handler_cmdlist,
-                                                   stdin=handler_stdin,
-                                                   on_success=afterwards,
-                                                   thread=overtakes))
-        else:
-            ui.notify('unknown mime type')
+                if child.returncode != 0:
+                    logging.error('Calling mailcap handler "%s" failed with code %d:',
+                                  h.cmd, e.returncode)
 
+        ui.run_task(view_attachment_task(h))
 
 @registerCommand(
     MODE, 'move', help='move focus in current buffer',
diff --git a/alot/db/message.py b/alot/db/message.py
index ff8cca70..e10e29a5 100644
--- a/alot/db/message.py
+++ b/alot/db/message.py
@@ -2,77 +2,54 @@
 # This file is released under the GNU GPL, version 3 or a later revision.
 # For further details see the COPYING file
 
-from   contextlib import ExitStack
 import email
 import email.charset as charset
 import email.policy
 import logging
-import mailcap
 import os
-import tempfile
+import subprocess
 
 from datetime import datetime
 
+from urwid.util import detected_encoding
+
 from .attachment import Attachment
 from .. import crypto
 from .. import helper
 from ..errors import GPGProblem
-from ..helper import parse_mailcap_nametemplate
-from ..helper import split_commandstring
 from ..settings.const import settings
+from ..utils.mailcap import MailcapHandler
 
 charset.add_charset('utf-8', charset.QP, charset.QP, 'utf-8')
 
 _APP_PGP_SIG = 'application/pgp-signature'
 _APP_PGP_ENC = 'application/pgp-encrypted'
 
-def _render_part_external(raw_payload, ctype, params, field_key='copiousoutput'):
+def _render_part_external(payload, ctype, params, filename):
     """
     renders a non-multipart email part into displayable plaintext by piping its
     payload through an external script. The handler itself is determined by
     the mailcap entry for this part's ctype.
     """
-    rendered_payload = None
-    # get mime handler
-    _, entry = settings.mailcap_find_match(ctype, key=field_key)
-    if entry is None:
-        return None
-
-    if isinstance(raw_payload, str):
-        raw_payload = raw_payload.encode('utf-8')
-
-    with ExitStack() as stack:
-        # read parameter, create handler command
-        parms = tuple('='.join(p) for p in params)
-
-        # in case the mailcap defined command contains no '%s',
-        # we pipe the files content to the handling command via stdin
-        if '%s' in entry['view']:
-            # open tempfile, respect mailcaps nametemplate
-            nametemplate   = entry.get('nametemplate', '%s')
-            prefix, suffix = parse_mailcap_nametemplate(nametemplate)
+    h = MailcapHandler(payload, ctype, params, filename, 'copiousoutput')
+    if not h or h.needs_terminal:
+        return
 
-            tmpfile = stack.enter_context(tempfile.NamedTemporaryFile(prefix = prefix, suffix = suffix))
+    def decode(buf):
+        return buf.decode(detected_encoding, errors = 'backslashreplace')
 
-            tmpfile.write(raw_payload)
-            tmpfile.flush()
+    with h:
+        logging.debug('Rendering part %s: %s', ctype, h.cmd)
 
-            tempfile_name = tmpfile.name
-            stdin         = None
-        else:
-            tempfile_name = None
-            stdin         = raw_payload
-
-        # create and call external command
-        cmd = mailcap.subst(entry['view'], ctype,
-                            filename = tempfile_name, plist = parms)
-        logging.debug('command: %s', cmd)
-        logging.debug('parms: %s', str(parms))
-        cmdlist = split_commandstring(cmd)
-        # call handler
-        stdout, _, _ = helper.call_cmd(cmdlist, stdin=stdin)
+        try:
+            result = subprocess.run(h.cmd, shell = True, check = True,
+                                    capture_output = True, input = h.stdin)
+        except subprocess.CalledProcessError as e:
+            logging.error('Calling mailcap handler "%s" failed with code %d: %s',
+                          h.cmd, e.returncode, decode(e.stderr))
+            return None
 
-        return stdout
+        return decode(result.stdout)
 
 class _MessageHeaders:
     _msg = None
@@ -170,7 +147,9 @@ class _MimeTree:
         content = self._part.get_content()
 
         # try procesing content with an external program
-        rendered = _render_part_external(content, self.content_type, self._part.get_params())
+        rendered = _render_part_external(content, self.content_type,
+                                         self._part.get_params(),
+                                         'copiousoutput')
         if rendered:
             return rendered
 
diff --git a/alot/helper.py b/alot/helper.py
index 27652988..9ba3c411 100644
--- a/alot/helper.py
+++ b/alot/helper.py
@@ -378,19 +378,6 @@ def shell_quote(text):
     return "'%s'" % text.replace("'", """'"'"'""")
 
 
-def parse_mailcap_nametemplate(tmplate='%s'):
-    """this returns a prefix and suffix to be used
-    in the tempfile module for a given mailcap nametemplate string"""
-    nt_list = tmplate.split('%s')
-    template_prefix = ''
-    template_suffix = ''
-    if len(nt_list) == 2:
-        template_suffix = nt_list[1]
-        template_prefix = nt_list[0]
-    else:
-        template_suffix = tmplate
-    return (template_prefix, template_suffix)
-
 def get_xdg_env(env_name, fallback):
     """ Used for XDG_* env variables to return fallback if unset *or* empty """
     env = os.environ.get(env_name)
diff --git a/alot/ui.py b/alot/ui.py
index 9a2d59fd..69c9cda3 100644
--- a/alot/ui.py
+++ b/alot/ui.py
@@ -104,6 +104,8 @@ class UI:
     # running asyncio event loop
     _loop             = None
 
+    _pending_tasks    = None
+
     def __init__(self, loop, dbman, initialcmdline):
         """
         :param dbman: :class:`~alot.db.DBManager`
@@ -133,6 +135,8 @@ class UI:
 
         self._loop = loop
 
+        self._pending_tasks = set()
+
         # define empty notification pile
         self._notification_bar = urwid.Pile([])
         self._footer_wgt       = urwid.Pile([self._notification_bar])
@@ -758,8 +762,18 @@ class UI:
         self._save_history_to_file(self.recipienthistory,
                                    self._recipients_hist_file, size=size)
 
+        logging.info('cancelling pending tasks: %s', self._pending_tasks)
+        for t in self._pending_tasks:
+            t.cancel()
+        await asyncio.gather(*self._pending_tasks)
+
         await self.dbman.shutdown()
 
+    def run_task(self, coro):
+        task = self._loop.create_task(coro)
+        self._pending_tasks.add(task)
+        task.add_done_callback(self._pending_tasks.discard)
+
     @staticmethod
     def _load_history_from_file(path, size=-1):
         """Load a history list from a file and split it into lines.
diff --git a/alot/utils/mailcap.py b/alot/utils/mailcap.py
new file mode 100644
index 00000000..31a50d70
--- /dev/null
+++ b/alot/utils/mailcap.py
@@ -0,0 +1,111 @@
+# Copyright (C) 2011-2012  Patrick Totzke <patricktotzke@gmail.com>
+# Copyright (C) 2021  Anton Khirnov <anton@khirnov.net>
+# This file is released under the GNU GPL, version 3 or a later revision.
+# For further details see the COPYING file
+
+import mailcap
+from tempfile import NamedTemporaryFile
+
+from urwid.util import detected_encoding
+
+from ..settings.const import settings
+
+def _parse_nametemplate(template):
+    """this returns a prefix and suffix to be used
+    in the tempfile module for a given mailcap nametemplate string"""
+    nt_list = template.split('%s')
+    template_prefix = ''
+    template_suffix = ''
+    if len(nt_list) == 2:
+        template_suffix = nt_list[1]
+        template_prefix = nt_list[0]
+    else:
+        template_suffix = template
+    return (template_prefix, template_suffix)
+
+class MailcapHandler:
+    """
+    A handler for externally processing a MIME part with given payload and
+    content-type ctype. Must be used as a context manager, since it may create a
+    temporary file that needs to be deleted.
+
+    If the handler requires a file, then this function will create a temporary
+    file and write the payload into it. Otherwise, the payload needs to be
+    provided on stdin.
+    """
+
+    _entry    = None
+
+    _payload  = None
+    _ctype    = None
+    _params   = None
+
+    _need_tmpfile = None
+    _tmpfile      = None
+
+    def __init__(self, payload, ctype, params, filename, field_key):
+        # find the mime handler
+        _, self._entry = settings.mailcap_find_match(ctype, key = field_key)
+        if self._entry is None:
+            return
+
+        if isinstance(payload, str):
+            payload = payload.encode(detected_encoding, errors = 'backslashreplace')
+        self._payload = payload
+
+        self._payload = payload
+        self._ctype   = ctype
+        self._params  = tuple('='.join(p) for p in params)
+
+        # in case the mailcap defined command contains no '%s',
+        # we pipe the files content to the handling command via stdin
+        self._need_tmpfile = '%s' in self._entry['view']
+
+    def __bool__(self):
+        return self._entry is not None
+
+    def __enter__(self):
+        """
+        The context manager manages the temporary file, if one is needed.
+        """
+        if not self._need_tmpfile:
+            return
+
+        nametemplate   = self._entry.get('nametemplate', '%s')
+        prefix, suffix = _parse_nametemplate(nametemplate)
+
+        fn_hook = settings.get_hook('sanitize_attachment_filename')
+        if fn_hook:
+            prefix, suffix = fn_hook(filename, prefix, suffix)
+
+        tmpfile = NamedTemporaryFile(prefix = prefix, suffix = suffix)
+
+        try:
+            tmpfile.write(self._payload)
+            tmpfile.flush()
+            self._tmpfile = tmpfile
+        except:
+            tmpfile.close()
+            raise
+
+    def __exit__(self, exc_type, exc_value, tb):
+        if self._tmpfile:
+            self._tmpfile.close()
+            self._tmpfile = None
+
+    @property
+    def cmd(self):
+        """ Shell command to run (str) """
+        fname = self._tmpfile.name if self._need_tmpfile else None
+        return mailcap.subst(self._entry['view'], self._ctype,
+                             plist = self._params, filename = fname)
+
+    @property
+    def stdin(self):
+        if self._need_tmpfile:
+            return None
+        return self._payload
+
+    @property
+    def needs_terminal(self):
+        return self._entry.get('needsterminal') is not None