pdf-converter: stop conversion when Cancel is clicked in GUI

debian/control

@@ -24,4 +24,6 @@ Depends:
  python3-pillow,
  python3-tqdm,
  ${misc:Depends}
+Recommends:
+ zenity
 Description: The Qubes service for converting untrusted PDF files into trusted ones

qubespdfconverter/client.py

@@ -491,11 +491,13 @@ async def run(self, archive, depth, in_place):
 
                 self.bar.set_job_status(Status.FAIL)
                 await ERROR_LOGS.put(f"{self.path.name}: {e}")
-                if self.proc.returncode is not None:
+                if self.proc.returncode is None:
                     await terminate_proc(self.proc)
                 raise
             except asyncio.CancelledError:
                 self.bar.set_job_status(Status.CANCELLED)
+                if self.proc.returncode is None:
+                    await terminate_proc(self.proc)
                 raise
 
         self.bar.set_job_status(Status.DONE)
@@ -599,10 +601,12 @@ async def run(params):
                                                  params["batch"],
                                                  params["in_place"])))
 
-    asyncio.get_running_loop().add_signal_handler(
-        signal.SIGINT,
-        lambda: asyncio.ensure_future(sigint_handler(tasks))
-    )
+    loop = asyncio.get_running_loop()
+    for sig in (signal.SIGINT, signal.SIGTERM):
+        loop.add_signal_handler(
+            sig,
+            lambda: asyncio.ensure_future(sigint_handler(tasks))
+        )
 
     results = await asyncio.gather(*tasks, return_exceptions=True)
     completed = results.count(None)

qubespdfconverter/tests/__init__.py

@@ -20,6 +20,7 @@
 # with this program; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 #
+import asyncio
 import os
 import unittest
 
@@ -135,6 +136,34 @@ def test_003_filename_with_spaces(self):
         self.assertCorrectlyTransformed('orig.pdf',
             'test with spaces.trusted.pdf')
 
+    def test_004_cancel_stops_conversion(self):
+        self.create_pdf('test.pdf', ['This is test'] * 500)
+        domains_before = set(self.app.domains)
+        p = self.vm.run(
+            'cp test.pdf orig.pdf; '
+            'timeout --signal=INT 20 qvm-convert-pdf test.pdf 2>&1',
+            passio_popen=True)
+        (stdout, _) = p.communicate()
+        self.assertNotEqual(p.returncode, 0,
+            'Expected non-zero exit from interrupted conversion: {}'.format(stdout))
+        self.assertNotEqual(
+            self.vm.run('test -r test.trusted.pdf', wait=True), 0,
+            'trusted pdf should not exist after cancel')
+        # DispVM cleanup in dom0 is asynchronous; poll until the DispVM
+        # disappears from self.app.domains (mirrors qubes-core-admin pattern),
+        # up to 10 seconds.
+        timeout = 10
+        while True:
+            domains_after = set(self.app.domains)
+            if domains_after == domains_before:
+                break
+            self.loop.run_until_complete(asyncio.sleep(1))
+            timeout -= 1
+            if timeout <= 0:
+                self.fail(
+                    'DispVM not cleaned up 10s after cancel: {}'.format(
+                        domains_after - domains_before))
+
 
 def list_tests():
     tests = [TC_00_PDFConverter]

qubespdfconverter/tests/test_client.py

@@ -0,0 +1,73 @@
+#!/usr/bin/python3
+
+import asyncio
+import signal
+import unittest
+from pathlib import Path
+from unittest import mock
+
+from qubespdfconverter.client import Job, PageError, run
+
+
+class DummyProc:
+    def __init__(self):
+        self.returncode = None
+        self.terminated = False
+        self.stdin = mock.Mock()
+        self.stdout = mock.Mock()
+
+    def terminate(self):
+        self.terminated = True
+        self.returncode = -signal.SIGTERM
+
+    async def wait(self):
+        if self.returncode is None:
+            self.returncode = 0
+
+
+class TC_ClientCancel(unittest.IsolatedAsyncioTestCase):
+    async def test_000_cancel_terminates_qrexec_proc(self):
+        job = Job(Path("/tmp/test.pdf"), 0)
+        proc = DummyProc()
+
+        with mock.patch("asyncio.create_subprocess_exec", new=mock.AsyncMock(return_value=proc)):
+            job._setup = mock.AsyncMock()
+            job._start = mock.AsyncMock(side_effect=asyncio.CancelledError)
+
+            with self.assertRaises(asyncio.CancelledError):
+                await job.run(Path("/tmp/archive"), depth=1, in_place=False)
+
+        self.assertTrue(proc.terminated)
+
+    async def test_001_failure_terminates_qrexec_proc(self):
+        job = Job(Path("/tmp/test.pdf"), 0)
+        proc = DummyProc()
+
+        with mock.patch("asyncio.create_subprocess_exec", new=mock.AsyncMock(return_value=proc)):
+            job._setup = mock.AsyncMock(side_effect=PageError)
+
+            with self.assertRaises(PageError):
+                await job.run(Path("/tmp/archive"), depth=1, in_place=False)
+
+        self.assertTrue(proc.terminated)
+
+    async def test_002_register_sigterm_handler(self):
+        loop = asyncio.get_running_loop()
+        add_handler_mock = mock.Mock()
+
+        with mock.patch.object(loop, "add_signal_handler", new=add_handler_mock):
+            result = await run({
+                "resolution": 300,
+                "files": [],
+                "archive": Path("/tmp/archive"),
+                "batch": 1,
+                "in_place": False,
+            })
+
+        self.assertFalse(result)
+        handled = {call.args[0] for call in add_handler_mock.mock_calls}
+        self.assertEqual(handled, {signal.SIGINT, signal.SIGTERM})
+
+
+if __name__ == "__main__":
+    unittest.main()

qvm-convert-pdf.gnome

@@ -20,10 +20,29 @@
 #
 #
 
+set -u
+
 if [ $# -ne 1 ]; then
 exit 1
 fi
 
 export PROGRESS_FOR_GUI="yes"
 
-/usr/bin/qvm-convert-pdf "$@" | zenity --progress --text="Converting PDF using Disposable VM..." --auto-close --auto-kill
+fifo="$(mktemp -u)"
+mkfifo "$fifo" || exit 1
+
+/usr/bin/qvm-convert-pdf "$@" >"$fifo" &
+converter_pid="$!"
+
+zenity --progress --text="Converting PDF using Disposable VM..." --auto-close <"$fifo"
+zenity_rc="$?"
+
+rm -f -- "$fifo"
+
+if [ "$zenity_rc" -ne 0 ]; then
+    kill "$converter_pid"
+    wait "$converter_pid"
+    exit 1
+fi
+
+wait "$converter_pid"

rpm_spec/qpdf-converter.spec.in

@@ -54,6 +54,7 @@ Requires:	python%{python3_pkgversion} >= 3.7
 Requires:	python%{python3_pkgversion}-pillow
 Requires:	python%{python3_pkgversion}-click
 Requires:	python%{python3_pkgversion}-tqdm
+Recommends:	zenity
 
 Source0: %{name}-%{version}.tar.gz