From 23f872823dcf6515f463e1f1ad6f31cdfd100bd8 Mon Sep 17 00:00:00 2001 From: Ludy Date: Wed, 21 Jan 2026 23:00:59 +0100 Subject: [PATCH] fix(pipeline): avoid bad multipart by letting RestTemplate set boundary (#5522) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description of Changes - **What was changed** - Updated `PipelineProcessor#sendWebRequest(...)` to stop forcing `Content-Type: multipart/form-data` and to stop manually writing the multipart body via `FormHttpMessageConverter`. - Switched to `RestTemplate#httpEntityCallback(...)` (`RequestCallback`) so Spring’s configured message converters handle multipart serialization (including boundary generation) correctly. - Made `X-API-KEY` header conditional (only added when non-null/non-empty). - Added a unit test (`sendWebRequestDoesNotForceContentType`) to ensure `Content-Type` is not explicitly set on the outgoing request headers and that the request succeeds end-to-end using mocked `RestTemplate` behavior. - **Why the change was made** - The server-side error `org.eclipse.jetty.http.BadMessageException: 400: bad multipart` with `EOFException: unexpected EOF` indicates the multipart request can be malformed (commonly due to boundary/content-type mismatches or prematurely terminated streams). Manually setting the multipart content type and writing the body can bypass Spring’s normal boundary handling and lead to invalid multipart payloads. Letting `RestTemplate` handle it ensures correct formatting. --- This pull request updates how multipart web requests are sent in the pipeline processor to avoid forcing the `Content-Type` header for multipart requests, allowing the message converter to set it automatically. It also adds a new test to ensure this behavior. The main changes are focused on improving multipart request handling and increasing test coverage. **Multipart request handling improvements:** * Refactored `PipelineProcessor.runPipelineAgainstFiles` to avoid explicitly setting the `Content-Type` header to `multipart/form-data`, letting the message converter set the appropriate boundary and content type instead. This helps prevent issues with incorrect boundaries and improves compatibility with multipart requests. * Updated imports in `PipelineProcessor.java` to remove unused `FormHttpMessageConverter` and add `RequestCallback`. **Testing enhancements:** * Added a new test `sendWebRequestDoesNotForceContentType` in `PipelineProcessorTest.java` to verify that the `Content-Type` header is not set explicitly and is left for the message converter to handle. This test uses mocking to simulate the request and response flow. * Added necessary imports in `PipelineProcessorTest.java` to support new test logic and mocking. [[1]](diffhunk://#diff-1a45e7b37023bd3a9580ab24d7025bb359cb8891c486111d372c0557f817b3edR7-R8) [[2]](diffhunk://#diff-1a45e7b37023bd3a9580ab24d7025bb359cb8891c486111d372c0557f817b3edR18-R27) --- ## Checklist ### General - [ ] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [ ] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md) (if applicable) - [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [ ] I have performed a self-review of my own code - [ ] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### Translations (if applicable) - [ ] I ran [`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md#6-testing) for more details. --- .../api/pipeline/PipelineProcessor.java | 18 ++-- .../api/pipeline/PipelineProcessorTest.java | 90 +++++++++++++++++++ 2 files changed, 100 insertions(+), 8 deletions(-) diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/pipeline/PipelineProcessor.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/pipeline/PipelineProcessor.java index f3a926de8..a0bb5b66b 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/api/pipeline/PipelineProcessor.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/pipeline/PipelineProcessor.java @@ -18,10 +18,10 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.io.FileSystemResource; import org.springframework.core.io.Resource; import org.springframework.http.*; -import org.springframework.http.converter.FormHttpMessageConverter; import org.springframework.stereotype.Service; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; +import org.springframework.web.client.RequestCallback; import org.springframework.web.client.RestTemplate; import org.springframework.web.multipart.MultipartFile; @@ -287,17 +287,19 @@ public class PipelineProcessor { // Set up headers, including API key HttpHeaders headers = new HttpHeaders(); String apiKey = getApiKeyForUser(); - headers.add("X-API-KEY", apiKey); - headers.setContentType(MediaType.MULTIPART_FORM_DATA); + if (apiKey != null && !apiKey.isEmpty()) { + headers.add("X-API-KEY", apiKey); + } + // Let the message converter set the multipart boundary/content type + HttpEntity> entity = new HttpEntity<>(body, headers); + + RequestCallback requestCallback = + restTemplate.httpEntityCallback(entity, Resource.class /* response type hint */); return restTemplate.execute( url, HttpMethod.POST, - request -> { - request.getHeaders().putAll(headers); - new FormHttpMessageConverter() - .write(body, MediaType.MULTIPART_FORM_DATA, request); - }, + requestCallback, response -> { try { TempFile tempFile = tempFileManager.createManagedTempFile("pipeline"); diff --git a/app/core/src/test/java/stirling/software/SPDF/controller/api/pipeline/PipelineProcessorTest.java b/app/core/src/test/java/stirling/software/SPDF/controller/api/pipeline/PipelineProcessorTest.java index 0811b7de6..d58770f45 100644 --- a/app/core/src/test/java/stirling/software/SPDF/controller/api/pipeline/PipelineProcessorTest.java +++ b/app/core/src/test/java/stirling/software/SPDF/controller/api/pipeline/PipelineProcessorTest.java @@ -4,6 +4,8 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; +import java.io.ByteArrayInputStream; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.List; @@ -13,12 +15,16 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; +import org.mockito.MockedConstruction; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.core.io.ByteArrayResource; import org.springframework.core.io.FileSystemResource; import org.springframework.core.io.Resource; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; +import org.springframework.http.client.ClientHttpResponse; +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.MultiValueMap; import jakarta.servlet.ServletContext; @@ -126,6 +132,90 @@ class PipelineProcessorTest { Files.deleteIfExists(tempPath); } + @Test + void sendWebRequestDoesNotForceContentType() throws Exception { + MultiValueMap body = new LinkedMultiValueMap<>(); + body.add( + "fileInput", + new ByteArrayResource("data".getBytes(StandardCharsets.UTF_8)) { + @Override + public String getFilename() { + return "input.pdf"; + } + }); + + Path tempPath = Files.createTempFile("pipeline-test", ".tmp"); + var tempFile = mock(stirling.software.common.util.TempFile.class); + when(tempFile.getPath()).thenReturn(tempPath); + when(tempFile.getFile()).thenReturn(tempPath.toFile()); + when(tempFileManager.createManagedTempFile("pipeline")).thenReturn(tempFile); + + var capturedHeaders = new org.springframework.http.HttpHeaders[1]; + + try (MockedConstruction ignored = + mockConstruction( + org.springframework.web.client.RestTemplate.class, + (mock, context) -> { + when(mock.httpEntityCallback(any(), eq(Resource.class))) + .thenAnswer( + invocation -> { + var entity = invocation.getArgument(0); + capturedHeaders[0] = + ((org.springframework.http.HttpEntity) + entity) + .getHeaders(); + return (org.springframework.web.client + .RequestCallback) + request -> {}; + }); + + when(mock.execute( + anyString(), + eq(org.springframework.http.HttpMethod.POST), + any(), + any())) + .thenAnswer( + invocation -> { + @SuppressWarnings("unchecked") + var extractor = + (org.springframework.web.client + .ResponseExtractor< + ResponseEntity>) + invocation.getArgument(3); + ClientHttpResponse response = + mock(ClientHttpResponse.class); + when(response.getBody()) + .thenReturn( + new ByteArrayInputStream( + "ok" + .getBytes( + StandardCharsets + .UTF_8))); + var headers = + new org.springframework.http.HttpHeaders(); + headers.add( + org.springframework.http.HttpHeaders + .CONTENT_DISPOSITION, + "attachment; filename=\"out.pdf\""); + when(response.getHeaders()).thenReturn(headers); + lenient() + .when(response.getStatusCode()) + .thenReturn(HttpStatus.OK); + return extractor.extractData(response); + }); + })) { + ResponseEntity response = + pipelineProcessor.sendWebRequest("http://localhost/api", body); + + assertNotNull(response); + assertEquals(HttpStatus.OK, response.getStatusCode()); + assertNotNull(response.getBody()); + assertNull(capturedHeaders[0].getContentType()); + } finally { + Files.deleteIfExists(tempPath); + } + } + private static class MyFileByteArrayResource extends ByteArrayResource { public MyFileByteArrayResource() { super("data".getBytes());