mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(scanner): clamp oversized inodes in upload and file event flows (#49)
Use BigInt stat reads for upload and file-event paths to preserve 64-bit inode values. Clamp inodes that exceed PostgreSQL bigint range to 0 before persistence and inode lookups, and add regression coverage for insert, upsert, restore, and move-detection paths. Fixes #48
This commit is contained in:
@@ -60,10 +60,10 @@ function makeService() {
|
||||
return new FileEventProcessorService(mockRepo as unknown as ScannerRepository);
|
||||
}
|
||||
|
||||
function makeFileStat(overrides: Partial<{ ino: number; size: number; mtime: Date; isDirectory: boolean; isFile: boolean }> = {}) {
|
||||
function makeFileStat(overrides: Partial<{ ino: number | bigint; size: number | bigint; mtime: Date; isDirectory: boolean; isFile: boolean }> = {}) {
|
||||
return {
|
||||
ino: overrides.ino ?? 1001,
|
||||
size: overrides.size ?? 12345,
|
||||
ino: overrides.ino ?? 1001n,
|
||||
size: overrides.size ?? 12345n,
|
||||
mtime: overrides.mtime ?? new Date('2024-01-01'),
|
||||
isDirectory: () => overrides.isDirectory ?? false,
|
||||
isFile: () => overrides.isFile ?? true,
|
||||
@@ -317,6 +317,20 @@ describe('handleCreate — file', () => {
|
||||
expect(result).toEqual({ type: 'book-restored', libraryId: 3, bookIds: [10] });
|
||||
});
|
||||
|
||||
it('clamps oversized MergerFS inode values to 0 when restoring existing file rows', async () => {
|
||||
const fileStat = makeFileStat({ ino: 14351917807348929000n, size: 50000n });
|
||||
mockStat.mockResolvedValue(fileStat);
|
||||
mockRepo.findBookFileByAbsolutePath.mockResolvedValue({
|
||||
file: { id: 42, bookId: 10 },
|
||||
libraryId: 3,
|
||||
} as any);
|
||||
mockRepo.findBookById.mockResolvedValue({ id: 10, status: 'missing', libraryId: 3 } as any);
|
||||
|
||||
await makeService().handleCreate('/books/Author/book.epub');
|
||||
|
||||
expect(mockRepo.updateBookFile).toHaveBeenCalledWith(42, expect.objectContaining({ ino: 0 }));
|
||||
});
|
||||
|
||||
it('restores own book before searching for folder-level missing books (Issue 25)', async () => {
|
||||
const fileStat = makeFileStat({ ino: 2000, size: 50000 });
|
||||
mockStat.mockResolvedValue(fileStat);
|
||||
@@ -538,6 +552,18 @@ describe('handleCreate — move detection', () => {
|
||||
expect(mockRepo.updateBookFolderPath).toHaveBeenCalledWith(40, '/books/renamed.epub');
|
||||
expect(result).toEqual({ type: 'book-moved', libraryId: 3, bookIds: [40] });
|
||||
});
|
||||
|
||||
it('skips inode-based move detection when inode exceeds PostgreSQL bigint range', async () => {
|
||||
const fileStat = makeFileStat({ ino: 14351917807348929000n });
|
||||
mockStat.mockResolvedValue(fileStat);
|
||||
mockRepo.findBookFileByAbsolutePath.mockResolvedValue(null);
|
||||
mockRepo.findMissingBookByFolderPath.mockResolvedValue(null);
|
||||
|
||||
const result = await makeService().handleCreate('/books/Author/book.epub');
|
||||
|
||||
expect(mockRepo.findBookFileWithContextByIno).not.toHaveBeenCalled();
|
||||
expect(result).toEqual({ type: 'noop' });
|
||||
});
|
||||
});
|
||||
|
||||
// ── handleCreate — directory move detection ─────────────────────────────────
|
||||
|
||||
@@ -1,9 +1,11 @@
|
||||
import { Injectable, Logger } from '@nestjs/common';
|
||||
import { readdir, stat } from 'fs/promises';
|
||||
import type { BigIntStats } from 'fs';
|
||||
import { dirname, join, relative } from 'path';
|
||||
import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils';
|
||||
|
||||
import { classifyFile, DEFAULT_FORMAT_PRIORITY } from './lib/classify';
|
||||
import { clampIno } from './lib/walk';
|
||||
import { ScannerRepository } from './scanner.repository';
|
||||
|
||||
export type FileEventResult =
|
||||
@@ -12,7 +14,7 @@ export type FileEventResult =
|
||||
| { type: 'book-moved'; libraryId: number; bookIds: number[] }
|
||||
| { type: 'noop' };
|
||||
|
||||
type FsStat = NonNullable<Awaited<ReturnType<typeof stat>>>;
|
||||
type FsStat = BigIntStats;
|
||||
|
||||
@Injectable()
|
||||
export class FileEventProcessorService {
|
||||
@@ -84,7 +86,7 @@ export class FileEventProcessorService {
|
||||
}
|
||||
|
||||
async handleCreate(absolutePath: string, scopeLibraryId?: number): Promise<FileEventResult> {
|
||||
const fileStat = await stat(absolutePath).catch(() => null);
|
||||
const fileStat = await stat(absolutePath, { bigint: true }).catch(() => null);
|
||||
if (!fileStat) return { type: 'noop' };
|
||||
|
||||
if (fileStat.isDirectory()) return this.handleCreateDir(absolutePath, scopeLibraryId);
|
||||
@@ -149,7 +151,7 @@ export class FileEventProcessorService {
|
||||
// for the wrong book), check if the inode belongs to a file already tracked by a
|
||||
// book at this folder path. This handles atomic-save patterns where an editor writes
|
||||
// a new inode for the same logical file.
|
||||
const ino = Number(fileStat.ino);
|
||||
const ino = clampIno(fileStat.ino);
|
||||
if (ino !== 0) {
|
||||
const folderPath = dirname(absolutePath);
|
||||
const ownBook =
|
||||
@@ -236,7 +238,7 @@ export class FileEventProcessorService {
|
||||
const existingContent: ((typeof files)[number] & { stat: FsStat })[] = [];
|
||||
|
||||
for (const file of files) {
|
||||
const s = await stat(file.absolutePath).catch(() => null);
|
||||
const s = await stat(file.absolutePath, { bigint: true }).catch(() => null);
|
||||
if (!s || !s.isFile()) continue;
|
||||
existingContent.push({ ...file, stat: s });
|
||||
}
|
||||
@@ -261,7 +263,7 @@ export class FileEventProcessorService {
|
||||
}
|
||||
|
||||
private async detectMovedFile(newAbsolutePath: string, fileStat: FsStat, scopeLibraryId?: number): Promise<FileEventResult> {
|
||||
const ino = Number(fileStat.ino);
|
||||
const ino = clampIno(fileStat.ino);
|
||||
if (ino === 0) return { type: 'noop' };
|
||||
|
||||
const match = await this.scannerRepo.findBookFileWithContextByIno(ino, scopeLibraryId);
|
||||
@@ -313,7 +315,7 @@ export class FileEventProcessorService {
|
||||
}
|
||||
|
||||
private statToFileInfo(s: FsStat): { ino: number; sizeBytes: number; mtime: Date } {
|
||||
return { ino: Number(s.ino), sizeBytes: Number(s.size), mtime: s.mtime };
|
||||
return { ino: clampIno(s.ino), sizeBytes: Number(s.size), mtime: s.mtime };
|
||||
}
|
||||
|
||||
private async detectMovedBooksInDir(dirPath: string, scopeLibraryId?: number): Promise<FileEventResult> {
|
||||
@@ -327,7 +329,7 @@ export class FileEventProcessorService {
|
||||
const { role } = classifyFile(fullPath);
|
||||
if (role !== 'content') continue;
|
||||
|
||||
const s = await stat(fullPath).catch(() => null);
|
||||
const s = await stat(fullPath, { bigint: true }).catch(() => null);
|
||||
if (!s) continue;
|
||||
|
||||
const result = await this.detectMovedFile(fullPath, s, scopeLibraryId);
|
||||
|
||||
@@ -75,7 +75,7 @@ describe('UploadProcessorService', () => {
|
||||
|
||||
orchestrator.scheduleIfEligible.mockResolvedValue(undefined);
|
||||
|
||||
mockStat.mockResolvedValue({ ino: 111, mtime: new Date('2024-01-01') } as Awaited<ReturnType<typeof stat>>);
|
||||
mockStat.mockResolvedValue({ ino: 111n, mtime: new Date('2024-01-01') } as Awaited<ReturnType<typeof stat>>);
|
||||
mockComputeFileHash.mockResolvedValue('hash-abc');
|
||||
|
||||
service = new UploadProcessorService(db as any, metadataService as any, orchestrator as any);
|
||||
@@ -133,6 +133,33 @@ describe('UploadProcessorService', () => {
|
||||
expect(orchestrator.scheduleIfEligible).toHaveBeenCalledWith(99, 1, 'event_import');
|
||||
});
|
||||
|
||||
it('clamps oversized MergerFS inode values to 0 before persisting', async () => {
|
||||
mockStat.mockResolvedValueOnce({ ino: 14351917807348929000n, mtime: new Date('2024-01-01') } as Awaited<ReturnType<typeof stat>>);
|
||||
|
||||
await service.createBookRecord(1, 2, '/folder', '/folder/book.epub', 'book/book.epub', 'epub', 12345);
|
||||
|
||||
expect(insertBookFilesValues).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
ino: 0,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('clamps oversized MergerFS inode values to 0 in existing-book upserts', async () => {
|
||||
selectLimit.mockResolvedValueOnce([{ id: 99 }]);
|
||||
mockStat.mockResolvedValueOnce({ ino: 14351917807348929000n, mtime: new Date('2024-01-01') } as Awaited<ReturnType<typeof stat>>);
|
||||
|
||||
await service.createBookRecord(1, 2, '/folder', '/folder/book2.epub', 'book/book2.epub', 'epub', 5000);
|
||||
|
||||
expect(insertBookFilesOnConflict).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
set: expect.objectContaining({
|
||||
ino: 0,
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
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 }]) });
|
||||
|
||||
@@ -10,6 +10,7 @@ import { bookFiles, bookMetadata, books } from '../../db/schema';
|
||||
import { BookMetadataFetchOrchestratorService } from '../book-metadata-fetch/book-metadata-fetch-orchestrator.service';
|
||||
import { MetadataService } from '../metadata/metadata.service';
|
||||
import { computeFileHash } from '../scanner/lib/hash';
|
||||
import { clampIno } from '../scanner/lib/walk';
|
||||
|
||||
type Db = NodePgDatabase<typeof schema>;
|
||||
|
||||
@@ -51,7 +52,8 @@ export class UploadProcessorService {
|
||||
format: string,
|
||||
sizeBytes: number,
|
||||
): Promise<{ bookId: number }> {
|
||||
const [fileStat, fileHash] = await Promise.all([stat(absolutePath), computeFileHash(absolutePath)]);
|
||||
const [fileStat, fileHash] = await Promise.all([stat(absolutePath, { bigint: true }), computeFileHash(absolutePath)]);
|
||||
const safeIno = clampIno(fileStat.ino);
|
||||
|
||||
const { bookId } = await this.db.transaction(async (tx) => {
|
||||
const [existingBook] = await tx
|
||||
@@ -66,7 +68,7 @@ export class UploadProcessorService {
|
||||
libraryFolderId,
|
||||
absolutePath,
|
||||
relPath,
|
||||
ino: fileStat.ino,
|
||||
ino: safeIno,
|
||||
sizeBytes,
|
||||
mtime: fileStat.mtime,
|
||||
fileHash,
|
||||
@@ -78,7 +80,7 @@ export class UploadProcessorService {
|
||||
.values(fileValues)
|
||||
.onConflictDoUpdate({
|
||||
target: bookFiles.absolutePath,
|
||||
set: { bookId: existingBook.id, libraryFolderId, relPath, ino: fileStat.ino, sizeBytes, mtime: fileStat.mtime, fileHash, format },
|
||||
set: { bookId: existingBook.id, libraryFolderId, relPath, ino: safeIno, sizeBytes, mtime: fileStat.mtime, fileHash, format },
|
||||
})
|
||||
.returning({ id: bookFiles.id });
|
||||
if (!file) throw new InternalServerErrorException('Failed to create book file');
|
||||
@@ -98,7 +100,7 @@ export class UploadProcessorService {
|
||||
libraryFolderId,
|
||||
absolutePath,
|
||||
relPath,
|
||||
ino: fileStat.ino,
|
||||
ino: safeIno,
|
||||
sizeBytes,
|
||||
mtime: fileStat.mtime,
|
||||
fileHash,
|
||||
|
||||
Reference in New Issue
Block a user