fix: harden uploads and CBR archive handling

Handle existing upload folder paths without creating duplicate books, tolerate partial RAR 1.5 archive reads, and clean up related production defaults and dashboard type safety.
This commit is contained in:
neon
2026-04-24 08:25:38 -06:00
parent 46208fc82d
commit ac1b2cedb4
10 changed files with 175 additions and 34 deletions
+2 -2
View File
@@ -1,5 +1,5 @@
# Immutable image reference built by CI (use a sha-* tag or @sha256 digest) # Docker image to run. Pin a sha-* tag or @sha256 digest for stricter deploys.
APP_IMAGE=ghcr.io/your-org/bookorbit:sha-<git-sha> APP_IMAGE=ghcr.io/neonsolstice/bookorbit:latest
# App publish port on host # App publish port on host
APP_PORT=3000 APP_PORT=3000
+2 -2
View File
@@ -228,13 +228,13 @@ Production uses the root compose file and env file:
- Compose: `docker-compose.yml` - Compose: `docker-compose.yml`
- Env file: `.env` (from `.env.example`) - Env file: `.env` (from `.env.example`)
- Image source: immutable images built in CI (`.github/workflows/container-image.yml`) - Image source: CI-published images (`latest`, sha tags, or digests)
Quick start: Quick start:
```bash ```bash
cp .env.example .env cp .env.example .env
# edit .env and set APP_IMAGE, secrets, APP_URL, and BOOKS_HOST_PATH # edit .env and set secrets, APP_URL, BOOKS_HOST_PATH, and optionally APP_IMAGE
pnpm prod:up pnpm prod:up
``` ```
+1 -1
View File
@@ -187,7 +187,7 @@ onUnmounted(() => stopLibraryUploadListener())
<!-- Dashboard / Book Dock / Authors --> <!-- Dashboard / Book Dock / Authors -->
<SidebarGroup> <SidebarGroup>
<SidebarGroupContent> <SidebarGroupContent>
<SidebarMenu> <SidebarMenu class="gap-0">
<SidebarNavItem <SidebarNavItem
:is-active="isDashboardActive" :is-active="isDashboardActive"
tooltip="Dashboard" tooltip="Dashboard"
@@ -14,20 +14,24 @@ const editing = ref(false)
const goalInput = ref('') const goalInput = ref('')
const saving = ref(false) const saving = ref(false)
const hasGoal = computed(() => data.value?.goalBooks != null && data.value.goalBooks > 0) const goalBooks = computed(() => data.value?.goalBooks ?? null)
const hasGoal = computed(() => goalBooks.value != null && goalBooks.value > 0)
const percentage = computed(() => { const percentage = computed(() => {
if (!data.value || !hasGoal.value) return 0 const goal = goalBooks.value
return Math.min(100, Math.round((data.value.completedBooks / data.value.goalBooks!) * 100)) if (!data.value || goal == null || goal <= 0) return 0
return Math.min(100, Math.round((data.value.completedBooks / goal) * 100))
}) })
const option = shallowRef({}) const option = shallowRef({})
watchEffect(() => { watchEffect(() => {
if (!data.value || !hasGoal.value) { if (!data.value || !hasGoal.value || goalBooks.value == null) {
option.value = {} option.value = {}
return return
} }
const goal = goalBooks.value
option.value = { option.value = {
tooltip: { show: false }, tooltip: { show: false },
series: [ series: [
@@ -43,9 +47,9 @@ watchEffect(() => {
label: { show: false }, label: { show: false },
labelLine: { show: false }, labelLine: { show: false },
data: [ data: [
{ value: Math.min(data.value.completedBooks, data.value.goalBooks), name: 'Completed' }, { value: Math.min(data.value.completedBooks, goal), name: 'Completed' },
{ {
value: Math.max(data.value.goalBooks - data.value.completedBooks, 0), value: Math.max(goal - data.value.completedBooks, 0),
name: 'Remaining', name: 'Remaining',
itemStyle: { color: readCssColor('--muted') }, itemStyle: { color: readCssColor('--muted') },
}, },
+11 -2
View File
@@ -106,8 +106,17 @@ BEGIN
) )
ON CONFLICT (user_id, book_id) DO UPDATE ON CONFLICT (user_id, book_id) DO UPDATE
SET status = 'read', SET status = 'read',
finished_at = COALESCE(user_book_status.finished_at, started_at = LEAST(
date_trunc('year', current_date) + (i * 14 * interval '1 day')), COALESCE(user_book_status.started_at, EXCLUDED.started_at),
COALESCE(user_book_status.finished_at, EXCLUDED.finished_at, EXCLUDED.started_at)
),
finished_at = GREATEST(
COALESCE(user_book_status.finished_at, EXCLUDED.finished_at),
LEAST(
COALESCE(user_book_status.started_at, EXCLUDED.started_at),
COALESCE(user_book_status.finished_at, EXCLUDED.finished_at, EXCLUDED.started_at)
)
),
updated_at = EXCLUDED.updated_at; updated_at = EXCLUDED.updated_at;
END LOOP; END LOOP;
RAISE NOTICE 'Year Projection: marked % books as read this year', i; RAISE NOTICE 'Year Projection: marked % books as read this year', i;
@@ -1,7 +1,7 @@
import { readFile } from 'fs/promises'; import { readFile } from 'fs/promises';
import { XMLParser } from 'fast-xml-parser'; import { XMLParser } from 'fast-xml-parser';
import { inflateRawSync } from 'zlib'; import { inflateRawSync } from 'zlib';
import { createExtractorFromData } from 'node-unrar-js'; import { createExtractorFromData, UnrarError } from 'node-unrar-js';
import { getSevenZip } from '../../../common/sevenzip'; import { getSevenZip } from '../../../common/sevenzip';
import { cleanupSevenZipArtifacts, createSevenZipTempId, type SevenZipInstance } from './sevenzip-vfs'; import { cleanupSevenZipArtifacts, createSevenZipTempId, type SevenZipInstance } from './sevenzip-vfs';
@@ -365,9 +365,15 @@ export async function extractCbrMetadata(absolutePath: string): Promise<ParsedCb
const { files } = extractor.extract({ files: (h) => h.name.toLowerCase() === 'comicinfo.xml' }); const { files } = extractor.extract({ files: (h) => h.name.toLowerCase() === 'comicinfo.xml' });
let xmlBuf: Buffer | undefined; let xmlBuf: Buffer | undefined;
try {
for (const file of files) { for (const file of files) {
if (!file.fileHeader.flags.directory && file.extraction) xmlBuf = Buffer.from(file.extraction); if (!file.fileHeader.flags.directory && file.extraction) xmlBuf = Buffer.from(file.extraction);
} }
} catch (err) {
// Some RAR 1.5 archives throw ERAR_BAD_DATA at the end-of-archive marker.
// If we already extracted the file we needed, use it.
if (!(err instanceof UnrarError)) throw err;
}
return xmlBuf ? parseComicInfoXml(xmlBuf) : null; return xmlBuf ? parseComicInfoXml(xmlBuf) : null;
} catch { } catch {
+17 -6
View File
@@ -1,5 +1,5 @@
import { readFile } from 'fs/promises'; import { readFile } from 'fs/promises';
import { createExtractorFromData } from 'node-unrar-js'; import { createExtractorFromData, UnrarError } from 'node-unrar-js';
import { isArchiveImageFile, isHiddenArchivePath } from './archive-image-utils'; import { isArchiveImageFile, isHiddenArchivePath } from './archive-image-utils';
function toArrayBuffer(buf: Buffer): ArrayBuffer { function toArrayBuffer(buf: Buffer): ArrayBuffer {
@@ -14,16 +14,27 @@ export async function extractCbrCover(absolutePath: string): Promise<Buffer | nu
// First pass: list files to find the first image, sorted naturally. // First pass: list files to find the first image, sorted naturally.
const listExtractor = await createExtractorFromData({ data: ab }); const listExtractor = await createExtractorFromData({ data: ab });
const { fileHeaders } = listExtractor.getFileList(); const { fileHeaders } = listExtractor.getFileList();
const images = [...fileHeaders] const images: { name: string }[] = [];
.filter((h) => !h.flags.directory && isArchiveImageFile(h.name) && !isHiddenArchivePath(h.name)) try {
.sort((a, b) => a.name.localeCompare(b.name, undefined, { numeric: true, sensitivity: 'base' })); for (const h of fileHeaders) {
if (!h.flags.directory && isArchiveImageFile(h.name) && !isHiddenArchivePath(h.name)) {
images.push(h);
}
}
} catch (err) {
// Some RAR 1.5 archives throw ERAR_BAD_DATA at the end-of-archive marker
// instead of ERAR_END_ARCHIVE. Accept partial results if we have images.
if (!(err instanceof UnrarError) || images.length === 0) throw err;
}
images.sort((a, b) => a.name.localeCompare(b.name, undefined, { numeric: true, sensitivity: 'base' }));
if (images.length === 0) return null; if (images.length === 0) return null;
const firstName = images[0].name; const firstName = images[0].name;
// Second pass: extract only that file. Must drain the generator fully to avoid WASM leak. // Second pass: pass an array so the extractor stops after the first match
// and never hits the malformed end-of-archive marker in some RAR 1.5 files.
const extractExtractor = await createExtractorFromData({ data: ab }); const extractExtractor = await createExtractorFromData({ data: ab });
const { files } = extractExtractor.extract({ files: (h) => h.name === firstName }); const { files } = extractExtractor.extract({ files: [firstName] });
let result: Uint8Array | undefined; let result: Uint8Array | undefined;
for (const file of files) { for (const file of files) {
if (!file.fileHeader.flags.directory) result = file.extraction; if (!file.fileHeader.flags.directory) result = file.extraction;
+30 -8
View File
@@ -1,9 +1,9 @@
import { Injectable, NotFoundException } from '@nestjs/common'; import { Injectable, NotFoundException, UnprocessableEntityException } from '@nestjs/common';
import { Readable } from 'stream'; import { Readable } from 'stream';
import { createReadStream } from 'fs'; import { createReadStream } from 'fs';
import { readFile } from 'fs/promises'; import { readFile } from 'fs/promises';
import { createInflateRaw } from 'zlib'; import { createInflateRaw } from 'zlib';
import { createExtractorFromData } from 'node-unrar-js'; import { createExtractorFromData, UnrarError } from 'node-unrar-js';
import { getSevenZip } from '../../../common/sevenzip'; import { getSevenZip } from '../../../common/sevenzip';
import { imageContentTypeFromPath } from '../../../common/image-content-type'; import { imageContentTypeFromPath } from '../../../common/image-content-type';
@@ -147,14 +147,27 @@ export class CbzService {
if (!this.rarCache.has(fileId)) { if (!this.rarCache.has(fileId)) {
const buf = await readFile(filePath); const buf = await readFile(filePath);
const ab = toArrayBuffer(buf); const ab = toArrayBuffer(buf);
const extractor = await createExtractorFromData({ data: ab }); const extractor = await createExtractorFromData({ data: ab });
const { fileHeaders } = extractor.getFileList(); const { fileHeaders } = extractor.getFileList();
const pages = [...fileHeaders]
.filter((h) => !h.flags.directory && isImage(h.name) && !isHidden(h.name))
.map((h) => h.name)
.sort(naturalSort);
const pages: string[] = [];
try {
for (const h of fileHeaders) {
if (!h.flags.directory && isImage(h.name) && !isHidden(h.name)) {
pages.push(h.name);
}
}
} catch (err) {
// Some RAR 1.5 archives throw ERAR_BAD_DATA at the end-of-archive
// marker instead of returning ERAR_END_ARCHIVE. If we already have
// pages, the archive is readable — accept the partial list.
if (!(err instanceof UnrarError) || pages.length === 0) {
throw err instanceof UnrarError ? new UnprocessableEntityException(`CBR archive is unreadable: ${err.message}`) : err;
}
}
pages.sort(naturalSort);
this.rarCache.set(fileId, { buffer: ab, pages }); this.rarCache.set(fileId, { buffer: ab, pages });
} }
return this.rarCache.get(fileId)!; return this.rarCache.get(fileId)!;
@@ -162,13 +175,22 @@ export class CbzService {
private async extractRarPage(buffer: ArrayBuffer, pageName: string): Promise<Uint8Array> { private async extractRarPage(buffer: ArrayBuffer, pageName: string): Promise<Uint8Array> {
const extractor = await createExtractorFromData({ data: buffer }); const extractor = await createExtractorFromData({ data: buffer });
const { files } = extractor.extract({ files: (h) => h.name === pageName }); // Pass an array so the extractor stops after the first match and never
// hits the malformed end-of-archive marker present in some RAR 1.5 files.
const { files } = extractor.extract({ files: [pageName] });
// Generators MUST be fully drained to avoid WASM memory leak // Generators MUST be fully drained to avoid WASM memory leak
let result: Uint8Array | undefined; let result: Uint8Array | undefined;
try {
for (const file of files) { for (const file of files) {
if (!file.fileHeader.flags.directory) result = file.extraction; if (!file.fileHeader.flags.directory) result = file.extraction;
} }
} catch (err) {
if (err instanceof UnrarError) {
throw new UnprocessableEntityException(`CBR page is unreadable: ${err.message}`);
}
throw err;
}
if (result === undefined) throw new NotFoundException(`Page not found in RAR archive`); if (result === undefined) throw new NotFoundException(`Page not found in RAR archive`);
return result; return result;
} }
@@ -23,10 +23,16 @@ describe('UploadProcessorService', () => {
const insertBookMetadataValues = vi.fn(); const insertBookMetadataValues = vi.fn();
const insertBookFilesReturning = vi.fn(); const insertBookFilesReturning = vi.fn();
const insertBookFilesValues = vi.fn(); const insertBookFilesValues = vi.fn();
const insertBookFilesOnConflict = vi.fn();
const updateBooksSet = vi.fn(); const updateBooksSet = vi.fn();
const updateBooksWhere = vi.fn(); const updateBooksWhere = vi.fn();
const selectFrom = vi.fn();
const selectWhere = vi.fn();
const selectLimit = vi.fn();
const tx = { const tx = {
select: vi.fn(() => ({ from: selectFrom })),
insert: vi.fn((table: unknown) => { insert: vi.fn((table: unknown) => {
if (table === books) { if (table === books) {
return { values: insertBooksValues }; return { values: insertBooksValues };
@@ -54,10 +60,15 @@ describe('UploadProcessorService', () => {
beforeEach(() => { beforeEach(() => {
vi.resetAllMocks(); vi.resetAllMocks();
selectFrom.mockReturnValue({ where: selectWhere });
selectWhere.mockReturnValue({ limit: selectLimit });
selectLimit.mockResolvedValue([]); // no existing book by default
insertBooksValues.mockReturnValue({ returning: insertBooksReturning }); insertBooksValues.mockReturnValue({ returning: insertBooksReturning });
insertBooksReturning.mockResolvedValue([{ id: 42 }]); insertBooksReturning.mockResolvedValue([{ id: 42 }]);
insertBookMetadataValues.mockResolvedValue(undefined); insertBookMetadataValues.mockResolvedValue(undefined);
insertBookFilesValues.mockReturnValue({ returning: insertBookFilesReturning }); insertBookFilesOnConflict.mockReturnValue({ returning: insertBookFilesReturning });
insertBookFilesValues.mockReturnValue({ returning: insertBookFilesReturning, onConflictDoUpdate: insertBookFilesOnConflict });
insertBookFilesReturning.mockResolvedValue([{ id: 420 }]); insertBookFilesReturning.mockResolvedValue([{ id: 420 }]);
updateBooksSet.mockReturnValue({ where: updateBooksWhere }); updateBooksSet.mockReturnValue({ where: updateBooksWhere });
updateBooksWhere.mockResolvedValue(undefined); updateBooksWhere.mockResolvedValue(undefined);
@@ -94,6 +105,44 @@ describe('UploadProcessorService', () => {
expect(orchestrator.scheduleIfEligible).toHaveBeenCalledWith(42, 1, 'event_import'); expect(orchestrator.scheduleIfEligible).toHaveBeenCalledWith(42, 1, 'event_import');
}); });
it('adds a file to an existing book when the folder path already exists in the library', async () => {
selectLimit.mockResolvedValueOnce([{ id: 99 }]);
const result = await service.createBookRecord(1, 2, '/folder', '/folder/book2.epub', 'book/book2.epub', 'epub', 5000);
expect(result).toEqual({ bookId: 99 });
expect(insertBooksValues).not.toHaveBeenCalled();
expect(insertBookMetadataValues).not.toHaveBeenCalled();
expect(updateBooksSet).not.toHaveBeenCalled();
expect(insertBookFilesValues).toHaveBeenCalledWith(
expect.objectContaining({
bookId: 99,
libraryFolderId: 2,
absolutePath: '/folder/book2.epub',
relPath: 'book/book2.epub',
format: 'epub',
role: 'content',
}),
);
expect(insertBookFilesOnConflict).toHaveBeenCalledWith(
expect.objectContaining({
target: bookFiles.absolutePath,
set: expect.objectContaining({ bookId: 99, libraryFolderId: 2, format: 'epub' }),
}),
);
expect(orchestrator.scheduleIfEligible).toHaveBeenCalledWith(99, 1, 'event_import');
});
it('refreshes a stale book_files record when the absolute path already exists in the db', async () => {
selectLimit.mockResolvedValueOnce([{ id: 99 }]);
insertBookFilesOnConflict.mockReturnValueOnce({ returning: vi.fn().mockResolvedValue([{ id: 777 }]) });
const result = await service.createBookRecord(1, 2, '/folder', '/folder/stale.epub', 'book/stale.epub', 'epub', 1000);
expect(result).toEqual({ bookId: 99 });
expect(insertBookFilesOnConflict).toHaveBeenCalled();
});
it('throws InternalServerErrorException when creating the book row fails', async () => { it('throws InternalServerErrorException when creating the book row fails', async () => {
insertBooksReturning.mockResolvedValueOnce([]); insertBooksReturning.mockResolvedValueOnce([]);
@@ -110,6 +159,15 @@ describe('UploadProcessorService', () => {
); );
}); });
it('throws InternalServerErrorException when adding a file to an existing book fails', async () => {
selectLimit.mockResolvedValueOnce([{ id: 99 }]);
insertBookFilesReturning.mockResolvedValueOnce([]);
await expect(service.createBookRecord(1, 2, '/folder', '/folder/book2.epub', 'book/book2.epub', 'epub', 5000)).rejects.toBeInstanceOf(
InternalServerErrorException,
);
});
it('logs but suppresses scheduling failures', async () => { it('logs but suppresses scheduling failures', async () => {
const warn = vi.spyOn((service as unknown as { logger: { warn: (m: string) => void } }).logger, 'warn').mockImplementation(); const warn = vi.spyOn((service as unknown as { logger: { warn: (m: string) => void } }).logger, 'warn').mockImplementation();
orchestrator.scheduleIfEligible.mockRejectedValue(new Error('queue offline')); orchestrator.scheduleIfEligible.mockRejectedValue(new Error('queue offline'));
@@ -1,6 +1,6 @@
import { Inject, Injectable, InternalServerErrorException, Logger, Optional } from '@nestjs/common'; import { Inject, Injectable, InternalServerErrorException, Logger, Optional } from '@nestjs/common';
import { stat } from 'fs/promises'; import { stat } from 'fs/promises';
import { eq } from 'drizzle-orm'; import { and, eq } from 'drizzle-orm';
import { NodePgDatabase } from 'drizzle-orm/node-postgres'; import { NodePgDatabase } from 'drizzle-orm/node-postgres';
import { DB } from '../../db'; import { DB } from '../../db';
@@ -53,6 +53,37 @@ export class UploadProcessorService {
const [fileStat, hash] = await Promise.all([stat(absolutePath), fingerprintFile(absolutePath)]); const [fileStat, hash] = await Promise.all([stat(absolutePath), fingerprintFile(absolutePath)]);
const { bookId } = await this.db.transaction(async (tx) => { const { bookId } = await this.db.transaction(async (tx) => {
const [existingBook] = await tx
.select({ id: books.id })
.from(books)
.where(and(eq(books.libraryId, libraryId), eq(books.folderPath, folderPath)))
.limit(1);
if (existingBook) {
const fileValues = {
bookId: existingBook.id,
libraryFolderId,
absolutePath,
relPath,
ino: fileStat.ino,
sizeBytes,
mtime: fileStat.mtime,
hash,
format,
role: 'content' as const,
};
const [file] = await tx
.insert(bookFiles)
.values(fileValues)
.onConflictDoUpdate({
target: bookFiles.absolutePath,
set: { bookId: existingBook.id, libraryFolderId, relPath, ino: fileStat.ino, sizeBytes, mtime: fileStat.mtime, hash, format },
})
.returning({ id: bookFiles.id });
if (!file) throw new InternalServerErrorException('Failed to create book file');
return { bookId: existingBook.id };
}
const [book] = await tx.insert(books).values({ libraryId, libraryFolderId, folderPath, status: 'present' }).returning({ id: books.id }); const [book] = await tx.insert(books).values({ libraryId, libraryFolderId, folderPath, status: 'present' }).returning({ id: books.id });
if (!book) throw new InternalServerErrorException('Failed to create book record'); if (!book) throw new InternalServerErrorException('Failed to create book record');