mirror of
https://github.com/Frooodle/Stirling-PDF.git
synced 2026-02-17 13:52:14 +01:00
fix(pipeline): avoid bad multipart by letting RestTemplate set boundary (#5522)
# 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.
This commit is contained in:
@@ -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<MultiValueMap<String, Object>> 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");
|
||||
|
||||
@@ -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<String, Object> 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<org.springframework.web.client.RestTemplate> 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<Resource>>)
|
||||
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<Resource> 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());
|
||||
|
||||
Reference in New Issue
Block a user