diff --git a/client/src/features/reader/cbz/CbzReaderView.spec.ts b/client/src/features/reader/cbz/CbzReaderView.spec.ts index c2b6e8547..625ea08dc 100644 --- a/client/src/features/reader/cbz/CbzReaderView.spec.ts +++ b/client/src/features/reader/cbz/CbzReaderView.spec.ts @@ -1,4 +1,4 @@ -import { computed, nextTick, ref } from 'vue' +import { computed, nextTick, onUnmounted, ref } from 'vue' import { flushPromises, shallowMount } from '@vue/test-utils' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -25,6 +25,7 @@ const mocks = vi.hoisted(() => ({ scrollToIndex: vi.fn<(index: number, options: { align: string }) => void>(), updateBookSettings: vi.fn<(patch: unknown) => void>(), resetBookSettings: vi.fn<() => void>(), + sessionEndSnapshots: [] as Array<{ percentage: number; pageNumber?: number | null }>, })) vi.mock('vue-i18n', () => ({ useI18n: () => ({ t: (key: string) => key }) })) @@ -127,8 +128,12 @@ vi.mock('../shared/composables/useReaderProgress', () => ({ }), })) +// Like the real composable, the session ends from an unmounted hook registered before the view's own. vi.mock('../shared/composables/useReadingSession', () => ({ - useReadingSession: () => ({ onActivity: vi.fn<() => void>(), elapsedMinutes: ref(0) }), + useReadingSession: (_fileId: number, getProgress: () => { percentage: number; pageNumber?: number | null }) => { + onUnmounted(() => mocks.sessionEndSnapshots.push(getProgress())) + return { onActivity: vi.fn<() => void>(), elapsedMinutes: ref(0) } + }, })) vi.mock('../shared/composables/useVisibility', () => ({ @@ -157,6 +162,7 @@ describe('CbzReaderView', () => { mocks.savedAutoAdvance = false mocks.seriesId = 42 mocks.nextBook = null + mocks.sessionEndSnapshots.length = 0 vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { callback(0) return 1 @@ -403,6 +409,23 @@ describe('CbzReaderView', () => { wrapper.unmount() }) + it('flushes a debounced page turn before the reading session ends', async () => { + mocks.savedMode = 'paginated' + mocks.pageCount = 6 + mocks.savedPageNumber = 2 + + const wrapper = shallowMount(CbzReaderView, { props: { bookId: 11, fileId: 22 } }) + await flushPromises() + await nextTick() + + window.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowRight' })) + await nextTick() + wrapper.unmount() + + expect(mocks.sessionEndSnapshots).toEqual([{ percentage: 50, pageNumber: 3 }]) + expect(mocks.progressSave).toHaveBeenCalledOnce() + }) + it('places the next book after the last page in continuous modes', async () => { mocks.savedMode = 'long-strip' mocks.pageCount = 3 diff --git a/client/src/features/reader/cbz/CbzReaderView.vue b/client/src/features/reader/cbz/CbzReaderView.vue index d8a4eaf51..7df94fe8d 100644 --- a/client/src/features/reader/cbz/CbzReaderView.vue +++ b/client/src/features/reader/cbz/CbzReaderView.vue @@ -1,5 +1,5 @@ diff --git a/client/src/features/reader/cbz/composables/useCbz.ts b/client/src/features/reader/cbz/composables/useCbz.ts index 24fef7a38..45deb27e4 100644 --- a/client/src/features/reader/cbz/composables/useCbz.ts +++ b/client/src/features/reader/cbz/composables/useCbz.ts @@ -1,5 +1,5 @@ import { ref } from 'vue' -import type { BookDetail } from '@bookorbit/types' +import type { BookDetail, ComicPageCountResponse } from '@bookorbit/types' import { api } from '@/lib/api' /** The series a book is read as part of: its first membership, matching how the book pages order them. */ @@ -27,7 +27,7 @@ export function useCbz(fileId: number, bookId: number) { loading.value = false return } - const [pagesData, bookData] = await Promise.all([pagesRes.json(), bookRes.ok ? bookRes.json() : null]) + const [pagesData, bookData] = await Promise.all([pagesRes.json() as Promise, bookRes.ok ? bookRes.json() : null]) pageCount.value = pagesData.pageCount bookTitle.value = bookData?.title ?? '' seriesId.value = primarySeriesId(bookData) diff --git a/client/src/features/reader/pdf-v4/PdfV4ReaderView.vue b/client/src/features/reader/pdf-v4/PdfV4ReaderView.vue index 5396bd429..5e13b4ad1 100644 --- a/client/src/features/reader/pdf-v4/PdfV4ReaderView.vue +++ b/client/src/features/reader/pdf-v4/PdfV4ReaderView.vue @@ -1,5 +1,5 @@ @@ -308,6 +325,7 @@ onUnmounted(() => { :book-id="props.bookId" :file-id="props.fileId" :initial-page="initialPage" + :initial-percentage="initialPercentage" :settings="effectiveSettings" :peek-mode="props.peekMode" @back="handleBack" diff --git a/client/src/features/reader/pdf-v4/__tests__/PdfReaderContent.spec.ts b/client/src/features/reader/pdf-v4/__tests__/PdfReaderContent.spec.ts new file mode 100644 index 000000000..c662bbd96 --- /dev/null +++ b/client/src/features/reader/pdf-v4/__tests__/PdfReaderContent.spec.ts @@ -0,0 +1,138 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { shallowMount } from '@vue/test-utils' +import { defineComponent, h, ref } from 'vue' +import type { PdfReaderSettings } from '@bookorbit/types' + +type LayoutReadyEvent = { documentId: string; isInitial: boolean; pageNumber: number; totalPages: number } +type PageChangeEvent = { documentId: string; pageNumber: number; totalPages: number } + +const mocks = vi.hoisted(() => ({ + layoutReadyListener: null as ((event: LayoutReadyEvent) => void) | null, + pageChangeListener: null as ((event: PageChangeEvent) => void) | null, + totalPages: 10, + scrollToPage: vi.fn<(options: { pageNumber: number }) => void>(), +})) + +vi.mock('@embedpdf/plugin-scroll/vue', async () => { + const { ScrollStrategy } = await import('@embedpdf/plugin-scroll') + return { + ScrollStrategy, + useScroll: () => ({ + state: ref({ currentPage: 1, totalPages: mocks.totalPages }), + provides: ref({ + // EmbedPDF reports the target page synchronously when a programmatic page change starts. + scrollToPage: (options: { pageNumber: number }) => { + mocks.scrollToPage(options) + mocks.pageChangeListener?.({ documentId: 'document-1', pageNumber: options.pageNumber, totalPages: mocks.totalPages }) + }, + getSpreadPagesWithRotatedSize: () => [], + getPageChangeState: () => ({ isChanging: false }), + }), + }), + useScrollCapability: () => ({ + provides: ref({ + onLayoutReady: (listener: (event: LayoutReadyEvent) => void) => { + mocks.layoutReadyListener = listener + return () => {} + }, + onPageChange: (listener: (event: PageChangeEvent) => void) => { + mocks.pageChangeListener = listener + return () => {} + }, + }), + }), + } +}) + +vi.mock('@embedpdf/plugin-zoom/vue', async () => { + const { ZoomMode } = await import('@embedpdf/plugin-zoom') + return { ZoomMode, useZoom: () => ({ state: ref({ currentZoomLevel: 1, zoomLevel: ZoomMode.FitPage }), provides: ref(null) }) } +}) + +vi.mock('@embedpdf/plugin-spread/vue', async () => { + const { SpreadMode } = await import('@embedpdf/plugin-spread') + return { SpreadMode, useSpread: () => ({ provides: ref(null) }) } +}) + +vi.mock('@embedpdf/plugin-rotate/vue', () => ({ useRotate: () => ({ rotation: ref(0), provides: ref(null) }) })) +vi.mock('@embedpdf/plugin-selection/vue', () => ({ useSelectionCapability: () => ({ provides: ref(null) }) })) +vi.mock('@embedpdf/plugin-annotation/vue', () => ({ useAnnotationCapability: () => ({ provides: ref(null) }) })) +vi.mock('@embedpdf/plugin-document-manager/vue', () => ({ + DocumentContent: defineComponent({ name: 'DocumentContent', props: ['documentId'], setup: () => () => h('div') }), +})) + +vi.mock('../composables/usePdfInteractionMode', () => ({ + usePdfInteractionMode: () => ({ isPanning: ref(false), togglePan: vi.fn<() => void>(), activateTextSelection: vi.fn<() => void>() }), +})) +vi.mock('../composables/usePdfResponsiveSpread', () => ({ usePdfResponsiveSpread: () => ({ apply: vi.fn<() => void>() }) })) +vi.mock('../../shared/composables/useFullscreen', () => ({ + useFullscreen: () => ({ isFullscreen: ref(false), isFullscreenSupported: ref(false), toggleFullscreen: vi.fn<() => void>() }), +})) +vi.mock('../composables/usePdfHighlights', () => ({ + usePdfHighlights: () => ({ + renderAll: vi.fn<() => void>(), + annotations: ref([]), + loadError: ref(null), + loading: ref(false), + loadingMore: ref(false), + hasMore: ref(false), + popupVisible: ref(false), + popupPosition: ref({ x: 0, y: 0 }), + popupShowBelow: ref(false), + selectedText: ref(''), + overlappingAnnotationId: ref(null), + isSaving: ref(false), + showNoteDialog: ref(false), + noteText: ref(''), + }), +})) + +const { default: PdfReaderContent } = await import('../components/PdfReaderContent.vue') + +const settings: PdfReaderSettings = { scrollMode: 'page', spread: 'none', zoomMode: 'fit-page', customScale: 1, rotation: 0 } + +function mountContent(initialPage: number | null, initialPercentage?: number) { + return shallowMount(PdfReaderContent, { + props: { documentId: 'document-1', bookId: 42, fileId: 101, initialPage, initialPercentage, settings }, + }) +} + +function layoutReady() { + mocks.layoutReadyListener?.({ documentId: 'document-1', isInitial: true, pageNumber: 1, totalPages: mocks.totalPages }) +} + +describe('PdfReaderContent', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.layoutReadyListener = null + mocks.pageChangeListener = null + mocks.totalPages = 10 + }) + + it.each([ + { initialPage: null, percentage: 37.5, expected: 4 }, + { initialPage: null, percentage: 100, expected: 10 }, + { initialPage: null, percentage: 0.4, expected: 1 }, + { initialPage: null, percentage: 0, expected: 1 }, + { initialPage: 3, percentage: 90, expected: 3 }, + { initialPage: 25, percentage: 10, expected: 10 }, + ])('opens $initialPage / $percentage% of 10 pages at page $expected', ({ initialPage, percentage, expected }) => { + mountContent(initialPage, percentage) + layoutReady() + + // Page 1 is where the document already is, so the reader only scrolls for a later page. + expect(mocks.scrollToPage.mock.calls[0]?.[0].pageNumber ?? 1).toBe(expected) + expect(mocks.scrollToPage.mock.calls.length).toBeLessThanOrEqual(1) + }) + + it('marks the opening page as restored and later turns as moves', () => { + const wrapper = mountContent(null, 37.5) + layoutReady() + mocks.pageChangeListener?.({ documentId: 'document-1', pageNumber: 5, totalPages: 10 }) + + expect(wrapper.emitted('pageChange')).toEqual([ + [4, 10, true], + [5, 10, false], + ]) + }) +}) diff --git a/client/src/features/reader/pdf-v4/__tests__/PdfV4ReaderView.spec.ts b/client/src/features/reader/pdf-v4/__tests__/PdfV4ReaderView.spec.ts index 2bcfdaf55..d028e4eab 100644 --- a/client/src/features/reader/pdf-v4/__tests__/PdfV4ReaderView.spec.ts +++ b/client/src/features/reader/pdf-v4/__tests__/PdfV4ReaderView.spec.ts @@ -1,6 +1,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { flushPromises, mount } from '@vue/test-utils' -import { defineComponent, h, ref } from 'vue' +import { defineComponent, h, onUnmounted, ref } from 'vue' import { ScrollStrategy } from '@embedpdf/plugin-scroll' import { SpreadMode } from '@embedpdf/plugin-spread' import { ZoomMode } from '@embedpdf/plugin-zoom' @@ -54,11 +54,17 @@ vi.mock('../../shared/composables/useReaderProgress', () => ({ })) const mockOnActivity = vi.fn<() => void>() +type SessionSnapshot = { percentage: number; pageNumber?: number | null } +const sessionEndSnapshots: SessionSnapshot[] = [] +// Like the real composable, the session ends from an unmounted hook registered before the view's own. vi.mock('../../shared/composables/useReadingSession', () => ({ - useReadingSession: () => ({ - onActivity: mockOnActivity, - elapsedMinutes: ref(0), - }), + useReadingSession: (_fileId: number, getProgress: () => SessionSnapshot) => { + onUnmounted(() => sessionEndSnapshots.push(getProgress())) + return { + onActivity: mockOnActivity, + elapsedMinutes: ref(0), + } + }, })) const mockUsePdfiumEngine = vi.fn<() => unknown>(() => ({ @@ -129,6 +135,7 @@ describe('PdfV4ReaderView', () => { mockRegistry.isDestroyed.mockReturnValue(false) mockRegistry.getPlugin.mockReturnValue({ provides: () => mockDocumentManager }) mockRoute.query = {} + sessionEndSnapshots.length = 0 mockProgressPageNumber.value = 5 mockProgressPercentage.value = 25 mockEffective.value = { @@ -147,7 +154,7 @@ describe('PdfV4ReaderView', () => { stubs: { PdfReaderContent: { name: 'PdfReaderContent', - props: ['documentId', 'initialPage', 'settings', 'peekMode'], + props: ['documentId', 'initialPage', 'initialPercentage', 'settings', 'peekMode'], emits: ['back', 'pageChange', 'retry', 'settingsChange', 'startReading'], template: '
', }, @@ -280,6 +287,57 @@ describe('PdfV4ReaderView', () => { expect(wrapper.getComponent({ name: 'PdfReaderContent' }).props('initialPage')).toBe(12) }) + it('opens at a saved page number before its percentage', async () => { + const wrapper = await mountReader() + + expect(wrapper.getComponent({ name: 'PdfReaderContent' }).props('initialPage')).toBe(5) + }) + + it('passes a percentage-only position through for the page estimate', async () => { + mockProgressPageNumber.value = null + mockProgressPercentage.value = 37.5 + const wrapper = await mountReader() + + const content = wrapper.getComponent({ name: 'PdfReaderContent' }) + expect(content.props('initialPage')).toBeNull() + expect(content.props('initialPercentage')).toBe(37.5) + }) + + it('treats a zero page number as no page so the percentage still places the reader', async () => { + mockProgressPageNumber.value = 0 + mockProgressPercentage.value = 60 + const wrapper = await mountReader() + + expect(wrapper.getComponent({ name: 'PdfReaderContent' }).props('initialPage')).toBeNull() + }) + + it('does not write progress for the page the reader opened on', async () => { + vi.useFakeTimers() + mockProgressPageNumber.value = null + mockProgressPercentage.value = 37.5 + const wrapper = await mountReader() + wrapper.getComponent({ name: 'PdfReaderContent' }).vm.$emit('pageChange', 4, 10, true) + await vi.advanceTimersByTimeAsync(2000) + wrapper.unmount() + + expect(mockProgressSave).not.toHaveBeenCalled() + expect(mockProgressPageNumber.value).toBe(4) + expect(mockProgressPercentage.value).toBe(40) + vi.useRealTimers() + }) + + it('flushes a debounced page turn before the reading session ends', async () => { + vi.useFakeTimers() + const wrapper = await mountReader() + wrapper.getComponent({ name: 'PdfReaderContent' }).vm.$emit('pageChange', 8, 20) + await vi.advanceTimersByTimeAsync(500) + wrapper.unmount() + + expect(sessionEndSnapshots).toEqual([{ percentage: 40, pageNumber: 8 }]) + expect(mockProgressSave).toHaveBeenCalledOnce() + vi.useRealTimers() + }) + it('debounces page progress saves', async () => { vi.useFakeTimers() const wrapper = await mountReader() diff --git a/client/src/features/reader/pdf-v4/components/PdfReaderContent.vue b/client/src/features/reader/pdf-v4/components/PdfReaderContent.vue index f2ffd0b55..254b35476 100644 --- a/client/src/features/reader/pdf-v4/components/PdfReaderContent.vue +++ b/client/src/features/reader/pdf-v4/components/PdfReaderContent.vue @@ -32,7 +32,8 @@ const props = defineProps<{ documentId: string bookId: number fileId: number - initialPage: number + initialPage: number | null + initialPercentage?: number settings: PdfReaderSettings peekMode?: boolean }>() @@ -41,7 +42,7 @@ const { t } = useI18n() const emit = defineEmits<{ back: [] - pageChange: [pageNumber: number, totalPages: number] + pageChange: [pageNumber: number, totalPages: number, restored: boolean] retry: [] settingsChange: [patch: Partial] startReading: [] @@ -61,6 +62,7 @@ const sidebar = usePdfSidebarLayout() const settingsOpen = ref(false) const pendingExternalUrl = ref(null) const restoredInitialPage = ref(false) +let restoringPage: number | null = null const viewerSurface = ref(null) const selectionPopup = ref<{ getElement: () => HTMLElement | null } | null>(null) const currentScrollMode = ref(props.settings.scrollMode) @@ -250,6 +252,17 @@ function handleRetryDocument() { emit('retry') } +// A percentage without a page number was written by a client that tracks no pages, so the page +// is estimated from it the way the iOS reader does. +function resolveInitialPage(totalPages: number) { + const total = Math.max(totalPages, 1) + const percentage = props.initialPercentage ?? 0 + let page = 1 + if (props.initialPage !== null) page = props.initialPage + else if (Number.isFinite(percentage) && percentage > 0) page = Math.round((percentage / 100) * total) + return Math.min(Math.max(page, 1), total) +} + function hasDocumentPages(documentState: DocumentState) { return (documentState.document?.pageCount ?? 0) > 0 } @@ -326,14 +339,19 @@ watch( if (event.documentId !== props.documentId || !event.isInitial) return highlights.renderAll() if (restoredInitialPage.value) return - const page = Math.min(Math.max(props.initialPage, 1), event.totalPages || 1) + const page = resolveInitialPage(event.totalPages) restoredInitialPage.value = true - if (page > 1) scroll.value?.scrollToPage({ pageNumber: page }) + if (page > 1) { + restoringPage = page + scroll.value?.scrollToPage({ pageNumber: page }) + } }) const unsubscribePage = capability.onPageChange((event) => { if (event.documentId !== props.documentId || !restoredInitialPage.value) return + const restored = event.pageNumber === restoringPage + restoringPage = null const range = getPageRange(event.pageNumber, event.totalPages) - emit('pageChange', range.end, event.totalPages) + emit('pageChange', range.end, event.totalPages, restored) }) onCleanup(() => { unsubscribeLayout() diff --git a/packages/types/src/comic.ts b/packages/types/src/comic.ts new file mode 100644 index 000000000..4cf79c026 --- /dev/null +++ b/packages/types/src/comic.ts @@ -0,0 +1,4 @@ +/** `GET /cbz/files/:fileId/pages`: how many readable pages a CBZ, CBR or CB7 file has. */ +export interface ComicPageCountResponse { + pageCount: number; +} diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 0ec914000..c7bea2ee6 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -49,6 +49,7 @@ export * from "./indexer-manager"; export * from "./request-credential"; export * from "./file-write"; export * from "./epub"; +export * from "./comic"; export * from "./author"; export * from "./metadata-score"; export * from "./metadata-reminders"; diff --git a/server/src/common/sevenzip.ts b/server/src/common/sevenzip.ts index b526c126b..23242fcda 100644 --- a/server/src/common/sevenzip.ts +++ b/server/src/common/sevenzip.ts @@ -12,7 +12,13 @@ export interface SevenZipFS { readFile(path: string): Uint8Array; /** An entry's size and kind without reading it, which is what bounds an extraction. */ stat(path: string): { mode: number; size: number }; + /** Like `stat`, but describes a symbolic link itself rather than what it points at. */ + lstat(path: string): { mode: number; size: number }; + /** Resolves a path; with `follow`, the returned path is the final target of any links. */ + lookupPath(path: string, opts?: { follow?: boolean }): { path: string }; isDir(mode: number): boolean; + isFile(mode: number): boolean; + isLink(mode: number): boolean; unlink(path: string): void; rmdir(path: string): void; } diff --git a/server/src/common/utils/bootstrap.utils.test.ts b/server/src/common/utils/bootstrap.utils.test.ts index 6e3643314..90f6104f2 100644 --- a/server/src/common/utils/bootstrap.utils.test.ts +++ b/server/src/common/utils/bootstrap.utils.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi } from 'vitest'; import Fastify, { type FastifyInstance } from 'fastify'; import fastifyHelmet from '@fastify/helmet'; +import fastifyCompress from '@fastify/compress'; import { request } from 'http'; import type { AddressInfo } from 'net'; import { @@ -11,6 +12,8 @@ import { buildEmptyJsonBodyStream, registerEmptyBodyContentTypeParser, applyDeclaredBodyLimit, + applyDeclaredCompressionOptOut, + registerDeclaredCompressionOptOuts, shouldInjectEmptyJsonBody, isSecureProtocol, applyConditionalHsts, @@ -611,3 +614,66 @@ describe('applyDeclaredBodyLimit', () => { expect(route.bodyLimit).toBe(1024); }); }); + +describe('applyDeclaredCompressionOptOut', () => { + it('copies a declared opt-out onto the route option @fastify/compress reads', () => { + const route: { config?: unknown; compress?: unknown } = { config: { compress: false } }; + applyDeclaredCompressionOptOut(route); + expect(route.compress).toBe(false); + }); + + it('leaves routes without an explicit opt-out on the global compression setting', () => { + const routes: Array<{ config?: unknown; compress?: unknown }> = [ + {}, + { config: {} }, + { config: { compress: true } }, + { config: { compress: 'no' } }, + ]; + for (const route of routes) { + applyDeclaredCompressionOptOut(route); + expect(route.compress).toBeUndefined(); + } + }); +}); + +describe('registerDeclaredCompressionOptOuts (integration)', () => { + const fb2Body = `${'

text

'.repeat(500)}
`; + + async function withCompressApp(run: (app: FastifyInstance) => Promise): Promise { + const app = Fastify({ logger: false }); + registerDeclaredCompressionOptOuts(app); + await app.register(fastifyCompress, { encodings: ['gzip', 'br'] }); + app.route({ + method: 'GET', + url: '/raw', + config: { compress: false }, + handler: (_request, reply) => reply.type('application/x-fictionbook+xml').send(fb2Body), + }); + app.route({ method: 'GET', url: '/default', handler: (_request, reply) => reply.type('application/x-fictionbook+xml').send(fb2Body) }); + + try { + await app.ready(); + await run(app); + } finally { + await app.close(); + } + } + + it('serves a route that opted out byte for byte with its Content-Length', async () => { + await withCompressApp(async (app) => { + const response = await app.inject({ method: 'GET', url: '/raw', headers: { 'accept-encoding': 'gzip, br' } }); + + expect(response.headers['content-encoding']).toBeUndefined(); + expect(response.headers['content-length']).toBe(String(Buffer.byteLength(fb2Body))); + expect(response.body).toBe(fb2Body); + }); + }); + + it('keeps compressing routes that did not opt out', async () => { + await withCompressApp(async (app) => { + const response = await app.inject({ method: 'GET', url: '/default', headers: { 'accept-encoding': 'gzip' } }); + + expect(response.headers['content-encoding']).toBe('gzip'); + }); + }); +}); diff --git a/server/src/common/utils/bootstrap.utils.ts b/server/src/common/utils/bootstrap.utils.ts index f1853d775..2502c065c 100644 --- a/server/src/common/utils/bootstrap.utils.ts +++ b/server/src/common/utils/bootstrap.utils.ts @@ -172,3 +172,17 @@ export function applyDeclaredBodyLimit(route: { config?: unknown; bodyLimit?: nu export function registerDeclaredBodyLimits(fastify: FastifyInstance): void { fastify.addHook('onRoute', applyDeclaredBodyLimit); } + +/** + * `@fastify/compress` reads its per-route opt-out from the route options, which Nest's + * `@RouteConfig({ compress: false })` cannot reach. Raw file routes need it: the plugin's default + * type check gzips `+xml` and `octet-stream` bodies (FB2, unknown formats) and drops Content-Length. + * Register before the compress plugin so this hook runs first. + */ +export function applyDeclaredCompressionOptOut(route: { config?: unknown; compress?: unknown }): void { + if ((route.config as { compress?: unknown } | undefined)?.compress === false) route.compress = false; +} + +export function registerDeclaredCompressionOptOuts(fastify: FastifyInstance): void { + fastify.addHook('onRoute', applyDeclaredCompressionOptOut); +} diff --git a/server/src/common/utils/file-range.utils.test.ts b/server/src/common/utils/file-range.utils.test.ts new file mode 100644 index 000000000..f923fc661 --- /dev/null +++ b/server/src/common/utils/file-range.utils.test.ts @@ -0,0 +1,133 @@ +import { describe, expect, it } from 'vitest'; + +import { fileValidators, parseSingleByteRange, planFileResponse } from './file-range.utils'; + +const SIZE = 1000; +const MTIME_MS = Date.UTC(2026, 0, 2, 3, 4, 5, 678); +const validators = fileValidators({ id: 42, size: SIZE, mtimeMs: MTIME_MS }); + +describe('fileValidators', () => { + it('builds a strong quoted ETag and an HTTP-date Last-Modified', () => { + expect(validators.etag).toMatch(/^"[0-9a-f]{64}"$/); + expect(validators.lastModified).toBe('Fri, 02 Jan 2026 03:04:05 GMT'); + }); + + it('is stable for the same file and changes when its id, size or mtime changes', () => { + expect(fileValidators({ id: 42, size: SIZE, mtimeMs: MTIME_MS }).etag).toBe(validators.etag); + expect(fileValidators({ id: 42, size: SIZE, mtimeMs: MTIME_MS + 0.4 }).etag).toBe(validators.etag); + expect(fileValidators({ id: 43, size: SIZE, mtimeMs: MTIME_MS }).etag).not.toBe(validators.etag); + expect(fileValidators({ id: 42, size: SIZE + 1, mtimeMs: MTIME_MS }).etag).not.toBe(validators.etag); + expect(fileValidators({ id: 42, size: SIZE, mtimeMs: MTIME_MS + 1 }).etag).not.toBe(validators.etag); + }); +}); + +describe('parseSingleByteRange', () => { + it('serves a closed range', () => { + expect(parseSingleByteRange('bytes=10-19', SIZE)).toEqual({ kind: 'partial', start: 10, end: 19 }); + }); + + it('serves an open-ended range to the last byte', () => { + expect(parseSingleByteRange('bytes=900-', SIZE)).toEqual({ kind: 'partial', start: 900, end: 999 }); + }); + + it('clamps an end past the file instead of rejecting it', () => { + expect(parseSingleByteRange('bytes=900-5000', SIZE)).toEqual({ kind: 'partial', start: 900, end: 999 }); + expect(parseSingleByteRange('bytes=0-999', SIZE)).toEqual({ kind: 'partial', start: 0, end: 999 }); + }); + + it('serves suffix ranges as the last N bytes', () => { + expect(parseSingleByteRange('bytes=-100', SIZE)).toEqual({ kind: 'partial', start: 900, end: 999 }); + expect(parseSingleByteRange('bytes=-5000', SIZE)).toEqual({ kind: 'partial', start: 0, end: 999 }); + }); + + it('rejects ranges that start at or past the end, and an empty suffix', () => { + expect(parseSingleByteRange('bytes=1000-', SIZE)).toEqual({ kind: 'unsatisfiable' }); + expect(parseSingleByteRange('bytes=1000-1200', SIZE)).toEqual({ kind: 'unsatisfiable' }); + expect(parseSingleByteRange('bytes=-0', SIZE)).toEqual({ kind: 'unsatisfiable' }); + expect(parseSingleByteRange('bytes=0-', 0)).toEqual({ kind: 'unsatisfiable' }); + expect(parseSingleByteRange('bytes=-10', 0)).toEqual({ kind: 'unsatisfiable' }); + }); + + it('ignores malformed, inverted, multi-range and foreign-unit headers', () => { + for (const header of ['bytes=', 'bytes=-', 'bytes=abc-', 'bytes=20-10', 'bytes=0-1,5-6', 'items=0-1', 'pages 0-1', 'xbytes=0-1']) { + expect(parseSingleByteRange(header, SIZE)).toBeNull(); + } + }); + + it('accepts the unit case-insensitively and tolerates surrounding whitespace', () => { + expect(parseSingleByteRange(' Bytes=0-0 ', SIZE)).toEqual({ kind: 'partial', start: 0, end: 0 }); + }); +}); + +describe('planFileResponse', () => { + it('sends the whole file when no range is asked for', () => { + expect(planFileResponse(SIZE, validators, {})).toEqual({ kind: 'full' }); + expect(planFileResponse(SIZE, validators, { range: '' })).toEqual({ kind: 'full' }); + }); + + it('serves a range without If-Range', () => { + expect(planFileResponse(SIZE, validators, { range: 'bytes=500-' })).toEqual({ kind: 'partial', start: 500, end: 999 }); + }); + + it('resumes when If-Range carries the current ETag', () => { + expect(planFileResponse(SIZE, validators, { range: 'bytes=500-', ifRange: validators.etag })).toEqual({ + kind: 'partial', + start: 500, + end: 999, + }); + }); + + it('resumes when If-Range carries the current Last-Modified date', () => { + expect(planFileResponse(SIZE, validators, { range: 'bytes=500-', ifRange: validators.lastModified })).toEqual({ + kind: 'partial', + start: 500, + end: 999, + }); + }); + + it('sends the whole file when If-Range names an older ETag or date', () => { + const older = fileValidators({ id: 42, size: SIZE, mtimeMs: MTIME_MS - 60_000 }); + expect(planFileResponse(SIZE, validators, { range: 'bytes=500-', ifRange: older.etag })).toEqual({ kind: 'full' }); + expect(planFileResponse(SIZE, validators, { range: 'bytes=500-', ifRange: older.lastModified })).toEqual({ kind: 'full' }); + expect(planFileResponse(SIZE, validators, { range: 'bytes=500-', ifRange: 'not a date' })).toEqual({ kind: 'full' }); + }); + + it('never resumes from a weak If-Range tag', () => { + expect(planFileResponse(SIZE, validators, { range: 'bytes=500-', ifRange: `W/${validators.etag}` })).toEqual({ kind: 'full' }); + }); + + it('sends the whole file rather than 416 when a stale If-Range resumes past the new end', () => { + const older = fileValidators({ id: 42, size: 5000, mtimeMs: MTIME_MS - 60_000 }); + expect(planFileResponse(SIZE, validators, { range: 'bytes=4000-', ifRange: older.etag })).toEqual({ kind: 'full' }); + }); + + it('answers 416 for an unsatisfiable range that If-Range still validates', () => { + expect(planFileResponse(SIZE, validators, { range: 'bytes=4000-', ifRange: validators.etag })).toEqual({ kind: 'unsatisfiable' }); + }); + + it('sends the whole file for a range it ignores', () => { + expect(planFileResponse(SIZE, validators, { range: 'bytes=0-1,5-6' })).toEqual({ kind: 'full' }); + }); + + it('answers 304 when If-None-Match lists the current ETag on a plain GET', () => { + expect(planFileResponse(SIZE, validators, { ifNoneMatch: validators.etag })).toEqual({ kind: 'not-modified' }); + expect(planFileResponse(SIZE, validators, { ifNoneMatch: `"other", W/${validators.etag}` })).toEqual({ kind: 'not-modified' }); + expect(planFileResponse(SIZE, validators, { ifNoneMatch: '*' })).toEqual({ kind: 'not-modified' }); + }); + + it('sends the file when If-None-Match does not match', () => { + expect(planFileResponse(SIZE, validators, { ifNoneMatch: '"other"' })).toEqual({ kind: 'full' }); + }); + + it('keeps serving ranges when If-None-Match accompanies a Range header', () => { + expect(planFileResponse(SIZE, validators, { range: 'bytes=0-9', ifNoneMatch: validators.etag })).toEqual({ + kind: 'partial', + start: 0, + end: 9, + }); + }); + + it('ignores If-Range without a Range header', () => { + expect(planFileResponse(SIZE, validators, { ifRange: '"stale"' })).toEqual({ kind: 'full' }); + }); +}); diff --git a/server/src/common/utils/file-range.utils.ts b/server/src/common/utils/file-range.utils.ts new file mode 100644 index 000000000..bca94a8b8 --- /dev/null +++ b/server/src/common/utils/file-range.utils.ts @@ -0,0 +1,83 @@ +import { createHash } from 'node:crypto'; + +export interface FileValidators { + etag: string; + lastModified: string; +} + +export interface FileConditionalHeaders { + range?: string; + ifRange?: string; + ifNoneMatch?: string; +} + +export type FileResponsePlan = + { kind: 'full' } | { kind: 'partial'; start: number; end: number } | { kind: 'not-modified' } | { kind: 'unsatisfiable' }; + +const SINGLE_BYTE_RANGE = /^bytes=(\d*)-(\d*)$/i; + +/** + * Strong validators for a file on disk. Range responses and resumed downloads (URLSession sends + * `If-Range` from resume data) splice byte offsets, so the ETag must change whenever the bytes can. + */ +export function fileValidators(file: { id: number; size: number; mtimeMs: number }): FileValidators { + const mtime = Math.floor(file.mtimeMs); + return { + etag: `"${createHash('sha256').update(`${file.id}:${file.size}:${mtime}`).digest('hex')}"`, + lastModified: new Date(mtime).toUTCString(), + }; +} + +export function planFileResponse(size: number, validators: FileValidators, headers: FileConditionalHeaders): FileResponsePlan { + const range = headers.range?.trim(); + if (!range) { + return headers.ifNoneMatch !== undefined && matchesAnyEntityTag(headers.ifNoneMatch, validators.etag) + ? { kind: 'not-modified' } + : { kind: 'full' }; + } + // A stale If-Range means the client's partial copy belongs to an older file: send all of it. + if (headers.ifRange !== undefined && !ifRangeMatches(headers.ifRange, validators)) return { kind: 'full' }; + return parseSingleByteRange(range, size) ?? { kind: 'full' }; +} + +/** + * RFC 9110 single byte range. Returns null for anything this server chooses to ignore (other units, + * multiple ranges, malformed or inverted specs), which the caller answers with the whole file. + */ +export function parseSingleByteRange(header: string, size: number): FileResponsePlan | null { + const match = SINGLE_BYTE_RANGE.exec(header.trim()); + if (!match) return null; + const [, first, last] = match; + if (!first && !last) return null; + + if (!first) { + const suffixLength = Number(last); + if (suffixLength === 0 || size === 0) return { kind: 'unsatisfiable' }; + return { kind: 'partial', start: Math.max(0, size - suffixLength), end: size - 1 }; + } + + const start = Number(first); + const end = last ? Number(last) : size - 1; + if (last && end < start) return null; + if (start >= size) return { kind: 'unsatisfiable' }; + return { kind: 'partial', start, end: Math.min(end, size - 1) }; +} + +function ifRangeMatches(ifRange: string, validators: FileValidators): boolean { + const value = ifRange.trim(); + // If-Range needs a strong comparison, so a weak tag never matches. + if (value.startsWith('"') || value.startsWith('W/')) return value === validators.etag; + const date = Date.parse(value); + return Number.isFinite(date) && date === Date.parse(validators.lastModified); +} + +function matchesAnyEntityTag(header: string, etag: string): boolean { + const value = header.trim(); + if (value === '*') return true; + const opaque = stripWeakPrefix(etag); + return value.split(',').some((candidate) => stripWeakPrefix(candidate.trim()) === opaque); +} + +function stripWeakPrefix(tag: string): string { + return tag.startsWith('W/') ? tag.slice(2) : tag; +} diff --git a/server/src/main.ts b/server/src/main.ts index 7c0e17e65..7e90f9ec6 100644 --- a/server/src/main.ts +++ b/server/src/main.ts @@ -23,6 +23,7 @@ import { buildEmptyJsonBodyStream, registerConditionalHsts, registerDeclaredBodyLimits, + registerDeclaredCompressionOptOuts, registerEmptyBodyContentTypeParser, shouldInjectEmptyJsonBody, shouldServeSpaFallback, @@ -42,6 +43,7 @@ async function bootstrap() { const standardFastify = fastify as unknown as FastifyInstance; registerDeclaredBodyLimits(fastify); + registerDeclaredCompressionOptOuts(fastify); // Fastify's default JSON parser rejects empty bodies, so we inject '{}' before parsing. fastify.addHook('preParsing', (request, _reply, payload, done) => { diff --git a/server/src/modules/book/book.controller.test.ts b/server/src/modules/book/book.controller.test.ts index f5acb0a67..21e2caf7a 100644 --- a/server/src/modules/book/book.controller.test.ts +++ b/server/src/modules/book/book.controller.test.ts @@ -10,6 +10,7 @@ import { AUDITABLE_KEY } from '../../common/decorators/auditable.decorator'; import { FORBIDDEN_PERMISSION_KEY } from '../../common/decorators/forbid-permission.decorator'; import { PERMISSION_KEY } from '../../common/decorators/require-permission.decorator'; import { BookController } from './book.controller'; +import { FASTIFY_ROUTE_CONFIG_METADATA } from '@nestjs/platform-fastify/constants.js'; import { EMPTY_CONTENT_FILTER_RULES } from '@bookorbit/types'; vi.mock('archiver', () => ({ @@ -335,6 +336,121 @@ describe('BookController', () => { expect(mockCreateReadStream).not.toHaveBeenCalled(); }); + describe('file byte routes', () => { + const MTIME_MS = Date.UTC(2026, 0, 2, 3, 4, 5); + + function mockFile(bookService: ReturnType['bookService'], format = 'pdf') { + bookService.getFileInfo.mockResolvedValue({ + path: `/tmp/book.${format}`, + size: 500, + mtimeMs: MTIME_MS, + format, + bookId: 5, + originalFilename: `book.${format}`, + mediaOverlayAvailable: false, + }); + bookService.resolveDownloadFilename.mockResolvedValue(`book.${format}`); + } + + it('sends validators and a revalidate-every-time cache policy with the whole file', async () => { + const { controller, bookService } = makeController(); + const { reply, headers } = makeReply(); + mockFile(bookService); + + await controller.serveFile(1, makeUser(), undefined, reply); + + expect(headers['ETag']).toMatch(/^"[0-9a-f]{64}"$/); + expect(headers['Last-Modified']).toBe('Fri, 02 Jan 2026 03:04:05 GMT'); + expect(headers['Cache-Control']).toBe('private, no-cache'); + expect(headers['Accept-Ranges']).toBe('bytes'); + expect(headers['Content-Length']).toBe(500); + expect(reply.status).not.toHaveBeenCalled(); + expect(mockCreateReadStream).toHaveBeenCalledWith('/tmp/book.pdf'); + }); + + it('resumes from If-Range when it carries the current ETag', async () => { + const { controller, bookService } = makeController(); + const first = makeReply(); + mockFile(bookService); + await controller.serveFile(1, makeUser(), undefined, first.reply); + const etag = first.headers['ETag'] as string; + mockCreateReadStream.mockClear(); + + const { reply, headers } = makeReply(); + await controller.serveFile(1, makeUser(), 'bytes=200-', reply, etag); + + expect(reply.status).toHaveBeenCalledWith(206); + expect(headers['Content-Range']).toBe('bytes 200-499/500'); + expect(headers['Content-Length']).toBe(300); + expect(mockCreateReadStream).toHaveBeenCalledWith('/tmp/book.pdf', { start: 200, end: 499 }); + }); + + it('restarts with the whole file when If-Range names a file that has since changed', async () => { + const { controller, bookService } = makeController(); + const { reply, headers } = makeReply(); + mockFile(bookService); + + await controller.serveFile(1, makeUser(), 'bytes=200-', reply, '"stale"'); + + expect(reply.status).not.toHaveBeenCalled(); + expect(headers['Content-Range']).toBeUndefined(); + expect(headers['Content-Length']).toBe(500); + expect(mockCreateReadStream).toHaveBeenCalledWith('/tmp/book.pdf'); + }); + + it('serves suffix ranges and clamps an end past the file', async () => { + const { controller, bookService } = makeController(); + mockFile(bookService); + + const suffix = makeReply(); + await controller.serveFile(1, makeUser(), 'bytes=-100', suffix.reply); + expect(suffix.headers['Content-Range']).toBe('bytes 400-499/500'); + + const clamped = makeReply(); + await controller.serveFile(1, makeUser(), 'bytes=450-9999', clamped.reply); + expect(clamped.reply.status).toHaveBeenCalledWith(206); + expect(clamped.headers['Content-Range']).toBe('bytes 450-499/500'); + expect(clamped.headers['Content-Length']).toBe(50); + }); + + it('answers 304 when If-None-Match matches on a plain GET', async () => { + const { controller, bookService } = makeController(); + const first = makeReply(); + mockFile(bookService); + await controller.serveFile(1, makeUser(), undefined, first.reply); + mockCreateReadStream.mockClear(); + + const { reply } = makeReply(); + await controller.serveFile(1, makeUser(), undefined, reply, undefined, first.headers['ETag'] as string); + + expect(reply.status).toHaveBeenCalledWith(304); + expect(mockCreateReadStream).not.toHaveBeenCalled(); + }); + + it('honours Range on the download route with the same validators as serve', async () => { + const { controller, bookService } = makeController(); + mockFile(bookService, 'epub'); + const served = makeReply(); + await controller.serveFile(1, makeUser(), undefined, served.reply); + mockCreateReadStream.mockClear(); + + const { reply, headers } = makeReply(); + await controller.downloadFile(1, makeUser(), reply, 'bytes=100-199', served.headers['ETag'] as string); + + expect(headers['ETag']).toBe(served.headers['ETag']); + expect(headers['Content-Disposition']).toContain('attachment'); + expect(reply.status).toHaveBeenCalledWith(206); + expect(headers['Content-Range']).toBe('bytes 100-199/500'); + expect(mockCreateReadStream).toHaveBeenCalledWith('/tmp/book.epub', { start: 100, end: 199 }); + }); + + it('opts every raw file route out of response compression', () => { + for (const handler of ['serveFile', 'downloadFile', 'downloadAudiolessEpub', 'getCover', 'getThumbnail'] as const) { + expect(Reflect.getMetadata(FASTIFY_ROUTE_CONFIG_METADATA, BookController.prototype[handler])).toEqual({ compress: false }); + } + }); + }); + it('streams server-sent events for bulk metadata refresh progress', async () => { const { controller, bookService } = makeController(); const { reply, raw } = makeReply(); diff --git a/server/src/modules/book/book.controller.ts b/server/src/modules/book/book.controller.ts index 61c610c13..4e028e03a 100644 --- a/server/src/modules/book/book.controller.ts +++ b/server/src/modules/book/book.controller.ts @@ -20,9 +20,11 @@ import { ZipArchive } from 'archiver'; import { createReadStream } from 'fs'; import { stat } from 'fs/promises'; import type { FastifyReply } from 'fastify'; +import { RouteConfig } from '@nestjs/platform-fastify'; import { CurrentUser } from '../../common/decorators/current-user.decorator'; import { RequirePermission } from '../../common/decorators/require-permission.decorator'; import { contentDispositionHeader } from '../../common/utils/content-disposition.utils'; +import { fileValidators, planFileResponse, type FileConditionalHeaders } from '../../common/utils/file-range.utils'; import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils'; import { Auditable } from '../../common/decorators/auditable.decorator'; import { ForbidPermission } from '../../common/decorators/forbid-permission.decorator'; @@ -340,6 +342,7 @@ export class BookController { } @Get(':id/cover') + @RouteConfig({ compress: false }) async getCover( @Param('id', ParseIntPipe) id: number, @CurrentUser() user: RequestUser, @@ -367,6 +370,7 @@ export class BookController { } @Get(':id/thumbnail') + @RouteConfig({ compress: false }) async getThumbnail( @Param('id', ParseIntPipe) id: number, @CurrentUser() user: RequestUser, @@ -396,52 +400,39 @@ export class BookController { // These MUST come before `:id/*` routes to avoid NestJS matching 'files' as :id. @Get('files/:fileId/serve') + @RouteConfig({ compress: false }) async serveFile( @Param('fileId', ParseIntPipe) fileId: number, @CurrentUser() user: RequestUser, @Headers('range') rangeHeader: string | undefined, @Res() reply: FastifyReply, + @Headers('if-range') ifRange?: string, + @Headers('if-none-match') ifNoneMatch?: string, ) { - const { path, size, format, originalFilename } = await this.bookService.getFileInfo(fileId, user); - if (resolveAudioMimeType(format)) throw new NotFoundException('File route not found'); - const mimeType = resolveBookMimeType(format); - const filename = originalFilename; + const file = await this.bookService.getFileInfo(fileId, user); + if (resolveAudioMimeType(file.format)) throw new NotFoundException('File route not found'); - reply.header('Accept-Ranges', 'bytes'); - reply.header('Content-Disposition', contentDispositionHeader('inline', filename, 'download')); - reply.type(mimeType); - - if (rangeHeader) { - const match = /bytes=(\d+)-(\d*)/.exec(rangeHeader); - if (match) { - const start = parseInt(match[1], 10); - const end = match[2] ? parseInt(match[2], 10) : size - 1; - if (start >= size || end < start || end >= size) { - reply.status(416); - reply.header('Content-Range', `bytes */${size}`); - reply.send(); - return; - } - reply.status(206); - reply.header('Content-Range', `bytes ${start}-${end}/${size}`); - reply.header('Content-Length', end - start + 1); - reply.send(createReadStream(path, { start, end })); - return; - } - } - - reply.header('Content-Length', size); - reply.send(createReadStream(path)); + reply.header('Content-Disposition', contentDispositionHeader('inline', file.originalFilename, 'download')); + reply.type(resolveBookMimeType(file.format)); + this.sendBookFile(reply, { id: fileId, path: file.path, size: file.size, mtimeMs: file.mtimeMs }, { range: rangeHeader, ifRange, ifNoneMatch }); } @Get('files/:fileId/download') @RequirePermission(Permission.LibraryDownload) - async downloadFile(@Param('fileId', ParseIntPipe) fileId: number, @CurrentUser() user: RequestUser, @Res() reply: FastifyReply) { + @RouteConfig({ compress: false }) + async downloadFile( + @Param('fileId', ParseIntPipe) fileId: number, + @CurrentUser() user: RequestUser, + @Res() reply: FastifyReply, + @Headers('range') rangeHeader?: string, + @Headers('if-range') ifRange?: string, + @Headers('if-none-match') ifNoneMatch?: string, + ) { const event = 'book.download_file'; const startedAt = Date.now(); this.logger.log(`[${event}] [start] fileId=${fileId} userId=${user.id} - download file started`); try { - const { path, size, format, bookId, mediaOverlayAvailable } = await this.bookService.getFileInfo(fileId, user); + const { path, size, mtimeMs, format, bookId, mediaOverlayAvailable } = await this.bookService.getFileInfo(fileId, user); const mimeType = resolveBookMimeType(format); const filename = await this.bookService.resolveDownloadFilename({ bookId, @@ -450,11 +441,9 @@ export class BookController { mediaOverlayAvailable, }); - reply.header('Accept-Ranges', 'bytes'); reply.header('Content-Disposition', contentDispositionHeader('attachment', filename, 'download')); reply.type(mimeType); - reply.header('Content-Length', size); - reply.send(createReadStream(path)); + this.sendBookFile(reply, { id: fileId, path, size, mtimeMs }, { range: rangeHeader, ifRange, ifNoneMatch }); this.logger.log( `[${event}] [end] fileId=${fileId} userId=${user.id} durationMs=${Date.now() - startedAt} sizeBytes=${size} - download file completed`, ); @@ -470,6 +459,7 @@ export class BookController { @Get('files/:fileId/audioless-epub/download') @RequirePermission(Permission.LibraryDownload) + @RouteConfig({ compress: false }) async downloadAudiolessEpub(@Param('fileId', ParseIntPipe) fileId: number, @CurrentUser() user: RequestUser, @Res() reply: FastifyReply) { const event = 'book.download_audioless_epub'; const startedAt = Date.now(); @@ -832,6 +822,38 @@ export class BookController { return this.bookService.getDetail(id, user); } + private sendBookFile( + reply: FastifyReply, + file: { id: number; path: string; size: number; mtimeMs: number }, + conditionalHeaders: FileConditionalHeaders, + ): void { + const validators = fileValidators(file); + reply.header('Accept-Ranges', 'bytes'); + reply.header('ETag', validators.etag); + reply.header('Last-Modified', validators.lastModified); + reply.header('Cache-Control', 'private, no-cache'); + + const plan = planFileResponse(file.size, validators, conditionalHeaders); + switch (plan.kind) { + case 'not-modified': + reply.status(304).send(); + return; + case 'unsatisfiable': + reply.status(416).header('Content-Range', `bytes */${file.size}`).send(); + return; + case 'partial': + reply.status(206); + reply.header('Content-Range', `bytes ${plan.start}-${plan.end}/${file.size}`); + reply.header('Content-Length', plan.end - plan.start + 1); + reply.send(createReadStream(file.path, { start: plan.start, end: plan.end })); + return; + case 'full': + reply.header('Content-Length', file.size); + reply.send(createReadStream(file.path)); + return; + } + } + private createSseStream(reply: FastifyReply): { send: (event: object) => void; isClosed: () => boolean; diff --git a/server/src/modules/book/book.service.test.ts b/server/src/modules/book/book.service.test.ts index dd23e9c85..82250f515 100644 --- a/server/src/modules/book/book.service.test.ts +++ b/server/src/modules/book/book.service.test.ts @@ -951,13 +951,14 @@ describe('BookService', () => { it('returns file info with unknown format fallback', async () => { const { service, bookRepo } = makeService(); bookRepo.findFileById.mockResolvedValue({ id: 10, absolutePath: '/books/test.book', format: null, bookId: 1, libraryId: 7 }); - mockStat.mockResolvedValue({ size: 1234 } as never); + mockStat.mockResolvedValue({ size: 1234, mtimeMs: 1_700_000_000_000 } as never); const result = await service.getFileInfo(10, makeUser()); expect(result).toEqual({ path: '/books/test.book', size: 1234, + mtimeMs: 1_700_000_000_000, format: 'unknown', bookId: 1, originalFilename: 'test.book', diff --git a/server/src/modules/book/book.service.ts b/server/src/modules/book/book.service.ts index 25fed5dd0..7042a9851 100644 --- a/server/src/modules/book/book.service.ts +++ b/server/src/modules/book/book.service.ts @@ -1453,11 +1453,20 @@ export class BookService { async getFileInfo( fileId: number, user: RequestUser, - ): Promise<{ path: string; size: number; format: string; bookId: number; originalFilename: string; mediaOverlayAvailable: boolean }> { + ): Promise<{ + path: string; + size: number; + mtimeMs: number; + format: string; + bookId: number; + originalFilename: string; + mediaOverlayAvailable: boolean; + }> { const file = await this.verifyFileAccess(fileId, user); let size: number; + let mtimeMs: number; try { - ({ size } = await stat(file.absolutePath)); + ({ size, mtimeMs } = await stat(file.absolutePath)); } catch (err) { if (this.isMissingFilesystemEntry(err)) { throw new NotFoundException(`File ${fileId} not found on disk`); @@ -1468,6 +1477,7 @@ export class BookService { return { path: file.absolutePath, size, + mtimeMs, format: file.format ?? 'unknown', bookId: file.bookId, originalFilename, diff --git a/server/src/modules/book/dto/book-dto-validation.test.ts b/server/src/modules/book/dto/book-dto-validation.test.ts index ddb54eeef..41d53283b 100644 --- a/server/src/modules/book/dto/book-dto-validation.test.ts +++ b/server/src/modules/book/dto/book-dto-validation.test.ts @@ -149,6 +149,17 @@ describe('Book DTO validation', () => { expect((await errorsFor(SaveProgressDto, { percentage: 50, koreaderProgress: 123 })).length).toBeGreaterThan(0); }); + it('accepts only integer page numbers that fit the reading_progress column', async () => { + expect((await errorsFor(SaveProgressDto, { percentage: 50 })).length).toBe(0); + expect((await errorsFor(SaveProgressDto, { percentage: 50, pageNumber: null })).length).toBe(0); + expect((await errorsFor(SaveProgressDto, { percentage: 50, pageNumber: 0 })).length).toBe(0); + expect((await errorsFor(SaveProgressDto, { percentage: 50, pageNumber: 2147483647 })).length).toBe(0); + + expect((await errorsFor(SaveProgressDto, { percentage: 50, pageNumber: 2.5 })).length).toBeGreaterThan(0); + expect((await errorsFor(SaveProgressDto, { percentage: 50, pageNumber: -1 })).length).toBeGreaterThan(0); + expect((await errorsFor(SaveProgressDto, { percentage: 50, pageNumber: 2147483648 })).length).toBeGreaterThan(0); + }); + it('requires non-empty search text and bounds search limit', async () => { const valid = plainToInstance(SearchBooksDto, { q: 'dune', limit: '20' }); const validErrors = await validate(valid); diff --git a/server/src/modules/book/dto/save-progress.dto.ts b/server/src/modules/book/dto/save-progress.dto.ts index 11655f545..24196ba73 100644 --- a/server/src/modules/book/dto/save-progress.dto.ts +++ b/server/src/modules/book/dto/save-progress.dto.ts @@ -1,4 +1,4 @@ -import { IsIn, IsNumber, IsOptional, IsString, Max, Min, ValidateIf } from 'class-validator'; +import { IsIn, IsInt, IsNumber, IsOptional, IsString, Max, Min, ValidateIf } from 'class-validator'; /** * Which of a file's two positions this write moved. Text is the default so a client that predates @@ -17,7 +17,9 @@ export class SaveProgressDto { cfi?: string | null; @ValidateIf((o: SaveProgressDto) => o.pageNumber != null) - @IsNumber() + @IsInt() + @Min(0) + @Max(2147483647) pageNumber?: number | null; @IsNumber() diff --git a/server/src/modules/koreader/koreader.service.test.ts b/server/src/modules/koreader/koreader.service.test.ts index f1d7794bd..6835baa82 100644 --- a/server/src/modules/koreader/koreader.service.test.ts +++ b/server/src/modules/koreader/koreader.service.test.ts @@ -1174,6 +1174,60 @@ describe('KoreaderService', () => { expect(mockPositionConverter.cfiPointToXpointer).not.toHaveBeenCalled(); }); + it.each(['pdf', 'cbz', 'cbr', 'cb7'])('serves the stored page to KOReader for a %s written by a BookOrbit client', async (format) => { + const readerTime = new Date('2026-02-01T11:00:00.000Z'); + mockRepo.resolveBookFileByHash.mockResolvedValue({ id: 10, format }); + mockRepo.getLatestDeviceProgress.mockResolvedValue(null); + mockRepo.getReadingProgress.mockResolvedValue({ percentage: 42, cfi: null, pageNumber: 37, koreaderProgress: null, updatedAt: readerTime }); + + await expect(service.getProgress(7, 'doc-hash')).resolves.toEqual({ + document: 'doc-hash', + percentage: 0.42, + progress: '37', + device: 'web', + device_id: 'bookorbit-web', + timestamp: Math.floor(readerTime.getTime() / 1000), + }); + expect(mockPositionConverter.cfiPointToXpointer).not.toHaveBeenCalled(); + }); + + it('keeps the page KOReader itself stored for a paged file', async () => { + mockRepo.resolveBookFileByHash.mockResolvedValue({ id: 10, format: 'pdf' }); + mockRepo.getLatestDeviceProgress.mockResolvedValue(null); + mockRepo.getReadingProgress.mockResolvedValue({ percentage: 12, pageNumber: 12, koreaderProgress: '12', updatedAt: new Date() }); + + await expect(service.getProgress(7, 'doc-hash')).resolves.toEqual(expect.objectContaining({ progress: '12' })); + }); + + it('prefers the stored page over a non-page pointer on a paged file', async () => { + mockRepo.resolveBookFileByHash.mockResolvedValue({ id: 10, format: 'cbz' }); + mockRepo.getLatestDeviceProgress.mockResolvedValue(null); + mockRepo.getReadingProgress.mockResolvedValue({ + percentage: 12, + pageNumber: 9, + koreaderProgress: '/body/DocFragment[2]/body', + updatedAt: new Date(), + }); + + await expect(service.getProgress(7, 'doc-hash')).resolves.toEqual(expect.objectContaining({ progress: '9' })); + }); + + it.each([null, 0])('serves no position for a paged file whose stored page is %s', async (pageNumber) => { + mockRepo.resolveBookFileByHash.mockResolvedValue({ id: 10, format: 'pdf' }); + mockRepo.getLatestDeviceProgress.mockResolvedValue(null); + mockRepo.getReadingProgress.mockResolvedValue({ percentage: 5, pageNumber, koreaderProgress: null, updatedAt: new Date() }); + + await expect(service.getProgress(7, 'doc-hash')).resolves.toEqual(expect.objectContaining({ progress: null, percentage: 0.05 })); + }); + + it('never turns a stored page into a position for a reflowable file', async () => { + mockRepo.resolveBookFileByHash.mockResolvedValue({ id: 10, format: 'epub' }); + mockRepo.getLatestDeviceProgress.mockResolvedValue(null); + mockRepo.getReadingProgress.mockResolvedValue({ percentage: 5, cfi: null, pageNumber: 4, koreaderProgress: null, updatedAt: new Date() }); + + await expect(service.getProgress(7, 'doc-hash')).resolves.toEqual(expect.objectContaining({ progress: null })); + }); + it('returns null when neither device nor web reader progress exists', async () => { mockRepo.resolveBookFileByHash.mockResolvedValue({ id: 10, bookId: 20, libraryId: 1 }); mockRepo.getLatestDeviceProgress.mockResolvedValue(null); diff --git a/server/src/modules/koreader/koreader.service.ts b/server/src/modules/koreader/koreader.service.ts index 03572411e..0145980a1 100644 --- a/server/src/modules/koreader/koreader.service.ts +++ b/server/src/modules/koreader/koreader.service.ts @@ -547,7 +547,13 @@ export class KoreaderService { if (readingProg) { let xpointer = readingProg.koreaderProgress ?? null; - if (!xpointer && readingProg.cfi && !isPagedReadingFormat(bookFile.format)) { + const paged = isPagedReadingFormat(bookFile.format); + // A BookOrbit client write clears koreaderProgress for a paged file and keeps only the + // page, so the page is what KOReader has to be sent back, in the form it reports itself. + if (paged && parseKoreaderPageNumber(xpointer) === null && isServablePageNumber(readingProg.pageNumber)) { + xpointer = String(readingProg.pageNumber); + } + if (!xpointer && readingProg.cfi && !paged) { const startedAt = Date.now(); try { const outcome = await this.positionConverter.cfiPointToXpointer({ bookFileId: bookFile.id, cfi: readingProg.cfi }); @@ -898,3 +904,7 @@ function toBookorbitPercentage(koreaderPct: number): number { function toKoreaderPercentage(bookorbitPct: number): number { return Math.round(bookorbitPct * 100) / 10000; } + +function isServablePageNumber(pageNumber: number | null | undefined): pageNumber is number { + return typeof pageNumber === 'number' && Number.isSafeInteger(pageNumber) && pageNumber >= 1; +} diff --git a/server/src/modules/reader/cbz/cb7-extraction.test.ts b/server/src/modules/reader/cbz/cb7-extraction.test.ts new file mode 100644 index 000000000..f21ecbd23 --- /dev/null +++ b/server/src/modules/reader/cbz/cb7-extraction.test.ts @@ -0,0 +1,86 @@ +import { compareComicPagePaths, listExtractedFiles, removeExtractedTree } from './cb7-extraction'; +import { FakeSevenZip } from './test-helpers'; + +/** The order CB7 pages had when the archive was extracted flat: file names alone, naturally sorted. */ +function flatOrder(names: string[]): string[] { + return [...names].sort((a, b) => a.localeCompare(b, undefined, { numeric: true, sensitivity: 'base' })); +} + +function basename(path: string): string { + return path.slice(path.lastIndexOf('/') + 1); +} + +describe('compareComicPagePaths', () => { + it('keeps the flat order for an archive with no folders', () => { + const names = ['10.jpg', '2.jpg', '1.jpg', 'cover.png', 'Page 3.jpg', 'page 20.jpg']; + + expect([...names].sort(compareComicPagePaths)).toEqual(flatOrder(names)); + }); + + it.each([ + [['vol2/003.jpg', 'vol1/001.jpg', 'vol10/002.jpg']], + [['b/c10.jpg', 'a/c9.jpg', 'c/c1.jpg', 'Z.jpg', 'a/b.jpg']], + [['scans/credits.png', 'chapter 1/p001.jpg', 'chapter 2/p002.jpg', 'chapter 10/p003.jpg', 'cover.jpg']], + ])('orders foldered pages exactly as the flat extraction did when file names are unique: %j', (paths) => { + const sorted = [...paths].sort(compareComicPagePaths).map(basename); + + expect(sorted).toEqual(flatOrder(paths.map(basename))); + }); + + it('keeps pages that share a file name, ordered by folder', () => { + const paths = ['ch2/001.jpg', 'ch10/001.jpg', 'ch1/002.jpg', 'ch1/001.jpg', 'ch2/002.jpg']; + + expect([...paths].sort(compareComicPagePaths)).toEqual(['ch1/001.jpg', 'ch2/001.jpg', 'ch10/001.jpg', 'ch1/002.jpg', 'ch2/002.jpg']); + }); +}); + +describe('listExtractedFiles', () => { + it('walks folders, including hidden ones, and returns paths relative to the root', () => { + const sevenZip = new FakeSevenZip(); + sevenZip.addFile('/work/001.jpg', 'a'); + sevenZip.addFile('/work/ch1/001.jpg', 'b'); + sevenZip.addFile('/work/ch1/deep/002.jpg', 'c'); + sevenZip.addFile('/work/.thumbs/t.jpg', 'd'); + sevenZip.addFile('/elsewhere/other.jpg', 'e'); + + expect(listExtractedFiles(sevenZip.FS, '/work').sort()).toEqual(['.thumbs/t.jpg', '001.jpg', 'ch1/001.jpg', 'ch1/deep/002.jpg']); + }); + + it('keeps links to files inside the root and drops links that leave it, dangle or name a folder', () => { + const sevenZip = new FakeSevenZip({ + 'ch1/001.jpg': 'page', + 'ch1/same.jpg': { link: '001.jpg' }, + 'ch1/up.jpg': { link: '../ch1/001.jpg' }, + 'ch1/escape.jpg': { link: '../../secret/page.jpg' }, + 'ch1/absolute.jpg': { link: '/secret/page.jpg' }, + 'ch1/dangling.jpg': { link: 'missing.jpg' }, + 'ch1/folder.jpg': { link: '../ch1' }, + }); + sevenZip.addFile('/secret/page.jpg', 'not yours'); + sevenZip.FS.mkdir('/work'); + sevenZip.callMain(['x', '/archive.7z', '-o/work', '-y']); + + expect(listExtractedFiles(sevenZip.FS, '/work').sort()).toEqual(['ch1/001.jpg', 'ch1/same.jpg', 'ch1/up.jpg']); + }); + + it('returns nothing for a root that does not exist', () => { + expect(listExtractedFiles(new FakeSevenZip().FS, '/missing')).toEqual([]); + }); +}); + +describe('removeExtractedTree', () => { + it('removes a nested tree, links included, and leaves everything else', () => { + const sevenZip = new FakeSevenZip({ 'a/b/1.jpg': 'x', 'a/2.jpg': 'y', 'link.jpg': { link: 'a/2.jpg' } }); + sevenZip.addFile('/keep/3.jpg', 'z'); + sevenZip.FS.mkdir('/work'); + sevenZip.callMain(['x', '/archive.7z', '-o/work', '-y']); + + removeExtractedTree(sevenZip.FS, '/work'); + + expect(sevenZip.paths()).toEqual(['/keep', '/keep/3.jpg']); + }); + + it('ignores a folder that is already gone', () => { + expect(() => removeExtractedTree(new FakeSevenZip().FS, '/missing')).not.toThrow(); + }); +}); diff --git a/server/src/modules/reader/cbz/cb7-extraction.ts b/server/src/modules/reader/cbz/cb7-extraction.ts new file mode 100644 index 000000000..c0eb9e068 --- /dev/null +++ b/server/src/modules/reader/cbz/cb7-extraction.ts @@ -0,0 +1,96 @@ +import type { SevenZipFS } from '../../../common/sevenzip'; + +function naturalCompare(a: string, b: string): number { + return a.localeCompare(b, undefined, { numeric: true, sensitivity: 'base' }); +} + +export function pathBasename(path: string): string { + return path.slice(path.lastIndexOf('/') + 1); +} + +/** + * Page order for a CB7 extracted with its folders kept. CB7 pages used to be extracted flat and + * sorted by file name alone, and saved reading positions are page indexes into that order, so the + * file name stays the primary key. The full path only separates pages that share a name, which the + * flat extraction collapsed into one. + */ +export function compareComicPagePaths(a: string, b: string): number { + return naturalCompare(pathBasename(a), pathBasename(b)) || naturalCompare(a, b); +} + +function entryKind(fs: SevenZipFS, root: string, path: string): 'dir' | 'file' | null { + try { + const { mode } = fs.lstat(path); + if (fs.isDir(mode)) return 'dir'; + if (fs.isFile(mode)) return 'file'; + if (!fs.isLink(mode)) return null; + // 7-Zip already refuses to extract a link that points outside the output folder. Checking + // the resolved target again keeps that true here, so a page can never read another file's + // working tree in the shared WASM filesystem. Links to folders are never followed. + const target = fs.lookupPath(path, { follow: true }).path; + return target.startsWith(`${root}/`) && fs.isFile(fs.stat(target).mode) ? 'file' : null; + } catch { + return null; + } +} + +/** Every readable file under `root`, as paths relative to it. Nothing is read into our heap. */ +export function listExtractedFiles(fs: SevenZipFS, root: string): string[] { + const files: string[] = []; + const dirs = ['']; + + for (let i = 0; i < dirs.length; i++) { + const relativeDir = dirs[i]!; + let names: string[]; + try { + names = fs.readdir(relativeDir ? `${root}/${relativeDir}` : root); + } catch { + continue; + } + + for (const name of names) { + if (name === '.' || name === '..') continue; + const relativePath = relativeDir ? `${relativeDir}/${name}` : name; + const kind = entryKind(fs, root, `${root}/${relativePath}`); + if (kind === 'dir') dirs.push(relativePath); + else if (kind === 'file') files.push(relativePath); + } + } + + return files; +} + +/** Best effort: a leftover tree only costs WASM memory, never a failed request. */ +export function removeExtractedTree(fs: SevenZipFS, path: string): void { + let names: string[]; + try { + names = fs.readdir(path).filter((name) => name !== '.' && name !== '..'); + } catch { + return; + } + + for (const name of names) { + const child = `${path}/${name}`; + let isDir: boolean; + try { + isDir = fs.isDir(fs.lstat(child).mode); + } catch { + continue; + } + if (isDir) { + removeExtractedTree(fs, child); + continue; + } + try { + fs.unlink(child); + } catch { + // Best effort, as above. + } + } + + try { + fs.rmdir(path); + } catch { + // Already gone, or still holds an entry that could not be removed. + } +} diff --git a/server/src/modules/reader/cbz/cbz.controller.test.ts b/server/src/modules/reader/cbz/cbz.controller.test.ts index 35ca14b22..09c05dd75 100644 --- a/server/src/modules/reader/cbz/cbz.controller.test.ts +++ b/server/src/modules/reader/cbz/cbz.controller.test.ts @@ -24,21 +24,45 @@ describe('CbzController', () => { expect(cbzService.getPageCount).toHaveBeenCalledWith(12, user); }); - it('sets response headers and streams page bytes', async () => { + it('asks clients to revalidate pages and streams page bytes', async () => { const user = { id: 7, isSuperuser: false, permissions: [] } as any; const stream = new PassThrough(); const reply = { header: vi.fn(), type: vi.fn(), + status: vi.fn(), send: vi.fn(), }; - cbzService.streamPage.mockResolvedValue({ stream, mimeType: 'image/png' }); + cbzService.streamPage.mockResolvedValue({ notModified: false, etag: '"page-tag"', stream, mimeType: 'image/png' }); await controller.getPage(12, 3, user, reply as any); - expect(cbzService.streamPage).toHaveBeenCalledWith(12, 3, user); - expect(reply.header).toHaveBeenCalledWith('Cache-Control', 'public, max-age=31536000, immutable'); + expect(cbzService.streamPage).toHaveBeenCalledWith(12, 3, user, undefined); + expect(reply.header).toHaveBeenCalledWith('Cache-Control', 'private, no-cache'); + expect(reply.header).toHaveBeenCalledWith('ETag', '"page-tag"'); expect(reply.type).toHaveBeenCalledWith('image/png'); + expect(reply.status).not.toHaveBeenCalled(); expect(reply.send).toHaveBeenCalledWith(stream); }); + + it('answers 304 with the validator when the client already holds the page', async () => { + const user = { id: 7, isSuperuser: false, permissions: [] } as any; + const reply = { + header: vi.fn(), + type: vi.fn(), + status: vi.fn(), + send: vi.fn(), + }; + reply.status.mockReturnValue(reply); + cbzService.streamPage.mockResolvedValue({ notModified: true, etag: '"page-tag"' }); + + await controller.getPage(12, 3, user, reply as any, '"page-tag"'); + + expect(cbzService.streamPage).toHaveBeenCalledWith(12, 3, user, '"page-tag"'); + expect(reply.header).toHaveBeenCalledWith('Cache-Control', 'private, no-cache'); + expect(reply.header).toHaveBeenCalledWith('ETag', '"page-tag"'); + expect(reply.status).toHaveBeenCalledWith(304); + expect(reply.send).toHaveBeenCalledWith(); + expect(reply.type).not.toHaveBeenCalled(); + }); }); diff --git a/server/src/modules/reader/cbz/cbz.controller.ts b/server/src/modules/reader/cbz/cbz.controller.ts index 302e1985e..a36e99506 100644 --- a/server/src/modules/reader/cbz/cbz.controller.ts +++ b/server/src/modules/reader/cbz/cbz.controller.ts @@ -1,15 +1,19 @@ -import { Controller, Get, Param, ParseIntPipe, Res } from '@nestjs/common'; +import { Controller, Get, Headers, Param, ParseIntPipe, Res } from '@nestjs/common'; +import type { ComicPageCountResponse } from '@bookorbit/types'; import type { FastifyReply } from 'fastify'; import { CurrentUser } from '../../../common/decorators/current-user.decorator'; import type { RequestUser } from '../../../common/types/request-user'; import { CbzService } from './cbz.service'; +/** Page URLs carry no version, so a page is revalidated on every use rather than trusted. */ +const PAGE_CACHE_CONTROL = 'private, no-cache'; + @Controller('cbz') export class CbzController { constructor(private readonly cbzService: CbzService) {} @Get('files/:fileId/pages') - async getPageCount(@Param('fileId', ParseIntPipe) fileId: number, @CurrentUser() user: RequestUser) { + async getPageCount(@Param('fileId', ParseIntPipe) fileId: number, @CurrentUser() user: RequestUser): Promise { return { pageCount: await this.cbzService.getPageCount(fileId, user) }; } @@ -19,10 +23,16 @@ export class CbzController { @Param('pageIndex', ParseIntPipe) pageIndex: number, @CurrentUser() user: RequestUser, @Res() reply: FastifyReply, + @Headers('if-none-match') ifNoneMatch?: string, ) { - const { stream, mimeType } = await this.cbzService.streamPage(fileId, pageIndex, user); - reply.header('Cache-Control', 'public, max-age=31536000, immutable'); - reply.type(mimeType); - reply.send(stream); + const page = await this.cbzService.streamPage(fileId, pageIndex, user, ifNoneMatch); + reply.header('Cache-Control', PAGE_CACHE_CONTROL); + reply.header('ETag', page.etag); + if (page.notModified) { + reply.status(304).send(); + return; + } + reply.type(page.mimeType); + reply.send(page.stream); } } diff --git a/server/src/modules/reader/cbz/cbz.service.sevenzip.test.ts b/server/src/modules/reader/cbz/cbz.service.sevenzip.test.ts new file mode 100644 index 000000000..58f85e910 --- /dev/null +++ b/server/src/modules/reader/cbz/cbz.service.sevenzip.test.ts @@ -0,0 +1,102 @@ +import { mkdtemp, rm, utimes, writeFile } from 'fs/promises'; +import { join } from 'path'; +import { tmpdir } from 'os'; + +import { createSevenZipTempId, getSevenZip } from '../../../common/sevenzip'; +import { CbzService, type ComicPageResult } from './cbz.service'; + +/** + * CB7 page listing walks a tree the real 7-Zip wrote, so these cases run against the real WASM + * module: the unit tests stand on a fake filesystem, and this is where the two would disagree. + */ +describe('CbzService CB7 pages against the real 7z module', () => { + const user = { id: 9, isSuperuser: false, permissions: [] } as any; + let workspace: string; + + beforeEach(async () => { + workspace = await mkdtemp(join(tmpdir(), 'bookorbit-cb7-real-')); + }); + + afterEach(async () => { + await rm(workspace, { recursive: true, force: true }); + }); + + async function buildSevenZip(files: Record): Promise { + const sevenZip = await getSevenZip(); + const root = `/${createSevenZipTempId('build')}`; + sevenZip.FS.mkdir(root); + for (const [name, contents] of Object.entries(files)) { + const segments = name.split('/'); + for (let depth = 1; depth < segments.length; depth++) { + try { + sevenZip.FS.mkdir(`${root}/${segments.slice(0, depth).join('/')}`); + } catch { + // already there + } + } + const fd = sevenZip.FS.open(`${root}/${name}`, 'w+'); + const bytes = new TextEncoder().encode(contents); + sevenZip.FS.write(fd, bytes, 0, bytes.length); + sevenZip.FS.close(fd); + } + sevenZip.callMain(['a', `${root}/comic.7z`, `${root}/*`, '-y']); + return sevenZip.FS.readFile(`${root}/comic.7z`); + } + + /** The page list the flat `e` extraction produced before folders were kept. */ + async function flatPageNames(archive: Uint8Array): Promise { + const sevenZip = await getSevenZip(); + const root = `/${createSevenZipTempId('flat')}`; + sevenZip.FS.mkdir(root); + const fd = sevenZip.FS.open(`${root}.7z`, 'w+'); + sevenZip.FS.write(fd, archive, 0, archive.length); + sevenZip.FS.close(fd); + sevenZip.callMain(['e', `${root}.7z`, `-o${root}`, '-y']); + return sevenZip.FS.readdir(root) + .filter((name) => name !== '.' && name !== '..' && /\.(jpe?g|png)$/i.test(name) && !name.startsWith('.')) + .sort((a, b) => a.localeCompare(b, undefined, { numeric: true, sensitivity: 'base' })); + } + + async function pageTexts(service: CbzService, fileId: number): Promise { + const count = await service.getPageCount(fileId, user); + const texts: string[] = []; + for (let index = 0; index < count; index++) { + const result: ComicPageResult = await service.streamPage(fileId, index, user); + if (result.notModified) throw new Error('expected page bytes'); + const chunks: Buffer[] = []; + for await (const chunk of result.stream) chunks.push(Buffer.from(chunk)); + texts.push(Buffer.concat(chunks).toString()); + } + return texts; + } + + it('keeps the flat page order for an archive whose page names are unique', async () => { + const files = { + 'Chapter 10/c10.jpg': 'c10.jpg', + 'Chapter 2/c2.jpg': 'c2.jpg', + 'cover.jpg': 'cover.jpg', + 'extras/.thumbs/z1.jpg': 'z1.jpg', + 'extras/credits.png': 'credits.png', + 'extras/._credits.png': 'hidden', + }; + const archive = await buildSevenZip(files); + const path = join(workspace, 'unique.cb7'); + await writeFile(path, archive); + const service = new CbzService({ verifyFileAccess: vi.fn().mockResolvedValue({ format: 'cb7', absolutePath: path }) } as any); + + await expect(pageTexts(service, 31)).resolves.toEqual(await flatPageNames(archive)); + }); + + it('serves pages that share a name in different folders, and rereads a rewritten file', async () => { + const path = join(workspace, 'nested.cb7'); + await writeFile(path, await buildSevenZip({ 'ch2/001.jpg': 'ch2-1', 'ch1/001.jpg': 'ch1-1', 'ch1/002.jpg': 'ch1-2' })); + const service = new CbzService({ verifyFileAccess: vi.fn().mockResolvedValue({ format: 'cb7', absolutePath: path }) } as any); + + await expect(pageTexts(service, 32)).resolves.toEqual(['ch1-1', 'ch2-1', 'ch1-2']); + + await writeFile(path, await buildSevenZip({ 'ch1/001.jpg': 'rewritten' })); + await utimes(path, new Date(), new Date(Date.now() + 60_000)); + + await expect(pageTexts(service, 32)).resolves.toEqual(['rewritten']); + }); +}); diff --git a/server/src/modules/reader/cbz/cbz.service.test.ts b/server/src/modules/reader/cbz/cbz.service.test.ts index fd1b6a140..36b73bde1 100644 --- a/server/src/modules/reader/cbz/cbz.service.test.ts +++ b/server/src/modules/reader/cbz/cbz.service.test.ts @@ -1,4 +1,4 @@ -vi.mock('fs/promises', () => ({ open: vi.fn(), readFile: vi.fn() })); +vi.mock('fs/promises', () => ({ open: vi.fn(), readFile: vi.fn(), stat: vi.fn() })); vi.mock('fs', () => ({ createReadStream: vi.fn() })); vi.mock('zlib', async () => { const actual = await vi.importActual('zlib'); @@ -16,22 +16,25 @@ vi.mock('node-unrar-js', () => { return { createExtractorFromData: vi.fn(), UnrarError: MockUnrarError }; }); -vi.mock('../../../common/sevenzip', () => ({ getSevenZip: vi.fn(), createSevenZipTempId: (prefix: string) => `${prefix}_test` })); +vi.mock('../../../common/sevenzip', () => ({ getSevenZip: vi.fn(), createSevenZipTempId: vi.fn() })); vi.mock('../../../common/comic-format-detect', () => ({ detectComicContainerFormat: vi.fn().mockImplementation((_path: string, fmt: string) => Promise.resolve(fmt)), })); import { NotFoundException, UnprocessableEntityException } from '@nestjs/common'; import { createReadStream } from 'fs'; -import { open, readFile } from 'fs/promises'; +import { open, readFile, stat } from 'fs/promises'; import { createExtractorFromData } from 'node-unrar-js'; import { createInflateRaw, deflateRawSync } from 'zlib'; -import { getSevenZip } from '../../../common/sevenzip'; +import { createSevenZipTempId, getSevenZip } from '../../../common/sevenzip'; import { detectComicContainerFormat } from '../../../common/comic-format-detect'; -import { CbzService } from './cbz.service'; +import { CbzService, matchesIfNoneMatch, type ComicPageResult } from './cbz.service'; +import { FakeSevenZip } from './test-helpers'; const mockReadFile = readFile as MockedFunction; +const mockStat = stat as MockedFunction; +const mockCreateSevenZipTempId = createSevenZipTempId as MockedFunction; const mockOpen = open as MockedFunction; const mockCreateReadStream = createReadStream as unknown as MockedFunction; const mockCreateInflateRaw = createInflateRaw as unknown as MockedFunction; @@ -121,6 +124,15 @@ function mockCbzFile(buf: Buffer): void { }); } +function fileStats(mtimeMs: number, size: number): Awaited> { + return { mtimeMs, size } as Awaited>; +} + +function served(result: ComicPageResult): Extract { + if (result.notModified) throw new Error('expected page bytes, got notModified'); + return result; +} + async function readStream(stream: NodeJS.ReadableStream): Promise { const chunks: Buffer[] = []; for await (const chunk of stream) { @@ -139,6 +151,9 @@ describe('CbzService', () => { vi.resetAllMocks(); service = new CbzService(bookService as any); mockDetectComicContainerFormat.mockImplementation((_path, fmt) => Promise.resolve(fmt)); + mockStat.mockResolvedValue(fileStats(1_000, 100)); + let tempIds = 0; + mockCreateSevenZipTempId.mockImplementation((prefix) => `${prefix}_${++tempIds}`); }); it('counts visible CBZ images and caches the parsed index', async () => { @@ -172,7 +187,7 @@ describe('CbzService', () => { mockCreateReadStream.mockReturnValue(rawStream as unknown as ReturnType); mockCreateInflateRaw.mockReturnValue(inflateStream as ReturnType); - const result = await service.streamPage(2, 0, user); + const result = served(await service.streamPage(2, 0, user)); expect(mockCreateReadStream).toHaveBeenCalledWith( '/books/two.cbz', @@ -190,7 +205,7 @@ describe('CbzService', () => { mockCbzFile(buildCbzBuffer([{ name: 'cover.png', data: Buffer.from([3, 3]), compression: 0 }])); mockCreateReadStream.mockReturnValue(rawStream as unknown as ReturnType); - const result = await service.streamPage(3, 0, user); + const result = served(await service.streamPage(3, 0, user)); expect(mockCreateInflateRaw).not.toHaveBeenCalled(); expect(result.stream).toBe(rawStream); @@ -239,7 +254,7 @@ describe('CbzService', () => { } as any); await expect(service.getPageCount(6, user)).resolves.toBe(2); - const streamed = await service.streamPage(6, 0, user); + const streamed = served(await service.streamPage(6, 0, user)); await expect(readStream(streamed.stream)).resolves.toEqual(Buffer.from([7, 8, 9])); expect(streamed.mimeType).toBe('image/jpeg'); await expect(service.getPageCount(6, user)).resolves.toBe(2); @@ -314,27 +329,52 @@ describe('CbzService', () => { await expect(service.streamPage(7, 1, user)).rejects.toThrow(NotFoundException); }); - it('extracts CB7 pages once and reuses cached page list', async () => { - const fsApi = { - open: vi.fn().mockReturnValue(3), - write: vi.fn(), - close: vi.fn(), - mkdir: vi.fn(), - readdir: vi.fn().mockReturnValue(['.', '..', '10.jpg', '2.png', '.hidden.jpg', 'notes.txt']), - readFile: vi.fn().mockReturnValue(Uint8Array.from([1, 2, 3])), - }; - const sevenZip = { FS: fsApi, callMain: vi.fn() }; - + it('extracts CB7 pages once with folders kept, reuses the page list and drops the archive copy', async () => { + const sevenZip = new FakeSevenZip({ '10.jpg': 'ten', '2.png': 'two', '.hidden.jpg': 'h', 'notes.txt': 'n' }); bookService.verifyFileAccess.mockResolvedValue({ format: 'cb7', absolutePath: '/books/eight.cb7' }); mockReadFile.mockResolvedValue(Buffer.from('7z') as unknown as Awaited>); mockGetSevenZip.mockResolvedValue(sevenZip as any); await expect(service.getPageCount(8, user)).resolves.toBe(2); - const result = await service.streamPage(8, 0, user); - await expect(readStream(result.stream)).resolves.toEqual(Buffer.from([1, 2, 3])); + const result = served(await service.streamPage(8, 0, user)); + await expect(readStream(result.stream)).resolves.toEqual(Buffer.from('two')); expect(result.mimeType).toBe('image/png'); - expect(fsApi.readFile).toHaveBeenCalledWith('/p8/2.png'); - expect(sevenZip.callMain).toHaveBeenCalledTimes(1); + expect(sevenZip.calls).toEqual([['x', '/comic8_1.7z', '-o/comic8_1', '-y']]); + expect(sevenZip.paths()).not.toContain('/comic8_1.7z'); + expect(mockReadFile).toHaveBeenCalledTimes(1); + }); + + it('keeps CB7 pages that share a name in different folders, in file-name order', async () => { + const sevenZip = new FakeSevenZip({ + 'ch2/001.jpg': 'c2p1', + 'ch1/002.jpg': 'c1p2', + 'ch1/001.jpg': 'c1p1', + 'ch1/.hidden.jpg': 'hidden', + '.thumbs/x.jpg': 'thumb', + }); + bookService.verifyFileAccess.mockResolvedValue({ format: 'cb7', absolutePath: '/books/nested.cb7' }); + mockReadFile.mockResolvedValue(Buffer.from('7z') as unknown as Awaited>); + mockGetSevenZip.mockResolvedValue(sevenZip as any); + + await expect(service.getPageCount(15, user)).resolves.toBe(4); + const pages: string[] = []; + for (let index = 0; index < 4; index++) { + pages.push((await readStream(served(await service.streamPage(15, index, user)).stream)).toString()); + } + expect(pages).toEqual(['c1p1', 'c2p1', 'c1p2', 'thumb']); + }); + + it('removes the CB7 working files when extraction fails', async () => { + const sevenZip = new FakeSevenZip({ '1.jpg': 'one' }); + vi.spyOn(sevenZip, 'callMain').mockImplementation(() => { + throw new Error('7z exited'); + }); + bookService.verifyFileAccess.mockResolvedValue({ format: 'cb7', absolutePath: '/books/broken.cb7' }); + mockReadFile.mockResolvedValue(Buffer.from('7z') as unknown as Awaited>); + mockGetSevenZip.mockResolvedValue(sevenZip as any); + + await expect(service.getPageCount(16, user)).rejects.toThrow('7z exited'); + expect(sevenZip.paths()).toEqual([]); }); it('throws for unsupported formats and CB7 index bounds', async () => { @@ -342,21 +382,13 @@ describe('CbzService', () => { .mockResolvedValueOnce({ format: 'pdf', absolutePath: '/books/nine.pdf' }) .mockResolvedValueOnce({ format: 'pdf', absolutePath: '/books/nine.pdf' }) .mockResolvedValueOnce({ format: 'cb7', absolutePath: '/books/nine.cb7' }); - - const fsApi = { - open: vi.fn().mockReturnValue(4), - write: vi.fn(), - close: vi.fn(), - mkdir: vi.fn(), - readdir: vi.fn().mockReturnValue(['.', '..', '1.jpg']), - readFile: vi.fn(), - }; - mockGetSevenZip.mockResolvedValue({ FS: fsApi, callMain: vi.fn() } as any); + mockGetSevenZip.mockResolvedValue(new FakeSevenZip({ '1.jpg': 'one' }) as any); mockReadFile.mockResolvedValue(Buffer.from('7z') as unknown as Awaited>); - await expect(service.getPageCount(9, user)).rejects.toThrow(NotFoundException); + await expect(service.getPageCount(9, user)).rejects.toThrow('Unsupported comic format: pdf'); await expect(service.streamPage(9, 0, user)).rejects.toThrow(NotFoundException); - await expect(service.streamPage(9, 1, user)).rejects.toThrow(NotFoundException); + await expect(service.streamPage(9, 1, user)).rejects.toThrow('Page 1 out of range'); + expect(mockStat).toHaveBeenCalledTimes(1); }); it('reads a CBZ-stored-as-RAR using the CBR reader when magic bytes detect RAR', async () => { @@ -373,7 +405,7 @@ describe('CbzService', () => { } as any); await expect(service.getPageCount(10, user)).resolves.toBe(1); - const result = await service.streamPage(10, 0, user); + const result = served(await service.streamPage(10, 0, user)); await expect(readStream(result.stream)).resolves.toEqual(Buffer.from([1, 2, 3])); expect(mockDetectComicContainerFormat).toHaveBeenCalledWith('/books/mislabelled.cbz', 'cbz'); }); @@ -386,7 +418,7 @@ describe('CbzService', () => { mockCreateReadStream.mockReturnValue(rawStream as unknown as ReturnType); await expect(service.getPageCount(11, user)).resolves.toBe(1); - const result = await service.streamPage(11, 0, user); + const result = served(await service.streamPage(11, 0, user)); expect(result.stream).toBe(rawStream); expect(mockDetectComicContainerFormat).toHaveBeenCalledWith('/books/mislabelled.cbr', 'cbr'); }); @@ -400,4 +432,167 @@ describe('CbzService', () => { expect(mockDetectComicContainerFormat).toHaveBeenCalledTimes(1); }); + + describe('when the file changes on disk', () => { + it('reuses a CBZ index while the file is unchanged and rebuilds it after a rewrite', async () => { + bookService.verifyFileAccess.mockResolvedValue({ format: 'cbz', absolutePath: '/books/patched.cbz' }); + mockCbzFile(buildCbzBuffer([{ name: '1.jpg', data: Buffer.from([1]) }])); + + await expect(service.getPageCount(20, user)).resolves.toBe(1); + await expect(service.getPageCount(20, user)).resolves.toBe(1); + expect(mockOpen).toHaveBeenCalledTimes(1); + + mockCbzFile( + buildCbzBuffer([ + { name: 'ComicInfo.xml', data: Buffer.from('') }, + { name: '1.jpg', data: Buffer.from([1]) }, + { name: '2.jpg', data: Buffer.from([2]) }, + ]), + ); + mockStat.mockResolvedValue(fileStats(2_000, 100)); + + await expect(service.getPageCount(20, user)).resolves.toBe(2); + expect(mockOpen).toHaveBeenCalledTimes(2); + expect(mockDetectComicContainerFormat).toHaveBeenCalledTimes(2); + }); + + it('treats a size change with the same mtime as a new version of a CBR', async () => { + bookService.verifyFileAccess.mockResolvedValue({ format: 'cbr', absolutePath: '/books/resized.cbr' }); + mockReadFile.mockResolvedValue(Buffer.from('rar') as unknown as Awaited>); + mockCreateExtractorFromData + .mockResolvedValueOnce({ getFileList: () => ({ fileHeaders: [{ name: '1.jpg', flags: { directory: false } }] }) } as any) + .mockResolvedValueOnce({ + getFileList: () => ({ + fileHeaders: [ + { name: '1.jpg', flags: { directory: false } }, + { name: '2.jpg', flags: { directory: false } }, + ], + }), + } as any); + + await expect(service.getPageCount(21, user)).resolves.toBe(1); + mockStat.mockResolvedValue(fileStats(1_000, 250)); + await expect(service.getPageCount(21, user)).resolves.toBe(2); + expect(mockReadFile).toHaveBeenCalledTimes(2); + }); + + it('drops the previous CB7 extraction and serves pages from the new one', async () => { + const sevenZip = new FakeSevenZip({ 'ch1/001.jpg': 'old' }); + bookService.verifyFileAccess.mockResolvedValue({ format: 'cb7', absolutePath: '/books/rewritten.cb7' }); + mockReadFile.mockResolvedValue(Buffer.from('7z') as unknown as Awaited>); + mockGetSevenZip.mockResolvedValue(sevenZip as any); + + await expect(service.getPageCount(22, user)).resolves.toBe(1); + expect(sevenZip.paths()).toContain('/comic22_1/ch1/001.jpg'); + + sevenZip.archive = { 'ch1/001.jpg': 'new', 'ch1/002.jpg': 'next' }; + mockStat.mockResolvedValue(fileStats(2_000, 100)); + + await expect(service.getPageCount(22, user)).resolves.toBe(2); + expect(sevenZip.paths().filter((path) => path.startsWith('/comic22_1'))).toEqual([]); + await expect(readStream(served(await service.streamPage(22, 0, user)).stream)).resolves.toEqual(Buffer.from('new')); + }); + }); + + describe('unreadable or missing files', () => { + it('answers 422 for a corrupt CBZ instead of an empty comic, and does not cache the failure', async () => { + bookService.verifyFileAccess.mockResolvedValue({ format: 'cbz', absolutePath: '/books/corrupt.cbz' }); + mockCbzFile(Buffer.from('this is not a zip archive, only some text')); + + await expect(service.getPageCount(23, user)).rejects.toThrow(UnprocessableEntityException); + await expect(service.streamPage(23, 0, user)).rejects.toThrow('CBZ archive is unreadable'); + + mockCbzFile(buildCbzBuffer([{ name: '1.jpg', data: Buffer.from([1]) }])); + await expect(service.getPageCount(23, user)).resolves.toBe(1); + }); + + it('still answers an empty page list for a readable CBZ with no images', async () => { + bookService.verifyFileAccess.mockResolvedValue({ format: 'cbz', absolutePath: '/books/empty.cbz' }); + mockCbzFile(buildCbzBuffer([{ name: 'ComicInfo.xml', data: Buffer.from('') }])); + + await expect(service.getPageCount(24, user)).resolves.toBe(0); + }); + + it('reports a comic missing on disk as not found without reading it', async () => { + bookService.verifyFileAccess.mockResolvedValue({ format: 'cbz', absolutePath: '/books/gone.cbz' }); + mockStat.mockRejectedValue(Object.assign(new Error('no such file'), { code: 'ENOENT' })); + + await expect(service.getPageCount(25, user)).rejects.toThrow('Comic file is missing on disk'); + expect(mockOpen).not.toHaveBeenCalled(); + }); + }); + + describe('page validators', () => { + beforeEach(() => { + bookService.verifyFileAccess.mockResolvedValue({ format: 'cbr', absolutePath: '/books/validated.cbr' }); + mockReadFile.mockResolvedValue(Buffer.from('rar') as unknown as Awaited>); + mockCreateExtractorFromData.mockImplementation( + () => + Promise.resolve({ + getFileList: () => ({ + fileHeaders: [ + { name: '1.jpg', flags: { directory: false } }, + { name: '2.jpg', flags: { directory: false } }, + ], + }), + extract: () => ({ files: [{ fileHeader: { flags: { directory: false } }, extraction: Uint8Array.from([1]) }] }), + }) as any, + ); + }); + + it('gives each page of each file version its own ETag', async () => { + const first = (await service.streamPage(26, 0, user)).etag; + const again = (await service.streamPage(26, 0, user)).etag; + const second = (await service.streamPage(26, 1, user)).etag; + const otherFile = (await service.streamPage(27, 0, user)).etag; + mockStat.mockResolvedValue(fileStats(2_000, 100)); + const rewritten = (await service.streamPage(26, 0, user)).etag; + + expect(first).toMatch(/^"[A-Za-z0-9_-]+"$/); + expect(again).toBe(first); + expect(new Set([first, second, otherFile, rewritten]).size).toBe(4); + }); + + it('answers notModified without extracting the page when the client already holds it', async () => { + const { etag } = await service.streamPage(26, 1, user); + const extractionsBefore = mockCreateExtractorFromData.mock.calls.length; + + await expect(service.streamPage(26, 1, user, etag)).resolves.toEqual({ notModified: true, etag }); + await expect(service.streamPage(26, 1, user, `"something-else", W/${etag}`)).resolves.toEqual({ notModified: true, etag }); + expect(mockCreateExtractorFromData).toHaveBeenCalledTimes(extractionsBefore); + expect(bookService.verifyFileAccess).toHaveBeenCalledTimes(3); + }); + + it('serves the page again once the file has changed', async () => { + const { etag } = await service.streamPage(26, 0, user); + mockStat.mockResolvedValue(fileStats(2_000, 100)); + + const result = await service.streamPage(26, 0, user, etag); + + expect(result.notModified).toBe(false); + expect(result.etag).not.toBe(etag); + }); + + it('checks access before answering notModified', async () => { + const { etag } = await service.streamPage(26, 0, user); + bookService.verifyFileAccess.mockRejectedValueOnce(new NotFoundException('No file with id 26')); + + await expect(service.streamPage(26, 0, user, etag)).rejects.toThrow(NotFoundException); + }); + }); + + describe('matchesIfNoneMatch', () => { + it.each([ + ['"abc"', true], + ['W/"abc"', true], + ['"x", "abc"', true], + ['"abcd"', false], + ['abc', false], + ['*', false], + [undefined, false], + ['', false], + ])('%j matches "abc": %s', (header, expected) => { + expect(matchesIfNoneMatch(header, '"abc"')).toBe(expected); + }); + }); }); diff --git a/server/src/modules/reader/cbz/cbz.service.ts b/server/src/modules/reader/cbz/cbz.service.ts index 968092a39..f352a6742 100644 --- a/server/src/modules/reader/cbz/cbz.service.ts +++ b/server/src/modules/reader/cbz/cbz.service.ts @@ -1,11 +1,13 @@ import { Injectable, NotFoundException, UnprocessableEntityException } from '@nestjs/common'; +import { createHash } from 'crypto'; import { Readable } from 'stream'; -import { readFile } from 'fs/promises'; +import { readFile, stat } from 'fs/promises'; import { createExtractorFromData, UnrarError } from 'node-unrar-js'; -import { getSevenZip } from '../../../common/sevenzip'; +import { createSevenZipTempId, getSevenZip } from '../../../common/sevenzip'; import { imageContentTypeFromPath } from '../../../common/image-content-type'; import { detectComicContainerFormat } from '../../../common/comic-format-detect'; import { createCbzZipEntryReadStream, readCbzZipIndex, type CbzZipEntry } from '../../../common/cbz-zip-reader'; +import { compareComicPagePaths, listExtractedFiles, pathBasename, removeExtractedTree } from './cb7-extraction'; import type { RequestUser } from '../../../common/types/request-user'; import { BookService } from '../../book/book.service'; @@ -53,6 +55,43 @@ function toArrayBuffer(buf: Buffer): ArrayBuffer { return buf.buffer.slice(buf.byteOffset, buf.byteOffset + buf.byteLength) as ArrayBuffer; } +// ── Freshness ────────────────────────────────────────────────────────────────── + +/** Bump when page listing or ordering changes, so a revalidation cannot keep a page from the old order. */ +const PAGE_ETAG_VERSION = 1; + +type ComicFormat = 'cbz' | 'cbr' | 'cb7'; + +function isComicFormat(format: string | null): format is ComicFormat { + return format === 'cbz' || format === 'cbr' || format === 'cb7'; +} + +/** Which version of a file a cache entry was built from. The metadata writer replaces files in place. */ +interface FileStamp { + mtimeMs: number; + size: number; +} + +interface Stamped { + stamp: FileStamp; + value: T; +} + +function sameStamp(a: FileStamp, b: FileStamp): boolean { + return a.mtimeMs === b.mtimeMs && a.size === b.size; +} + +function pageEtag(fileId: number, stamp: FileStamp, pageIndex: number): string { + const digest = createHash('sha1').update(`${PAGE_ETAG_VERSION}:${fileId}:${stamp.mtimeMs}:${stamp.size}:${pageIndex}`).digest('base64url'); + return `"${digest}"`; +} + +/** RFC 9110 weak comparison, which is what If-None-Match uses. */ +export function matchesIfNoneMatch(header: string | undefined, etag: string): boolean { + if (!header) return false; + return header.split(',').some((candidate) => candidate.trim().replace(/^W\//, '') === etag); +} + // ── Service ──────────────────────────────────────────────────────────────────── interface RarCache { @@ -60,80 +99,132 @@ interface RarCache { pages: string[]; } +interface SevenZipPages { + /** This extraction's own folder in the shared WASM filesystem. */ + dir: string; + /** Paths relative to `dir`, in reading order. */ + pages: string[]; +} + +export type ComicPageResult = + { notModified: true; etag: string } | { notModified: false; etag: string; stream: NodeJS.ReadableStream; mimeType: string }; + +interface OpenedComic { + absolutePath: string; + format: ComicFormat; + stamp: FileStamp; +} + @Injectable() export class CbzService { // CBZ: byte-offset index per fileId - private readonly zipIndex = new Map(); + private readonly zipIndex = new Map>(); // CBR: archive ArrayBuffer + sorted page names per fileId - private readonly rarCache = new Map(); - // CB7: sorted page names per fileId (extracted files live in WASM VFS) - private readonly sevenZPages = new Map(); + private readonly rarCache = new Map>(); + // CB7: extracted folder + sorted page paths per fileId (extracted files live in WASM VFS) + private readonly sevenZPages = new Map>(); // Resolved actual format per fileId (magic bytes override stored extension) - private readonly resolvedFormat = new Map(); + private readonly resolvedFormat = new Map>(); constructor(private readonly bookService: BookService) {} - private async getFile(fileId: number, user: RequestUser) { - return this.bookService.verifyFileAccess(fileId, user); + private async openComic(fileId: number, user: RequestUser): Promise { + const file = await this.bookService.verifyFileAccess(fileId, user); + if (!isComicFormat(file.format)) { + throw new NotFoundException(`Unsupported comic format: ${file.format ?? ''}`); + } + const stamp = await this.statFile(fileId, file.absolutePath); + const format = await this.resolveFormat(fileId, file.absolutePath, file.format, stamp); + return { absolutePath: file.absolutePath, format, stamp }; } - private async resolveFormat(fileId: number, absolutePath: string, storedFmt: string | null): Promise { - if (!this.resolvedFormat.has(fileId)) { - let resolved: string; - if (storedFmt === 'cbz' || storedFmt === 'cbr' || storedFmt === 'cb7') { - resolved = await detectComicContainerFormat(absolutePath, storedFmt); - } else { - resolved = storedFmt ?? ''; - } - this.resolvedFormat.set(fileId, resolved); + private async statFile(fileId: number, absolutePath: string): Promise { + let stats: Awaited>; + try { + stats = await stat(absolutePath); + } catch (err) { + const code = (err as NodeJS.ErrnoException | undefined)?.code; + if (code === 'ENOENT' || code === 'ENOTDIR') throw new NotFoundException('Comic file is missing on disk'); + throw err; } - return this.resolvedFormat.get(fileId)!; + + const stamp = { mtimeMs: Number(stats.mtimeMs), size: Number(stats.size) }; + if (this.hasStaleEntry(fileId, stamp)) await this.invalidate(fileId); + return stamp; + } + + private hasStaleEntry(fileId: number, stamp: FileStamp): boolean { + const entries = [this.zipIndex.get(fileId), this.rarCache.get(fileId), this.sevenZPages.get(fileId), this.resolvedFormat.get(fileId)]; + return entries.some((entry) => entry !== undefined && !sameStamp(entry.stamp, stamp)); + } + + private async invalidate(fileId: number): Promise { + const sevenZip = this.sevenZPages.get(fileId); + this.zipIndex.delete(fileId); + this.rarCache.delete(fileId); + this.sevenZPages.delete(fileId); + this.resolvedFormat.delete(fileId); + if (sevenZip) removeExtractedTree((await getSevenZip()).FS, sevenZip.value.dir); + } + + private async resolveFormat(fileId: number, absolutePath: string, storedFmt: ComicFormat, stamp: FileStamp): Promise { + const cached = this.resolvedFormat.get(fileId); + if (cached && sameStamp(cached.stamp, stamp)) return cached.value; + + const resolved = await detectComicContainerFormat(absolutePath, storedFmt); + this.resolvedFormat.set(fileId, { stamp, value: resolved }); + return resolved; } // ── CBZ ────────────────────────────────────────────────────────────────────── - private async getCbzIndex(fileId: number, filePath: string): Promise { - if (!this.zipIndex.has(fileId)) { - const index = await readCbzZipIndex(filePath); - const entries = - index?.entries - .filter((entry) => !entry.name.endsWith('/') && !isHidden(entry.name) && isImage(entry.name)) - .filter((entry) => (entry.compression === 0 || entry.compression === 8) && entry.compressedSize > 0) - .sort((a, b) => naturalSort(a.name, b.name)) ?? []; - this.zipIndex.set(fileId, entries); - } - return this.zipIndex.get(fileId)!; + private async getCbzIndex(fileId: number, filePath: string, stamp: FileStamp): Promise { + const cached = this.zipIndex.get(fileId); + if (cached && sameStamp(cached.stamp, stamp)) return cached.value; + + const index = await readCbzZipIndex(filePath); + // Not cached: the reader also answers null for a transient read error, which a retry should get past. + if (!index) throw new UnprocessableEntityException('CBZ archive is unreadable'); + + const entries = index.entries + .filter((entry) => !entry.name.endsWith('/') && !isHidden(entry.name) && isImage(entry.name)) + .filter((entry) => (entry.compression === 0 || entry.compression === 8) && entry.compressedSize > 0) + .sort((a, b) => naturalSort(a.name, b.name)); + this.zipIndex.set(fileId, { stamp, value: entries }); + return entries; } // ── CBR ────────────────────────────────────────────────────────────────────── - private async getRarCache(fileId: number, filePath: string): Promise { - if (!this.rarCache.has(fileId)) { - const buf = await readFile(filePath); - const ab = toArrayBuffer(buf); + private async getRarCache(fileId: number, filePath: string, stamp: FileStamp): Promise { + const cached = this.rarCache.get(fileId); + if (cached && sameStamp(cached.stamp, stamp)) return cached.value; - const pages: string[] = []; - try { - const extractor = await createExtractorFromData({ data: ab }); - const { fileHeaders } = extractor.getFileList(); - 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 (!isUnrarError(err) || pages.length === 0) { - throw isUnrarError(err) ? unreadableCbrArchive(err) : err; + const buf = await readFile(filePath); + const ab = toArrayBuffer(buf); + + const pages: string[] = []; + try { + const extractor = await createExtractorFromData({ data: ab }); + const { fileHeaders } = extractor.getFileList(); + for (const h of fileHeaders) { + if (!h.flags.directory && isImage(h.name) && !isHidden(h.name)) { + pages.push(h.name); } } - - pages.sort(naturalSort); - this.rarCache.set(fileId, { buffer: ab, pages }); + } 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 (!isUnrarError(err) || pages.length === 0) { + throw isUnrarError(err) ? unreadableCbrArchive(err) : err; + } } - return this.rarCache.get(fileId)!; + + pages.sort(naturalSort); + const value = { buffer: ab, pages }; + this.rarCache.set(fileId, { stamp, value }); + return value; } private async extractRarPage(buffer: ArrayBuffer, pageName: string): Promise { @@ -159,80 +250,97 @@ export class CbzService { // ── CB7 ────────────────────────────────────────────────────────────────────── - private async getSevenZPages(fileId: number, filePath: string): Promise { - if (!this.sevenZPages.has(fileId)) { - const sz = await getSevenZip(); - const archivePath = `/a${fileId}`; - const outDir = `/p${fileId}`; + private async getSevenZPages(fileId: number, filePath: string, stamp: FileStamp): Promise { + const cached = this.sevenZPages.get(fileId); + if (cached && sameStamp(cached.stamp, stamp)) return cached.value; - const buf = await readFile(filePath); + const sz = await getSevenZip(); + const buf = await readFile(filePath); + + // A folder per extraction, so a rebuild after the file changed never shares a tree with + // requests still reading the previous one. + const dir = `/${createSevenZipTempId(`comic${fileId}`)}`; + const archivePath = `${dir}.7z`; + + try { const fd = sz.FS.open(archivePath, 'w+'); sz.FS.write(fd, buf, 0, buf.length); sz.FS.close(fd); + sz.FS.mkdir(dir); - try { - sz.FS.mkdir(outDir); - } catch { - // already exists from a previous run - } - - // callMain is synchronous — blocks the event loop while 7z extracts. + // callMain is synchronous - blocks the event loop while 7z extracts. // Acceptable for a local single-user server; revisit with worker_threads if needed. - sz.callMain(['e', archivePath, `-o${outDir}`, '-y']); - - const files = sz.FS.readdir(outDir); - const pages = files.filter((f) => f !== '.' && f !== '..' && isImage(f) && !isHidden(f)).sort(naturalSort); - - this.sevenZPages.set(fileId, pages); + // `x` keeps folders: `e` flattened them, so two pages sharing a name overwrote each other. + sz.callMain(['x', archivePath, `-o${dir}`, '-y']); + } catch (err) { + removeExtractedTree(sz.FS, dir); + throw err; + } finally { + try { + sz.FS.unlink(archivePath); + } catch { + // never written + } } - return this.sevenZPages.get(fileId)!; + + // Only the file name decides hidden, as it did when the archive was extracted flat. + const pages = listExtractedFiles(sz.FS, dir) + .filter((path) => { + const name = pathBasename(path); + return isImage(name) && !name.startsWith('.'); + }) + .sort(compareComicPagePaths); + + const previous = this.sevenZPages.get(fileId); + const value = { dir, pages }; + this.sevenZPages.set(fileId, { stamp, value }); + if (previous && previous.value.dir !== dir) removeExtractedTree(sz.FS, previous.value.dir); + return value; } // ── Public API ──────────────────────────────────────────────────────────────── async getPageCount(fileId: number, user: RequestUser): Promise { - const file = await this.getFile(fileId, user); - const fmt = await this.resolveFormat(fileId, file.absolutePath, file.format); + const comic = await this.openComic(fileId, user); - if (fmt === 'cbz') return (await this.getCbzIndex(fileId, file.absolutePath)).length; - if (fmt === 'cbr') return (await this.getRarCache(fileId, file.absolutePath)).pages.length; - if (fmt === 'cb7') return (await this.getSevenZPages(fileId, file.absolutePath)).length; - - throw new NotFoundException(`Unsupported comic format: ${fmt}`); + if (comic.format === 'cbz') return (await this.getCbzIndex(fileId, comic.absolutePath, comic.stamp)).length; + if (comic.format === 'cbr') return (await this.getRarCache(fileId, comic.absolutePath, comic.stamp)).pages.length; + return (await this.getSevenZPages(fileId, comic.absolutePath, comic.stamp)).pages.length; } - async streamPage(fileId: number, pageIndex: number, user: RequestUser): Promise<{ stream: NodeJS.ReadableStream; mimeType: string }> { - const file = await this.getFile(fileId, user); - const fmt = await this.resolveFormat(fileId, file.absolutePath, file.format); + /** + * Answers `notModified` without reading the archive when `ifNoneMatch` names this exact page of + * this exact version of the file, so a client revalidating a page it already has costs a stat. + */ + async streamPage(fileId: number, pageIndex: number, user: RequestUser, ifNoneMatch?: string): Promise { + const comic = await this.openComic(fileId, user); + const etag = pageEtag(fileId, comic.stamp, pageIndex); + if (matchesIfNoneMatch(ifNoneMatch, etag)) return { notModified: true, etag }; - if (fmt === 'cbz') { - const entries = await this.getCbzIndex(fileId, file.absolutePath); + if (comic.format === 'cbz') { + const entries = await this.getCbzIndex(fileId, comic.absolutePath, comic.stamp); if (pageIndex < 0 || pageIndex >= entries.length) { throw new NotFoundException(`Page ${pageIndex} out of range`); } const entry = entries[pageIndex]; - return { stream: createCbzZipEntryReadStream(file.absolutePath, entry), mimeType: mimeForExt(entry.name) }; + return { notModified: false, etag, stream: createCbzZipEntryReadStream(comic.absolutePath, entry), mimeType: mimeForExt(entry.name) }; } - if (fmt === 'cbr') { - const { buffer, pages } = await this.getRarCache(fileId, file.absolutePath); + if (comic.format === 'cbr') { + const { buffer, pages } = await this.getRarCache(fileId, comic.absolutePath, comic.stamp); if (pageIndex < 0 || pageIndex >= pages.length) { throw new NotFoundException(`Page ${pageIndex} out of range`); } const data = await this.extractRarPage(buffer, pages[pageIndex]); - return { stream: Readable.from(Buffer.from(data)), mimeType: mimeForExt(pages[pageIndex]) }; + return { notModified: false, etag, stream: Readable.from(Buffer.from(data)), mimeType: mimeForExt(pages[pageIndex]) }; } - if (fmt === 'cb7') { - const pages = await this.getSevenZPages(fileId, file.absolutePath); - if (pageIndex < 0 || pageIndex >= pages.length) { - throw new NotFoundException(`Page ${pageIndex} out of range`); - } - const sz = await getSevenZip(); - const data = sz.FS.readFile(`/p${fileId}/${pages[pageIndex]}`); - return { stream: Readable.from(Buffer.from(data)), mimeType: mimeForExt(pages[pageIndex]) }; + const sz = await getSevenZip(); + const { dir, pages } = await this.getSevenZPages(fileId, comic.absolutePath, comic.stamp); + if (pageIndex < 0 || pageIndex >= pages.length) { + throw new NotFoundException(`Page ${pageIndex} out of range`); } - - throw new NotFoundException(`Unsupported comic format: ${fmt}`); + const data = sz.FS.readFile(`${dir}/${pages[pageIndex]}`); + return { notModified: false, etag, stream: Readable.from(Buffer.from(data)), mimeType: mimeForExt(pages[pageIndex]) }; } } diff --git a/server/src/modules/reader/cbz/test-helpers.ts b/server/src/modules/reader/cbz/test-helpers.ts new file mode 100644 index 000000000..5ec0e0f2c --- /dev/null +++ b/server/src/modules/reader/cbz/test-helpers.ts @@ -0,0 +1,128 @@ +import type { SevenZipFS, SevenZipModule } from '../../../common/sevenzip'; + +const DIR_MODE = 0o040000; +const FILE_MODE = 0o100000; +const LINK_MODE = 0o120000; + +interface FakeNode { + mode: number; + data?: Uint8Array; + target?: string; +} + +/** An archive entry: file contents, or `{ link }` for a symbolic link with that target. */ +export type FakeArchiveEntry = string | { link: string }; + +function parentOf(path: string): string { + const slash = path.lastIndexOf('/'); + return slash <= 0 ? '/' : path.slice(0, slash); +} + +function normalize(path: string): string { + const parts: string[] = []; + for (const part of path.split('/')) { + if (!part || part === '.') continue; + if (part === '..') parts.pop(); + else parts.push(part); + } + return `/${parts.join('/')}`; +} + +function errno(code: string): Error { + return Object.assign(new Error(code), { code }); +} + +/** + * A small in-memory stand-in for the 7z-wasm module: enough of the Emscripten filesystem for the + * comic reader, and a `callMain` whose `x` writes `archive` under the `-o` folder with its folders + * kept, the way 7-Zip does. + */ +export class FakeSevenZip implements SevenZipModule { + readonly nodes = new Map([['/', { mode: DIR_MODE }]]); + readonly calls: string[][] = []; + private readonly handles: string[] = []; + + constructor(public archive: Record = {}) {} + + readonly FS: SevenZipFS = { + open: (path) => { + this.nodes.set(path, { mode: FILE_MODE, data: new Uint8Array() }); + return this.handles.push(path) - 1; + }, + write: (fd, buf, offset, length) => { + this.nodes.get(this.handles[fd]!)!.data = Uint8Array.from(buf.subarray(offset, offset + length)); + return length; + }, + close: () => undefined, + mkdir: (path) => { + if (this.nodes.has(path)) throw errno('EEXIST'); + this.nodes.set(path, { mode: DIR_MODE }); + }, + readdir: (path) => { + if (this.nodes.get(path)?.mode !== DIR_MODE) throw errno('ENOTDIR'); + const children = [...this.nodes.keys()].filter((key) => key !== '/' && key !== path && parentOf(key) === path); + return ['.', '..', ...children.map((key) => key.slice(key.lastIndexOf('/') + 1))]; + }, + readFile: (path) => { + const node = this.nodes.get(this.FS.lookupPath(path, { follow: true }).path); + if (node?.mode !== FILE_MODE) throw errno('ENOENT'); + return node.data!; + }, + stat: (path) => this.FS.lstat(this.FS.lookupPath(path, { follow: true }).path), + lstat: (path) => { + const node = this.nodes.get(path); + if (!node) throw errno('ENOENT'); + return { mode: node.mode, size: node.data?.length ?? 0 }; + }, + lookupPath: (path, opts) => { + let current = path; + for (let hops = 0; hops < 8; hops++) { + const node = this.nodes.get(current); + if (!node) throw errno('ENOENT'); + if (node.mode !== LINK_MODE || !opts?.follow) return { path: current }; + current = node.target!.startsWith('/') ? normalize(node.target!) : normalize(`${parentOf(current)}/${node.target}`); + } + throw errno('ELOOP'); + }, + isDir: (mode) => mode === DIR_MODE, + isFile: (mode) => mode === FILE_MODE, + isLink: (mode) => mode === LINK_MODE, + unlink: (path) => { + const node = this.nodes.get(path); + if (!node || node.mode === DIR_MODE) throw errno('EISDIR'); + this.nodes.delete(path); + }, + rmdir: (path) => { + if (this.nodes.get(path)?.mode !== DIR_MODE) throw errno('ENOTDIR'); + if (this.FS.readdir(path).length > 2) throw errno('ENOTEMPTY'); + this.nodes.delete(path); + }, + }; + + callMain(args: string[]): void { + this.calls.push(args); + const outArg = args.find((arg) => arg.startsWith('-o')); + if (args[0] !== 'x' || !outArg) return; + const outDir = outArg.slice(2); + for (const [entryPath, entry] of Object.entries(this.archive)) { + const path = `${outDir}/${entryPath}`; + this.ensureDir(parentOf(path)); + this.nodes.set(path, typeof entry === 'string' ? { mode: FILE_MODE, data: Buffer.from(entry) } : { mode: LINK_MODE, target: entry.link }); + } + } + + addFile(path: string, contents: string): void { + this.ensureDir(parentOf(path)); + this.nodes.set(path, { mode: FILE_MODE, data: Buffer.from(contents) }); + } + + paths(): string[] { + return [...this.nodes.keys()].filter((key) => key !== '/').sort(); + } + + private ensureDir(path: string): void { + if (path === '/' || this.nodes.has(path)) return; + this.ensureDir(parentOf(path)); + this.nodes.set(path, { mode: DIR_MODE }); + } +} diff --git a/server/test/reader-format-delivery.e2e-spec.ts b/server/test/reader-format-delivery.e2e-spec.ts index cdc0c3103..8bdb74a35 100644 --- a/server/test/reader-format-delivery.e2e-spec.ts +++ b/server/test/reader-format-delivery.e2e-spec.ts @@ -338,9 +338,18 @@ describe('Reader format delivery (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () => }); expect(firstPageResponse.statusCode).toBe(200); expect(firstPageResponse.headers['content-type']).toContain('image/png'); - expect(firstPageResponse.headers['cache-control']).toBe('public, max-age=31536000, immutable'); + expect(firstPageResponse.headers['cache-control']).toBe('private, no-cache'); + expect(firstPageResponse.headers.etag).toEqual(expect.stringMatching(/^".+"$/)); expect(responseBuffer(firstPageResponse)).toEqual(PAGE_PNG); + const revalidatedPageResponse = await ctx.app.inject({ + method: 'GET', + url: `/api/v1/cbz/files/${sharedCbz.bookFileId}/pages/0`, + headers: { ...authHeader(viewer.accessToken), 'if-none-match': String(firstPageResponse.headers.etag) }, + }); + expect(revalidatedPageResponse.statusCode).toBe(304); + expect(revalidatedPageResponse.headers.etag).toBe(firstPageResponse.headers.etag); + const secondPageResponse = await ctx.app.inject({ method: 'GET', url: `/api/v1/cbz/files/${sharedCbz.bookFileId}/pages/1`,