From 0b738ba49021d2e2e1dcba3e8bd55cc7cae2b669 Mon Sep 17 00:00:00 2001
From: deeplow <deeplower@protonmail.com>
Date: Wed, 2 Nov 2022 16:50:39 +0000
Subject: [PATCH] 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           |  1 +
 dangerzone/document.py |  9 ++++-----
 dangerzone/errors.py   |  2 +-
 tests/__init__.py      |  7 -------
 tests/test_document.py | 14 ++++++++------
 5 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6c3ead8..bd4a241 100644
--- a/CHANGELOG.md
+++ b/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))
diff --git a/dangerzone/document.py b/dangerzone/document.py
index b49b512..d1e311c 100644
--- a/dangerzone/document.py
+++ b/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:
-            with open(filename, "wb"):
-                pass
-        except PermissionError as e:
-            raise errors.UnwriteableOutputFileException() from e
+        if not os.access(Path(filename).parent, os.W_OK):
+            # in unwriteable directory
+            raise errors.UnwriteableOutputDirException()
 
     @property
     def input_filename(self) -> str:
diff --git a/dangerzone/errors.py b/dangerzone/errors.py
index 00eb702..1e6811a 100644
--- a/dangerzone/errors.py
+++ b/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:
diff --git a/tests/__init__.py b/tests/__init__.py
index dd32eac..2733a0f 100644
--- a/tests/__init__.py
+++ b/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"
diff --git a/tests/test_document.py b/tests/test_document.py
index 9e9d23b..3b0bdba 100644
--- a/tests/test_document.py
+++ b/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:
-    d = Document()
-    with pytest.raises(errors.UnwriteableOutputFileException) as e:
-        d.output_filename = unwriteable_pdf
-    assert "Safe PDF filename is not writable" in str(e.value)
+@pytest.mark.skipif(platform.system() == "Windows", reason="Unix-specific")
+def test_output_file_unwriteable_dir(sample_doc: str, tmp_path: Path) -> None:
+    # make parent dir unwriteable
+    sample_doc_safe = str(tmp_path / "document-safe.pdf")
+    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: