Fix isolation provider tests

Conversions methods had changed and that was part of the reason why
the tests were failing. Furthermore, due to the `provider.proc`, which
stores the associated qrexec / container process, "server" exceptions
raise a IterruptedConversion error (now ConverterProcException), which
then requires interpretation of the process exit code to obtain the
"real" exception.

dangerzone/conversion/errors.py

@@ -1,4 +1,4 @@
-from typing import List, Optional, Type
+from typing import List, Optional, Type, Union
 
 # XXX: errors start at 128 for conversion-related issues
 ERROR_SHIFT = 128
@@ -86,8 +86,8 @@ class PageCountMismatch(PagesException):
     )
 
 
-class InterruptedConversion(ConversionException):
+class ConverterProcException(ConversionException):
-    """Protocol received num of bytes different than expected"""
+    """Some exception occurred in the converter"""
 
     error_code = ERROR_SHIFT + 60
     error_message = (
@@ -100,7 +100,9 @@ class UnexpectedConversionError(ConversionException):
     error_message = "Some unexpected error occurred while converting the document"
 
 
-def exception_from_error_code(error_code: int) -> Optional[ConversionException]:
+def exception_from_error_code(
+    error_code: int,
+) -> Union[ConversionException, ValueError]:
     """returns the conversion exception corresponding to the error code"""
     for cls in ConversionException.get_subclasses():
         if cls.error_code == error_code:

dangerzone/isolation_provider/base.py

@@ -27,7 +27,7 @@ def read_bytes(f: IO[bytes], size: int, timeout: float, exact: bool = True) -> b
     """Read bytes from a file-like object."""
     buf = nonblocking_read(f, size, timeout)
     if exact and len(buf) != size:
-        raise errors.InterruptedConversion
+        raise errors.ConverterProcException()
     return buf
 
 
@@ -80,11 +80,9 @@ class IsolationProvider(ABC):
             document.mark_as_safe()
             if document.archive_after_conversion:
                 document.archive()
-        except errors.InterruptedConversion:
+        except errors.ConverterProcException:
-            assert self.proc is not None
+            exception = self.get_proc_exception()
-            error_code = self.proc.wait(3)
+            self.print_progress(document, True, str(exception), 0)
-            # XXX Reconstruct exception from error code
-            exception = errors.exception_from_error_code(error_code)
             document.mark_as_failed()
         except errors.ConversionException as e:
             self.print_progress(document, True, str(e), 0)
@@ -105,7 +103,7 @@ class IsolationProvider(ABC):
                 self.proc.stdin.write(f.read())
                 self.proc.stdin.close()
             except BrokenPipeError as e:
-                raise errors.InterruptedConversion()
+                raise errors.ConverterProcException()
 
             # Get file size (in MiB)
             size = os.path.getsize(document.input_filename) / 1024**2
@@ -188,6 +186,12 @@ class IsolationProvider(ABC):
         if self.progress_callback:
             self.progress_callback(error, text, percentage)
 
+    def get_proc_exception(self) -> Exception:
+        """Returns an exception associated with a process exit code"""
+        assert self.proc
+        error_code = self.proc.wait(3)
+        return errors.exception_from_error_code(error_code)
+
     @abstractmethod
     def get_max_parallel_conversions(self) -> int:
         pass

dangerzone/isolation_provider/container.py

@@ -237,7 +237,7 @@ class Container(IsolationProvider):
         error_code = pixels_to_pdf_proc.wait()
         if error_code != 0:
             log.error("pixels-to-pdf failed")
-            raise errors.exception_from_error_code(error_code)  # type: ignore [misc]
+            raise errors.exception_from_error_code(error_code)
         else:
             # Move the final file to the right place
             if os.path.exists(document.output_filename):

tests/isolation_provider/base.py

@@ -18,14 +18,15 @@ from .. import pdf_11k_pages, sanitized_text, uncommon_text
 )
 @pytest.mark.skipif(not running_on_qubes(), reason="Not on a Qubes system")
 class IsolationProviderTest:
-    def test_max_pages_received(
+    def test_max_pages_server_enforcement(
         self,
         pdf_11k_pages: str,
         provider: base.IsolationProvider,
         mocker: MockerFixture,
+        tmpdir: str,
     ) -> None:
         provider.progress_callback = mocker.MagicMock()
         doc = Document(pdf_11k_pages)
-        with pytest.raises(errors.MaxPagesException):
+        with pytest.raises(errors.ConverterProcException):
-            success = provider._convert(doc, ocr_lang=None)
+            provider.doc_to_pixels(doc, tmpdir)
-            assert not success
+            assert provider.get_proc_exception() == errors.MaxPagesException

tests/isolation_provider/test_qubes.py

@@ -30,11 +30,12 @@ def provider() -> Qubes:
 
 @pytest.mark.skipif(not running_on_qubes(), reason="Not on a Qubes system")
 class TestQubes(IsolationProviderTest):
-    def test_max_pages_client_side_enforcement(
+    def test_max_pages_client_enforcement(
         self,
         sample_doc: str,
         provider: Qubes,
         mocker: MockerFixture,
+        tmpdir: str,
     ) -> None:
         provider.progress_callback = mocker.MagicMock()
         mocker.patch(
@@ -42,8 +43,7 @@ class TestQubes(IsolationProviderTest):
         )  # sample_doc has 4 pages > 1
         doc = Document(sample_doc)
         with pytest.raises(errors.MaxPagesException):
-            success = provider._convert(doc, ocr_lang=None)
+            provider.doc_to_pixels(doc, tmpdir)
-            assert not success
 
     def test_max_dimensions(
         self,
@@ -51,14 +51,13 @@ class TestQubes(IsolationProviderTest):
         sample_bad_height: str,
         provider: Qubes,
         mocker: MockerFixture,
+        tmpdir: str,
     ) -> None:
         provider.progress_callback = mocker.MagicMock()
         with pytest.raises(errors.MaxPageWidthException):
-            success = provider._convert(Document(sample_bad_width), ocr_lang=None)
+            provider.doc_to_pixels(Document(sample_bad_width), tmpdir)
-            assert not success
         with pytest.raises(errors.MaxPageHeightException):
-            success = provider._convert(Document(sample_bad_height), ocr_lang=None)
+            provider.doc_to_pixels(Document(sample_bad_height), tmpdir)
-            assert not success
 
     def test_out_of_ram(
         self,
@@ -66,6 +65,7 @@ class TestQubes(IsolationProviderTest):
         mocker: MockerFixture,
         monkeypatch: MonkeyPatch,
         sample_doc: str,
+        tmpdir: str,
     ) -> None:
         provider.progress_callback = mocker.MagicMock()
 
@@ -85,6 +85,7 @@ class TestQubes(IsolationProviderTest):
             provider, "start_doc_to_pixels_proc", start_doc_to_pixels_proc
         )
 
-        with pytest.raises(errors.QubesQrexecFailed) as e:
+        with pytest.raises(errors.ConverterProcException) as e:
             doc = Document(sample_doc)
-            provider._convert(doc, ocr_lang=None)
+            provider.doc_to_pixels(doc, tmpdir)
+            assert provider.get_proc_exception() == errors.QubesQrexecFailed