From 9b853935bb3a793bd1ec73ba8ab50f921d29511a Mon Sep 17 00:00:00 2001 From: neon <18559658+neonsolstice@users.noreply.github.com> Date: Mon, 2 Nov 2026 01:54:30 +0000 Subject: [PATCH] fix(reader): sort comic names before their extensions Compare extensionless archive entry paths first so a cover named IMG.jpg precedes page names that extend IMG with a suffix. Keep cover extraction and CBZ/CBR page delivery aligned, and invalidate old page ETags. Closes #2035 --- server/src/common/archive-entry-sort.ts | 16 ++++++++ .../metadata/lib/archive-image-utils.test.ts | 5 +++ .../metadata/lib/archive-image-utils.ts | 4 +- .../modules/metadata/lib/cover-cbr.test.ts | 17 ++++++++ .../modules/metadata/lib/cover-cbz.test.ts | 9 +++++ .../modules/reader/cbz/cbz.service.test.ts | 40 +++++++++++++++++++ server/src/modules/reader/cbz/cbz.service.ts | 11 ++--- 7 files changed, 92 insertions(+), 10 deletions(-) create mode 100644 server/src/common/archive-entry-sort.ts diff --git a/server/src/common/archive-entry-sort.ts b/server/src/common/archive-entry-sort.ts new file mode 100644 index 000000000..1f7df1b77 --- /dev/null +++ b/server/src/common/archive-entry-sort.ts @@ -0,0 +1,16 @@ +const ARCHIVE_ENTRY_COLLATOR = new Intl.Collator(undefined, { numeric: true, sensitivity: 'base' }); + +function extensionIndex(path: string): number { + const separator = Math.max(path.lastIndexOf('/'), path.lastIndexOf('\\')); + const dot = path.lastIndexOf('.'); + return dot > separator + 1 ? dot : path.length; +} + +export function compareArchiveEntryNames(a: string, b: string): number { + const aExtension = extensionIndex(a); + const bExtension = extensionIndex(b); + return ( + ARCHIVE_ENTRY_COLLATOR.compare(a.slice(0, aExtension), b.slice(0, bExtension)) || + ARCHIVE_ENTRY_COLLATOR.compare(a.slice(aExtension), b.slice(bExtension)) + ); +} diff --git a/server/src/modules/metadata/lib/archive-image-utils.test.ts b/server/src/modules/metadata/lib/archive-image-utils.test.ts index 45522b5bd..fb88ed41d 100644 --- a/server/src/modules/metadata/lib/archive-image-utils.test.ts +++ b/server/src/modules/metadata/lib/archive-image-utils.test.ts @@ -21,4 +21,9 @@ describe('archive-image-utils', () => { const entries = ['page10.jpg', 'Page1.JPG', 'page2.jpg']; expect(entries.toSorted(compareArchiveEntryNames)).toEqual(['Page1.JPG', 'page2.jpg', 'page10.jpg']); }); + + it('sorts a complete base filename before a filename that extends its stem', () => { + const entries = ['IMG_0004_Seite_001_1L.jpg', 'IMG.jpg']; + expect(entries.toSorted(compareArchiveEntryNames)).toEqual(['IMG.jpg', 'IMG_0004_Seite_001_1L.jpg']); + }); }); diff --git a/server/src/modules/metadata/lib/archive-image-utils.ts b/server/src/modules/metadata/lib/archive-image-utils.ts index 6dd567eca..03b8fdcf8 100644 --- a/server/src/modules/metadata/lib/archive-image-utils.ts +++ b/server/src/modules/metadata/lib/archive-image-utils.ts @@ -1,8 +1,6 @@ const ARCHIVE_IMAGE_EXTENSIONS = new Set(['.jpg', '.jpeg', '.png', '.webp', '.gif', '.bmp']); -export function compareArchiveEntryNames(a: string, b: string): number { - return a.localeCompare(b, undefined, { numeric: true, sensitivity: 'base' }); -} +export { compareArchiveEntryNames } from '../../../common/archive-entry-sort'; export function isArchiveImageFile(name: string): boolean { const dot = name.lastIndexOf('.'); diff --git a/server/src/modules/metadata/lib/cover-cbr.test.ts b/server/src/modules/metadata/lib/cover-cbr.test.ts index dd07d3254..e66fa3a35 100644 --- a/server/src/modules/metadata/lib/cover-cbr.test.ts +++ b/server/src/modules/metadata/lib/cover-cbr.test.ts @@ -33,6 +33,23 @@ describe('extractCbrCover', () => { await expect(extractCbrCover('/book.cbr')).resolves.toEqual(Buffer.from([7, 7, 7])); }); + it('chooses a complete base filename before a filename that extends its stem', async () => { + const extract = vi.fn(() => ({ files: [{ fileHeader: { flags: { directory: false } }, extraction: Uint8Array.from([1]) }] })); + mockCreateExtractor + .mockResolvedValueOnce({ + getFileList: () => ({ + fileHeaders: [ + { name: 'IMG_0004_Seite_001_1L.jpg', flags: { directory: false } }, + { name: 'IMG.jpg', flags: { directory: false } }, + ], + }), + } as any) + .mockResolvedValueOnce({ extract } as any); + + await expect(extractCbrCover('/book.cbr')).resolves.toEqual(Buffer.from([1])); + expect(extract).toHaveBeenCalledWith({ files: ['IMG.jpg'] }); + }); + it('returns null when archive has no visible image files', async () => { mockCreateExtractor.mockResolvedValue({ getFileList: () => ({ fileHeaders: [{ name: 'notes.txt', flags: { directory: false } }] }), diff --git a/server/src/modules/metadata/lib/cover-cbz.test.ts b/server/src/modules/metadata/lib/cover-cbz.test.ts index bbb7f8f68..ce7027218 100644 --- a/server/src/modules/metadata/lib/cover-cbz.test.ts +++ b/server/src/modules/metadata/lib/cover-cbz.test.ts @@ -133,6 +133,15 @@ describe('extractCbzCover', () => { await expect(extractCbzCover('/book.cbz')).resolves.toEqual(Buffer.from([0x02])); }); + it('chooses a complete base filename before a filename that extends its stem', async () => { + mockZip([ + { name: 'IMG_0004_Seite_001_1L.jpg', data: Buffer.from([0x04]) }, + { name: 'IMG.jpg', data: Buffer.from([0x01]) }, + ]); + + await expect(extractCbzCover('/book.cbz')).resolves.toEqual(Buffer.from([0x01])); + }); + it('skips hidden image files and returns first visible image', async () => { mockZip([ { name: '.hidden.jpg', data: Buffer.from([0xaa]) }, diff --git a/server/src/modules/reader/cbz/cbz.service.test.ts b/server/src/modules/reader/cbz/cbz.service.test.ts index 36b73bde1..e50df76a4 100644 --- a/server/src/modules/reader/cbz/cbz.service.test.ts +++ b/server/src/modules/reader/cbz/cbz.service.test.ts @@ -198,6 +198,24 @@ describe('CbzService', () => { expect(result.mimeType).toBe('image/jpeg'); }); + it('streams a complete base filename before a filename that extends its stem', async () => { + const rawStream = { kind: 'raw-stream' }; + + bookService.verifyFileAccess.mockResolvedValue({ format: 'cbz', absolutePath: '/books/prefix.cbz' }); + mockCbzFile( + buildCbzBuffer([ + { name: 'IMG_0004_Seite_001_1L.jpg', data: Buffer.from([4]), compression: 8 }, + { name: 'IMG.jpg', data: Buffer.from([1]), compression: 0 }, + ]), + ); + mockCreateReadStream.mockReturnValue(rawStream as unknown as ReturnType); + + const result = served(await service.streamPage(24, 0, user)); + + expect(mockCreateInflateRaw).not.toHaveBeenCalled(); + expect(result.stream).toBe(rawStream); + }); + it('streams STORED CBZ entries without inflate', async () => { const rawStream = { kind: 'raw-stream' }; @@ -261,6 +279,28 @@ describe('CbzService', () => { expect(mockReadFile).toHaveBeenCalledTimes(1); }); + it('streams a complete CBR base filename before a filename that extends its stem', async () => { + const extract = vi.fn(() => ({ + files: [{ fileHeader: { flags: { directory: false } }, extraction: Uint8Array.from([1]) }], + })); + bookService.verifyFileAccess.mockResolvedValue({ format: 'cbr', absolutePath: '/books/prefix.cbr' }); + mockReadFile.mockResolvedValue(Buffer.from('rar-bytes') as unknown as Awaited>); + mockCreateExtractorFromData + .mockResolvedValueOnce({ + getFileList: () => ({ + fileHeaders: [ + { name: 'IMG_0004_Seite_001_1L.jpg', flags: { directory: false } }, + { name: 'IMG.jpg', flags: { directory: false } }, + ], + }), + } as any) + .mockResolvedValueOnce({ extract } as any); + + await service.streamPage(25, 0, user); + + expect(extract).toHaveBeenCalledWith({ files: ['IMG.jpg'] }); + }); + it('throws when CBR extraction does not produce the requested file', async () => { bookService.verifyFileAccess.mockResolvedValue({ format: 'cbr', absolutePath: '/books/seven.cbr' }); mockReadFile.mockResolvedValue(Buffer.from('rar') as unknown as Awaited>); diff --git a/server/src/modules/reader/cbz/cbz.service.ts b/server/src/modules/reader/cbz/cbz.service.ts index f352a6742..ca285765e 100644 --- a/server/src/modules/reader/cbz/cbz.service.ts +++ b/server/src/modules/reader/cbz/cbz.service.ts @@ -7,6 +7,7 @@ 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 { compareArchiveEntryNames } from '../../../common/archive-entry-sort'; import { compareComicPagePaths, listExtractedFiles, pathBasename, removeExtractedTree } from './cb7-extraction'; import type { RequestUser } from '../../../common/types/request-user'; @@ -25,10 +26,6 @@ function isHidden(name: string): boolean { return name.split('/').some((p) => p.startsWith('.')); } -function naturalSort(a: string, b: string): number { - return a.localeCompare(b, undefined, { numeric: true, sensitivity: 'base' }); -} - function mimeForExt(name: string): string { return imageContentTypeFromPath(name); } @@ -58,7 +55,7 @@ function toArrayBuffer(buf: Buffer): 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; +const PAGE_ETAG_VERSION = 2; type ComicFormat = 'cbz' | 'cbr' | 'cb7'; @@ -189,7 +186,7 @@ export class CbzService { 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)); + .sort((a, b) => compareArchiveEntryNames(a.name, b.name)); this.zipIndex.set(fileId, { stamp, value: entries }); return entries; } @@ -221,7 +218,7 @@ export class CbzService { } } - pages.sort(naturalSort); + pages.sort(compareArchiveEntryNames); const value = { buffer: ab, pages }; this.rarCache.set(fileId, { stamp, value }); return value;