lelii
/
humainflow-service
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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) <noreply@anthropic.com>
This commit is contained in:
Lucio Lelii 2026-09-14 12:19:20 +02:00
parent cc820554ac
commit 01d739ee7d
2 changed files with 59 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
src/main/java/it/cnr/isti/workflow/manager/controllers/ExecutionsController.java
View File

@ -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.
*
* <p>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<File> 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);
}

35
src/test/java/it/cnr/isti/workflow/manager/controllers/ExecutionControllerTest.java
View File

@ -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<it.cnr.isti.workflow.manager.blocks.types.MCPAgentBlockType> 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<FlowView> 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()