mirror of
https://github.com/Stirling-Tools/Stirling-PDF.git
synced 2026-09-03 05:10:16 +03:00
refactor(get-info): remove redundant PDF validation logic (#7213)
# Description of Changes Could not get past validation, since very few endpoint have such validation, i think redundant. Changes: * Removed the `validatePdfFile` method, which previously checked for file presence, size limits, and content type, from `GetInfoOnPDF.java`. * Deleted the invocation of `validatePdfFile` and its associated error handling from the `getPdfInfo` method, so uploaded files are no longer validated at this layer. <!-- Please provide a summary of the changes, including: - What was changed - Why the change was made - Any challenges encountered Closes #(issue_number) --> --- ## Checklist ### General - [X] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [X] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/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) - [X] I have performed a self-review of my own code - [X] 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) - [X] I have run `task check` to verify linters, typechecks, and tests pass - [X] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md#7-testing) for more details.
This commit is contained in:
-28
@@ -61,7 +61,6 @@ import stirling.software.common.model.api.PDFFile;
|
||||
import stirling.software.common.model.tool.ToolFormat;
|
||||
import stirling.software.common.model.tool.ToolIO;
|
||||
import stirling.software.common.service.CustomPDFDocumentFactory;
|
||||
import stirling.software.common.util.ExceptionUtils;
|
||||
import stirling.software.common.util.RegexPatternUtils;
|
||||
import stirling.software.common.util.WebResponseUtils;
|
||||
|
||||
@@ -270,25 +269,6 @@ public class GetInfoOnPDF {
|
||||
}
|
||||
}
|
||||
|
||||
private static void validatePdfFile(MultipartFile file) {
|
||||
if (file == null || file.isEmpty()) {
|
||||
throw new IllegalArgumentException("PDF file is required");
|
||||
}
|
||||
|
||||
if (file.getSize() > MAX_FILE_SIZE) {
|
||||
throw ExceptionUtils.createIllegalArgumentException(
|
||||
"error.fileSizeLimit",
|
||||
"File size ({0} bytes) exceeds maximum allowed size ({1} bytes)",
|
||||
file.getSize(),
|
||||
MAX_FILE_SIZE);
|
||||
}
|
||||
|
||||
String contentType = file.getContentType();
|
||||
if (contentType != null && !"application/pdf".equals(contentType)) {
|
||||
log.warn("File content type is {}, expected application/pdf", contentType);
|
||||
}
|
||||
}
|
||||
|
||||
private static ResponseEntity<byte[]> createErrorResponse(String errorMessage) {
|
||||
try {
|
||||
ObjectNode errorNode = objectMapper.createObjectNode();
|
||||
@@ -1104,14 +1084,6 @@ public class GetInfoOnPDF {
|
||||
public ResponseEntity<byte[]> getPdfInfo(@ModelAttribute PDFFile request) throws IOException {
|
||||
MultipartFile inputFile = request.getFileInput();
|
||||
|
||||
// Validate input
|
||||
try {
|
||||
validatePdfFile(inputFile);
|
||||
} catch (IllegalArgumentException e) {
|
||||
log.error("Invalid PDF file: {}", e.getMessage());
|
||||
return createErrorResponse("Invalid PDF file: " + e.getMessage());
|
||||
}
|
||||
|
||||
List<PDFVerificationResult> verificationResults = null;
|
||||
try {
|
||||
verificationResults = veraPDFService.validatePDF(inputFile.getInputStream());
|
||||
|
||||
-15
@@ -264,21 +264,6 @@ class GetInfoOnPDFMoreTest {
|
||||
@DisplayName("error handling")
|
||||
class Errors {
|
||||
|
||||
@Test
|
||||
@DisplayName("empty file input yields an error response")
|
||||
void emptyFile() throws Exception {
|
||||
MockMultipartFile mf =
|
||||
new MockMultipartFile("fileInput", "x.pdf", "application/pdf", new byte[0]);
|
||||
PDFFile request = new PDFFile();
|
||||
request.setFileInput(mf);
|
||||
ResponseEntity<byte[]> resp = getInfoOnPDF.getPdfInfo(request);
|
||||
// createErrorResponse returns HTTP 200 with a JSON body carrying an "error" field.
|
||||
assertThat(resp.getBody()).isNotNull();
|
||||
JsonNode body = om.readTree(resp.getBody());
|
||||
assertThat(body.has("error")).isTrue();
|
||||
assertThat(body.get("error").asText("")).contains("Invalid");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("veraPDF failure is swallowed and a report is still produced")
|
||||
void veraPdfFailureSwallowed() throws Exception {
|
||||
|
||||
-76
@@ -556,24 +556,6 @@ class GetInfoOnPDFTest {
|
||||
@DisplayName("Validation and Error Handling Tests")
|
||||
class ValidationErrorTests {
|
||||
|
||||
@Test
|
||||
@DisplayName("Should reject null file")
|
||||
void testValidation_NullFile() throws IOException {
|
||||
PDFFile request = new PDFFile();
|
||||
request.setFileInput(null);
|
||||
|
||||
ResponseEntity<byte[]> response = getInfoOnPDF.getPdfInfo(request);
|
||||
|
||||
Assertions.assertEquals(
|
||||
HttpStatus.OK, response.getStatusCode()); // Returns error JSON with 200
|
||||
String jsonResponse = new String(response.getBody(), StandardCharsets.UTF_8);
|
||||
JsonNode jsonNode = objectMapper.readTree(jsonResponse);
|
||||
|
||||
Assertions.assertTrue(jsonNode.has("error"));
|
||||
Assertions.assertTrue(
|
||||
jsonNode.get("error").asText("").contains("PDF file is required"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Should reject empty file")
|
||||
void testValidation_EmptyFile() throws IOException {
|
||||
@@ -591,64 +573,6 @@ class GetInfoOnPDFTest {
|
||||
|
||||
Assertions.assertTrue(jsonNode.has("error"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Should reject file that exceeds max size")
|
||||
void testValidation_TooLargeFile() throws IOException {
|
||||
MultipartFile largeFile =
|
||||
new MultipartFile() {
|
||||
@Override
|
||||
public String getName() {
|
||||
return "file";
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getOriginalFilename() {
|
||||
return "large.pdf";
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getContentType() {
|
||||
return MediaType.APPLICATION_PDF_VALUE;
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean isEmpty() {
|
||||
return false;
|
||||
}
|
||||
|
||||
@Override
|
||||
public long getSize() {
|
||||
// Report 101 MB without allocating memory
|
||||
return 101L * 1024L * 1024L;
|
||||
}
|
||||
|
||||
@Override
|
||||
public byte[] getBytes() {
|
||||
return new byte[0];
|
||||
}
|
||||
|
||||
@Override
|
||||
public java.io.InputStream getInputStream() {
|
||||
return java.io.InputStream.nullInputStream();
|
||||
}
|
||||
|
||||
@Override
|
||||
public void transferTo(java.io.File dest) throws IllegalStateException {}
|
||||
};
|
||||
|
||||
PDFFile request = new PDFFile();
|
||||
request.setFileInput(largeFile);
|
||||
|
||||
ResponseEntity<byte[]> response = getInfoOnPDF.getPdfInfo(request);
|
||||
|
||||
String jsonResponse = new String(response.getBody(), StandardCharsets.UTF_8);
|
||||
JsonNode jsonNode = objectMapper.readTree(jsonResponse);
|
||||
|
||||
Assertions.assertTrue(jsonNode.has("error"));
|
||||
Assertions.assertTrue(
|
||||
jsonNode.get("error").asText("").contains("exceeds maximum allowed size"));
|
||||
}
|
||||
}
|
||||
|
||||
@Nested
|
||||
|
||||
Reference in New Issue
Block a user