From 01d739ee7db7d81d1ed1898bae95e4b2c31e0576 Mon Sep 17 00:00:00 2001 From: Lucio Lelii Date: Mon, 14 Sep 2026 12:19:20 +0200 Subject: [PATCH] Stop a short input name from failing a file upload The uploaded file's temp name used the input's own name as the prefix handed to File.createTempFile, which rejects anything shorter than three characters. Uploading to an input called "dc" therefore threw IllegalArgumentException - past the IOException catch, so the client saw only a 500 that reads as "Failed to upload file" in the UI, with nothing naming the real cause. A filename carrying a path separator failed the same way. Sanitise and pad both parts: neither is the uploader's mistake to pay for, and nothing downstream reads meaning out of the temp name beyond the extension, which is preserved. Co-Authored-By: Claude Opus 5 (1M context) --- .../controllers/ExecutionsController.java | 26 ++++++++++++-- .../controllers/ExecutionControllerTest.java | 35 +++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/main/java/it/cnr/isti/workflow/manager/controllers/ExecutionsController.java b/src/main/java/it/cnr/isti/workflow/manager/controllers/ExecutionsController.java index 9411e8d..df3b0a7 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/controllers/ExecutionsController.java +++ b/src/main/java/it/cnr/isti/workflow/manager/controllers/ExecutionsController.java @@ -336,7 +336,7 @@ public class ExecutionsController { public ExecutionView prepareFileInputs(@PathVariable String executionId, @PathVariable String nodeId, @PathVariable String inputName, @RequestParam MultipartFile file, @AuthenticationPrincipal LoginEntity userDetails) { try { - File myFile = File.createTempFile(inputName, file.getOriginalFilename()); + File myFile = createUploadTempFile(inputName, file.getOriginalFilename()); file.transferTo(myFile); return ExecutionView.fromExecution( executionService.prepareInput(visibleExecution(executionId, userDetails).getId(), nodeId, inputName, myFile)); @@ -346,6 +346,28 @@ public class ExecutionsController { } + /** + * Names the temp file an upload lands in. + * + *

Both parts are sanitised because neither is the uploader's mistake to pay for: + * {@link File#createTempFile} rejects a prefix shorter than three characters, so an input named + * "dc" used to fail the whole upload with a 500 that reads as "Failed to upload file" in the UI, + * and a filename carrying a path separator fails the same way. The names only make the temp file + * readable - nothing downstream reads meaning out of them beyond the extension. + */ + private static File createUploadTempFile(String inputName, String originalFilename) throws IOException { + StringBuilder prefix = new StringBuilder(sanitizeTempNamePart(inputName)); + while (prefix.length() < 3) { + prefix.append('_'); + } + String suffix = sanitizeTempNamePart(originalFilename); + return File.createTempFile(prefix.toString(), suffix.isEmpty() ? ".tmp" : suffix); + } + + private static String sanitizeTempNamePart(String value) { + return value == null ? "" : value.replaceAll("[^A-Za-z0-9._-]", ""); + } + @PutMapping(path = "{executionId}/node/{nodeId}/input/{inputName}/files", consumes = "multipart/form-data") @Operation(summary = "Prepares file array inputs", description = "Prepares multiple file inputs for a specific execution node input.") public ExecutionView prepareFileArrayInputs(@PathVariable String executionId, @PathVariable String nodeId, @@ -354,7 +376,7 @@ public class ExecutionsController { List preparedFiles = new ArrayList<>(); try { for (MultipartFile file : files) { - File myFile = File.createTempFile(inputName, file.getOriginalFilename()); + File myFile = createUploadTempFile(inputName, file.getOriginalFilename()); file.transferTo(myFile); preparedFiles.add(myFile); } diff --git a/src/test/java/it/cnr/isti/workflow/manager/controllers/ExecutionControllerTest.java b/src/test/java/it/cnr/isti/workflow/manager/controllers/ExecutionControllerTest.java index 56dc73e..d47fa49 100644 --- a/src/test/java/it/cnr/isti/workflow/manager/controllers/ExecutionControllerTest.java +++ b/src/test/java/it/cnr/isti/workflow/manager/controllers/ExecutionControllerTest.java @@ -744,6 +744,41 @@ public class ExecutionControllerTest { .get(new it.cnr.isti.workflow.manager.executions.FieldKey(block.getId(), "response"))); } + @Test + public void fileInputUploadAcceptsAnInputNameShorterThanATempFilePrefix() { + // File.createTempFile rejects a prefix shorter than three characters, and the input's name + // was handed to it verbatim: uploading to an input called "dc" threw IllegalArgumentException + // past the IOException catch, so the client only saw a 500 reading "Failed to upload file". + Block agentBlock = + blocksController.create(it.cnr.isti.workflow.manager.blocks.configurations.MCPAgentBlockConfiguration.builder() + .name("Read uploaded doc") + .model("llama3.1:8b") + .prompt("Summarize the attached document") + .uploadInputs(List.of(new it.cnr.isti.workflow.manager.blocks.configurations.MCPAgentUploadInput( + "dc", + it.cnr.isti.workflow.manager.blocks.configurations.MCPAgentUploadInput.MCPAgentUploadKind.DOCUMENT, + false))) + .build()); + + FlowCreateRequest request = new FlowCreateRequest( + "Short Upload Input Flow", + "Flow whose upload input name is shorter than a temp file prefix", + FlowData.builder().block(agentBlock).build()); + ResponseEntity createdFlow = flowController.createFlow(request, testUser()); + ExecutionView execution = executionsController.create(createdFlow.getBody().id(), testUser()); + + org.springframework.mock.web.MockMultipartFile upload = new org.springframework.mock.web.MockMultipartFile( + "file", "plan.pdf", "application/pdf", "%PDF-1.7\nbody".getBytes(java.nio.charset.StandardCharsets.US_ASCII)); + + ExecutionView withFile = executionsController.prepareFileInputs(execution.getId(), agentBlock.getId(), "dc", + upload, testUser()); + + Object storedInput = withFile.getContext().getInputs().values().stream().findFirst().orElse(null); + org.junit.jupiter.api.Assertions.assertNotNull(storedInput); + org.junit.jupiter.api.Assertions.assertTrue(String.valueOf(storedInput).endsWith("plan.pdf"), + "the uploaded file keeps its extension, so whatever reads it back still sees a PDF: " + storedInput); + } + @Test public void cancelExecutionClearsRuntimeStateAndMarksExecutionCancelled() { LLMDescriptor llmDescriptor = LLMDescriptor.builder()