diff --git a/dangerzone/conversion/errors.py b/dangerzone/conversion/errors.py index 61e48c253..235ffc9b2 100644 --- a/dangerzone/conversion/errors.py +++ b/dangerzone/conversion/errors.py @@ -1,3 +1,4 @@ +import subprocess from typing import List, Optional, Type, Union # XXX: errors start at 128 for conversion-related issues @@ -7,6 +8,23 @@ MAX_PAGE_HEIGHT = 10000 +class InterruptedConversionException(Exception): + """Data received was less than expected""" + + def __init__(self) -> None: + super().__init__( + "Something interrupted the conversion and it could not be completed." + ) + + +class ConverterProcException(Exception): + """Some exception occurred in the converter""" + + def __init__(self, proc: subprocess.Popen) -> None: + self.proc = proc + super().__init__() + + class ConversionException(Exception): error_message = "Unspecified error" error_code = ERROR_SHIFT @@ -86,15 +104,6 @@ class PageCountMismatch(PagesException): ) -class ConverterProcException(ConversionException): - """Some exception occurred in the converter""" - - error_code = ERROR_SHIFT + 60 - error_message = ( - "Something interrupted the conversion and it could not be completed." - ) - - class UnexpectedConversionError(ConversionException): error_code = ERROR_SHIFT + 100 error_message = "Some unexpected error occurred while converting the document" diff --git a/dangerzone/isolation_provider/base.py b/dangerzone/isolation_provider/base.py index f637ff382..dfdcd8463 100644 --- a/dangerzone/isolation_provider/base.py +++ b/dangerzone/isolation_provider/base.py @@ -27,7 +27,7 @@ def read_bytes(f: IO[bytes], size: int, exact: bool = True) -> bytes: """Read bytes from a file-like object.""" buf = f.read(size) if exact and len(buf) != size: - raise errors.ConverterProcException() + raise errors.InterruptedConversionException() return buf @@ -35,7 +35,7 @@ def read_int(f: IO[bytes]) -> int: """Read 2 bytes from a file-like object, and decode them as int.""" untrusted_int = f.read(INT_BYTES) if len(untrusted_int) != INT_BYTES: - raise errors.ConverterProcException() + raise errors.InterruptedConversionException() return int.from_bytes(untrusted_int, "big", signed=False) @@ -52,7 +52,6 @@ class IsolationProvider(ABC): def __init__(self) -> None: self.percentage = 0.0 - self.proc: Optional[subprocess.Popen] = None if getattr(sys, "dangerzone_dev", False) == True: self.proc_stderr = subprocess.PIPE @@ -80,8 +79,8 @@ def convert( document.mark_as_safe() if document.archive_after_conversion: document.archive() - except errors.ConverterProcException: - exception = self.get_proc_exception() + except errors.ConverterProcException as e: + exception = self.get_proc_exception(e.proc) self.print_progress(document, True, str(exception), 0) document.mark_as_failed() except errors.ConversionException as e: @@ -96,16 +95,16 @@ def convert( def doc_to_pixels(self, document: Document, tempdir: str) -> None: with open(document.input_filename, "rb") as f: - self.proc = self.start_doc_to_pixels_proc() + p = self.start_doc_to_pixels_proc() try: - assert self.proc.stdin is not None - self.proc.stdin.write(f.read()) - self.proc.stdin.close() + assert p.stdin is not None + p.stdin.write(f.read()) + p.stdin.close() except BrokenPipeError as e: - raise errors.ConverterProcException() + raise errors.ConverterProcException(p) - assert self.proc.stdout - n_pages = read_int(self.proc.stdout) + assert p.stdout + n_pages = read_int(p.stdout) if n_pages == 0 or n_pages > errors.MAX_PAGES: raise errors.MaxPagesException() percentage_per_page = 50.0 / n_pages @@ -114,8 +113,8 @@ def doc_to_pixels(self, document: Document, tempdir: str) -> None: text = f"Converting page {page}/{n_pages} to pixels" self.print_progress(document, False, text, self.percentage) - width = read_int(self.proc.stdout) - height = read_int(self.proc.stdout) + width = read_int(p.stdout) + height = read_int(p.stdout) if not (1 <= width <= errors.MAX_PAGE_WIDTH): raise errors.MaxPageWidthException() if not (1 <= height <= errors.MAX_PAGE_HEIGHT): @@ -123,7 +122,7 @@ def doc_to_pixels(self, document: Document, tempdir: str) -> None: num_pixels = width * height * 3 # three color channels untrusted_pixels = read_bytes( - self.proc.stdout, + p.stdout, num_pixels, ) @@ -138,16 +137,16 @@ def doc_to_pixels(self, document: Document, tempdir: str) -> None: self.percentage += percentage_per_page # Ensure nothing else is read after all bitmaps are obtained - self.proc.stdout.close() + p.stdout.close() # TODO handle leftover code input text = "Converted document to pixels" self.print_progress(document, False, text, self.percentage) if getattr(sys, "dangerzone_dev", False): - assert self.proc.stderr - untrusted_log = read_debug_text(self.proc.stderr, MAX_CONVERSION_LOG_CHARS) - self.proc.stderr.close() + assert p.stderr + untrusted_log = read_debug_text(p.stderr, MAX_CONVERSION_LOG_CHARS) + p.stderr.close() log.info( f"Conversion output (doc to pixels)\n{self.sanitize_conversion_str(untrusted_log)}" ) @@ -173,10 +172,9 @@ def print_progress( if self.progress_callback: self.progress_callback(error, text, percentage) - def get_proc_exception(self) -> Exception: + def get_proc_exception(self, p: subprocess.Popen) -> Exception: """Returns an exception associated with a process exit code""" - assert self.proc - error_code = self.proc.wait(3) + error_code = p.wait(3) return errors.exception_from_error_code(error_code) @abstractmethod diff --git a/dangerzone/isolation_provider/qubes.py b/dangerzone/isolation_provider/qubes.py index 4c098f775..d77c13bc1 100644 --- a/dangerzone/isolation_provider/qubes.py +++ b/dangerzone/isolation_provider/qubes.py @@ -23,10 +23,6 @@ class Qubes(IsolationProvider): """Uses a disposable qube for performing the conversion""" - def __init__(self) -> None: - self.proc: Optional[subprocess.Popen] = None - super().__init__() - def install(self) -> bool: return True diff --git a/tests/isolation_provider/base.py b/tests/isolation_provider/base.py index d03a2b115..b9071b0f6 100644 --- a/tests/isolation_provider/base.py +++ b/tests/isolation_provider/base.py @@ -1,7 +1,9 @@ import os +import subprocess import pytest from colorama import Style +from pytest import MonkeyPatch from pytest_mock import MockerFixture from dangerzone.conversion import errors @@ -30,13 +32,25 @@ def test_max_pages_server_enforcement( pdf_11k_pages: str, provider: base.IsolationProvider, mocker: MockerFixture, + monkeypatch: MonkeyPatch, tmpdir: str, ) -> None: provider.progress_callback = mocker.MagicMock() doc = Document(pdf_11k_pages) - with pytest.raises(errors.ConverterProcException): + + proc = None + provider.old_start_doc_to_pixels_proc = provider.start_doc_to_pixels_proc # type: ignore [attr-defined] + + def start_doc_to_pixels_proc() -> subprocess.Popen: + proc = provider.old_start_doc_to_pixels_proc() # type: ignore [attr-defined] + return proc + + monkeypatch.setattr( + provider, "start_doc_to_pixels_proc", start_doc_to_pixels_proc + ) + with pytest.raises(errors.InterruptedConversionException): provider.doc_to_pixels(doc, tmpdir) - assert provider.get_proc_exception() == errors.MaxPagesException + assert provider.get_proc_exception(proc) == errors.MaxPagesException # type: ignore [arg-type] def test_max_pages_client_enforcement( self, diff --git a/tests/isolation_provider/test_qubes.py b/tests/isolation_provider/test_qubes.py index bf8b6afea..c21614a7b 100644 --- a/tests/isolation_provider/test_qubes.py +++ b/tests/isolation_provider/test_qubes.py @@ -40,8 +40,10 @@ def test_out_of_ram( ) -> None: provider.progress_callback = mocker.MagicMock() + proc = None + def start_doc_to_pixels_proc() -> subprocess.Popen: - p = subprocess.Popen( + proc = subprocess.Popen( # XXX error 126 simulates a qrexec-policy failure. Source: # https://github.com/QubesOS/qubes-core-qrexec/blob/fdcbfd7/daemon/qrexec-daemon.c#L1022 ["exit 126"], @@ -50,13 +52,13 @@ def start_doc_to_pixels_proc() -> subprocess.Popen: stderr=subprocess.PIPE, shell=True, ) - return p + return proc monkeypatch.setattr( provider, "start_doc_to_pixels_proc", start_doc_to_pixels_proc ) - with pytest.raises(errors.ConverterProcException) as e: + with pytest.raises(errors.InterruptedConversionException) as e: doc = Document(sample_doc) provider.doc_to_pixels(doc, tmpdir) - assert provider.get_proc_exception() == errors.QubesQrexecFailed + assert provider.get_proc_exception(proc) == errors.QubesQrexecFailed # type: ignore [arg-type]