Merge pull request #2184 from freedomofpress/revert-magicbyte-check

Revert magicbytes (-M) check for print conversion

export/securedrop_export/print/service.py

@@ -314,10 +314,7 @@ def _needs_pdf_conversion(self, filename: Path):
             # b'filename that may have spaces.docx: application/bla\n'
             # use magic bytes (-M) for filetype detection
             mimetype = (
-                subprocess.check_output(["mimetype", "-M", filename])
-                .decode()
-                .split(":")[-1]
-                .strip()
+                subprocess.check_output(["mimetype", filename]).decode().split(":")[-1].strip()
             )
         except subprocess.CalledProcessError:
             logger.error(f"Could not process mimetype of {filename}")

export/tests/print/test_service.py

@@ -20,7 +20,8 @@
 SAMPLE_OUTPUT_UNSUPPORTED_PRINTER = b"network beh\nnetwork https\nnetwork ipp\nnetwork ipps\nnetwork http\nnetwork\nnetwork ipp14\ndirect usb://Canon/QL-700%?serial=A00000A000000\nnetwork lpd"  # noqa
 
 SUPPORTED_MIMETYPE_COUNT = 107  # Mimetypes in the sample LibreOffice .desktop files
-SAMPLE_ODT_FILENAME = "Sample_Print.odt"  # see export/tests/files
+SAMPLE_FILES_SUPPORTED = Path.cwd() / "tests" / "files" / "samples_supported"
+SAMPLE_FILES_UNSUPPORTED = Path.cwd() / "tests" / "files" / "samples_unsupported"
 
 
 class TestPrint:
@@ -163,27 +164,31 @@ def test__get_supported_mimetypes_libreoffice_integration(self, capsys):
         assert "application/vnd.oasis.opendocument.text" in mimes
         assert "application/vnd.openxmlformats-officedocument.wordprocessingml.document" in mimes
 
-    def test__print_file_with_libreoffice_conversion_integration(self, capsys):
+    @pytest.mark.parametrize("sample_file", [i for i in os.listdir(SAMPLE_FILES_SUPPORTED)])
+    def test__print_file_with_libreoffice_conversion_integration(self, sample_file, capsys):
         apps = Path("/usr/share/applications")
         if not (apps / "libreoffice-writer.desktop").exists():
             pytest.skip("libreoffice doesn't appear to be installed")
 
-        # Set up a sample print directory with a real .odt file
+        # Set up a sample print directory with a real file
         print_dir = tempfile.TemporaryDirectory()
-        filepath = Path.cwd() / "tests" / "files" / SAMPLE_ODT_FILENAME
+        filepath = SAMPLE_FILES_SUPPORTED / sample_file
         shutil.copy(filepath, print_dir.name)
 
-        target = Path(print_dir.name, SAMPLE_ODT_FILENAME)
-        expected_conversion_file = target.parent / "print-pdf" / (target.stem + ".pdf")
+        target = Path(print_dir.name, sample_file)
+
+        if self.service._needs_pdf_conversion(target):
+            expected = target.parent / "print-pdf" / (target.stem + ".pdf")
+        else:
+            expected = target
 
         with (
             mock.patch("subprocess.check_call") as mock_print_xpp,
-            mock.patch("securedrop_export.print.service.logger.info") as log,
             mock.patch.object(self.service, "_wait_for_print") as mock_wait_for_print,
         ):
             self.service._print_file(target)
 
-        assert expected_conversion_file.exists()
+        assert expected.exists()
         assert mock_wait_for_print.call_count == 1
         assert mock_print_xpp.call_count == 1
         mock_print_xpp.assert_has_calls(
@@ -193,18 +198,29 @@ def test__print_file_with_libreoffice_conversion_integration(self, capsys):
                         "xpp",
                         "-P",
                         "sdw-printer",
-                        expected_conversion_file,
+                        expected,
                     ],
                 ),
             ]
         )
-        assert log.call_count == 2
-        log.assert_has_calls(
-            [
-                mock.call("Convert to pdf for printing"),
-                mock.call("Sending file to printer sdw-printer"),
-            ]
-        )
+
+    @pytest.mark.parametrize("sample_file", [i for i in os.listdir(SAMPLE_FILES_UNSUPPORTED)])
+    def test__print_file_unsupported_integration(self, sample_file, capsys):
+        apps = Path("/usr/share/applications")
+        if not (apps / "libreoffice-writer.desktop").exists():
+            pytest.skip("libreoffice doesn't appear to be installed")
+
+        # Set up a sample print directory with a real file
+        print_dir = tempfile.TemporaryDirectory()
+        filepath = SAMPLE_FILES_UNSUPPORTED / sample_file
+        shutil.copy(filepath, print_dir.name)
+
+        target = Path(print_dir.name, sample_file)
+
+        with pytest.raises(ExportException) as ex:
+            self.service._print_file(target)
+
+        assert ex.value.sdstatus == Status.ERROR_MIMETYPE_UNSUPPORTED
 
     @mock.patch("subprocess.run")
     def test_install_printer_ppd_laserjet(self, mocker):