diff --git a/debian/control b/debian/control index a9ceddd..8f42cc1 100644 --- a/debian/control +++ b/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 diff --git a/qubespdfconverter/client.py b/qubespdfconverter/client.py index 082b0f4..52883cf 100644 --- a/qubespdfconverter/client.py +++ b/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) diff --git a/qubespdfconverter/tests/__init__.py b/qubespdfconverter/tests/__init__.py index 15655f1..6be3ff4 100644 --- a/qubespdfconverter/tests/__init__.py +++ b/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] diff --git a/qubespdfconverter/tests/test_client.py b/qubespdfconverter/tests/test_client.py new file mode 100644 index 0000000..a930e98 --- /dev/null +++ b/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() diff --git a/qvm-convert-pdf.gnome b/qvm-convert-pdf.gnome index f1801d0..e1fdd8a 100755 --- a/qvm-convert-pdf.gnome +++ b/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" diff --git a/rpm_spec/qpdf-converter.spec.in b/rpm_spec/qpdf-converter.spec.in index 7890568..91abb0d 100644 --- a/rpm_spec/qpdf-converter.spec.in +++ b/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