Ensure temporary files are properly cleaned up after metadata updates (#1226)

This commit is contained in:
Aditya Chandel
2025-09-28 01:18:18 -06:00
committed by GitHub
parent a2d6b98063
commit c23a626b7c
2 changed files with 98 additions and 41 deletions

View File

@@ -3,7 +3,11 @@ package com.adityachandel.booklore.service.metadata.writer;
import com.adityachandel.booklore.model.MetadataClearFlags;
import com.adityachandel.booklore.model.entity.BookMetadataEntity;
import com.adityachandel.booklore.model.enums.BookFileType;
import com.github.junrar.Archive;
import com.github.junrar.rarfile.FileHeader;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.compress.archivers.sevenz.SevenZArchiveEntry;
import org.apache.commons.compress.archivers.sevenz.SevenZFile;
import org.springframework.stereotype.Component;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
@@ -17,26 +21,19 @@ import javax.xml.transform.Transformer;
import javax.xml.transform.TransformerFactory;
import javax.xml.transform.dom.DOMSource;
import javax.xml.transform.stream.StreamResult;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.*;
import java.nio.file.Files;
import java.nio.file.LinkOption;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.nio.file.StandardOpenOption;
import java.time.LocalDate;
import java.util.Comparator;
import java.util.Enumeration;
import java.util.Locale;
import java.util.Set;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;
import java.util.zip.ZipOutputStream;
import com.github.junrar.Archive;
import com.github.junrar.rarfile.FileHeader;
import org.apache.commons.compress.archivers.sevenz.SevenZFile;
import org.apache.commons.compress.archivers.sevenz.SevenZArchiveEntry;
@Slf4j
@Component
@@ -45,6 +42,8 @@ public class CbxMetadataWriter implements MetadataWriter {
@Override
public void writeMetadataToFile(File file, BookMetadataEntity metadata, String thumbnailUrl, boolean restoreMode, MetadataClearFlags clearFlags) {
Path backup = null;
Path tempDir = null;
Path tempFile = null;
boolean writeSucceeded = false;
try {
// Create a backup next to the source file (temp name, safe to delete later)
@@ -82,7 +81,8 @@ public class CbxMetadataWriter implements MetadataWriter {
SevenZArchiveEntry existing = null;
for (SevenZArchiveEntry e : sevenZ.getEntries()) {
if (e != null && !e.isDirectory() && isComicInfoName(e.getName())) {
existing = e; break;
existing = e;
break;
}
}
if (existing != null) {
@@ -147,18 +147,19 @@ public class CbxMetadataWriter implements MetadataWriter {
// Repack depending on container type; always write to a temp target then atomic move
if (isCbz) {
Path temp = Files.createTempFile("cbx_edit", ".cbz");
repackZipReplacingComicInfo(file.toPath(), temp, xmlBytes);
atomicReplace(temp, file.toPath());
tempFile = Files.createTempFile("cbx_edit", ".cbz");
repackZipReplacingComicInfo(file.toPath(), tempFile, xmlBytes);
atomicReplace(tempFile, file.toPath());
tempFile = null; // Successfully moved, don't delete in finally
writeSucceeded = true;
return;
}
if (isCb7) {
// Convert to CBZ with updated ComicInfo.xml
Path tempZip = Files.createTempFile("cbx_edit", ".cbz");
tempFile = Files.createTempFile("cbx_edit", ".cbz");
try (SevenZFile sevenZ = new SevenZFile(file);
ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(tempZip))) {
ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(tempFile))) {
for (SevenZArchiveEntry e : sevenZ.getEntries()) {
if (e.isDirectory()) continue;
String entryName = e.getName();
@@ -178,8 +179,12 @@ public class CbxMetadataWriter implements MetadataWriter {
zos.closeEntry();
}
Path target = file.toPath().resolveSibling(stripExtension(file.getName()) + ".cbz");
atomicReplace(tempZip, target);
try { Files.deleteIfExists(file.toPath()); } catch (Exception ignored) {}
atomicReplace(tempFile, target);
tempFile = null; // Successfully moved, don't delete in finally
try {
Files.deleteIfExists(file.toPath());
} catch (Exception ignored) {
}
writeSucceeded = true;
return;
}
@@ -189,7 +194,7 @@ public class CbxMetadataWriter implements MetadataWriter {
boolean rarAvailable = isRarAvailable(rarBin);
if (rarAvailable) {
Path tempDir = Files.createTempDirectory("cbx_rar_");
tempDir = Files.createTempDirectory("cbx_rar_");
try {
// Extract entire RAR into a temp directory
try (Archive archive = new Archive(file)) {
@@ -234,20 +239,16 @@ public class CbxMetadataWriter implements MetadataWriter {
log.warn("RAR creation failed with exit code {}. Falling back to CBZ conversion for {}", code, file.getName());
}
} finally {
try { // cleanup temp dir
java.nio.file.Files.walk(tempDir)
.sorted(java.util.Comparator.reverseOrder())
.forEach(path -> { try { Files.deleteIfExists(path); } catch (Exception ignore) {} });
} catch (Exception ignore) {}
// tempDir cleanup will be handled in outer finally block
}
} else {
log.warn("`rar` binary not found. Falling back to CBZ conversion for {}", file.getName());
}
// Fallback: convert the CBR to CBZ containing updated ComicInfo.xml
Path tempZip = Files.createTempFile("cbx_edit", ".cbz");
tempFile = Files.createTempFile("cbx_edit", ".cbz");
try (Archive archive = new Archive(file);
ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(tempZip))) {
ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(tempFile))) {
for (FileHeader fh : archive.getFileHeaders()) {
if (fh.isDirectory()) continue;
String entryName = fh.getFileName();
@@ -265,8 +266,12 @@ public class CbxMetadataWriter implements MetadataWriter {
zos.closeEntry();
}
Path target = file.toPath().resolveSibling(stripExtension(file.getName()) + ".cbz");
atomicReplace(tempZip, target);
try { Files.deleteIfExists(file.toPath()); } catch (Exception ignored) {}
atomicReplace(tempFile, target);
tempFile = null; // Successfully moved, don't delete in finally
try {
Files.deleteIfExists(file.toPath());
} catch (Exception ignored) {
}
writeSucceeded = true;
} catch (Exception e) {
// Attempt to restore the original file from backup
@@ -280,8 +285,27 @@ public class CbxMetadataWriter implements MetadataWriter {
}
log.warn("Failed to write metadata for {}: {}", file.getName(), e.getMessage(), e);
} finally {
// Clean up temporary file if it wasn't successfully moved
if (tempFile != null) {
try {
Files.deleteIfExists(tempFile);
} catch (Exception e) {
log.warn("Failed to delete temp file: {}", tempFile, e);
}
}
// Clean up temporary directory if it was created
if (tempDir != null) {
deleteDirectoryRecursively(tempDir);
}
// Clean up backup file if write succeeded
if (writeSucceeded && backup != null) {
try { Files.deleteIfExists(backup); } catch (Exception ignore) {}
try {
Files.deleteIfExists(backup);
} catch (Exception e) {
log.warn("Failed to delete backup file: {}", backup, e);
}
}
}
}
@@ -451,4 +475,20 @@ public class CbxMetadataWriter implements MetadataWriter {
public BookFileType getSupportedBookType() {
return BookFileType.CBX;
}
private void deleteDirectoryRecursively(Path dir) {
try (var pathStream = Files.walk(dir)) {
pathStream
.sorted(Comparator.reverseOrder())
.forEach(path -> {
try {
Files.delete(path);
} catch (IOException e) {
log.warn("Failed to delete temp file/directory: {}", path, e);
}
});
} catch (IOException e) {
log.warn("Failed to clean up temporary directory: {}", dir, e);
}
}
}

View File

@@ -32,6 +32,7 @@ import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.util.Comparator;
import java.util.List;
import java.util.Objects;
import java.util.UUID;
@@ -51,7 +52,7 @@ public class EpubMetadataWriter implements MetadataWriter {
log.warn("Failed to create backup of EPUB {}: {}", epubFile.getName(), ex.getMessage());
return;
}
Path tempDir;
Path tempDir = null;
try {
tempDir = Files.createTempDirectory("epub_edit_" + UUID.randomUUID());
ZipFile zipFile = new ZipFile(epubFile);
@@ -190,6 +191,9 @@ public class EpubMetadataWriter implements MetadataWriter {
}
}
} finally {
if (tempDir != null) {
deleteDirectoryRecursively(tempDir);
}
if (backupFile.exists()) {
try {
Files.delete(backupFile.toPath());
@@ -252,9 +256,10 @@ public class EpubMetadataWriter implements MetadataWriter {
return;
}
Path tempDir = null;
try {
File epubFile = new File(bookEntity.getFullFilePath().toUri());
Path tempDir = Files.createTempDirectory("epub_cover_" + UUID.randomUUID());
tempDir = Files.createTempDirectory("epub_cover_" + UUID.randomUUID());
new ZipFile(epubFile).extractAll(tempDir.toString());
File opfFile = findOpfFile(tempDir.toFile());
@@ -286,6 +291,10 @@ public class EpubMetadataWriter implements MetadataWriter {
} catch (Exception e) {
log.warn("Failed to update EPUB with uploaded cover image: {}", e.getMessage(), e);
} finally {
if (tempDir != null) {
deleteDirectoryRecursively(tempDir);
}
}
}
@@ -295,9 +304,10 @@ public class EpubMetadataWriter implements MetadataWriter {
log.warn("Cover update via URL failed: empty or null URL.");
return;
}
Path tempDir = null;
try {
File epubFile = new File(bookEntity.getFullFilePath().toUri());
Path tempDir = Files.createTempDirectory("epub_cover_url_" + UUID.randomUUID());
tempDir = Files.createTempDirectory("epub_cover_url_" + UUID.randomUUID());
new ZipFile(epubFile).extractAll(tempDir.toString());
File opfFile = findOpfFile(tempDir.toFile());
@@ -333,6 +343,10 @@ public class EpubMetadataWriter implements MetadataWriter {
log.info("Cover image updated in EPUB via URL: {}", epubFile.getName());
} catch (Exception e) {
log.warn("Failed to update EPUB with cover from URL: {}", e.getMessage(), e);
} finally {
if (tempDir != null) {
deleteDirectoryRecursively(tempDir);
}
}
}
@@ -524,16 +538,19 @@ public class EpubMetadataWriter implements MetadataWriter {
return null;
}
private String getIdentifierByScheme(Element metadataElement, String scheme) {
NodeList identifiers = metadataElement.getElementsByTagNameNS("*", "identifier");
for (int i = 0; i < identifiers.getLength(); i++) {
Element idElement = (Element) identifiers.item(i);
String schemeAttr = idElement.getAttributeNS(OPF_NS, "scheme");
if (scheme.equalsIgnoreCase(schemeAttr)) {
return idElement.getTextContent();
}
private void deleteDirectoryRecursively(Path dir) {
try (var pathStream = Files.walk(dir)) {
pathStream
.sorted(Comparator.reverseOrder())
.forEach(path -> {
try {
Files.delete(path);
} catch (IOException e) {
log.warn("Failed to delete temp file/directory: {}", path, e);
}
});
} catch (IOException e) {
log.warn("Failed to clean up temporary directory: {}", dir, e);
}
return null;
}
}