diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/PdfOverlayController.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/PdfOverlayController.java index 50b42d89d..4aab03ff5 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/api/PdfOverlayController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/PdfOverlayController.java @@ -10,6 +10,7 @@ import java.util.Map; import org.apache.pdfbox.Loader; import org.apache.pdfbox.multipdf.Overlay; +import org.apache.pdfbox.pdfwriter.compress.CompressParameters; import org.apache.pdfbox.pdmodel.PDDocument; import org.springframework.core.io.Resource; import org.springframework.http.MediaType; @@ -157,7 +158,10 @@ public class PdfOverlayController { PDDocument singlePageDocument = new PDDocument()) { singlePageDocument.addPage(overlayPdf.getPage(pageCountInCurrentOverlay)); File tempFile = Files.createTempFile("overlay-page-", ".pdf").toFile(); - singlePageDocument.save(tempFile); + // NO_COMPRESSION: this single-page doc holds a page copied from overlayPdf. + // PDFBox 3.0.7's compressed writer (PDFBOX-6203) drops shared resources imported + // across documents, corrupting overlay fonts. Revert once on 3.0.8. + singlePageDocument.save(tempFile, CompressParameters.NO_COMPRESSION); overlayGuide.put(basePageIndex, tempFile.getAbsolutePath()); tempFiles.add(tempFile); // Keep track of the temporary file for cleanup diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/RearrangePagesPDFController.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/RearrangePagesPDFController.java index 1e87a15d5..6dd7aacd7 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/api/RearrangePagesPDFController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/RearrangePagesPDFController.java @@ -6,11 +6,9 @@ import java.util.Collections; import java.util.List; import java.util.Locale; -import org.apache.pdfbox.cos.COSName; import org.apache.pdfbox.pdmodel.PDDocument; -import org.apache.pdfbox.pdmodel.PDDocumentCatalog; import org.apache.pdfbox.pdmodel.PDPage; -import org.apache.pdfbox.pdmodel.interactive.form.PDAcroForm; +import org.apache.pdfbox.pdmodel.PDPageTree; import org.springframework.core.io.Resource; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; @@ -262,38 +260,31 @@ public class RearrangePagesPDFController { } log.info("newPageOrder = {}", newPageOrder); log.info("totalPages = {}", totalPages); - // Create a new list to hold the pages in the new order - List newPages = new ArrayList<>(); - for (int i = 0; i < newPageOrder.size(); i++) { - newPages.add(document.getPage(newPageOrder.get(i))); + + // Snapshot the desired pages before mutating the source document's page tree. + List newPages = new ArrayList<>(newPageOrder.size()); + for (Integer idx : newPageOrder) { + newPages.add(document.getPage(idx)); } - // Create a new document based on the original one - try (PDDocument rearrangedDocument = - pdfDocumentFactory.createNewDocumentBasedOnOldDocument(document)) { - - // Add the pages in the new order - for (PDPage page : newPages) { - rearrangedDocument.addPage(page); - } - - PDDocumentCatalog sourceCatalog = document.getDocumentCatalog(); - if (sourceCatalog != null) { - PDAcroForm sourceForm = sourceCatalog.getAcroForm(null); - if (sourceForm != null) { - rearrangedDocument - .getDocumentCatalog() - .getCOSObject() - .setItem(COSName.ACRO_FORM, sourceForm.getCOSObject()); - } - } - - return WebResponseUtils.pdfDocToWebResponse( - rearrangedDocument, - GeneralUtils.generateFilename( - pdfFile.getOriginalFilename(), "_rearranged.pdf"), - tempFileManager); + // Rearrange in-place on the source document rather than copying pages into a + // freshly-created PDDocument. Copying pages across documents triggers a PDFBox + // 3.0.7 compressed-save regression (PDFBOX-6203, fixed for 3.0.8) where shared + // resource objects (fonts, etc.) imported from the source can be silently + // dropped from the output, producing pages with "font not found" errors. + PDPageTree pages = document.getPages(); + for (int i = totalPages - 1; i >= 0; i--) { + pages.remove(i); } + for (PDPage page : newPages) { + pages.add(page); + } + + return WebResponseUtils.pdfDocToWebResponse( + document, + GeneralUtils.generateFilename( + pdfFile.getOriginalFilename(), "_rearranged.pdf"), + tempFileManager); } } catch (IOException e) { ExceptionUtils.logException("document rearrangement", e); diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/misc/AutoSplitPdfController.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/misc/AutoSplitPdfController.java index d9714f061..ad5776b3c 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/api/misc/AutoSplitPdfController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/misc/AutoSplitPdfController.java @@ -14,6 +14,7 @@ import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; import org.apache.pdfbox.cos.COSName; +import org.apache.pdfbox.pdfwriter.compress.CompressParameters; import org.apache.pdfbox.pdmodel.PDDocument; import org.apache.pdfbox.pdmodel.PDPage; import org.apache.pdfbox.pdmodel.graphics.image.PDImageXObject; @@ -357,7 +358,10 @@ public class AutoSplitPdfController { for (int i = 0; i < splitDocuments.size(); i++) { String fileName = filename + "_" + (i + 1) + ".pdf"; zipOut.putNextEntry(new ZipEntry(fileName)); - splitDocuments.get(i).save(zipOut); + // NO_COMPRESSION: split docs are built by addPage()-ing pages copied from the + // source document. PDFBox 3.0.7's compressed writer (PDFBOX-6203) drops shared + // resources imported across documents, corrupting fonts. Revert once on 3.0.8. + splitDocuments.get(i).save(zipOut, CompressParameters.NO_COMPRESSION); zipOut.closeEntry(); } } diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/misc/OCRController.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/misc/OCRController.java index 64ed5b79e..4660d5f12 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/api/misc/OCRController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/misc/OCRController.java @@ -17,6 +17,7 @@ import javax.imageio.ImageIO; import org.apache.pdfbox.io.IOUtils; import org.apache.pdfbox.multipdf.PDFMergerUtility; +import org.apache.pdfbox.pdfwriter.compress.CompressParameters; import org.apache.pdfbox.pdmodel.PDDocument; import org.apache.pdfbox.pdmodel.PDPage; import org.apache.pdfbox.rendering.PDFRenderer; @@ -427,7 +428,10 @@ public class OCRController { // Save original page without OCR as fallback try (PDDocument pageDoc = new PDDocument()) { pageDoc.addPage(page); - pageDoc.save(pageOutputPath); + // NO_COMPRESSION: page is copied from another document; + // PDFBox 3.0.7 compressed writer (PDFBOX-6203) drops shared + // resources, corrupting fonts. Revert once on 3.0.8. + pageDoc.save(pageOutputPath, CompressParameters.NO_COMPRESSION); } } @@ -437,7 +441,10 @@ public class OCRController { // Save original page without OCR try (PDDocument pageDoc = new PDDocument()) { pageDoc.addPage(page); - pageDoc.save(pageOutputPath); + // NO_COMPRESSION: page is copied from another document; PDFBox 3.0.7 + // compressed writer (PDFBOX-6203) drops shared resources, corrupting + // fonts on retained text pages. Revert once on 3.0.8. + pageDoc.save(pageOutputPath, CompressParameters.NO_COMPRESSION); merger.addSource(pageOutputPath); } } diff --git a/app/core/src/test/java/stirling/software/SPDF/controller/api/RearrangePagesPDFControllerTest.java b/app/core/src/test/java/stirling/software/SPDF/controller/api/RearrangePagesPDFControllerTest.java index c31cae295..a225c3fc5 100644 --- a/app/core/src/test/java/stirling/software/SPDF/controller/api/RearrangePagesPDFControllerTest.java +++ b/app/core/src/test/java/stirling/software/SPDF/controller/api/RearrangePagesPDFControllerTest.java @@ -4,10 +4,14 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.*; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; import java.nio.file.Files; +import java.util.ArrayList; +import java.util.List; +import org.apache.pdfbox.Loader; import org.apache.pdfbox.pdmodel.PDDocument; import org.apache.pdfbox.pdmodel.PDPage; import org.junit.jupiter.api.BeforeEach; @@ -56,6 +60,38 @@ class RearrangePagesPDFControllerTest { "fileInput", "test.pdf", MediaType.APPLICATION_PDF_VALUE, new byte[] {1, 2, 3}); } + /** Build a real, in-memory PDDocument with the requested number of blank pages. */ + private PDDocument buildRealPdf(int pageCount) throws IOException { + PDDocument doc = new PDDocument(); + for (int i = 0; i < pageCount; i++) { + doc.addPage(new PDPage()); + } + return doc; + } + + /** + * Returns the underlying {@link org.apache.pdfbox.cos.COSDictionary} for each page in document + * order. PDPageTree returns a fresh PDPage wrapper per get(), so comparing wrappers with + * assertSame is unreliable - the COSDictionary identity is the stable handle. + */ + private List snapshotCosPages(PDDocument doc) { + List snapshot = new ArrayList<>(); + for (PDPage p : doc.getPages()) { + snapshot.add(p.getCOSObject()); + } + return snapshot; + } + + private List reloadAndSnapshot(ResponseEntity response) throws IOException { + try (var in = response.getBody().getInputStream(); + var baos = new ByteArrayOutputStream()) { + in.transferTo(baos); + try (PDDocument out = Loader.loadPDF(baos.toByteArray())) { + return snapshotCosPages(out); + } + } + } + @Test void testDeletePages_Success() throws IOException { MockMultipartFile file = createMockPdf(); @@ -83,27 +119,23 @@ class RearrangePagesPDFControllerTest { request.setPageNumbers(""); request.setCustomMode("REVERSE_ORDER"); - PDDocument mockDoc = mock(PDDocument.class); - PDDocument mockNewDoc = mock(PDDocument.class); - PDPage page0 = mock(PDPage.class); - PDPage page1 = mock(PDPage.class); - PDPage page2 = mock(PDPage.class); + try (PDDocument realDoc = buildRealPdf(3)) { + List originals = snapshotCosPages(realDoc); + when(pdfDocumentFactory.load(file)).thenReturn(realDoc); - when(pdfDocumentFactory.load(file)).thenReturn(mockDoc); - when(mockDoc.getNumberOfPages()).thenReturn(3); - when(mockDoc.getPage(0)).thenReturn(page0); - when(mockDoc.getPage(1)).thenReturn(page1); - when(mockDoc.getPage(2)).thenReturn(page2); - when(pdfDocumentFactory.createNewDocumentBasedOnOldDocument(mockDoc)) - .thenReturn(mockNewDoc); + ResponseEntity response = controller.rearrangePages(request); - ResponseEntity response = controller.rearrangePages(request); - - assertNotNull(response); - assertEquals(200, response.getStatusCode().value()); - verify(mockNewDoc).addPage(page2); - verify(mockNewDoc).addPage(page1); - verify(mockNewDoc).addPage(page0); + assertNotNull(response); + assertEquals(200, response.getStatusCode().value()); + List finalOrder = reloadAndSnapshot(response); + assertEquals(3, finalOrder.size()); + // We can no longer compare references after a save/reload, so compare via + // the in-memory snapshot taken *after* the controller mutated the source. + List mutatedSource = snapshotCosPages(realDoc); + assertSame(originals.get(2), mutatedSource.get(0)); + assertSame(originals.get(1), mutatedSource.get(1)); + assertSame(originals.get(0), mutatedSource.get(2)); + } } @Test @@ -114,25 +146,18 @@ class RearrangePagesPDFControllerTest { request.setPageNumbers(""); request.setCustomMode("REMOVE_FIRST"); - PDDocument mockDoc = mock(PDDocument.class); - PDDocument mockNewDoc = mock(PDDocument.class); - PDPage page0 = mock(PDPage.class); - PDPage page1 = mock(PDPage.class); - PDPage page2 = mock(PDPage.class); + try (PDDocument realDoc = buildRealPdf(3)) { + List originals = snapshotCosPages(realDoc); + when(pdfDocumentFactory.load(file)).thenReturn(realDoc); - when(pdfDocumentFactory.load(file)).thenReturn(mockDoc); - when(mockDoc.getNumberOfPages()).thenReturn(3); - when(mockDoc.getPage(1)).thenReturn(page1); - when(mockDoc.getPage(2)).thenReturn(page2); - when(pdfDocumentFactory.createNewDocumentBasedOnOldDocument(mockDoc)) - .thenReturn(mockNewDoc); + ResponseEntity response = controller.rearrangePages(request); - ResponseEntity response = controller.rearrangePages(request); - - assertNotNull(response); - verify(mockNewDoc).addPage(page1); - verify(mockNewDoc).addPage(page2); - verify(mockNewDoc, never()).addPage(page0); + assertNotNull(response); + List mutated = snapshotCosPages(realDoc); + assertEquals(2, mutated.size()); + assertSame(originals.get(1), mutated.get(0)); + assertSame(originals.get(2), mutated.get(1)); + } } @Test @@ -143,23 +168,18 @@ class RearrangePagesPDFControllerTest { request.setPageNumbers(""); request.setCustomMode("REMOVE_LAST"); - PDDocument mockDoc = mock(PDDocument.class); - PDDocument mockNewDoc = mock(PDDocument.class); - PDPage page0 = mock(PDPage.class); - PDPage page1 = mock(PDPage.class); + try (PDDocument realDoc = buildRealPdf(3)) { + List originals = snapshotCosPages(realDoc); + when(pdfDocumentFactory.load(file)).thenReturn(realDoc); - when(pdfDocumentFactory.load(file)).thenReturn(mockDoc); - when(mockDoc.getNumberOfPages()).thenReturn(3); - when(mockDoc.getPage(0)).thenReturn(page0); - when(mockDoc.getPage(1)).thenReturn(page1); - when(pdfDocumentFactory.createNewDocumentBasedOnOldDocument(mockDoc)) - .thenReturn(mockNewDoc); + ResponseEntity response = controller.rearrangePages(request); - ResponseEntity response = controller.rearrangePages(request); - - assertNotNull(response); - verify(mockNewDoc).addPage(page0); - verify(mockNewDoc).addPage(page1); + assertNotNull(response); + List mutated = snapshotCosPages(realDoc); + assertEquals(2, mutated.size()); + assertSame(originals.get(0), mutated.get(0)); + assertSame(originals.get(1), mutated.get(1)); + } } @Test @@ -170,21 +190,19 @@ class RearrangePagesPDFControllerTest { request.setPageNumbers(""); request.setCustomMode("REMOVE_FIRST_AND_LAST"); - PDDocument mockDoc = mock(PDDocument.class); - PDDocument mockNewDoc = mock(PDDocument.class); - PDPage page1 = mock(PDPage.class); + try (PDDocument realDoc = buildRealPdf(4)) { + List originals = snapshotCosPages(realDoc); + when(pdfDocumentFactory.load(file)).thenReturn(realDoc); - when(pdfDocumentFactory.load(file)).thenReturn(mockDoc); - when(mockDoc.getNumberOfPages()).thenReturn(4); - when(mockDoc.getPage(1)).thenReturn(page1); - when(mockDoc.getPage(2)).thenReturn(page1); - when(pdfDocumentFactory.createNewDocumentBasedOnOldDocument(mockDoc)) - .thenReturn(mockNewDoc); + ResponseEntity response = controller.rearrangePages(request); - ResponseEntity response = controller.rearrangePages(request); - - assertNotNull(response); - assertEquals(200, response.getStatusCode().value()); + assertNotNull(response); + assertEquals(200, response.getStatusCode().value()); + List mutated = snapshotCosPages(realDoc); + assertEquals(2, mutated.size()); + assertSame(originals.get(1), mutated.get(0)); + assertSame(originals.get(2), mutated.get(1)); + } } @Test @@ -195,23 +213,15 @@ class RearrangePagesPDFControllerTest { request.setPageNumbers(""); request.setCustomMode("DUPLEX_SORT"); - PDDocument mockDoc = mock(PDDocument.class); - PDDocument mockNewDoc = mock(PDDocument.class); - PDPage page0 = mock(PDPage.class); - PDPage page1 = mock(PDPage.class); - PDPage page2 = mock(PDPage.class); - PDPage page3 = mock(PDPage.class); + try (PDDocument realDoc = buildRealPdf(4)) { + when(pdfDocumentFactory.load(file)).thenReturn(realDoc); - when(pdfDocumentFactory.load(file)).thenReturn(mockDoc); - when(mockDoc.getNumberOfPages()).thenReturn(4); - when(mockDoc.getPage(anyInt())).thenReturn(page0); - when(pdfDocumentFactory.createNewDocumentBasedOnOldDocument(mockDoc)) - .thenReturn(mockNewDoc); + ResponseEntity response = controller.rearrangePages(request); - ResponseEntity response = controller.rearrangePages(request); - - assertNotNull(response); - assertEquals(200, response.getStatusCode().value()); + assertNotNull(response); + assertEquals(200, response.getStatusCode().value()); + assertEquals(4, realDoc.getNumberOfPages()); + } } @Test @@ -222,20 +232,15 @@ class RearrangePagesPDFControllerTest { request.setPageNumbers(""); request.setCustomMode("BOOKLET_SORT"); - PDDocument mockDoc = mock(PDDocument.class); - PDDocument mockNewDoc = mock(PDDocument.class); - PDPage page = mock(PDPage.class); + try (PDDocument realDoc = buildRealPdf(4)) { + when(pdfDocumentFactory.load(file)).thenReturn(realDoc); - when(pdfDocumentFactory.load(file)).thenReturn(mockDoc); - when(mockDoc.getNumberOfPages()).thenReturn(4); - when(mockDoc.getPage(anyInt())).thenReturn(page); - when(pdfDocumentFactory.createNewDocumentBasedOnOldDocument(mockDoc)) - .thenReturn(mockNewDoc); + ResponseEntity response = controller.rearrangePages(request); - ResponseEntity response = controller.rearrangePages(request); - - assertNotNull(response); - assertEquals(200, response.getStatusCode().value()); + assertNotNull(response); + assertEquals(200, response.getStatusCode().value()); + assertEquals(4, realDoc.getNumberOfPages()); + } } @Test @@ -246,20 +251,15 @@ class RearrangePagesPDFControllerTest { request.setPageNumbers(""); request.setCustomMode("ODD_EVEN_SPLIT"); - PDDocument mockDoc = mock(PDDocument.class); - PDDocument mockNewDoc = mock(PDDocument.class); - PDPage page = mock(PDPage.class); + try (PDDocument realDoc = buildRealPdf(4)) { + when(pdfDocumentFactory.load(file)).thenReturn(realDoc); - when(pdfDocumentFactory.load(file)).thenReturn(mockDoc); - when(mockDoc.getNumberOfPages()).thenReturn(4); - when(mockDoc.getPage(anyInt())).thenReturn(page); - when(pdfDocumentFactory.createNewDocumentBasedOnOldDocument(mockDoc)) - .thenReturn(mockNewDoc); + ResponseEntity response = controller.rearrangePages(request); - ResponseEntity response = controller.rearrangePages(request); - - assertNotNull(response); - assertEquals(200, response.getStatusCode().value()); + assertNotNull(response); + assertEquals(200, response.getStatusCode().value()); + assertEquals(4, realDoc.getNumberOfPages()); + } } @Test @@ -270,24 +270,20 @@ class RearrangePagesPDFControllerTest { request.setPageNumbers("3,1,2"); request.setCustomMode("custom"); - PDDocument mockDoc = mock(PDDocument.class); - PDDocument mockNewDoc = mock(PDDocument.class); - PDPage page0 = mock(PDPage.class); - PDPage page1 = mock(PDPage.class); - PDPage page2 = mock(PDPage.class); + try (PDDocument realDoc = buildRealPdf(3)) { + List originals = snapshotCosPages(realDoc); + when(pdfDocumentFactory.load(file)).thenReturn(realDoc); - when(pdfDocumentFactory.load(file)).thenReturn(mockDoc); - when(mockDoc.getNumberOfPages()).thenReturn(3); - when(mockDoc.getPage(0)).thenReturn(page0); - when(mockDoc.getPage(1)).thenReturn(page1); - when(mockDoc.getPage(2)).thenReturn(page2); - when(pdfDocumentFactory.createNewDocumentBasedOnOldDocument(mockDoc)) - .thenReturn(mockNewDoc); + ResponseEntity response = controller.rearrangePages(request); - ResponseEntity response = controller.rearrangePages(request); - - assertNotNull(response); - assertEquals(200, response.getStatusCode().value()); + assertNotNull(response); + assertEquals(200, response.getStatusCode().value()); + List mutated = snapshotCosPages(realDoc); + assertEquals(3, mutated.size()); + assertSame(originals.get(2), mutated.get(0)); + assertSame(originals.get(0), mutated.get(1)); + assertSame(originals.get(1), mutated.get(2)); + } } @Test @@ -298,21 +294,15 @@ class RearrangePagesPDFControllerTest { request.setPageNumbers("3"); request.setCustomMode("DUPLICATE"); - PDDocument mockDoc = mock(PDDocument.class); - PDDocument mockNewDoc = mock(PDDocument.class); - PDPage page = mock(PDPage.class); + try (PDDocument realDoc = buildRealPdf(2)) { + when(pdfDocumentFactory.load(file)).thenReturn(realDoc); - when(pdfDocumentFactory.load(file)).thenReturn(mockDoc); - when(mockDoc.getNumberOfPages()).thenReturn(2); - when(mockDoc.getPage(anyInt())).thenReturn(page); - when(pdfDocumentFactory.createNewDocumentBasedOnOldDocument(mockDoc)) - .thenReturn(mockNewDoc); + ResponseEntity response = controller.rearrangePages(request); - ResponseEntity response = controller.rearrangePages(request); - - assertNotNull(response); - // 2 pages * 3 duplicates = 6 addPage calls - verify(mockNewDoc, times(6)).addPage(page); + assertNotNull(response); + // 2 pages * 3 duplicates = 6 final pages + assertEquals(6, realDoc.getNumberOfPages()); + } } @Test @@ -323,19 +313,14 @@ class RearrangePagesPDFControllerTest { request.setPageNumbers(""); request.setCustomMode("SIDE_STITCH_BOOKLET_SORT"); - PDDocument mockDoc = mock(PDDocument.class); - PDDocument mockNewDoc = mock(PDDocument.class); - PDPage page = mock(PDPage.class); + try (PDDocument realDoc = buildRealPdf(4)) { + when(pdfDocumentFactory.load(file)).thenReturn(realDoc); - when(pdfDocumentFactory.load(file)).thenReturn(mockDoc); - when(mockDoc.getNumberOfPages()).thenReturn(4); - when(mockDoc.getPage(anyInt())).thenReturn(page); - when(pdfDocumentFactory.createNewDocumentBasedOnOldDocument(mockDoc)) - .thenReturn(mockNewDoc); + ResponseEntity response = controller.rearrangePages(request); - ResponseEntity response = controller.rearrangePages(request); - - assertNotNull(response); - assertEquals(200, response.getStatusCode().value()); + assertNotNull(response); + assertEquals(200, response.getStatusCode().value()); + assertEquals(4, realDoc.getNumberOfPages()); + } } }