Do not create outfile files when checking if writeable

Checking if files were writeable created files in the process. In the
case where someone adds a list of N files to dangerzone but exits before
converting, they would be left with N 0-byte files for the -safe
version. Now they don't.

Fixes #214

CHANGELOG.md

@@ -7,6 +7,7 @@
 - Feature: Add Debian Bookworm (12) support
 - Reinstate Ubuntu Focal support ([issue #206](https://github.com/freedomofpress/dangerzone/issues/206))
 - Feature: support multiple input documents in the CLI-version
+- Bug fix: Failed execution no longer produces an empty "safe" documents ([issue #214](https://github.com/freedomofpress/dangerzone/issues/214))
 
 ## Dangerzone 0.3.2
 - Bug fix: some non-ascii characters like “ would prevent Dangerzone from working  ([issue #144](https://github.com/freedomofpress/dangerzone/issues/144))

dangerzone/document.py

@@ -5,6 +5,7 @@ import platform
 import secrets
 import stat
 import tempfile
+from pathlib import Path
 from typing import Optional
 
 import appdirs
@@ -62,11 +63,9 @@ class Document:
     def validate_output_filename(filename: str) -> None:
         if not filename.endswith(".pdf"):
             raise errors.NonPDFOutputFileException()
-        try:
+        if not os.access(Path(filename).parent, os.W_OK):
-            with open(filename, "wb"):
+            # in unwriteable directory
-                pass
+            raise errors.UnwriteableOutputDirException()
-        except PermissionError as e:
-            raise errors.UnwriteableOutputFileException() from e
 
     @property
     def input_filename(self) -> str:

dangerzone/errors.py

@@ -35,7 +35,7 @@ class NonPDFOutputFileException(DocumentFilenameException):
         super().__init__("Safe PDF filename must end in '.pdf'")
 
 
-class UnwriteableOutputFileException(DocumentFilenameException):
+class UnwriteableOutputDirException(DocumentFilenameException):
     """Exception for when the output file is not writeable."""
 
     def __init__(self) -> None:

tests/__init__.py

@@ -30,13 +30,6 @@ def sample_doc() -> str:
     return str(test_docs_dir.joinpath(BASIC_SAMPLE))
 
 
-@pytest.fixture
-def unwriteable_pdf(tmp_path: Path) -> str:
-    file_path = tmp_path / "document.pdf"
-    file_path.touch(mode=0o400)
-    return str(file_path)
-
-
 @pytest.fixture
 def unreadable_pdf(tmp_path: Path) -> str:
     file_path = tmp_path / "document.pdf"

tests/test_document.py

@@ -8,7 +8,7 @@ import pytest
 from dangerzone import errors
 from dangerzone.document import Document
 
-from . import sample_doc, unreadable_pdf, unwriteable_pdf
+from . import sample_doc, unreadable_pdf
 
 
 def test_input_sample_init(sample_doc: str) -> None:
@@ -43,11 +43,13 @@ def test_input_file_unreadable(unreadable_pdf: str) -> None:
         Document(unreadable_pdf)
 
 
-def test_output_file_unwriteable(unwriteable_pdf: str) -> None:
+@pytest.mark.skipif(platform.system() == "Windows", reason="Unix-specific")
-    d = Document()
+def test_output_file_unwriteable_dir(sample_doc: str, tmp_path: Path) -> None:
-    with pytest.raises(errors.UnwriteableOutputFileException) as e:
+    # make parent dir unwriteable
-        d.output_filename = unwriteable_pdf
+    sample_doc_safe = str(tmp_path / "document-safe.pdf")
-    assert "Safe PDF filename is not writable" in str(e.value)
+    os.chmod(tmp_path, 0o400)
+    with pytest.raises(errors.UnwriteableOutputDirException) as e:
+        d = Document(sample_doc, sample_doc_safe)
 
 
 def test_output(tmp_path: Path) -> None: