fix(backend): move IOException into service, add content-type whitelist to attachFile
- DocumentService.attachFile() now catches IOException internally and re-throws as DomainException.internal — the IOException no longer leaks through the service boundary - DocumentController.attachFile() is now a plain delegate (no try/catch) - ALLOWED_CONTENT_TYPES whitelist (PDF/JPEG/PNG/TIFF) is now enforced on the attachFile endpoint, matching the existing quick-upload validation - Added 5 DocumentService unit tests for attachFile (notFound, status transition PLACEHOLDER→UPLOADED, no-change when already UPLOADED, field assignment from upload result, IOException→DomainException) - Added controller tests: 400 on disallowed content type, 404 on missing doc Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -131,23 +131,23 @@ public class DocumentController {
|
|||||||
|
|
||||||
// --- ATTACH FILE ---
|
// --- ATTACH FILE ---
|
||||||
|
|
||||||
|
private static final Set<String> ALLOWED_CONTENT_TYPES = Set.of(
|
||||||
|
"application/pdf", "image/jpeg", "image/png", "image/tiff");
|
||||||
|
|
||||||
@PostMapping(value = "/{id}/file", consumes = MediaType.MULTIPART_FORM_DATA_VALUE)
|
@PostMapping(value = "/{id}/file", consumes = MediaType.MULTIPART_FORM_DATA_VALUE)
|
||||||
@RequirePermission(Permission.WRITE_ALL)
|
@RequirePermission(Permission.WRITE_ALL)
|
||||||
public Document attachFile(
|
public Document attachFile(
|
||||||
@PathVariable UUID id,
|
@PathVariable UUID id,
|
||||||
@RequestPart("file") MultipartFile file) {
|
@RequestPart("file") MultipartFile file) {
|
||||||
try {
|
String contentType = file.getContentType();
|
||||||
return documentService.attachFile(id, file);
|
if (contentType == null || !ALLOWED_CONTENT_TYPES.contains(contentType)) {
|
||||||
} catch (IOException e) {
|
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Unsupported file type: " + contentType);
|
||||||
throw DomainException.internal(ErrorCode.FILE_UPLOAD_FAILED, "Failed to upload file: " + e.getMessage());
|
|
||||||
}
|
}
|
||||||
|
return documentService.attachFile(id, file);
|
||||||
}
|
}
|
||||||
|
|
||||||
// --- QUICK UPLOAD ---
|
// --- QUICK UPLOAD ---
|
||||||
|
|
||||||
private static final Set<String> ALLOWED_CONTENT_TYPES = Set.of(
|
|
||||||
"application/pdf", "image/jpeg", "image/png", "image/tiff");
|
|
||||||
|
|
||||||
public record UploadError(String filename, String code) {}
|
public record UploadError(String filename, String code) {}
|
||||||
public record QuickUploadResult(List<Document> created, List<Document> updated, List<UploadError> errors) {}
|
public record QuickUploadResult(List<Document> created, List<Document> updated, List<UploadError> errors) {}
|
||||||
|
|
||||||
|
|||||||
@@ -287,10 +287,15 @@ public class DocumentService {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Transactional
|
@Transactional
|
||||||
public Document attachFile(UUID id, MultipartFile file) throws IOException {
|
public Document attachFile(UUID id, MultipartFile file) {
|
||||||
Document doc = documentRepository.findById(id)
|
Document doc = documentRepository.findById(id)
|
||||||
.orElseThrow(() -> DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + id));
|
.orElseThrow(() -> DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + id));
|
||||||
FileService.UploadResult upload = fileService.uploadFile(file, file.getOriginalFilename());
|
FileService.UploadResult upload;
|
||||||
|
try {
|
||||||
|
upload = fileService.uploadFile(file, file.getOriginalFilename());
|
||||||
|
} catch (IOException e) {
|
||||||
|
throw DomainException.internal(ErrorCode.FILE_UPLOAD_FAILED, "Failed to upload file: " + e.getMessage());
|
||||||
|
}
|
||||||
doc.setFilePath(upload.s3Key());
|
doc.setFilePath(upload.s3Key());
|
||||||
doc.setFileHash(upload.fileHash());
|
doc.setFileHash(upload.fileHash());
|
||||||
doc.setOriginalFilename(file.getOriginalFilename());
|
doc.setOriginalFilename(file.getOriginalFilename());
|
||||||
|
|||||||
@@ -4,6 +4,8 @@ import org.junit.jupiter.api.Test;
|
|||||||
import org.raddatz.familienarchiv.dto.DocumentSearchResult;
|
import org.raddatz.familienarchiv.dto.DocumentSearchResult;
|
||||||
import org.raddatz.familienarchiv.dto.DocumentVersionSummary;
|
import org.raddatz.familienarchiv.dto.DocumentVersionSummary;
|
||||||
import org.raddatz.familienarchiv.dto.IncompleteDocumentDTO;
|
import org.raddatz.familienarchiv.dto.IncompleteDocumentDTO;
|
||||||
|
import org.raddatz.familienarchiv.exception.DomainException;
|
||||||
|
import org.raddatz.familienarchiv.exception.ErrorCode;
|
||||||
import org.raddatz.familienarchiv.model.Document;
|
import org.raddatz.familienarchiv.model.Document;
|
||||||
import org.raddatz.familienarchiv.model.DocumentStatus;
|
import org.raddatz.familienarchiv.model.DocumentStatus;
|
||||||
import org.raddatz.familienarchiv.model.DocumentVersion;
|
import org.raddatz.familienarchiv.model.DocumentVersion;
|
||||||
@@ -593,6 +595,33 @@ class DocumentControllerTest {
|
|||||||
.andExpect(jsonPath("$.status").value("UPLOADED"));
|
.andExpect(jsonPath("$.status").value("UPLOADED"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@WithMockUser(authorities = "WRITE_ALL")
|
||||||
|
void attachFile_returns400_whenContentTypeIsNotWhitelisted() throws Exception {
|
||||||
|
org.springframework.mock.web.MockMultipartFile htmlFile =
|
||||||
|
new org.springframework.mock.web.MockMultipartFile(
|
||||||
|
"file", "evil.html", "text/html", "<script>alert(1)</script>".getBytes());
|
||||||
|
|
||||||
|
mockMvc.perform(multipart("/api/documents/" + UUID.randomUUID() + "/file").file(htmlFile))
|
||||||
|
.andExpect(status().isBadRequest());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@WithMockUser(authorities = "WRITE_ALL")
|
||||||
|
void attachFile_returns404_whenDocumentDoesNotExist() throws Exception {
|
||||||
|
UUID id = UUID.randomUUID();
|
||||||
|
when(documentService.attachFile(eq(id), any()))
|
||||||
|
.thenThrow(DomainException.notFound(
|
||||||
|
ErrorCode.DOCUMENT_NOT_FOUND,
|
||||||
|
"Document not found: " + id));
|
||||||
|
|
||||||
|
org.springframework.mock.web.MockMultipartFile file =
|
||||||
|
new org.springframework.mock.web.MockMultipartFile("file", "brief.pdf", "application/pdf", new byte[]{1});
|
||||||
|
|
||||||
|
mockMvc.perform(multipart("/api/documents/" + id + "/file").file(file))
|
||||||
|
.andExpect(status().isNotFound());
|
||||||
|
}
|
||||||
|
|
||||||
// ─── GET /api/documents/{id}/versions/{versionId} ────────────────────────
|
// ─── GET /api/documents/{id}/versions/{versionId} ────────────────────────
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
@@ -1428,4 +1428,75 @@ class DocumentServiceTest {
|
|||||||
new MatchOffset(10, 4) // "Welt"
|
new MatchOffset(10, 4) // "Welt"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ─── attachFile ───────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void attachFile_throwsNotFound_whenDocumentDoesNotExist() {
|
||||||
|
UUID id = UUID.randomUUID();
|
||||||
|
when(documentRepository.findById(id)).thenReturn(Optional.empty());
|
||||||
|
|
||||||
|
MockMultipartFile file = new MockMultipartFile("file", "brief.pdf", "application/pdf", new byte[]{1});
|
||||||
|
assertThatThrownBy(() -> documentService.attachFile(id, file))
|
||||||
|
.isInstanceOf(DomainException.class)
|
||||||
|
.hasMessageContaining(id.toString());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void attachFile_setsStatusToUploaded_whenDocumentIsPlaceholder() throws Exception {
|
||||||
|
UUID id = UUID.randomUUID();
|
||||||
|
Document placeholder = Document.builder().id(id).status(DocumentStatus.PLACEHOLDER).build();
|
||||||
|
when(documentRepository.findById(id)).thenReturn(Optional.of(placeholder));
|
||||||
|
when(fileService.uploadFile(any(), any())).thenReturn(new FileService.UploadResult("s3/key.pdf", "abc123"));
|
||||||
|
when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
||||||
|
|
||||||
|
MockMultipartFile file = new MockMultipartFile("file", "brief.pdf", "application/pdf", new byte[]{1});
|
||||||
|
Document result = documentService.attachFile(id, file);
|
||||||
|
|
||||||
|
assertThat(result.getStatus()).isEqualTo(DocumentStatus.UPLOADED);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void attachFile_doesNotChangeStatus_whenAlreadyUploaded() throws Exception {
|
||||||
|
UUID id = UUID.randomUUID();
|
||||||
|
Document uploaded = Document.builder().id(id).status(DocumentStatus.UPLOADED).build();
|
||||||
|
when(documentRepository.findById(id)).thenReturn(Optional.of(uploaded));
|
||||||
|
when(fileService.uploadFile(any(), any())).thenReturn(new FileService.UploadResult("s3/key.pdf", "abc123"));
|
||||||
|
when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
||||||
|
|
||||||
|
MockMultipartFile file = new MockMultipartFile("file", "brief.pdf", "application/pdf", new byte[]{1});
|
||||||
|
Document result = documentService.attachFile(id, file);
|
||||||
|
|
||||||
|
assertThat(result.getStatus()).isEqualTo(DocumentStatus.UPLOADED);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void attachFile_setsFilePathAndContentType_fromUploadResult() throws Exception {
|
||||||
|
UUID id = UUID.randomUUID();
|
||||||
|
Document doc = Document.builder().id(id).status(DocumentStatus.PLACEHOLDER).build();
|
||||||
|
when(documentRepository.findById(id)).thenReturn(Optional.of(doc));
|
||||||
|
when(fileService.uploadFile(any(), any()))
|
||||||
|
.thenReturn(new FileService.UploadResult("s3/brief.pdf", "deadbeef"));
|
||||||
|
when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
||||||
|
|
||||||
|
MockMultipartFile file = new MockMultipartFile("file", "brief.pdf", "application/pdf", new byte[]{1});
|
||||||
|
Document result = documentService.attachFile(id, file);
|
||||||
|
|
||||||
|
assertThat(result.getFilePath()).isEqualTo("s3/brief.pdf");
|
||||||
|
assertThat(result.getFileHash()).isEqualTo("deadbeef");
|
||||||
|
assertThat(result.getContentType()).isEqualTo("application/pdf");
|
||||||
|
assertThat(result.getOriginalFilename()).isEqualTo("brief.pdf");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void attachFile_throwsDomainException_whenFileUploadFails() throws Exception {
|
||||||
|
UUID id = UUID.randomUUID();
|
||||||
|
Document doc = Document.builder().id(id).status(DocumentStatus.PLACEHOLDER).build();
|
||||||
|
when(documentRepository.findById(id)).thenReturn(Optional.of(doc));
|
||||||
|
when(fileService.uploadFile(any(), any())).thenThrow(new java.io.IOException("storage unavailable"));
|
||||||
|
|
||||||
|
MockMultipartFile file = new MockMultipartFile("file", "brief.pdf", "application/pdf", new byte[]{1});
|
||||||
|
assertThatThrownBy(() -> documentService.attachFile(id, file))
|
||||||
|
.isInstanceOf(DomainException.class);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user