From 634523dac91eb4921e2055f19805c4b9b348939b Mon Sep 17 00:00:00 2001 From: Alex Pyrgiotis Date: Mon, 19 Feb 2024 15:06:31 +0200 Subject: [PATCH] Get underlying error when conversion fails When we get an early EOF from the converter process, we should immediately get the exit code of that process, to find out the actual underlying error. Currently, the exception we raise masks the underlying error. Raise a ConverterProcException, that in turns makes our error handling code read the exit code of the spawned process, and converts it to a helpful error message. Fixes #714 --- dangerzone/conversion/errors.py | 14 ++------------ dangerzone/isolation_provider/base.py | 8 ++++---- tests/isolation_provider/base.py | 2 +- tests/isolation_provider/test_qubes.py | 2 +- 4 files changed, 8 insertions(+), 18 deletions(-) diff --git a/dangerzone/conversion/errors.py b/dangerzone/conversion/errors.py index 235ffc9..2156087 100644 --- a/dangerzone/conversion/errors.py +++ b/dangerzone/conversion/errors.py @@ -8,21 +8,11 @@ MAX_PAGE_WIDTH = 10000 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__() + def __init__(self) -> None: + super().__init__("The process spawned for the conversion has exited early") class ConversionException(Exception): diff --git a/dangerzone/isolation_provider/base.py b/dangerzone/isolation_provider/base.py index 6becee6..d378eee 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.InterruptedConversionException() + raise errors.ConverterProcException() 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.InterruptedConversionException() + raise errors.ConverterProcException() return int.from_bytes(untrusted_int, "big", signed=False) @@ -80,7 +80,7 @@ class IsolationProvider(ABC): if document.archive_after_conversion: document.archive() except errors.ConverterProcException as e: - exception = self.get_proc_exception(e.proc) + exception = self.get_proc_exception(conversion_proc) self.print_progress(document, True, str(exception), 0) document.mark_as_failed() except errors.ConversionException as e: @@ -103,7 +103,7 @@ class IsolationProvider(ABC): p.stdin.write(f.read()) p.stdin.close() except BrokenPipeError as e: - raise errors.ConverterProcException(p) + raise errors.ConverterProcException() assert p.stdout n_pages = read_int(p.stdout) diff --git a/tests/isolation_provider/base.py b/tests/isolation_provider/base.py index b9071b0..997471f 100644 --- a/tests/isolation_provider/base.py +++ b/tests/isolation_provider/base.py @@ -48,7 +48,7 @@ class IsolationProviderTest: monkeypatch.setattr( provider, "start_doc_to_pixels_proc", start_doc_to_pixels_proc ) - with pytest.raises(errors.InterruptedConversionException): + with pytest.raises(errors.ConverterProcException): provider.doc_to_pixels(doc, tmpdir) assert provider.get_proc_exception(proc) == errors.MaxPagesException # type: ignore [arg-type] diff --git a/tests/isolation_provider/test_qubes.py b/tests/isolation_provider/test_qubes.py index c21614a..c6dfd7f 100644 --- a/tests/isolation_provider/test_qubes.py +++ b/tests/isolation_provider/test_qubes.py @@ -58,7 +58,7 @@ class TestQubes(IsolationProviderTest): provider, "start_doc_to_pixels_proc", start_doc_to_pixels_proc ) - with pytest.raises(errors.InterruptedConversionException) as e: + with pytest.raises(errors.ConverterProcException) as e: doc = Document(sample_doc) provider.doc_to_pixels(doc, tmpdir) assert provider.get_proc_exception(proc) == errors.QubesQrexecFailed # type: ignore [arg-type]