From 575684ee4bdbaadc29a2acd12ee000fc2549030e Mon Sep 17 00:00:00 2001 From: James Brunton Date: Mon, 11 May 2026 10:57:41 +0100 Subject: [PATCH] Add edit text support to stirling engine (#6245) # Description of Changes Hooks up the (alpha) PDF Editor backend to the AI engine Edit Agent via an intermediary API which is easier for the agent to call. It suffers from all the same issues that the PDF Editor does in actually editing the text, but should also benefit from any fixes to that. It also adds protection against the underlying tools misbehaving by hanging, and fixes a hanging bug in the PDF Editor. --------- Co-authored-by: EthanHealy01 <80844253+EthanHealy01@users.noreply.github.com> --- .../common/model/ApplicationProperties.java | 14 + .../model/api/general/EditTextOperation.java | 19 + .../common/service/InternalApiClient.java | 72 +- .../service/InternalApiTimeoutException.java | 36 + .../StringToArrayListPropertyEditor.java | 24 +- .../common/service/InternalApiClientTest.java | 90 ++- .../StringToArrayListPropertyEditorTest.java | 4 +- .../controller/api/EditTextController.java | 333 +++++++++ .../api/security/RedactController.java | 4 +- .../model/api/general/EditTextRequest.java | 32 + .../service/PdfJsonConversionService.java | 128 +++- .../api/EditTextControllerTest.java | 664 ++++++++++++++++++ ...onConversionServiceUnicodeParsingTest.java | 161 +++++ .../service/AiWorkflowService.java | 45 +- .../service/AiWorkflowServiceTest.java | 155 ++++ engine/src/stirling/models/tool_models.py | 24 + engine/tests/test_pdf_edit_agent.py | 184 ++++- 17 files changed, 1937 insertions(+), 52 deletions(-) create mode 100644 app/common/src/main/java/stirling/software/common/model/api/general/EditTextOperation.java create mode 100644 app/common/src/main/java/stirling/software/common/service/InternalApiTimeoutException.java create mode 100644 app/core/src/main/java/stirling/software/SPDF/controller/api/EditTextController.java create mode 100644 app/core/src/main/java/stirling/software/SPDF/model/api/general/EditTextRequest.java create mode 100644 app/core/src/test/java/stirling/software/SPDF/controller/api/EditTextControllerTest.java create mode 100644 app/core/src/test/java/stirling/software/SPDF/service/PdfJsonConversionServiceUnicodeParsingTest.java diff --git a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java index e8a77bd08..f5319ff2a 100644 --- a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java +++ b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java @@ -76,6 +76,7 @@ public class ApplicationProperties { private ProcessExecutor processExecutor = new ProcessExecutor(); private PdfEditor pdfEditor = new PdfEditor(); private AiEngine aiEngine = new AiEngine(); + private InternalApi internalApi = new InternalApi(); @Bean public PropertySource dynamicYamlPropertySource(ConfigurableEnvironment environment) @@ -246,6 +247,19 @@ public class ApplicationProperties { private int longRunningTimeoutSeconds = 600; } + /** + * HTTP timeouts for loopback calls to internal Stirling API endpoints, used by the AI workflow + * executor and the pipeline processor. A bounded read timeout prevents a hung tool (e.g. an + * infinite loop in a PDF processing service) from stalling the entire chat workflow forever. + * Tools that legitimately need longer than the read timeout should be invoked through the async + * job executor instead of synchronously. + */ + @Data + public static class InternalApi { + private int connectTimeoutSeconds = 10; + private int readTimeoutSeconds = 300; + } + @Data public static class Legal { private String termsAndConditions; diff --git a/app/common/src/main/java/stirling/software/common/model/api/general/EditTextOperation.java b/app/common/src/main/java/stirling/software/common/model/api/general/EditTextOperation.java new file mode 100644 index 000000000..76815c5fb --- /dev/null +++ b/app/common/src/main/java/stirling/software/common/model/api/general/EditTextOperation.java @@ -0,0 +1,19 @@ +package stirling.software.common.model.api.general; + +import io.swagger.v3.oas.annotations.media.Schema; + +import lombok.Data; +import lombok.EqualsAndHashCode; + +@Data +@EqualsAndHashCode +public class EditTextOperation { + + @Schema(description = "The literal text to find.", requiredMode = Schema.RequiredMode.REQUIRED) + private String find; + + @Schema( + description = "The replacement text. May be empty to delete the matched text.", + requiredMode = Schema.RequiredMode.REQUIRED) + private String replace; +} diff --git a/app/common/src/main/java/stirling/software/common/service/InternalApiClient.java b/app/common/src/main/java/stirling/software/common/service/InternalApiClient.java index 70a20a6d5..f72eca14d 100644 --- a/app/common/src/main/java/stirling/software/common/service/InternalApiClient.java +++ b/app/common/src/main/java/stirling/software/common/service/InternalApiClient.java @@ -5,6 +5,7 @@ import java.io.UncheckedIOException; import java.net.URLDecoder; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.time.Duration; import java.util.regex.Pattern; import org.springframework.beans.factory.annotation.Autowired; @@ -12,15 +13,18 @@ import org.springframework.core.env.Environment; import org.springframework.core.io.FileSystemResource; import org.springframework.core.io.Resource; import org.springframework.http.*; +import org.springframework.http.client.SimpleClientHttpRequestFactory; import org.springframework.stereotype.Service; import org.springframework.util.MultiValueMap; import org.springframework.web.client.RequestCallback; +import org.springframework.web.client.ResourceAccessException; import org.springframework.web.client.RestTemplate; import jakarta.servlet.ServletContext; import lombok.extern.slf4j.Slf4j; +import stirling.software.common.model.ApplicationProperties; import stirling.software.common.model.enumeration.Role; import stirling.software.common.util.TempFile; import stirling.software.common.util.TempFileManager; @@ -50,16 +54,29 @@ public class InternalApiClient { private final UserServiceInterface userService; private final TempFileManager tempFileManager; private final Environment environment; + private final Duration readTimeout; + private final RestTemplate restTemplate; public InternalApiClient( ServletContext servletContext, @Autowired(required = false) UserServiceInterface userService, TempFileManager tempFileManager, - Environment environment) { + Environment environment, + ApplicationProperties applicationProperties) { this.servletContext = servletContext; this.userService = userService; this.tempFileManager = tempFileManager; this.environment = environment; + ApplicationProperties.InternalApi internalApi = applicationProperties.getInternalApi(); + // A bounded read timeout is what protects the workflow when an internal tool hangs + // (e.g. an infinite loop in a PDF processing service). The connect timeout is short + // because this is a loopback call; if connecting takes longer than a few seconds the + // local server is itself unhealthy. + this.readTimeout = Duration.ofSeconds(internalApi.getReadTimeoutSeconds()); + SimpleClientHttpRequestFactory factory = new SimpleClientHttpRequestFactory(); + factory.setConnectTimeout(Duration.ofSeconds(internalApi.getConnectTimeoutSeconds())); + factory.setReadTimeout(readTimeout); + this.restTemplate = new RestTemplate(factory); } /** @@ -74,7 +91,6 @@ public class InternalApiClient { validateUrl(endpointPath); String url = getBaseUrl() + endpointPath; - RestTemplate restTemplate = new RestTemplate(); HttpHeaders headers = new HttpHeaders(); String apiKey = getApiKeyForUser(); if (apiKey != null && !apiKey.isEmpty()) { @@ -84,26 +100,38 @@ public class InternalApiClient { HttpEntity> entity = new HttpEntity<>(body, headers); RequestCallback requestCallback = restTemplate.httpEntityCallback(entity, Resource.class); - return restTemplate.execute( - url, - HttpMethod.POST, - requestCallback, - response -> { - try { - TempFile tempFile = tempFileManager.createManagedTempFile("internal-api"); - Files.copy( - response.getBody(), - tempFile.getPath(), - java.nio.file.StandardCopyOption.REPLACE_EXISTING); - String filename = extractFilename(response.getHeaders()); - TempFileResource resource = new TempFileResource(tempFile, filename); - return ResponseEntity.status(response.getStatusCode()) - .headers(response.getHeaders()) - .body(resource); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - }); + try { + return restTemplate.execute( + url, + HttpMethod.POST, + requestCallback, + response -> { + try { + TempFile tempFile = + tempFileManager.createManagedTempFile("internal-api"); + Files.copy( + response.getBody(), + tempFile.getPath(), + java.nio.file.StandardCopyOption.REPLACE_EXISTING); + String filename = extractFilename(response.getHeaders()); + TempFileResource resource = new TempFileResource(tempFile, filename); + return ResponseEntity.status(response.getStatusCode()) + .headers(response.getHeaders()) + .body(resource); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }); + } catch (ResourceAccessException e) { + // RestTemplate wraps low-level I/O failures in ResourceAccessException. Only the + // SocketTimeoutException-rooted case is a real timeout; other I/O failures (connection + // refused, DNS, etc.) propagate as-is so the upstream generic handler can describe + // them accurately. + if (e.getCause() instanceof java.net.SocketTimeoutException) { + throw new InternalApiTimeoutException(endpointPath, readTimeout, e); + } + throw e; + } } /** diff --git a/app/common/src/main/java/stirling/software/common/service/InternalApiTimeoutException.java b/app/common/src/main/java/stirling/software/common/service/InternalApiTimeoutException.java new file mode 100644 index 000000000..b17262c24 --- /dev/null +++ b/app/common/src/main/java/stirling/software/common/service/InternalApiTimeoutException.java @@ -0,0 +1,36 @@ +package stirling.software.common.service; + +import java.time.Duration; + +/** + * Thrown when an internal Stirling tool invocation exceeds its configured read timeout or otherwise + * fails at the transport layer. Distinguishes a hung/timed-out tool from a tool that returned a + * non-2xx HTTP response (which is reported via the response status itself), so callers can present + * a clear "the tool didn't respond in time" message to the user instead of a generic stack trace. + */ +public class InternalApiTimeoutException extends RuntimeException { + + private final String endpointPath; + private final Duration readTimeout; + + public InternalApiTimeoutException(String endpointPath, Duration readTimeout, Throwable cause) { + super(buildMessage(endpointPath, readTimeout, cause), cause); + this.endpointPath = endpointPath; + this.readTimeout = readTimeout; + } + + public String getEndpointPath() { + return endpointPath; + } + + public Duration getReadTimeout() { + return readTimeout; + } + + private static String buildMessage(String endpointPath, Duration readTimeout, Throwable cause) { + String reason = cause != null && cause.getMessage() != null ? cause.getMessage() : ""; + return String.format( + "Internal tool %s did not respond within %ds (%s)", + endpointPath, readTimeout.toSeconds(), reason); + } +} diff --git a/app/common/src/main/java/stirling/software/common/util/propertyeditor/StringToArrayListPropertyEditor.java b/app/common/src/main/java/stirling/software/common/util/propertyeditor/StringToArrayListPropertyEditor.java index faaa1926f..6941e6b6d 100644 --- a/app/common/src/main/java/stirling/software/common/util/propertyeditor/StringToArrayListPropertyEditor.java +++ b/app/common/src/main/java/stirling/software/common/util/propertyeditor/StringToArrayListPropertyEditor.java @@ -6,15 +6,18 @@ import java.util.List; import lombok.extern.slf4j.Slf4j; -import stirling.software.common.model.api.security.RedactionArea; - -import tools.jackson.core.type.TypeReference; import tools.jackson.databind.DeserializationFeature; +import tools.jackson.databind.JavaType; import tools.jackson.databind.ObjectMapper; import tools.jackson.databind.json.JsonMapper; +/** + * Spring property editor that decodes a JSON string into a typed {@link ArrayList}. Used to bind + * complex list parameters (e.g. {@code List}, {@code List}) from + * multipart form fields, where Spring's default binding cannot deserialize a JSON array. + */ @Slf4j -public class StringToArrayListPropertyEditor extends PropertyEditorSupport { +public class StringToArrayListPropertyEditor extends PropertyEditorSupport { private final ObjectMapper objectMapper = JsonMapper.builder() @@ -22,6 +25,12 @@ public class StringToArrayListPropertyEditor extends PropertyEditorSupport { .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) .build(); + private final Class elementType; + + public StringToArrayListPropertyEditor(Class elementType) { + this.elementType = elementType; + } + @Override public void setAsText(String text) throws IllegalArgumentException { if (text == null || text.trim().isEmpty()) { @@ -29,8 +38,11 @@ public class StringToArrayListPropertyEditor extends PropertyEditorSupport { return; } try { - TypeReference> typeRef = new TypeReference<>() {}; - List list = objectMapper.readValue(text, typeRef); + JavaType listType = + objectMapper + .getTypeFactory() + .constructCollectionType(ArrayList.class, elementType); + List list = objectMapper.readValue(text, listType); setValue(list); } catch (Exception e) { log.error("Exception while converting {}", e); diff --git a/app/common/src/test/java/stirling/software/common/service/InternalApiClientTest.java b/app/common/src/test/java/stirling/software/common/service/InternalApiClientTest.java index 6f18ca079..06543e47b 100644 --- a/app/common/src/test/java/stirling/software/common/service/InternalApiClientTest.java +++ b/app/common/src/test/java/stirling/software/common/service/InternalApiClientTest.java @@ -22,11 +22,13 @@ import org.springframework.mock.env.MockEnvironment; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.web.client.RequestCallback; +import org.springframework.web.client.ResourceAccessException; import org.springframework.web.client.ResponseExtractor; import org.springframework.web.client.RestTemplate; import jakarta.servlet.ServletContext; +import stirling.software.common.model.ApplicationProperties; import stirling.software.common.util.TempFile; import stirling.software.common.util.TempFileManager; @@ -42,8 +44,19 @@ class InternalApiClientTest { @BeforeEach void setUp() { lenient().when(servletContext.getContextPath()).thenReturn(""); + client = newClient(); + } + + /** + * Build a fresh client. Tests that use {@link org.mockito.Mockito#mockConstruction} to + * intercept {@link RestTemplate} must call this from inside their {@code mockConstruction} + * block, since the client now caches one RestTemplate per instance at construction time. + */ + private InternalApiClient newClient() { MockEnvironment environment = new MockEnvironment().withProperty("server.port", "8080"); - client = new InternalApiClient(servletContext, userService, tempFileManager, environment); + ApplicationProperties applicationProperties = new ApplicationProperties(); + return new InternalApiClient( + servletContext, userService, tempFileManager, environment, applicationProperties); } @Test @@ -75,7 +88,10 @@ class InternalApiClientTest { .thenAnswer(inv -> fakeOkResponse(inv.getArgument(3))); })) { - ResponseEntity response = client.post("/api/v1/general/merge-pdfs", body); + // Reconstruct the client so its cached RestTemplate is the mocked one. + InternalApiClient mockedClient = newClient(); + ResponseEntity response = + mockedClient.post("/api/v1/general/merge-pdfs", body); assertNotNull(response); assertEquals(HttpStatus.OK, response.getStatusCode()); @@ -86,6 +102,72 @@ class InternalApiClientTest { } } + @Test + void postWrapsSocketTimeoutAsInternalApiTimeoutException() { + // Simulates the read-timeout case: RestTemplate wraps SocketTimeoutException in + // ResourceAccessException when the underlying socket times out. The client must repackage + // that into a typed timeout exception that carries the failing endpoint and the configured + // read timeout, so the workflow layer can surface a clean "tool didn't respond" message + // to the user. + MultiValueMap body = new LinkedMultiValueMap<>(); + body.add("fileInput", namedResource("input.pdf", "data")); + + try (var ignored = + mockConstruction( + RestTemplate.class, + (rt, ctx) -> { + when(rt.httpEntityCallback(any(), eq(Resource.class))) + .thenAnswer(inv -> (RequestCallback) req -> {}); + when(rt.execute(anyString(), eq(HttpMethod.POST), any(), any())) + .thenThrow( + new ResourceAccessException( + "I/O error on POST request: Read timed out", + new java.net.SocketTimeoutException( + "Read timed out"))); + })) { + + InternalApiClient mockedClient = newClient(); + InternalApiTimeoutException thrown = + assertThrows( + InternalApiTimeoutException.class, + () -> mockedClient.post("/api/v1/general/merge-pdfs", body)); + + assertEquals("/api/v1/general/merge-pdfs", thrown.getEndpointPath()); + assertNotNull(thrown.getReadTimeout()); + assertTrue(thrown.getMessage().contains("/api/v1/general/merge-pdfs")); + } + } + + @Test + void postRethrowsNonTimeoutResourceAccessExceptionAsIs() { + // ResourceAccessException covers more than just timeouts (e.g. connection refused, DNS + // failure). Only SocketTimeoutException-rooted failures are timeouts; everything else + // must propagate so the upstream generic handler can label it correctly instead of lying + // about a "tool didn't respond" timeout. + MultiValueMap body = new LinkedMultiValueMap<>(); + body.add("fileInput", namedResource("input.pdf", "data")); + + try (var ignored = + mockConstruction( + RestTemplate.class, + (rt, ctx) -> { + when(rt.httpEntityCallback(any(), eq(Resource.class))) + .thenAnswer(inv -> (RequestCallback) req -> {}); + when(rt.execute(anyString(), eq(HttpMethod.POST), any(), any())) + .thenThrow( + new ResourceAccessException( + "I/O error on POST request: Connection refused", + new java.net.ConnectException( + "Connection refused"))); + })) { + + InternalApiClient mockedClient = newClient(); + assertThrows( + ResourceAccessException.class, + () -> mockedClient.post("/api/v1/general/merge-pdfs", body)); + } + } + @Test void postRejectsDisallowedPath() { MultiValueMap body = new LinkedMultiValueMap<>(); @@ -125,8 +207,10 @@ class InternalApiClientTest { .thenAnswer(inv -> fakeOkResponse(inv.getArgument(3))); })) { + // Reconstruct the client so its cached RestTemplate is the mocked one. + InternalApiClient mockedClient = newClient(); ResponseEntity response = - client.post("/api/v1/ai/tools/pdf-comment-agent", body); + mockedClient.post("/api/v1/ai/tools/pdf-comment-agent", body); assertNotNull(response); assertEquals(HttpStatus.OK, response.getStatusCode()); diff --git a/app/common/src/test/java/stirling/software/common/util/propertyeditor/StringToArrayListPropertyEditorTest.java b/app/common/src/test/java/stirling/software/common/util/propertyeditor/StringToArrayListPropertyEditorTest.java index f68879271..a2e03f086 100644 --- a/app/common/src/test/java/stirling/software/common/util/propertyeditor/StringToArrayListPropertyEditorTest.java +++ b/app/common/src/test/java/stirling/software/common/util/propertyeditor/StringToArrayListPropertyEditorTest.java @@ -11,11 +11,11 @@ import stirling.software.common.model.api.security.RedactionArea; class StringToArrayListPropertyEditorTest { - private StringToArrayListPropertyEditor editor; + private StringToArrayListPropertyEditor editor; @BeforeEach void setUp() { - editor = new StringToArrayListPropertyEditor(); + editor = new StringToArrayListPropertyEditor<>(RedactionArea.class); } @Test diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/EditTextController.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/EditTextController.java new file mode 100644 index 000000000..84f4c8f6e --- /dev/null +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/EditTextController.java @@ -0,0 +1,333 @@ +package stirling.software.SPDF.controller.api; + +import java.io.OutputStream; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Objects; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.springframework.core.io.Resource; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.WebDataBinder; +import org.springframework.web.bind.annotation.InitBinder; +import org.springframework.web.bind.annotation.ModelAttribute; +import org.springframework.web.multipart.MultipartFile; + +import io.github.pixee.security.Filenames; +import io.swagger.v3.oas.annotations.Operation; + +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; + +import stirling.software.SPDF.config.swagger.StandardPdfResponse; +import stirling.software.SPDF.model.api.general.EditTextRequest; +import stirling.software.SPDF.model.json.PdfJsonDocument; +import stirling.software.SPDF.model.json.PdfJsonPage; +import stirling.software.SPDF.model.json.PdfJsonTextElement; +import stirling.software.SPDF.service.PdfJsonConversionService; +import stirling.software.common.annotations.AutoJobPostMapping; +import stirling.software.common.annotations.api.GeneralApi; +import stirling.software.common.model.api.general.EditTextOperation; +import stirling.software.common.util.ExceptionUtils; +import stirling.software.common.util.GeneralUtils; +import stirling.software.common.util.TempFile; +import stirling.software.common.util.TempFileManager; +import stirling.software.common.util.WebResponseUtils; +import stirling.software.common.util.propertyeditor.StringToArrayListPropertyEditor; + +/** + * Find/replace text editing for PDFs. Round-trips through {@link PdfJsonConversionService}: the + * input PDF is parsed into the editable JSON model, find/replace operations are applied to the + * {@code text} field of each text element, and the mutated model is rebuilt into a PDF. + * + *

Matching joins all text elements on a page into a single string before searching, so find + * strings can span multiple visual runs (titles split per word, kerning-broken phrases, etc.). + * + *

For cross-element matches the entire replacement is written into the first matched element, + * any intermediate elements are emptied, and the suffix of the last matched element is preserved. + * This anchors the new text at the leftmost match position and lets the font lay it out as one run, + * which is more visually reliable than redistributing words to the original per-element X positions + * (those positions are calibrated to the original word widths and rarely match the replacement + * widths). Centered or tracked layouts will become left-aligned at the original first-word position + * when their text is replaced - the trade-off is accepted in favour of avoiding glyph overlaps and + * gaps from variable-width word distribution. + */ +@Slf4j +@GeneralApi +@RequiredArgsConstructor +public class EditTextController { + + private static final Pattern FILE_EXTENSION_PATTERN = Pattern.compile("[.][^.]+$"); + + private final PdfJsonConversionService pdfJsonConversionService; + private final TempFileManager tempFileManager; + + @InitBinder + public void initBinder(WebDataBinder binder) { + binder.registerCustomEditor( + List.class, + "edits", + new StringToArrayListPropertyEditor<>(EditTextOperation.class)); + } + + @AutoJobPostMapping(consumes = "multipart/form-data", value = "/edit-text") + @StandardPdfResponse + @Operation( + summary = "Edit text in a PDF via find and replace", + description = + "Applies an ordered list of find/replace operations to the text in a PDF and" + + " returns the edited PDF. Useful for find-and-replace, bulk renames" + + " (e.g. updating a company name throughout a document), and copy" + + " editing where the AI agent has identified specific replacements." + + " Matching is performed against the joined text of each page, so" + + " find strings can span multiple visual runs (titles split per word," + + " kerning-broken phrases). Cross-element matches are written as a" + + " single replacement run anchored at the leftmost matched position;" + + " centered or tracked text may shift left when its content changes." + + " Input:PDF Output:PDF Type:SISO") + public ResponseEntity editText(@ModelAttribute EditTextRequest request) + throws Exception { + MultipartFile inputFile = request.getFileInput(); + if (inputFile == null) { + throw ExceptionUtils.createFileNullOrEmptyException(); + } + List edits = request.getEdits(); + if (edits == null || edits.isEmpty()) { + throw ExceptionUtils.createIllegalArgumentException( + "error.editText.no.edits", + "No find/replace operations provided for text editing"); + } + for (EditTextOperation edit : edits) { + if (edit == null || edit.getFind() == null || edit.getFind().isEmpty()) { + throw ExceptionUtils.createIllegalArgumentException( + "error.editText.empty.find", "Each edit must have a non-empty find string"); + } + } + + boolean wholeWordSearch = Boolean.TRUE.equals(request.getWholeWordSearch()); + List compiledEdits = compileEdits(edits, wholeWordSearch); + + PdfJsonDocument document = pdfJsonConversionService.convertPdfToJsonDocument(inputFile); + Set pageFilter = resolvePageFilter(request, document); + int modifiedSpans = applyEdits(document, compiledEdits, pageFilter); + log.info( + "edit-text: modified {} text span(s) using {} edit(s) on {} page(s)", + modifiedSpans, + compiledEdits.size(), + pageFilter == null + ? document.getPages() == null ? 0 : document.getPages().size() + : pageFilter.size()); + + String docName = buildOutputFilename(inputFile); + TempFile tempOut = tempFileManager.createManagedTempFile(".pdf"); + try (OutputStream os = Files.newOutputStream(tempOut.getPath())) { + pdfJsonConversionService.convertJsonToPdf(document, os); + } catch (Exception e) { + tempOut.close(); + throw e; + } + return WebResponseUtils.pdfFileToWebResponse(tempOut, docName); + } + + private List compileEdits( + List edits, boolean wholeWordSearch) { + return edits.stream().map(edit -> compileEdit(edit, wholeWordSearch)).toList(); + } + + private CompiledEdit compileEdit(EditTextOperation edit, boolean wholeWordSearch) { + // Always treat the user-supplied find string as a literal: Pattern.quote escapes any + // regex metacharacters, so the constructed pattern can only ever do a literal match + // (optionally bounded by our own word-boundary anchors). This rules out catastrophic + // backtracking from a malicious find string. + String findRaw = edit.getFind(); + String replacement = Objects.toString(edit.getReplace(), ""); + String regex = Pattern.quote(findRaw); + if (wholeWordSearch) { + // Use lookarounds rather than \b so the bound works even when the find string starts + // or ends with a non-word character (e.g. "-foo" or "$id"). \b only fires at a + // word/non-word transition, which would never trigger for a find that starts with a + // non-word char and was preceded by another non-word char in the source text. + regex = "(? resolvePageFilter(EditTextRequest request, PdfJsonDocument document) { + String pageNumbers = request.getPageNumbers(); + int totalPages = document.getPages() == null ? 0 : document.getPages().size(); + if (totalPages == 0) { + return Collections.emptySet(); + } + if (pageNumbers == null || pageNumbers.isBlank() || "all".equalsIgnoreCase(pageNumbers)) { + return null; + } + List pages = GeneralUtils.parsePageList(pageNumbers, totalPages, true); + return new HashSet<>(pages); + } + + private int applyEdits( + PdfJsonDocument document, List edits, Set pageFilter) { + List pages = document.getPages(); + if (pages == null) { + return 0; + } + int modifiedSpans = 0; + // The page filter is built from parsePageList against pages.size(), so it returns 1-based + // positional page numbers. Match against the same positional numbering here rather than + // the per-page pageNumber field, which can become non-sequential after split/merge round + // trips and would misalign the filter. + for (int i = 0; i < pages.size(); i++) { + int positionalPageNumber = i + 1; + if (pageFilter != null && !pageFilter.contains(positionalPageNumber)) { + continue; + } + modifiedSpans += applyEditsToPage(pages.get(i), edits); + } + return modifiedSpans; + } + + private int applyEditsToPage(PdfJsonPage page, List edits) { + List elements = page.getTextElements(); + if (elements == null || elements.isEmpty()) { + return 0; + } + Set modifiedIndices = new HashSet<>(); + for (CompiledEdit edit : edits) { + applyEditToPage(elements, edit, modifiedIndices); + } + for (Integer index : modifiedIndices) { + // Char codes were captured for the original glyph sequence; clear them so the rebuild + // re-encodes from the new text via the font. + elements.get(index).setCharCodes(null); + } + return modifiedIndices.size(); + } + + /** + * Apply a single edit across the page by matching against the concatenation of all element + * texts, then writing the replacement back into the originating element(s). + */ + private void applyEditToPage( + List elements, CompiledEdit edit, Set modifiedIndices) { + StringBuilder joined = new StringBuilder(); + int[] starts = new int[elements.size()]; + int[] ends = new int[elements.size()]; + for (int i = 0; i < elements.size(); i++) { + starts[i] = joined.length(); + String text = elements.get(i).getText(); + if (text != null) { + joined.append(text); + } + ends[i] = joined.length(); + } + + Matcher matcher = edit.pattern().matcher(joined); + List spans = new ArrayList<>(); + StringBuffer interpolation = new StringBuffer(); + int previousAppendPosition = 0; + while (matcher.find()) { + if (matcher.start() == matcher.end()) { + // Skip zero-length matches (e.g. /a*/ on empty input) — they cannot be applied. + continue; + } + int sizeBefore = interpolation.length(); + matcher.appendReplacement(interpolation, edit.replacement()); + int prefixLength = matcher.start() - previousAppendPosition; + String actualReplacement = + interpolation.substring(sizeBefore + prefixLength, interpolation.length()); + spans.add(new MatchSpan(matcher.start(), matcher.end(), actualReplacement)); + previousAppendPosition = matcher.end(); + } + + // Apply right-to-left so earlier match positions stay valid as we mutate elements. + for (int i = spans.size() - 1; i >= 0; i--) { + MatchSpan span = spans.get(i); + int firstElement = findElementForCharIndex(starts, ends, span.start()); + int lastElement = findElementForCharIndex(starts, ends, span.end() - 1); + if (firstElement < 0 || lastElement < 0) { + continue; + } + applyMatchToElements( + elements, starts, span, firstElement, lastElement, modifiedIndices); + } + } + + /** + * Find the element whose text covers the character at {@code charIndex} in the joined string. + * Returns -1 if no element covers that index (which should not happen for valid match spans). + */ + private static int findElementForCharIndex(int[] starts, int[] ends, int charIndex) { + for (int i = 0; i < starts.length; i++) { + if (starts[i] <= charIndex && charIndex < ends[i]) { + return i; + } + } + return -1; + } + + private static void applyMatchToElements( + List elements, + int[] starts, + MatchSpan span, + int firstElement, + int lastElement, + Set modifiedIndices) { + if (firstElement == lastElement) { + PdfJsonTextElement element = elements.get(firstElement); + String text = nullToEmpty(element.getText()); + int matchStartInElement = span.start() - starts[firstElement]; + int matchEndInElement = span.end() - starts[firstElement]; + element.setText( + text.substring(0, matchStartInElement) + + span.replacement() + + text.substring(matchEndInElement)); + modifiedIndices.add(firstElement); + return; + } + + // Cross-element match: write the whole replacement into the first matched element, empty + // any intermediate elements, and keep only the suffix of the last matched element. The + // JSON->PDF rebuild concatenates per-token text, so the font lays out the replacement as + // one continuous run anchored at the first element's X position. + String firstText = nullToEmpty(elements.get(firstElement).getText()); + int firstSplit = span.start() - starts[firstElement]; + elements.get(firstElement).setText(firstText.substring(0, firstSplit) + span.replacement()); + modifiedIndices.add(firstElement); + + for (int mid = firstElement + 1; mid < lastElement; mid++) { + elements.get(mid).setText(""); + modifiedIndices.add(mid); + } + + String lastText = nullToEmpty(elements.get(lastElement).getText()); + int lastSplit = span.end() - starts[lastElement]; + elements.get(lastElement).setText(lastText.substring(lastSplit)); + modifiedIndices.add(lastElement); + } + + private static String nullToEmpty(String value) { + return value != null ? value : ""; + } + + private String buildOutputFilename(MultipartFile inputFile) { + String originalName = inputFile.getOriginalFilename(); + String baseName = + (originalName != null && !originalName.isBlank()) + ? FILE_EXTENSION_PATTERN + .matcher(Filenames.toSimpleFileName(originalName)) + .replaceFirst("") + : "document"; + return baseName + "_edited.pdf"; + } + + private record CompiledEdit(Pattern pattern, String replacement) {} + + private record MatchSpan(int start, int end, String replacement) {} +} diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/security/RedactController.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/security/RedactController.java index bcb7f5e52..c371982f7 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/api/security/RedactController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/security/RedactController.java @@ -96,7 +96,9 @@ public class RedactController { @InitBinder public void initBinder(WebDataBinder binder) { binder.registerCustomEditor( - List.class, "redactions", new StringToArrayListPropertyEditor()); + List.class, + "redactions", + new StringToArrayListPropertyEditor<>(RedactionArea.class)); } @AutoJobPostMapping(value = "/redact", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) diff --git a/app/core/src/main/java/stirling/software/SPDF/model/api/general/EditTextRequest.java b/app/core/src/main/java/stirling/software/SPDF/model/api/general/EditTextRequest.java new file mode 100644 index 000000000..8130d0edc --- /dev/null +++ b/app/core/src/main/java/stirling/software/SPDF/model/api/general/EditTextRequest.java @@ -0,0 +1,32 @@ +package stirling.software.SPDF.model.api.general; + +import java.util.List; + +import io.swagger.v3.oas.annotations.media.Schema; + +import lombok.Data; +import lombok.EqualsAndHashCode; + +import stirling.software.SPDF.model.api.PDFWithPageNums; +import stirling.software.common.model.api.general.EditTextOperation; + +@Data +@EqualsAndHashCode(callSuper = true) +public class EditTextRequest extends PDFWithPageNums { + + @Schema( + description = + "Ordered list of find/replace operations. Each replaces every occurrence on" + + " the selected pages, in order; later operations see the result of" + + " earlier ones (so 'foo'->'foos' then 'foos'->'bars' turns 'foo'" + + " into 'bars').", + requiredMode = Schema.RequiredMode.REQUIRED) + private List edits; + + @Schema( + description = + "Whether matches must be whole words (boundaries determined by non-word" + + " characters)", + defaultValue = "false") + private Boolean wholeWordSearch; +} diff --git a/app/core/src/main/java/stirling/software/SPDF/service/PdfJsonConversionService.java b/app/core/src/main/java/stirling/software/SPDF/service/PdfJsonConversionService.java index 9a87167d7..331c79008 100644 --- a/app/core/src/main/java/stirling/software/SPDF/service/PdfJsonConversionService.java +++ b/app/core/src/main/java/stirling/software/SPDF/service/PdfJsonConversionService.java @@ -252,6 +252,17 @@ public class PdfJsonConversionService { convertPdfToJson(file, null, false, out); } + /** + * Converts a PDF to the editable {@link PdfJsonDocument} model. Convenience wrapper around + * {@link #convertPdfToJson(MultipartFile, OutputStream)} for callers that need to inspect or + * mutate the document in memory before round-tripping it back to PDF. + */ + public PdfJsonDocument convertPdfToJsonDocument(MultipartFile file) throws IOException { + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + convertPdfToJson(file, null, false, buffer); + return objectMapper.readValue(buffer.toByteArray(), PdfJsonDocument.class); + } + public void convertPdfToJson(MultipartFile file, boolean lightweight, OutputStream out) throws IOException { convertPdfToJson(file, null, lightweight, out); @@ -620,6 +631,13 @@ public class PdfJsonConversionService { } byte[] jsonBytes = file.getBytes(); PdfJsonDocument pdfJson = objectMapper.readValue(jsonBytes, PdfJsonDocument.class); + convertJsonToPdf(pdfJson, out); + } + + public void convertJsonToPdf(PdfJsonDocument pdfJson, OutputStream out) throws IOException { + if (pdfJson == null) { + throw ExceptionUtils.createNullArgumentException("document"); + } List fontModels = pdfJson.getFonts(); if (fontModels == null) { @@ -2076,14 +2094,29 @@ public class PdfJsonConversionService { byte[] toUnicodeBytes = Base64.getDecoder().decode(toUnicodeBase64); String toUnicodeStr = new String(toUnicodeBytes, StandardCharsets.UTF_8); - // Parse ToUnicode CMap for bfchar and bfrange + // Parse ToUnicode CMap for bfchar and bfrange. Both sides go through + // parseToUnicodeCodepoint so an 8-hex-char surrogate-pair value on either side + // (e.g. "D837DF0E") is decoded as a single supplementary codepoint instead of + // overflowing Integer.parseInt and aborting the whole mapping build. java.util.regex.Pattern bfcharPattern = java.util.regex.Pattern.compile("<([0-9A-Fa-f]+)>\\s*<([0-9A-Fa-f]+)>"); java.util.regex.Matcher matcher = bfcharPattern.matcher(toUnicodeStr); while (matcher.find()) { - int charCode = Integer.parseInt(matcher.group(1), 16); - int unicode = Integer.parseInt(matcher.group(2), 16); - charCodeToUnicode.put(charCode, unicode); + try { + int charCode = parseToUnicodeCodepoint(matcher.group(1)); + int unicode = parseToUnicodeCodepoint(matcher.group(2)); + charCodeToUnicode.put(charCode, unicode); + } catch (NumberFormatException entryEx) { + // Tolerate a single malformed entry: log and skip rather than aborting the + // entire ToUnicode CMap (which would force the whole font onto the slow + // raw-bytes fallback path). + log.debug( + "Skipping malformed ToUnicode entry <{}> <{}> in font {}: {}", + matcher.group(1), + matcher.group(2), + font.getName(), + entryEx.getMessage()); + } } // Build JSON mapping: CharCode → CID → GID → Unicode @@ -2136,6 +2169,42 @@ public class PdfJsonConversionService { } } + /** + * Parse a hex string from a PDF ToUnicode CMap into a single Unicode codepoint. Handles three + * cases: a single BMP code unit (4 hex chars), a UTF-16 surrogate pair encoding a supplementary + * codepoint above U+FFFF (8 hex chars, e.g. {@code D837DF0E} for U+1F40E), and multi-codepoint + * mappings (longer; returns the first codepoint as a best-effort representative). + * + *

Without this, {@code Integer.parseInt("D837DF0E", 16)} overflows because the value is ~3.6 + * billion, throwing {@link NumberFormatException} and forcing the conversion to fall back to a + * raw ToUnicode payload that the JSON→PDF rebuild then fails to use efficiently. + */ + static int parseToUnicodeCodepoint(String hex) { + if (hex == null || hex.isEmpty()) { + throw new NumberFormatException("Empty ToUnicode hex value"); + } + if (hex.length() <= 4) { + return Integer.parseInt(hex, 16); + } + // Treat the hex string as UTF-16BE: pairs of hex digits form bytes, four hex digits form + // one UTF-16 code unit. The PDF ToUnicode CMap convention requires an even number of bytes + // (i.e. a multiple of four hex characters) for multi-unit values. + if (hex.length() % 4 != 0) { + throw new NumberFormatException( + "ToUnicode hex value not a multiple of 4 chars: " + hex); + } + int unitCount = hex.length() / 4; + char[] units = new char[unitCount]; + for (int i = 0; i < unitCount; i++) { + units[i] = (char) Integer.parseInt(hex.substring(i * 4, i * 4 + 4), 16); + } + // codePointAt assembles a surrogate pair into a supplementary codepoint when the + // high/low surrogates appear in sequence; for any other multi-unit sequence it returns + // the first BMP codepoint, which is the right best-effort fallback for ligature + // decompositions (one charCode -> several Unicode chars). + return new String(units).codePointAt(0); + } + private PdfJsonFontCidSystemInfo extractCidSystemInfo(COSDictionary fontDictionary) { if (fontDictionary == null) { return null; @@ -4256,12 +4325,8 @@ public class PdfJsonConversionService { return 0; } if (font != null) { - try (InputStream inputStream = new ByteArrayInputStream(value.getBytes())) { - int count = 0; - int code; - while ((code = font.readCode(inputStream)) != -1) { - count++; - } + try (ByteArrayInputStream inputStream = new ByteArrayInputStream(value.getBytes())) { + int count = countCodesProtected(inputStream, font::readCode); if (count > 0) { return count; } @@ -4273,6 +4338,49 @@ public class PdfJsonConversionService { return Math.max(1, bytes.length); } + /** + * Functional accessor for {@link PDFont#readCode(InputStream)} so the bounded counting loop can + * be exercised in isolation without instantiating a {@link PDFont}. + */ + @FunctionalInterface + interface CodeReader { + int readCode(InputStream stream) throws IOException; + } + + /** + * Count how many codes the supplied {@code reader} can extract from {@code inputStream}, with + * two safety nets that PDFBox's raw {@link PDFont#readCode(InputStream)} loop lacks: + * + *

    + *
  1. Stop when the stream is empty (a corrupt CMap can otherwise loop forever returning + * successfully-matched zero-bytes from an exhausted {@link ByteArrayInputStream}). + *
  2. Stop when a {@code readCode} call did not consume any bytes, even if it returned a + * non-{@code -1} value. + *
+ * + *

Both conditions were observed in the wild on round-tripped fallback fonts where the + * embedded ToUnicode CMap matched 0x00 sequences, hanging the JSON→PDF rebuild. + */ + static int countCodesProtected(ByteArrayInputStream inputStream, CodeReader reader) + throws IOException { + int count = 0; + int previousAvailable = inputStream.available(); + while (previousAvailable > 0) { + int code = reader.readCode(inputStream); + if (code == -1) { + break; + } + int currentAvailable = inputStream.available(); + if (currentAvailable >= previousAvailable) { + // No progress made; break to avoid infinite loop on corrupt CMaps. + break; + } + count++; + previousAvailable = currentAvailable; + } + return count; + } + private MergedText mergeText(List elements) { StringBuilder builder = new StringBuilder(); List combinedCodes = new ArrayList<>(); diff --git a/app/core/src/test/java/stirling/software/SPDF/controller/api/EditTextControllerTest.java b/app/core/src/test/java/stirling/software/SPDF/controller/api/EditTextControllerTest.java new file mode 100644 index 000000000..007871fba --- /dev/null +++ b/app/core/src/test/java/stirling/software/SPDF/controller/api/EditTextControllerTest.java @@ -0,0 +1,664 @@ +package stirling.software.SPDF.controller.api; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.OutputStream; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.List; + +import org.apache.pdfbox.pdmodel.PDDocument; +import org.apache.pdfbox.pdmodel.PDPage; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.core.io.Resource; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.mock.web.MockMultipartFile; +import org.springframework.web.multipart.MultipartFile; + +import stirling.software.SPDF.model.api.general.EditTextRequest; +import stirling.software.SPDF.model.json.PdfJsonDocument; +import stirling.software.SPDF.model.json.PdfJsonPage; +import stirling.software.SPDF.model.json.PdfJsonTextElement; +import stirling.software.SPDF.service.PdfJsonConversionService; +import stirling.software.common.model.api.general.EditTextOperation; +import stirling.software.common.util.TempFile; +import stirling.software.common.util.TempFileManager; + +@ExtendWith(MockitoExtension.class) +class EditTextControllerTest { + + @Mock private PdfJsonConversionService pdfJsonConversionService; + @Mock private TempFileManager tempFileManager; + + @InjectMocks private EditTextController controller; + + @BeforeEach + void setUp() throws Exception { + lenient() + .when(tempFileManager.createManagedTempFile(anyString())) + .thenAnswer( + inv -> { + File f = + Files.createTempFile("test", inv.getArgument(0)) + .toFile(); + TempFile tf = mock(TempFile.class); + lenient().when(tf.getFile()).thenReturn(f); + lenient().when(tf.getPath()).thenReturn(f.toPath()); + return tf; + }); + } + + private static MultipartFile pdfFile() { + return new MockMultipartFile( + "fileInput", "doc.pdf", "application/pdf", "stub-pdf-bytes".getBytes()); + } + + private static EditTextOperation edit(String find, String replace) { + EditTextOperation op = new EditTextOperation(); + op.setFind(find); + op.setReplace(replace); + return op; + } + + private static PdfJsonTextElement textElement(String text) { + PdfJsonTextElement el = new PdfJsonTextElement(); + el.setText(text); + el.setCharCodes(new int[] {1, 2, 3}); + return el; + } + + private static PdfJsonDocument documentWithElements(List elements) { + PdfJsonDocument doc = new PdfJsonDocument(); + PdfJsonPage page = new PdfJsonPage(); + page.setPageNumber(1); + page.setTextElements(new ArrayList<>(elements)); + doc.setPages(new ArrayList<>(List.of(page))); + return doc; + } + + private static PdfJsonDocument documentWithText(String... textsByPage) { + PdfJsonDocument doc = new PdfJsonDocument(); + List pages = new ArrayList<>(); + for (int i = 0; i < textsByPage.length; i++) { + PdfJsonPage page = new PdfJsonPage(); + page.setPageNumber(i + 1); + PdfJsonTextElement el = new PdfJsonTextElement(); + el.setText(textsByPage[i]); + el.setCharCodes(new int[] {1, 2, 3}); + page.setTextElements(new ArrayList<>(List.of(el))); + pages.add(page); + } + doc.setPages(pages); + return doc; + } + + /** Build a minimal real PDF so tests can run end-to-end without mocking the conversion. */ + private static byte[] buildEmptyPdf() throws Exception { + try (PDDocument doc = new PDDocument(); + ByteArrayOutputStream baos = new ByteArrayOutputStream()) { + doc.addPage(new PDPage()); + doc.save(baos); + return baos.toByteArray(); + } + } + + @Test + void editText_nullFileInputThrows() { + EditTextRequest request = new EditTextRequest(); + request.setFileInput(null); + request.setEdits(List.of(edit("foo", "bar"))); + + assertThrows(IllegalArgumentException.class, () -> controller.editText(request)); + } + + @Test + void editText_emptyEditsThrows() { + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of()); + + assertThrows(IllegalArgumentException.class, () -> controller.editText(request)); + } + + @Test + void editText_nullEditsThrows() { + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(null); + + assertThrows(IllegalArgumentException.class, () -> controller.editText(request)); + } + + @Test + void editText_emptyFindStringThrows() { + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("", "replacement"))); + + assertThrows(IllegalArgumentException.class, () -> controller.editText(request)); + } + + @Test + void editText_findStringWithRegexMetacharsIsTreatedLiterally() throws Exception { + // Find strings are always treated as literals (Pattern.quote'd internally) — no ReDoS + // exposure from regex metacharacters supplied by the caller. + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("(unclosed", "fixed"))); + + PdfJsonDocument input = documentWithText("text with (unclosed paren"); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + controller.editText(request); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PdfJsonDocument.class); + org.mockito.Mockito.verify(pdfJsonConversionService) + .convertJsonToPdf(captor.capture(), any(OutputStream.class)); + assertEquals( + "text with fixed paren", + captor.getValue().getPages().get(0).getTextElements().get(0).getText()); + } + + @Test + void editText_literalFindReplace_mutatesMatchingSpansAndClearsCharCodes() throws Exception { + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("foo", "bar"))); + + PdfJsonDocument input = documentWithText("foo and foo", "no match here"); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + ResponseEntity response = controller.editText(request); + + assertEquals(HttpStatus.OK, response.getStatusCode()); + assertNotNull(response.getBody()); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PdfJsonDocument.class); + org.mockito.Mockito.verify(pdfJsonConversionService) + .convertJsonToPdf(captor.capture(), any(OutputStream.class)); + PdfJsonDocument mutated = captor.getValue(); + + assertEquals("bar and bar", mutated.getPages().get(0).getTextElements().get(0).getText()); + assertNull(mutated.getPages().get(0).getTextElements().get(0).getCharCodes()); + + assertEquals("no match here", mutated.getPages().get(1).getTextElements().get(0).getText()); + // Char codes preserved on unmodified spans. + assertNotNull(mutated.getPages().get(1).getTextElements().get(0).getCharCodes()); + } + + @Test + void editText_wholeWordSearch_doesNotMatchInsideWords() throws Exception { + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("cat", "dog"))); + request.setWholeWordSearch(true); + + PdfJsonDocument input = documentWithText("the cat is in the catalogue"); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + controller.editText(request); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PdfJsonDocument.class); + org.mockito.Mockito.verify(pdfJsonConversionService) + .convertJsonToPdf(captor.capture(), any(OutputStream.class)); + assertEquals( + "the dog is in the catalogue", + captor.getValue().getPages().get(0).getTextElements().get(0).getText()); + } + + @Test + void editText_wholeWordSearch_matchesFindStartingWithNonWordChar() throws Exception { + // Regression: \b only fires on a word/non-word transition. A find that starts with a + // non-word char (e.g. "-foo") preceded by another non-word char in the source (a space) + // would never match under \b. The lookaround-based bound handles this correctly. + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("-foo", "-bar"))); + request.setWholeWordSearch(true); + + PdfJsonDocument input = documentWithText("space then -foo here"); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + controller.editText(request); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PdfJsonDocument.class); + org.mockito.Mockito.verify(pdfJsonConversionService) + .convertJsonToPdf(captor.capture(), any(OutputStream.class)); + assertEquals( + "space then -bar here", + captor.getValue().getPages().get(0).getTextElements().get(0).getText()); + } + + @Test + void editText_wholeWordSearch_doesNotMatchWhenAdjacentToWordChar() throws Exception { + // The lookaround bound must still reject matches that are part of a larger word. + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("-foo", "-bar"))); + request.setWholeWordSearch(true); + + PdfJsonDocument input = documentWithText("inline-foo should not match"); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + controller.editText(request); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PdfJsonDocument.class); + org.mockito.Mockito.verify(pdfJsonConversionService) + .convertJsonToPdf(captor.capture(), any(OutputStream.class)); + // "inline-foo" has 'e' (word) before '-foo' so the lookbehind blocks the match. The + // trailing 'o' is followed by a space (non-word) so the trailing lookahead would pass on + // its own; the leading lookbehind is what rejects it. + assertEquals( + "inline-foo should not match", + captor.getValue().getPages().get(0).getTextElements().get(0).getText()); + } + + @Test + void editText_pageFilter_onlyAffectsListedPages() throws Exception { + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("foo", "bar"))); + request.setPageNumbers("2"); + + PdfJsonDocument input = documentWithText("foo on page 1", "foo on page 2", "foo on page 3"); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + controller.editText(request); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PdfJsonDocument.class); + org.mockito.Mockito.verify(pdfJsonConversionService) + .convertJsonToPdf(captor.capture(), any(OutputStream.class)); + PdfJsonDocument mutated = captor.getValue(); + + assertEquals("foo on page 1", mutated.getPages().get(0).getTextElements().get(0).getText()); + assertEquals("bar on page 2", mutated.getPages().get(1).getTextElements().get(0).getText()); + assertEquals("foo on page 3", mutated.getPages().get(2).getTextElements().get(0).getText()); + } + + @Test + void editText_pageRange_appliesToAllPagesInRange() throws Exception { + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("foo", "bar"))); + request.setPageNumbers("2-3"); + + PdfJsonDocument input = + documentWithText("foo page 1", "foo page 2", "foo page 3", "foo page 4"); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + controller.editText(request); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PdfJsonDocument.class); + org.mockito.Mockito.verify(pdfJsonConversionService) + .convertJsonToPdf(captor.capture(), any(OutputStream.class)); + PdfJsonDocument mutated = captor.getValue(); + + assertTrue(mutated.getPages().get(0).getTextElements().get(0).getText().startsWith("foo")); + assertTrue(mutated.getPages().get(1).getTextElements().get(0).getText().startsWith("bar")); + assertTrue(mutated.getPages().get(2).getTextElements().get(0).getText().startsWith("bar")); + assertTrue(mutated.getPages().get(3).getTextElements().get(0).getText().startsWith("foo")); + } + + @Test + void editText_orderedEdits_applyInSequence() throws Exception { + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("foo", "bar"), edit("bar", "baz"))); + + PdfJsonDocument input = documentWithText("foo"); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + controller.editText(request); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PdfJsonDocument.class); + org.mockito.Mockito.verify(pdfJsonConversionService) + .convertJsonToPdf(captor.capture(), any(OutputStream.class)); + assertEquals("baz", captor.getValue().getPages().get(0).getTextElements().get(0).getText()); + } + + @Test + void editText_replaceWithEmptyString_deletesMatch() throws Exception { + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("DRAFT ", ""))); + + PdfJsonDocument input = documentWithText("DRAFT Confidential Memo"); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + controller.editText(request); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PdfJsonDocument.class); + org.mockito.Mockito.verify(pdfJsonConversionService) + .convertJsonToPdf(captor.capture(), any(OutputStream.class)); + assertEquals( + "Confidential Memo", + captor.getValue().getPages().get(0).getTextElements().get(0).getText()); + } + + @Test + void editText_noMatches_returnsPdfWithoutMutation() throws Exception { + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("notfound", "replacement"))); + + PdfJsonDocument input = documentWithText("nothing to match"); + int[] originalCodes = input.getPages().get(0).getTextElements().get(0).getCharCodes(); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + ResponseEntity response = controller.editText(request); + + assertEquals(HttpStatus.OK, response.getStatusCode()); + // Char codes left intact when nothing was replaced. + assertEquals( + originalCodes, input.getPages().get(0).getTextElements().get(0).getCharCodes()); + assertEquals( + "nothing to match", input.getPages().get(0).getTextElements().get(0).getText()); + } + + @Test + void editText_dollarInLiteralReplacement_isQuoted() throws Exception { + // Without quoting, '$1' would be interpreted as a backreference and crash. + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("price", "$100"))); + + PdfJsonDocument input = documentWithText("the price is set"); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + controller.editText(request); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PdfJsonDocument.class); + org.mockito.Mockito.verify(pdfJsonConversionService) + .convertJsonToPdf(captor.capture(), any(OutputStream.class)); + assertEquals( + "the $100 is set", + captor.getValue().getPages().get(0).getTextElements().get(0).getText()); + } + + @Test + void editText_emptyDocument_returnsResponseWithoutErrors() throws Exception { + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("foo", "bar"))); + + PdfJsonDocument input = new PdfJsonDocument(); + input.setPages(new ArrayList<>()); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + ResponseEntity response = controller.editText(request); + + assertEquals(HttpStatus.OK, response.getStatusCode()); + assertNotNull(response.getBody()); + } + + @Test + void editText_textElementWithNullText_isSkipped() throws Exception { + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("foo", "bar"))); + + PdfJsonDocument input = new PdfJsonDocument(); + PdfJsonPage page = new PdfJsonPage(); + page.setPageNumber(1); + PdfJsonTextElement nullText = new PdfJsonTextElement(); + nullText.setText(null); + PdfJsonTextElement realText = new PdfJsonTextElement(); + realText.setText("foo here"); + page.setTextElements(new ArrayList<>(List.of(nullText, realText))); + input.setPages(new ArrayList<>(List.of(page))); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + controller.editText(request); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PdfJsonDocument.class); + org.mockito.Mockito.verify(pdfJsonConversionService) + .convertJsonToPdf(captor.capture(), any(OutputStream.class)); + PdfJsonDocument mutated = captor.getValue(); + assertNull(mutated.getPages().get(0).getTextElements().get(0).getText()); + assertEquals("bar here", mutated.getPages().get(0).getTextElements().get(1).getText()); + } + + @Test + void editText_crossElement_matchSpansTwoElements() throws Exception { + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("Hello World", "Goodbye Earth"))); + + PdfJsonDocument input = + documentWithElements(List.of(textElement("Hello "), textElement("World"))); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + controller.editText(request); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PdfJsonDocument.class); + org.mockito.Mockito.verify(pdfJsonConversionService) + .convertJsonToPdf(captor.capture(), any(OutputStream.class)); + List elements = captor.getValue().getPages().get(0).getTextElements(); + + // Whole replacement lands in the first matched element; the second is emptied. The + // JSON->PDF rebuild concatenates per-token text so the font lays out the replacement as + // one run anchored at the first element's X position. + assertEquals("Goodbye Earth", elements.get(0).getText()); + assertEquals("", elements.get(1).getText()); + assertNull(elements.get(0).getCharCodes()); + assertNull(elements.get(1).getCharCodes()); + } + + @Test + void editText_crossElement_matchSpansFiveElementsLikeFragmentedTitle() throws Exception { + // Reproduces the real-world case: a multi-word title fragmented one word per text span. + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits( + List.of(edit("The Free Adobe Acrobat Alternative", "The PDF automation pipeline"))); + + PdfJsonDocument input = + documentWithElements( + List.of( + textElement("The "), + textElement("Free "), + textElement("Adobe "), + textElement("Acrobat "), + textElement("Alternative"))); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + controller.editText(request); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PdfJsonDocument.class); + org.mockito.Mockito.verify(pdfJsonConversionService) + .convertJsonToPdf(captor.capture(), any(OutputStream.class)); + List elements = captor.getValue().getPages().get(0).getTextElements(); + + // Whole replacement is written into the first matched element; the remaining four are + // emptied. Centered titles will become left-aligned at the original first-word X position. + assertEquals("The PDF automation pipeline", elements.get(0).getText()); + for (int i = 1; i < elements.size(); i++) { + assertEquals("", elements.get(i).getText(), "element " + i + " should be empty"); + } + } + + @Test + void editText_crossElement_preservesPrefixAndSuffix() throws Exception { + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("Hello World", "Goodbye Earth"))); + + PdfJsonDocument input = + documentWithElements( + List.of(textElement("Greeting: Hello "), textElement("World! And more"))); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + controller.editText(request); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PdfJsonDocument.class); + org.mockito.Mockito.verify(pdfJsonConversionService) + .convertJsonToPdf(captor.capture(), any(OutputStream.class)); + List elements = captor.getValue().getPages().get(0).getTextElements(); + + // First element keeps its prefix and gets the entire replacement; last element keeps its + // suffix only. + assertEquals("Greeting: Goodbye Earth", elements.get(0).getText()); + assertEquals("! And more", elements.get(1).getText()); + } + + @Test + void editText_matchInOneElementOfMany_onlyTouchesThatElement() throws Exception { + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("World", "Earth"))); + + PdfJsonDocument input = + documentWithElements( + List.of( + textElement("Hello "), + textElement("World!"), + textElement(" Goodbye"))); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + controller.editText(request); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PdfJsonDocument.class); + org.mockito.Mockito.verify(pdfJsonConversionService) + .convertJsonToPdf(captor.capture(), any(OutputStream.class)); + List elements = captor.getValue().getPages().get(0).getTextElements(); + + assertEquals("Hello ", elements.get(0).getText()); + assertEquals("Earth!", elements.get(1).getText()); + assertEquals(" Goodbye", elements.get(2).getText()); + // Only the modified element's char codes get cleared. + assertNotNull(elements.get(0).getCharCodes()); + assertNull(elements.get(1).getCharCodes()); + assertNotNull(elements.get(2).getCharCodes()); + } + + @Test + void editText_crossElement_multipleMatchesAppliedRightToLeft() throws Exception { + // Two matches in the same page text. Right-to-left application keeps earlier indices + // valid as later matches are written. + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("foo bar", "X"))); + + PdfJsonDocument input = + documentWithElements( + List.of( + textElement("foo "), + textElement("bar baz "), + textElement("foo "), + textElement("bar"))); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + controller.editText(request); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PdfJsonDocument.class); + org.mockito.Mockito.verify(pdfJsonConversionService) + .convertJsonToPdf(captor.capture(), any(OutputStream.class)); + List elements = captor.getValue().getPages().get(0).getTextElements(); + + // Joined was "foo bar baz foo bar"; both "foo bar" runs replaced with "X". + StringBuilder joined = new StringBuilder(); + for (PdfJsonTextElement el : elements) { + joined.append(el.getText() == null ? "" : el.getText()); + } + assertEquals("X baz X", joined.toString()); + } + + @Test + void editText_subWordFragmentation_writesIntoFirstElement() throws Exception { + // When the matched text is split into many character-level spans (typical of Type3 glyph + // runs), the whole replacement is dumped into the first matched element and the rest are + // emptied. The font lays out the replacement as one run at the first glyph's X position. + EditTextRequest request = new EditTextRequest(); + request.setFileInput(pdfFile()); + request.setEdits(List.of(edit("Hello World", "Goodbye Earth"))); + + // 11 sub-word elements covering "Hello World". + PdfJsonDocument input = + documentWithElements( + List.of( + textElement("H"), + textElement("e"), + textElement("l"), + textElement("l"), + textElement("o"), + textElement(" "), + textElement("W"), + textElement("o"), + textElement("r"), + textElement("l"), + textElement("d"))); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + controller.editText(request); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PdfJsonDocument.class); + org.mockito.Mockito.verify(pdfJsonConversionService) + .convertJsonToPdf(captor.capture(), any(OutputStream.class)); + List elements = captor.getValue().getPages().get(0).getTextElements(); + + // Entire replacement goes into the first matched element; the other 10 are emptied. + assertEquals("Goodbye Earth", elements.get(0).getText()); + for (int i = 1; i < elements.size(); i++) { + assertEquals("", elements.get(i).getText(), "element " + i + " should be empty"); + } + } + + @Test + void editText_outputFilenameDerivedFromInput() throws Exception { + EditTextRequest request = new EditTextRequest(); + request.setFileInput( + new MockMultipartFile( + "fileInput", "report.pdf", "application/pdf", buildEmptyPdf())); + request.setEdits(List.of(edit("anything", "x"))); + + PdfJsonDocument input = documentWithText("nothing matches"); + when(pdfJsonConversionService.convertPdfToJsonDocument(any(MultipartFile.class))) + .thenReturn(input); + + ResponseEntity response = controller.editText(request); + String contentDisposition = + response.getHeaders() + .getFirst(org.springframework.http.HttpHeaders.CONTENT_DISPOSITION); + assertNotNull(contentDisposition); + assertTrue(contentDisposition.contains("report_edited.pdf")); + assertFalse(contentDisposition.contains(".pdf_edited.pdf")); + } +} diff --git a/app/core/src/test/java/stirling/software/SPDF/service/PdfJsonConversionServiceUnicodeParsingTest.java b/app/core/src/test/java/stirling/software/SPDF/service/PdfJsonConversionServiceUnicodeParsingTest.java new file mode 100644 index 000000000..af81ea047 --- /dev/null +++ b/app/core/src/test/java/stirling/software/SPDF/service/PdfJsonConversionServiceUnicodeParsingTest.java @@ -0,0 +1,161 @@ +package stirling.software.SPDF.service; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.time.Duration; + +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link PdfJsonConversionService#parseToUnicodeCodepoint(String)}. + * + *

The function exists because PDF ToUnicode CMap entries can encode supplementary-plane + * codepoints (above U+FFFF) as UTF-16 surrogate pairs, e.g. {@code } for U+1F40E. The + * naive implementation that called {@link Integer#parseInt(String, int)} on the whole hex string + * threw {@link NumberFormatException} for any 8-char value because they overflow {@code int}. That + * triggered a fallback that left fonts with no proper CID mapping and made downstream JSON→PDF + * rebuilds slow/hung when rendering text with those fonts. + */ +class PdfJsonConversionServiceUnicodeParsingTest { + + @Test + void parsesSingleBmpCodeUnit() { + // Latin capital A. + assertEquals(0x41, PdfJsonConversionService.parseToUnicodeCodepoint("0041")); + } + + @Test + void parsesShortBmpHexValue() { + // Some ToUnicode entries omit leading zeros for low codepoints. + assertEquals(0x41, PdfJsonConversionService.parseToUnicodeCodepoint("41")); + } + + @Test + void parsesSupplementaryCodepointFromSurrogatePair() { + // U+1F40E HORSE encoded as UTF-16 surrogate pair D83D DC0E. The bug we are fixing was + // that Integer.parseInt("D83DDC0E", 16) overflows because D83DDC0E > Integer.MAX_VALUE. + assertEquals(0x1F40E, PdfJsonConversionService.parseToUnicodeCodepoint("D83DDC0E")); + } + + @Test + void parsesSurrogatePairFromUserReportedHang() { + // The exact hex value from the user's hang reproducer log: + // "Failed to build Unicode mapping for font NotoSans-Regular: For input string: + // \"D837DF0E\" under radix 16" + // D837 DF0E decodes to U+1DF0E (CJK supplementary). Important assertion: it returns a + // valid codepoint instead of overflowing Integer.parseInt and throwing. + int expected = new String(new char[] {(char) 0xD837, (char) 0xDF0E}).codePointAt(0); + assertEquals(0x1DF0E, expected); // sanity check on the test setup itself + assertEquals(expected, PdfJsonConversionService.parseToUnicodeCodepoint("D837DF0E")); + } + + @Test + void parsesLigatureDecompositionAsFirstCodepoint() { + // A ToUnicode entry can map one charCode to multiple Unicode chars (a ligature). PDF + // spec allows e.g. <0041 0042> for "AB". Our best-effort behavior is to return the + // first codepoint so the mapping is at least roughly meaningful for search/copy. + assertEquals(0x41, PdfJsonConversionService.parseToUnicodeCodepoint("00410042")); + } + + @Test + void rejectsEmptyHex() { + assertThrows( + NumberFormatException.class, + () -> PdfJsonConversionService.parseToUnicodeCodepoint("")); + } + + @Test + void rejectsNullHex() { + assertThrows( + NumberFormatException.class, + () -> PdfJsonConversionService.parseToUnicodeCodepoint(null)); + } + + @Test + void rejectsOddByteCountAboveBmp() { + // 6 hex chars is 3 bytes — not a valid UTF-16BE sequence. + assertThrows( + NumberFormatException.class, + () -> PdfJsonConversionService.parseToUnicodeCodepoint("D83DDC")); + } + + @Test + void countCodesProtectedTerminatesWhenReaderMakesNoProgress() { + // Reproduces the user's hang: PDFBox's CMap.readCode can return a successful code (0) + // from a stream where no bytes were consumed (corrupt codespace matching 0x00 bytes from + // the buffer's uninitialized region after EOF). Without the no-progress guard, the + // counting loop in countGlyphs ran forever. + ByteArrayInputStream stream = new ByteArrayInputStream(new byte[] {1, 2, 3, 4}); + PdfJsonConversionService.CodeReader reader = in -> 0; // never reads, always "succeeds" + + int count = + assertTimeoutPreemptively( + Duration.ofSeconds(2), + () -> PdfJsonConversionService.countCodesProtected(stream, reader)); + + // First iteration sees no progress and breaks immediately. + assertEquals(0, count); + } + + @Test + void countCodesProtectedTerminatesOnEmptyStream() { + ByteArrayInputStream stream = new ByteArrayInputStream(new byte[0]); + PdfJsonConversionService.CodeReader reader = + in -> { + throw new AssertionError("reader must not be called when stream is empty"); + }; + + int count = + assertTimeoutPreemptively( + Duration.ofSeconds(2), + () -> PdfJsonConversionService.countCodesProtected(stream, reader)); + + assertEquals(0, count); + } + + @Test + void countCodesProtectedHonorsExplicitMinusOneReturn() throws IOException { + ByteArrayInputStream stream = new ByteArrayInputStream(new byte[] {1, 2, 3}); + PdfJsonConversionService.CodeReader reader = + in -> { + int b = in.read(); + return b == -1 ? -1 : b; + }; + + int count = PdfJsonConversionService.countCodesProtected(stream, reader); + + assertEquals(3, count); + } + + @Test + void countCodesProtectedTerminatesIfReaderReadsThenStops() throws IOException { + // A reader that consumes one byte then hits a corrupt-CMap pattern returning 0 without + // consuming further must still terminate after counting the consumed bytes. + ByteArrayInputStream stream = new ByteArrayInputStream(new byte[] {1, 2, 3, 4}); + PdfJsonConversionService.CodeReader reader = + new PdfJsonConversionService.CodeReader() { + boolean firstCall = true; + + @Override + public int readCode(InputStream in) throws IOException { + if (firstCall) { + firstCall = false; + return in.read(); + } + return 0; // simulates corrupt CMap thereafter + } + }; + + int count = + assertTimeoutPreemptively( + Duration.ofSeconds(2), + () -> PdfJsonConversionService.countCodesProtected(stream, reader)); + + assertEquals(1, count); + } +} diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/service/AiWorkflowService.java b/app/proprietary/src/main/java/stirling/software/proprietary/service/AiWorkflowService.java index c77da89ce..64de88e4d 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/service/AiWorkflowService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/service/AiWorkflowService.java @@ -30,6 +30,7 @@ import lombok.extern.slf4j.Slf4j; import stirling.software.common.service.CustomPDFDocumentFactory; import stirling.software.common.service.FileStorage; import stirling.software.common.service.InternalApiClient; +import stirling.software.common.service.InternalApiTimeoutException; import stirling.software.common.service.ToolMetadataService; import stirling.software.common.util.ExceptionUtils; import stirling.software.common.util.TempFile; @@ -323,10 +324,12 @@ public class AiWorkflowService { result.files(), inputFileNames(filesById), result.report())); + } catch (InternalApiTimeoutException e) { + log.error("Tool {} timed out: {}", endpointPath, e.getMessage()); + return new WorkflowState.Terminal(cannotContinue(toolTimeoutMessage(endpointPath, e))); } catch (Exception e) { log.error("Failed to execute tool {}: {}", endpointPath, e.getMessage(), e); - return new WorkflowState.Terminal( - cannotContinue("Tool execution failed: " + e.getMessage())); + return new WorkflowState.Terminal(cannotContinue(toolFailureMessage(endpointPath, e))); } } @@ -414,6 +417,10 @@ public class AiWorkflowService { return new WorkflowState.Terminal( buildCompletedResponse( summary, currentFiles, inputFileNames(filesById), lastReport)); + } catch (InternalApiTimeoutException e) { + log.error("Plan step on tool {} timed out: {}", e.getEndpointPath(), e.getMessage()); + return new WorkflowState.Terminal( + cannotContinue(toolTimeoutMessage(e.getEndpointPath(), e))); } catch (Exception e) { log.error("Failed to execute plan: {}", e.getMessage(), e); return new WorkflowState.Terminal( @@ -425,6 +432,20 @@ public class AiWorkflowService { return filesById.values().stream().map(MultipartFile::getOriginalFilename).toList(); } + private static String toolTimeoutMessage(String endpointPath, InternalApiTimeoutException e) { + return String.format( + "The %s tool did not respond within %d seconds and was aborted. The underlying" + + " operation may be hung; try again, run on a smaller file, or use a" + + " different approach.", + endpointPath, e.getReadTimeout().toSeconds()); + } + + private static String toolFailureMessage(String endpointPath, Throwable cause) { + String reason = + cause.getMessage() != null ? cause.getMessage() : cause.getClass().getSimpleName(); + return String.format("The %s tool failed: %s", endpointPath, reason); + } + /** * Execute a single tool step. If the endpoint accepts multiple files, all files are sent in one * call. Otherwise, the endpoint is called once per file. ZIP responses are unpacked so each @@ -477,8 +498,15 @@ public class AiWorkflowService { } for (Map.Entry entry : parameters.entrySet()) { if (entry.getValue() instanceof List list) { - for (Object item : list) { - body.add(entry.getKey(), item); + if (containsStructuredElements(list)) { + // Endpoints binding lists of structured objects (e.g. /security/redact's + // redactions, /general/edit-text's edits) parse a single JSON string field via + // a property editor. Pre-serialize the whole list so binding succeeds. + body.add(entry.getKey(), objectMapper.writeValueAsString(list)); + } else { + for (Object item : list) { + body.add(entry.getKey(), item); + } } } else { body.add(entry.getKey(), entry.getValue()); @@ -529,6 +557,15 @@ public class AiWorkflowService { } } + private static boolean containsStructuredElements(List list) { + for (Object item : list) { + if (item instanceof Map || item instanceof List) { + return true; + } + } + return false; + } + private List toResources(Map filesById) throws IOException { List resources = new ArrayList<>(); for (MultipartFile file : filesById.values()) { diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/service/AiWorkflowServiceTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/service/AiWorkflowServiceTest.java index b0ffbf19d..3b450bc6c 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/service/AiWorkflowServiceTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/service/AiWorkflowServiceTest.java @@ -2,6 +2,7 @@ package stirling.software.proprietary.service; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; @@ -30,6 +31,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.io.TempDir; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.core.io.ByteArrayResource; @@ -44,6 +46,7 @@ import stirling.software.common.service.CustomPDFDocumentFactory; import stirling.software.common.service.FileStorage; import stirling.software.common.service.FileStorage.StoredFile; import stirling.software.common.service.InternalApiClient; +import stirling.software.common.service.InternalApiTimeoutException; import stirling.software.common.service.ToolMetadataService; import stirling.software.common.util.TempFileManager; import stirling.software.common.util.TempFileRegistry; @@ -251,6 +254,158 @@ class AiWorkflowServiceTest { verify(internalApiClient, times(1)).post(eq(COMPRESS_ENDPOINT), any()); } + @Test + void parameterListOfStructuredObjectsIsJsonEncodedAsSingleField() throws IOException { + // Endpoints like /general/edit-text and /security/redact bind a List + // from a single JSON form field via a property editor. The plan executor must + // pre-serialize such lists rather than splitting them into repeated form fields. + String editTextEndpoint = "/api/v1/general/edit-text"; + MockMultipartFile input = pdf("input.pdf", "bytes"); + stubOrchestrator( + """ + { + "outcome":"plan", + "summary":"Find and replace", + "steps":[ + {"tool":"%s","parameters":{ + "edits":[{"find":"foo","replace":"bar"},{"find":"baz","replace":"qux"}], + "useRegex":false + }} + ] + } + """ + .formatted(editTextEndpoint)); + when(toolMetadataService.isMultiInput(editTextEndpoint)).thenReturn(false); + when(toolMetadataService.shouldUnpackZipResponse(editTextEndpoint)).thenReturn(false); + stubEndpoint(editTextEndpoint, pdfResource("edited", "edited.pdf")); + stubFileStorage(); + + service.orchestrate(requestFor(input, "find and replace")); + + @SuppressWarnings("unchecked") + ArgumentCaptor> bodyCaptor = + ArgumentCaptor.forClass(MultiValueMap.class); + verify(internalApiClient).post(eq(editTextEndpoint), bodyCaptor.capture()); + MultiValueMap body = bodyCaptor.getValue(); + + // The structured-list field must be serialized as ONE JSON-string entry, not multiple + // entries. + List editsValues = body.get("edits"); + assertNotNull(editsValues); + assertEquals(1, editsValues.size()); + assertEquals( + "[{\"find\":\"foo\",\"replace\":\"bar\"},{\"find\":\"baz\",\"replace\":\"qux\"}]", + editsValues.get(0)); + + // Primitive fields keep the original behavior (single value, not JSON-wrapped). + List useRegex = body.get("useRegex"); + assertNotNull(useRegex); + assertEquals(1, useRegex.size()); + assertEquals(false, useRegex.get(0)); + } + + @Test + void parameterListOfPrimitivesIsSentAsRepeatedFormFields() throws IOException { + // Endpoints like /misc/ocr-pdf bind List via Spring's repeated-form-field + // convention. The executor must preserve that behavior for primitive lists. + String ocrEndpoint = "/api/v1/misc/ocr-pdf"; + MockMultipartFile input = pdf("input.pdf", "bytes"); + stubOrchestrator( + """ + { + "outcome":"plan", + "summary":"OCR the document", + "steps":[ + {"tool":"%s","parameters":{"languages":["eng","fra","deu"]}} + ] + } + """ + .formatted(ocrEndpoint)); + when(toolMetadataService.isMultiInput(ocrEndpoint)).thenReturn(false); + when(toolMetadataService.shouldUnpackZipResponse(ocrEndpoint)).thenReturn(false); + stubEndpoint(ocrEndpoint, pdfResource("ocred", "ocred.pdf")); + stubFileStorage(); + + service.orchestrate(requestFor(input, "ocr")); + + @SuppressWarnings("unchecked") + ArgumentCaptor> bodyCaptor = + ArgumentCaptor.forClass(MultiValueMap.class); + verify(internalApiClient).post(eq(ocrEndpoint), bodyCaptor.capture()); + List languages = bodyCaptor.getValue().get("languages"); + assertNotNull(languages); + assertEquals(List.of("eng", "fra", "deu"), languages); + } + + @Test + void toolTimeoutSurfacesAsCannotContinueWithFriendlyMessage() throws IOException { + // When an internal tool hangs, InternalApiClient throws InternalApiTimeoutException after + // its read timeout fires. The workflow must convert that into a clean CANNOT_CONTINUE + // outcome so the user sees an actionable message rather than the request hanging forever. + MockMultipartFile input = pdf("input.pdf", "bytes"); + stubOrchestrator( + """ + {"outcome":"tool_call","tool":"%s","parameters":{"angle":90},"rationale":"Rotating"} + """ + .formatted(ROTATE_ENDPOINT)); + when(toolMetadataService.isMultiInput(ROTATE_ENDPOINT)).thenReturn(false); + when(internalApiClient.post(eq(ROTATE_ENDPOINT), any())) + .thenThrow( + new InternalApiTimeoutException( + ROTATE_ENDPOINT, + java.time.Duration.ofSeconds(300), + new java.io.IOException("Read timed out"))); + + AiWorkflowResponse result = service.orchestrate(requestFor(input, "rotate")); + + assertEquals(AiWorkflowOutcome.CANNOT_CONTINUE, result.getOutcome()); + assertNotNull(result.getReason()); + assertTrue( + result.getReason().contains(ROTATE_ENDPOINT), + "reason should mention the failing tool path: " + result.getReason()); + assertTrue( + result.getReason().contains("300"), + "reason should mention the timeout duration: " + result.getReason()); + verify(internalApiClient, times(1)).post(eq(ROTATE_ENDPOINT), any()); + } + + @Test + void planTimeoutOnLaterStepStillSurfacesCleanly() throws IOException { + // Multi-step plans must also handle a hung tool gracefully (no partial leaks) and the + // failure message must identify which step failed so the user can iterate. + MockMultipartFile input = pdf("input.pdf", "bytes"); + stubOrchestrator( + """ + { + "outcome":"plan", + "summary":"Rotate then compress", + "steps":[ + {"tool":"%s","parameters":{"angle":90}}, + {"tool":"%s","parameters":{}} + ] + } + """ + .formatted(ROTATE_ENDPOINT, COMPRESS_ENDPOINT)); + when(toolMetadataService.isMultiInput(anyString())).thenReturn(false); + when(toolMetadataService.shouldUnpackZipResponse(anyString())).thenReturn(false); + stubEndpoint(ROTATE_ENDPOINT, pdfResource("rotated", "rotated.pdf")); + // No fileStorage stub here: the second step times out before any output reaches storage. + when(internalApiClient.post(eq(COMPRESS_ENDPOINT), any())) + .thenThrow( + new InternalApiTimeoutException( + COMPRESS_ENDPOINT, + java.time.Duration.ofSeconds(300), + new java.io.IOException("Read timed out"))); + + AiWorkflowResponse result = service.orchestrate(requestFor(input, "rotate then compress")); + + assertEquals(AiWorkflowOutcome.CANNOT_CONTINUE, result.getOutcome()); + assertTrue(result.getReason().contains(COMPRESS_ENDPOINT)); + // The first step actually executed; the failure happened on the second. + verify(internalApiClient, times(1)).post(eq(ROTATE_ENDPOINT), any()); + verify(internalApiClient, times(1)).post(eq(COMPRESS_ENDPOINT), any()); + } + @Test void toolCallWithoutEndpointFallsBackToCannotContinue() throws IOException { MockMultipartFile input = pdf("input.pdf", "bytes"); diff --git a/engine/src/stirling/models/tool_models.py b/engine/src/stirling/models/tool_models.py index 2750f222c..2e3caad86 100644 --- a/engine/src/stirling/models/tool_models.py +++ b/engine/src/stirling/models/tool_models.py @@ -373,6 +373,25 @@ class EditTableOfContentsParams(ApiModel): ) +class EditTextOperation(ApiModel): + find: str = Field(..., description="The literal text to find.") + replace: str = Field(..., description="The replacement text. May be empty to delete the matched text.") + + +class EditTextParams(ApiModel): + edits: list[EditTextOperation] | None = Field( + None, + description="Ordered list of find/replace operations. Each replaces every occurrence on the selected pages, in order; later operations see the result of earlier ones (so 'foo'->'foos' then 'foos'->'bars' turns 'foo' into 'bars').", + ) + page_numbers: str | None = Field( + "all", + description="The pages to select, Supports ranges (e.g., '1,3,5-9'), or 'all' or functions in the format 'an+b' where 'a' is the multiplier of the page number 'n', and 'b' is a constant (e.g., '2n+1', '3n', '6n-5')", + ) + whole_word_search: bool | None = Field( + False, description="Whether matches must be whole words (boundaries determined by non-word characters)" + ) + + class EmlToPdfParams(ApiModel): download_html: bool | None = Field( None, description="Download HTML intermediate file instead of PDF", examples=[False] @@ -1105,6 +1124,7 @@ class Model( | BookletImpositionParams | CropParams | EditTableOfContentsParams + | EditTextParams | MergePdfsParams | MultiPageLayoutParams | OverlayPdfsParams @@ -1172,6 +1192,7 @@ class Model( | BookletImpositionParams | CropParams | EditTableOfContentsParams + | EditTextParams | MergePdfsParams | MultiPageLayoutParams | OverlayPdfsParams @@ -1240,6 +1261,7 @@ type ParamToolModel = ( | BookletImpositionParams | CropParams | EditTableOfContentsParams + | EditTextParams | MergePdfsParams | MultiPageLayoutParams | OverlayPdfsParams @@ -1309,6 +1331,7 @@ class ToolEndpoint(StrEnum): BOOKLET_IMPOSITION = "/api/v1/general/booklet-imposition" CROP = "/api/v1/general/crop" EDIT_TABLE_OF_CONTENTS = "/api/v1/general/edit-table-of-contents" + EDIT_TEXT = "/api/v1/general/edit-text" MERGE_PDFS = "/api/v1/general/merge-pdfs" MULTI_PAGE_LAYOUT = "/api/v1/general/multi-page-layout" OVERLAY_PDFS = "/api/v1/general/overlay-pdfs" @@ -1376,6 +1399,7 @@ OPERATIONS: dict[ToolEndpoint, ParamToolModelType] = { ToolEndpoint.BOOKLET_IMPOSITION: BookletImpositionParams, ToolEndpoint.CROP: CropParams, ToolEndpoint.EDIT_TABLE_OF_CONTENTS: EditTableOfContentsParams, + ToolEndpoint.EDIT_TEXT: EditTextParams, ToolEndpoint.MERGE_PDFS: MergePdfsParams, ToolEndpoint.MULTI_PAGE_LAYOUT: MultiPageLayoutParams, ToolEndpoint.OVERLAY_PDFS: OverlayPdfsParams, diff --git a/engine/tests/test_pdf_edit_agent.py b/engine/tests/test_pdf_edit_agent.py index a40b671db..312e5a64c 100644 --- a/engine/tests/test_pdf_edit_agent.py +++ b/engine/tests/test_pdf_edit_agent.py @@ -21,8 +21,15 @@ from stirling.contracts import ( SupportedCapability, ToolOperationStep, ) -from stirling.models import OPERATIONS, FileId -from stirling.models.tool_models import Angle, FlattenParams, RotatePdfParams, ToolEndpoint +from stirling.models import OPERATIONS, FileId, ParamToolModel +from stirling.models.tool_models import ( + Angle, + EditTextOperation, + EditTextParams, + FlattenParams, + RotatePdfParams, + ToolEndpoint, +) from stirling.services.runtime import AppRuntime @@ -35,8 +42,11 @@ class ParameterSelectorCall: class RecordingParameterSelector: - def __init__(self) -> None: + """Test double that records calls and returns predetermined parameter objects per index.""" + + def __init__(self, params_by_index: list[ParamToolModel] | None = None) -> None: self.calls: list[ParameterSelectorCall] = [] + self._params_by_index = params_by_index async def select( self, @@ -44,7 +54,7 @@ class RecordingParameterSelector: operation_plan: list[ToolEndpoint], operation_index: int, generated_steps: list[ToolOperationStep], - ) -> RotatePdfParams | FlattenParams: + ) -> ParamToolModel: self.calls.append( ParameterSelectorCall( request=request, @@ -53,6 +63,8 @@ class RecordingParameterSelector: generated_steps=list(generated_steps), ) ) + if self._params_by_index is not None: + return self._params_by_index[operation_index] if operation_index == 0: return RotatePdfParams(angle=Angle(90)) return FlattenParams(flatten_only_forms=False, render_dpi=None) @@ -393,3 +405,167 @@ async def test_pdf_edit_agent_rejects_plan_referencing_unavailable_operations( assert "not available" in response.reason assert "COMPRESS_PDF" in response.reason assert parameter_selector.calls == [] + + +@pytest.mark.anyio +async def test_pdf_edit_agent_supports_literal_find_replace(runtime: AppRuntime) -> None: + params = EditTextParams( + edits=[EditTextOperation(find="2025", replace="2026")], + page_numbers="all", + whole_word_search=False, + ) + parameter_selector = RecordingParameterSelector([params]) + agent = StubPdfEditAgent( + runtime, + PdfEditPlanSelection( + operations=[ToolEndpoint.EDIT_TEXT], + summary="Replace 2025 with 2026 throughout the document.", + ), + parameter_selector=parameter_selector, + ) + + response = await agent.handle( + PdfEditRequest( + user_message="Change every 2025 to 2026.", + files=[AiFile(id=FileId("contract-id"), name="contract.pdf")], + ) + ) + + assert isinstance(response, EditPlanResponse) + assert len(response.steps) == 1 + step = response.steps[0] + assert step.tool == ToolEndpoint.EDIT_TEXT + assert isinstance(step.parameters, EditTextParams) + assert step.parameters.edits == [EditTextOperation(find="2025", replace="2026")] + + +@pytest.mark.anyio +async def test_pdf_edit_agent_supports_copy_edit_using_page_text(runtime: AppRuntime) -> None: + page_text = [ + ExtractedFileText( + file_name="memo.pdf", + pages=[ + PdfTextSelection( + page_number=3, + text="The quick brown fox jumps over the lazy dog.", + ) + ], + ) + ] + params = EditTextParams( + edits=[ + EditTextOperation(find="quick", replace="slow"), + EditTextOperation(find="lazy", replace="energetic"), + ], + page_numbers="3", + whole_word_search=False, + ) + parameter_selector = RecordingParameterSelector([params]) + agent = StubPdfEditAgent( + runtime, + PdfEditPlanSelection( + operations=[ToolEndpoint.EDIT_TEXT], + summary="Fix typos on page 3.", + ), + parameter_selector=parameter_selector, + ) + + response = await agent.handle( + PdfEditRequest( + user_message="Fix typos on page 3.", + files=[AiFile(id=FileId("memo-id"), name="memo.pdf")], + page_text=page_text, + ) + ) + + assert isinstance(response, EditPlanResponse) + assert len(parameter_selector.calls) == 1 + # The parameter selector receives the extracted page text, which is what enables free-form + # copy-editing: it can read the current text and propose specific edits. + assert parameter_selector.calls[0].request.page_text == page_text + step = response.steps[0] + assert step.tool == ToolEndpoint.EDIT_TEXT + assert isinstance(step.parameters, EditTextParams) + assert step.parameters.page_numbers == "3" + assert step.parameters.edits is not None + assert len(step.parameters.edits) == 2 + + +@pytest.mark.anyio +async def test_pdf_edit_agent_supports_natural_language_directed_edit(runtime: AppRuntime) -> None: + page_text = [ + ExtractedFileText( + file_name="agreement.pdf", + pages=[ + PdfTextSelection( + page_number=1, + text="This agreement is between OldCompany Inc. and the client.", + ) + ], + ) + ] + params = EditTextParams( + edits=[EditTextOperation(find="OldCompany Inc.", replace="Acme Corp")], + page_numbers="all", + whole_word_search=False, + ) + parameter_selector = RecordingParameterSelector([params]) + agent = StubPdfEditAgent( + runtime, + PdfEditPlanSelection( + operations=[ToolEndpoint.EDIT_TEXT], + summary="Update the company name to Acme Corp.", + ), + parameter_selector=parameter_selector, + ) + + response = await agent.handle( + PdfEditRequest( + user_message="Update the company name to Acme Corp.", + files=[AiFile(id=FileId("agreement-id"), name="agreement.pdf")], + page_text=page_text, + ) + ) + + assert isinstance(response, EditPlanResponse) + step = response.steps[0] + assert step.tool == ToolEndpoint.EDIT_TEXT + assert isinstance(step.parameters, EditTextParams) + # The exact find string came from interpreting the user's intent against the extracted text. + assert step.parameters.edits is not None + assert step.parameters.edits[0].find == "OldCompany Inc." + assert step.parameters.edits[0].replace == "Acme Corp" + + +@pytest.mark.anyio +async def test_pdf_edit_agent_composes_edit_text_with_other_operations(runtime: AppRuntime) -> None: + """EDIT_TEXT can appear alongside other operations in a single plan.""" + edit_params = EditTextParams( + edits=[EditTextOperation(find="DRAFT", replace="")], + page_numbers="all", + whole_word_search=False, + ) + parameter_selector = RecordingParameterSelector([edit_params, RotatePdfParams(angle=Angle(90))]) + agent = StubPdfEditAgent( + runtime, + PdfEditPlanSelection( + operations=[ToolEndpoint.EDIT_TEXT, ToolEndpoint.ROTATE_PDF], + summary="Remove DRAFT marker, then rotate.", + ), + parameter_selector=parameter_selector, + ) + + response = await agent.handle( + PdfEditRequest( + user_message="Remove the DRAFT watermark text and then rotate.", + files=[AiFile(id=FileId("draft-id"), name="draft.pdf")], + ) + ) + + assert isinstance(response, EditPlanResponse) + assert [step.tool for step in response.steps] == [ + ToolEndpoint.EDIT_TEXT, + ToolEndpoint.ROTATE_PDF, + ] + assert isinstance(response.steps[0].parameters, EditTextParams) + assert isinstance(response.steps[1].parameters, RotatePdfParams)