Save edited PDF to temp file before replacing original

src-ui/src/app/components/document-detail/document-detail.component.html

@@ -59,7 +59,7 @@
       </button>
 
       <button ngbDropdownItem (click)="editPdf()" [disabled]="!userIsOwner || !userCanEdit || originalContentRenderType !== ContentRenderType.PDF">
-        <i-bs name="pencil"></i-bs>&nbsp;<ng-container i18n>Edit PDF</ng-container>
+        <i-bs name="pencil"></i-bs>&nbsp;<ng-container i18n>PDF Editor</ng-container>
       </button>
     </div>
   </div>

src-ui/src/app/components/document-detail/document-detail.component.ts

@@ -1356,7 +1356,7 @@ export class DocumentDetailComponent
       size: 'xl',
       scrollable: true,
     })
-    modal.componentInstance.title = $localize`Edit PDF`
+    modal.componentInstance.title = $localize`PDF Editor`
     modal.componentInstance.btnCaption = $localize`Proceed`
     modal.componentInstance.documentID = this.document.id
     modal.componentInstance.confirmClicked

src/documents/bulk_edit.py

@@ -532,7 +532,7 @@ def edit_pdf(
                 logger.error(
                     "Update requested but multiple output documents specified",
                 )
-                return "ERROR"
+                raise ValueError("Multiple output documents specified")
 
             for op in operations:
                 dst = pdf_docs[op.get("doc", 0)]
@@ -542,9 +542,13 @@ def edit_pdf(
                     dst.pages[-1].rotate(op["rotate"], relative=True)
 
         if update_document:
+            temp_path = doc.source_path.with_suffix(".tmp.pdf")
             pdf = pdf_docs[0]
             pdf.remove_unreferenced_resources()
-            pdf.save(doc.source_path)
+            # save the edited PDF to a temporary file in case of errors
+            pdf.save(temp_path)
+            # replace the original document with the edited one
+            temp_path.replace(doc.source_path)
             doc.checksum = hashlib.md5(doc.source_path.read_bytes()).hexdigest()
             doc.page_count = len(pdf.pages)
             doc.save()
@@ -583,7 +587,9 @@ def edit_pdf(
 
     except Exception as e:
         logger.exception(f"Error editing document {doc.id}: {e}")
-        return "ERROR"
+        raise ValueError(
+            f"An error occurred while editing the document: {e}",
+        ) from e
 
     return "OK"
 

src/documents/serialisers.py

@@ -1524,7 +1524,7 @@ class BulkEditSerializer(
         else:
             parameters["archive_fallback"] = False
 
-    def _validate_parameters_edit_pdf(self, parameters):
+    def _validate_parameters_edit_pdf(self, parameters, document_id):
         if "operations" not in parameters:
             raise serializers.ValidationError("operations not specified")
         if not isinstance(parameters["operations"], list):
@@ -1556,6 +1556,15 @@ class BulkEditSerializer(
                     "update_document only allowed with a single output document",
                 )
 
+        doc = Document.objects.get(id=document_id)
+        # doc existence is already validated
+        if doc.page_count:
+            for op in parameters["operations"]:
+                if op["page"] < 1 or op["page"] > doc.page_count:
+                    raise serializers.ValidationError(
+                        f"Page {op['page']} is out of bounds for document with {doc.page_count} pages.",
+                    )
+
     def validate(self, attrs):
         method = attrs["method"]
         parameters = attrs["parameters"]
@@ -1595,7 +1604,7 @@ class BulkEditSerializer(
                 raise serializers.ValidationError(
                     "Edit PDF method only supports one document",
                 )
-            self._validate_parameters_edit_pdf(parameters)
+            self._validate_parameters_edit_pdf(parameters, attrs["documents"][0])
 
         return attrs
 

src/documents/tests/test_api_bulk_edit.py

@@ -41,6 +41,7 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase):
             title="B",
             correspondent=self.c1,
             document_type=self.dt1,
+            page_count=5,
         )
         self.doc3 = Document.objects.create(
             checksum="C",
@@ -1555,6 +1556,32 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase):
             response.content,
         )
 
+    @mock.patch("documents.serialisers.bulk_edit.edit_pdf")
+    def test_edit_pdf_page_out_of_bounds(self, m):
+        """
+        GIVEN:
+            - API data for editing PDF is called
+            - The page number is out of bounds
+        WHEN:
+            - API is called
+        THEN:
+            - The API fails with a correct error code
+        """
+        self.setup_mock(m, "edit_pdf")
+        response = self.client.post(
+            "/api/documents/bulk_edit/",
+            json.dumps(
+                {
+                    "documents": [self.doc2.id],
+                    "method": "edit_pdf",
+                    "parameters": {"operations": [{"page": 99}]},
+                },
+            ),
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertIn(b"out of bounds", response.content)
+
     @override_settings(AUDIT_LOG_ENABLED=True)
     def test_bulk_edit_audit_log_enabled_simple_field(self):
         """

src/documents/tests/test_bulk_edit.py

@@ -1032,8 +1032,8 @@ class TestPDFActions(DirectoriesMixin, TestCase):
             {"page": 9999},  # invalid page, forces error during PDF load
         ]
         with self.assertLogs("paperless.bulk_edit", level="ERROR"):
-            result = bulk_edit.edit_pdf(doc_ids, operations)
+            with self.assertRaises(Exception):
-            self.assertEqual(result, "ERROR")
+                bulk_edit.edit_pdf(doc_ids, operations)
         mock_group.assert_not_called()
         mock_consume_file.assert_not_called()
 
@@ -1058,7 +1058,7 @@ class TestPDFActions(DirectoriesMixin, TestCase):
             {"page": 2, "doc": 1},
         ]
         with self.assertLogs("paperless.bulk_edit", level="ERROR"):
-            result = bulk_edit.edit_pdf(doc_ids, operations, update_document=True)
+            with self.assertRaises(ValueError):
-            self.assertEqual(result, "ERROR")
+                bulk_edit.edit_pdf(doc_ids, operations, update_document=True)
         mock_group.assert_not_called()
         mock_consume_file.assert_not_called()

src/documents/views.py

@@ -1427,7 +1427,6 @@ class BulkEditView(PassUserMixin):
                     )
                 }
 
-            # TODO: parameter validation
             result = method(documents, **parameters)
 
             if settings.AUDIT_LOG_ENABLED and modified_field: