diff --git a/src/main/java/com/clearfolio/viewer/artifact/FileSystemArtifactStore.java b/src/main/java/com/clearfolio/viewer/artifact/FileSystemArtifactStore.java index 7e756fa7a..1ee598fd5 100644 --- a/src/main/java/com/clearfolio/viewer/artifact/FileSystemArtifactStore.java +++ b/src/main/java/com/clearfolio/viewer/artifact/FileSystemArtifactStore.java @@ -65,11 +65,15 @@ public FileSystemArtifactStore(Path rootDir) { */ @Override public void putPdf(UUID docId, byte[] pdfBytes) { + requireDocId(docId); byte[] copy = pdfBytes.clone(); + byte[] metadata = metadataBytes(docId, copy); + ArtifactFilesSnapshot previous = snapshotExistingFiles(docId); try { bytesWriter.write(pdfPath(docId), copy); - bytesWriter.write(metadataPath(docId), metadataBytes(docId, copy)); + bytesWriter.write(metadataPath(docId), metadata); } catch (IOException ex) { + rollbackPartialWrite(docId, previous, ex); throw new IllegalStateException("failed to persist artifact for docId " + docId, ex); } cache.put(docId, copy); @@ -80,6 +84,7 @@ public void putPdf(UUID docId, byte[] pdfBytes) { */ @Override public Optional getPdf(UUID docId) { + requireDocId(docId); byte[] cached = cache.get(docId); if (cached != null) { return Optional.of(cached.clone()); @@ -102,6 +107,7 @@ public Optional getPdf(UUID docId) { */ @Override public void deletePdf(UUID docId) { + requireDocId(docId); try { Files.deleteIfExists(pdfPath(docId)); Files.deleteIfExists(metadataPath(docId)); @@ -112,12 +118,64 @@ public void deletePdf(UUID docId) { } } + private ArtifactFilesSnapshot snapshotExistingFiles(UUID docId) { + try { + return new ArtifactFilesSnapshot( + existingFileBytes(pdfPath(docId)), + existingFileBytes(metadataPath(docId)) + ); + } catch (IOException ex) { + throw new IllegalStateException("failed to snapshot existing artifact for docId " + docId, ex); + } + } + + private static byte[] existingFileBytes(Path path) throws IOException { + return Files.exists(path) ? Files.readAllBytes(path) : null; + } + + private void rollbackPartialWrite( + UUID docId, + ArtifactFilesSnapshot previous, + IOException writeFailure + ) { + restorePartialFile(pdfPath(docId), previous.pdfBytes(), writeFailure); + restorePartialFile(metadataPath(docId), previous.metadataBytes(), writeFailure); + } + + private static void restorePartialFile(Path path, byte[] previousBytes, IOException writeFailure) { + try { + if (previousBytes == null) { + Files.deleteIfExists(path); + } else { + Files.write(path, previousBytes); + } + } catch (IOException rollbackFailure) { + writeFailure.addSuppressed(rollbackFailure); + } + } + private Path pdfPath(UUID docId) { - return rootDir.resolve(docId + PDF_SUFFIX); + return resolveArtifactPath(rootDir, docId, PDF_SUFFIX); } private Path metadataPath(UUID docId) { - return rootDir.resolve(docId + METADATA_SUFFIX); + return resolveArtifactPath(rootDir, docId, METADATA_SUFFIX); + } + + static Path resolveArtifactPath(Path configuredRoot, UUID docId, String suffix) { + requireDocId(docId); + Path normalizedRoot = configuredRoot.toAbsolutePath().normalize(); + Path candidate = normalizedRoot.resolve(docId + suffix).normalize(); + if (!candidate.startsWith(normalizedRoot)) { + throw new IllegalArgumentException("artifact path must remain within configured root"); + } + return candidate; + } + + private static void requireDocId(UUID docId) { + if (docId == null) { + throw new IllegalArgumentException("docId is required"); + } } private static byte[] metadataBytes(UUID docId, byte[] pdfBytes) { @@ -137,4 +195,7 @@ private static String sha256Hex(final byte[] bytes) { throw new IllegalStateException("SHA-256 digest unavailable", ex); } } + + private record ArtifactFilesSnapshot(byte[] pdfBytes, byte[] metadataBytes) { + } } diff --git a/src/test/java/com/clearfolio/viewer/artifact/FileSystemArtifactStorePartialWriteTest.java b/src/test/java/com/clearfolio/viewer/artifact/FileSystemArtifactStorePartialWriteTest.java new file mode 100644 index 000000000..eea10e01b --- /dev/null +++ b/src/test/java/com/clearfolio/viewer/artifact/FileSystemArtifactStorePartialWriteTest.java @@ -0,0 +1,198 @@ +package com.clearfolio.viewer.artifact; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Optional; +import java.util.UUID; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Verifies that a failed multi-file artifact write does not publish orphaned + * PDF bytes without the matching metadata sidecar. + */ +class FileSystemArtifactStorePartialWriteTest { + + @TempDir + Path temporaryDirectory; + + @Test + void metadataWriteFailureRollsBackNewArtifactBytes() throws Exception { + Path root = temporaryDirectory.resolve("artifacts"); + UUID docId = UUID.randomUUID(); + FileSystemArtifactStore store = new FileSystemArtifactStore( + root, + (path, bytes) -> { + if (path.getFileName().toString().endsWith(".meta.properties")) { + throw new IOException("metadata disk full"); + } + Files.write(path, bytes); + }, + Files::readAllBytes + ); + + IllegalStateException error = assertThrows( + IllegalStateException.class, + () -> store.putPdf(docId, "%PDF-1.7\npartial-write".getBytes(StandardCharsets.UTF_8)) + ); + + assertEquals("failed to persist artifact for docId " + docId, error.getMessage()); + assertTrue(Files.notExists(root.resolve(docId + ".pdf"))); + assertTrue(Files.notExists(root.resolve(docId + ".meta.properties"))); + assertEquals(Optional.empty(), store.getPdf(docId)); + } + + @Test + void failedReplacementPreservesPreviouslyCommittedArtifactAcrossRestart() throws Exception { + Path root = temporaryDirectory.resolve("replacement"); + UUID docId = UUID.randomUUID(); + byte[] originalPdf = "%PDF-1.7\noriginal".getBytes(StandardCharsets.UTF_8); + byte[] replacementPdf = "%PDF-1.7\nreplacement".getBytes(StandardCharsets.UTF_8); + new FileSystemArtifactStore(root).putPdf(docId, originalPdf); + byte[] originalMetadata = Files.readAllBytes(root.resolve(docId + ".meta.properties")); + + FileSystemArtifactStore failingStore = new FileSystemArtifactStore( + root, + (path, bytes) -> { + if (path.getFileName().toString().endsWith(".meta.properties")) { + throw new IOException("metadata disk full"); + } + Files.write(path, bytes); + }, + Files::readAllBytes + ); + + assertThrows(IllegalStateException.class, () -> failingStore.putPdf(docId, replacementPdf)); + + assertTrue(Files.exists(root.resolve(docId + ".pdf"))); + assertTrue(Files.exists(root.resolve(docId + ".meta.properties"))); + assertArrayEquals(originalMetadata, Files.readAllBytes(root.resolve(docId + ".meta.properties"))); + FileSystemArtifactStore restartedStore = new FileSystemArtifactStore(root); + assertArrayEquals(originalPdf, restartedStore.getPdf(docId).orElseThrow()); + } + + @Test + void rollbackFailureIsRetainedAsSuppressedEvidence() throws Exception { + Path root = temporaryDirectory.resolve("rollback-failure"); + UUID docId = UUID.randomUUID(); + FileSystemArtifactStore store = new FileSystemArtifactStore( + root, + (path, bytes) -> { + if (path.getFileName().toString().endsWith(".pdf")) { + Files.createDirectory(path); + Files.writeString(path.resolve("retained"), "evidence"); + return; + } + throw new IOException("metadata disk full"); + }, + Files::readAllBytes + ); + + IllegalStateException error = assertThrows( + IllegalStateException.class, + () -> store.putPdf(docId, "%PDF-1.7\nrollback-failure".getBytes(StandardCharsets.UTF_8)) + ); + + assertEquals("failed to persist artifact for docId " + docId, error.getMessage()); + assertEquals(1, error.getCause().getSuppressed().length); + assertTrue(Files.exists(root.resolve(docId + ".pdf").resolve("retained"))); + } + + @Test + void metadataRollbackFailureStillAttemptsPdfCleanup() throws Exception { + Path root = temporaryDirectory.resolve("independent-rollback"); + UUID docId = UUID.randomUUID(); + Path pdfPath = root.resolve(docId + ".pdf"); + Path metadataPath = root.resolve(docId + ".meta.properties"); + FileSystemArtifactStore store = new FileSystemArtifactStore( + root, + (path, bytes) -> { + if (path.equals(pdfPath)) { + Files.write(path, bytes); + return; + } + Files.createDirectory(metadataPath); + Files.writeString(metadataPath.resolve("retained"), "evidence"); + throw new IOException("metadata disk full"); + }, + Files::readAllBytes + ); + + IllegalStateException error = assertThrows( + IllegalStateException.class, + () -> store.putPdf(docId, "%PDF-1.7\nindependent-rollback".getBytes(StandardCharsets.UTF_8)) + ); + + assertEquals(1, error.getCause().getSuppressed().length); + assertTrue(Files.exists(metadataPath.resolve("retained"))); + assertTrue(Files.notExists(pdfPath)); + } + + @Test + void unreadableExistingArtifactSnapshotFailsClosedBeforeReplacement() throws Exception { + Path root = temporaryDirectory.resolve("snapshot-failure"); + Files.createDirectories(root); + UUID docId = UUID.randomUUID(); + Path pdfPath = root.resolve(docId + ".pdf"); + Files.createDirectory(pdfPath); + Files.writeString(pdfPath.resolve("retained"), "evidence"); + FileSystemArtifactStore store = new FileSystemArtifactStore(root); + + IllegalStateException error = assertThrows( + IllegalStateException.class, + () -> store.putPdf(docId, "%PDF-1.7\nreplacement".getBytes(StandardCharsets.UTF_8)) + ); + + assertEquals("failed to snapshot existing artifact for docId " + docId, error.getMessage()); + assertTrue(Files.isDirectory(pdfPath)); + assertTrue(Files.exists(pdfPath.resolve("retained"))); + assertTrue(Files.notExists(root.resolve(docId + ".meta.properties"))); + } + + @Test + void nullDocumentIdentityFailsClosedBeforeFilesystemAccess() { + Path root = temporaryDirectory.resolve("null-authority"); + FileSystemArtifactStore store = new FileSystemArtifactStore(root); + byte[] pdf = "%PDF-1.7\nnull-authority".getBytes(StandardCharsets.UTF_8); + + IllegalArgumentException putError = assertThrows( + IllegalArgumentException.class, + () -> store.putPdf(null, pdf) + ); + IllegalArgumentException getError = assertThrows( + IllegalArgumentException.class, + () -> store.getPdf(null) + ); + IllegalArgumentException deleteError = assertThrows( + IllegalArgumentException.class, + () -> store.deletePdf(null) + ); + + assertEquals("docId is required", putError.getMessage()); + assertEquals("docId is required", getError.getMessage()); + assertEquals("docId is required", deleteError.getMessage()); + assertTrue(Files.notExists(root.resolve("null.pdf"))); + assertTrue(Files.notExists(root.resolve("null.meta.properties"))); + } + + @Test + void artifactPathResolutionRejectsEscapesFromConfiguredRoot() { + Path root = temporaryDirectory.resolve("path-confinement"); + UUID docId = UUID.randomUUID(); + + IllegalArgumentException error = assertThrows( + IllegalArgumentException.class, + () -> FileSystemArtifactStore.resolveArtifactPath(root, docId, "/../../outside") + ); + + assertEquals("artifact path must remain within configured root", error.getMessage()); + } +}