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)