fix(cover): re-encode custom covers as baseline jpegs

Re-encode uploaded progressive JPEG custom cover images to baseline JPEG format so they render correctly across OPDS clients that lack progressive JPEG support.

Closes #782

server/src/modules/cover/cover.service.test.ts

@@ -1,9 +1,9 @@
 import { type LookupAddress, lookup } from 'dns/promises';
-import { mkdir, readdir, readFile, unlink, writeFile } from 'fs/promises';
+import { mkdir, readdir, readFile, rename, unlink, writeFile } from 'fs/promises';
 import type { CoverSearchResult } from '@bookorbit/types';
 
 import type { RequestUser } from '../../common/types/request-user';
-import { coverDirPath, generateThumbnail, imageExt } from '../metadata/lib/cover';
+import { coverDirPath, generateThumbnail, imageExt, normalizeProgressiveJpeg } from '../metadata/lib/cover';
 import { COVER_CUSTOM_FILE_PREFIX, COVER_PROXY_MAX_IMAGE_BYTES, COVER_PROXY_USER_AGENT, COVER_THUMBNAIL_FILE_NAME } from './constants';
 import { CoverService } from './cover.service';
 import type { CoverProviderRegistry } from './provider-registry';
@@ -19,6 +19,7 @@ vi.mock('fs/promises', () => ({
   mkdir: vi.fn(),
   readdir: vi.fn(),
   readFile: vi.fn(),
+  rename: vi.fn(),
   writeFile: vi.fn(),
   unlink: vi.fn(),
 }));
@@ -27,6 +28,7 @@ vi.mock('../metadata/lib/cover', () => ({
   coverDirPath: vi.fn(),
   generateThumbnail: vi.fn(),
   imageExt: vi.fn(),
+  normalizeProgressiveJpeg: vi.fn(),
 }));
 
 function makeResult(url: string, previewUrl: string): CoverSearchResult {
@@ -134,10 +136,12 @@ describe('CoverService', () => {
   let fetchMock: ReturnType<typeof vi.fn>;
 
   beforeEach(() => {
+    vi.clearAllMocks();
     lookupMock.mockReset();
     lookupMock.mockResolvedValue([{ address: '93.184.216.34', family: 4 } as LookupAddress]);
     fetchMock = vi.fn();
     global.fetch = fetchMock as unknown as typeof fetch;
+    vi.mocked(normalizeProgressiveJpeg).mockImplementation((bytes) => Promise.resolve(bytes));
   });
 
   afterEach(() => {
@@ -471,22 +475,92 @@ describe('CoverService', () => {
 
     it('saves custom cover file and thumbnail when unlocked', async () => {
       const { service, mockDb, updateSet } = createMutationService();
+      const source = Buffer.from('image-data');
+      const normalized = Buffer.from('normalized-image');
 
       vi.mocked(coverDirPath).mockReturnValue('/tmp/books/covers/12');
+      vi.mocked(normalizeProgressiveJpeg).mockResolvedValue(normalized);
       vi.mocked(imageExt).mockReturnValue('jpg');
       vi.mocked(generateThumbnail).mockResolvedValue(Buffer.from('thumb'));
       vi.mocked(readdir).mockResolvedValue([] as never);
       vi.mocked(mkdir).mockResolvedValue(undefined as never);
       vi.mocked(writeFile).mockResolvedValue(undefined);
+      vi.mocked(rename).mockResolvedValue(undefined);
+
+      await service.uploadCover(12, source, 'image/jpeg', makeUser());
+
+      expect(normalizeProgressiveJpeg).toHaveBeenCalledWith(source);
+      expect(generateThumbnail).toHaveBeenCalledWith(normalized);
+      expect(mkdir).toHaveBeenCalledWith('/tmp/books/covers/12', { recursive: true });
+      expect(writeFile).toHaveBeenCalledTimes(2);
+      expect(writeFile).toHaveBeenCalledWith(expect.stringMatching(/^\/tmp\/books\/covers\/12\/\.cover-upload-.*\.jpg\.tmp$/), normalized);
+      expect(writeFile).toHaveBeenCalledWith(
+        expect.stringMatching(/^\/tmp\/books\/covers\/12\/\.cover-upload-.*\.thumbnail\.tmp$/),
+        Buffer.from('thumb'),
+      );
+      expect(rename).toHaveBeenNthCalledWith(
+        1,
+        expect.stringMatching(/^\/tmp\/books\/covers\/12\/\.cover-upload-.*\.thumbnail\.tmp$/),
+        `/tmp/books/covers/12/${COVER_THUMBNAIL_FILE_NAME}`,
+      );
+      expect(rename).toHaveBeenNthCalledWith(
+        2,
+        expect.stringMatching(/^\/tmp\/books\/covers\/12\/\.cover-upload-.*\.jpg\.tmp$/),
+        `/tmp/books/covers/12/${COVER_CUSTOM_FILE_PREFIX}jpg`,
+      );
+      expect(mockDb.update).toHaveBeenCalledWith(books);
+      expect(updateSet).toHaveBeenCalledWith({ updatedAt: expect.any(Date) });
+    });
+
+    it('preserves the existing cover when image normalization fails', async () => {
+      const { service, mockDb } = createMutationService();
+      vi.mocked(normalizeProgressiveJpeg).mockRejectedValue(new Error('invalid image'));
+
+      await expect(service.uploadCover(12, Buffer.from('invalid'), 'image/jpeg', makeUser())).rejects.toThrow('Invalid image file');
+
+      expect(mkdir).not.toHaveBeenCalled();
+      expect(writeFile).not.toHaveBeenCalled();
+      expect(rename).not.toHaveBeenCalled();
+      expect(unlink).not.toHaveBeenCalled();
+      expect(mockDb.insert).not.toHaveBeenCalled();
+    });
+
+    it('removes obsolete custom-cover formats only after replacing both files', async () => {
+      const { service } = createMutationService();
+
+      vi.mocked(coverDirPath).mockReturnValue('/tmp/books/covers/12');
+      vi.mocked(imageExt).mockReturnValue('jpg');
+      vi.mocked(generateThumbnail).mockResolvedValue(Buffer.from('thumb'));
+      vi.mocked(readdir).mockResolvedValue(['cover_custom.png', 'cover_custom.jpg'] as never);
+      vi.mocked(mkdir).mockResolvedValue(undefined as never);
+      vi.mocked(writeFile).mockResolvedValue(undefined);
+      vi.mocked(rename).mockResolvedValue(undefined);
+      vi.mocked(unlink).mockResolvedValue(undefined);
 
       await service.uploadCover(12, Buffer.from('image-data'), 'image/jpeg', makeUser());
 
-      expect(mkdir).toHaveBeenCalledWith('/tmp/books/covers/12', { recursive: true });
-      expect(writeFile).toHaveBeenCalledTimes(2);
-      expect(writeFile).toHaveBeenCalledWith(`/tmp/books/covers/12/${COVER_CUSTOM_FILE_PREFIX}jpg`, Buffer.from('image-data'));
-      expect(writeFile).toHaveBeenCalledWith(`/tmp/books/covers/12/${COVER_THUMBNAIL_FILE_NAME}`, Buffer.from('thumb'));
-      expect(mockDb.update).toHaveBeenCalledWith(books);
-      expect(updateSet).toHaveBeenCalledWith({ updatedAt: expect.any(Date) });
+      expect(unlink).toHaveBeenCalledWith('/tmp/books/covers/12/cover_custom.png');
+      expect(unlink).not.toHaveBeenCalledWith('/tmp/books/covers/12/cover_custom.jpg');
+      expect(vi.mocked(unlink).mock.invocationCallOrder[0]).toBeGreaterThan(vi.mocked(rename).mock.invocationCallOrder[1]);
+    });
+
+    it('keeps the old custom cover when atomic replacement fails', async () => {
+      const { service, mockDb } = createMutationService();
+
+      vi.mocked(coverDirPath).mockReturnValue('/tmp/books/covers/12');
+      vi.mocked(imageExt).mockReturnValue('jpg');
+      vi.mocked(generateThumbnail).mockResolvedValue(Buffer.from('thumb'));
+      vi.mocked(mkdir).mockResolvedValue(undefined as never);
+      vi.mocked(writeFile).mockResolvedValue(undefined);
+      vi.mocked(rename).mockResolvedValueOnce(undefined).mockRejectedValueOnce(new Error('rename failed'));
+      vi.mocked(unlink).mockRejectedValue(new Error('cleanup failed'));
+
+      await expect(service.uploadCover(12, Buffer.from('image-data'), 'image/jpeg', makeUser())).rejects.toThrow('rename failed');
+
+      expect(readdir).not.toHaveBeenCalled();
+      expect(unlink).not.toHaveBeenCalledWith('/tmp/books/covers/12/cover_custom.jpg');
+      expect(unlink).toHaveBeenCalledWith(expect.stringMatching(/^\/tmp\/books\/covers\/12\/\.cover-upload-.*\.jpg\.tmp$/));
+      expect(mockDb.insert).not.toHaveBeenCalled();
     });
 
     it('removes custom cover and restores extracted if available', async () => {

server/src/modules/cover/cover.service.ts

@@ -1,10 +1,11 @@
 import { BadRequestException, Inject, Injectable, Logger, NotFoundException } from '@nestjs/common';
 import { ConfigService } from '@nestjs/config';
 import { CoverSearchResult } from '@bookorbit/types';
+import { randomUUID } from 'crypto';
 import { eq } from 'drizzle-orm';
 import { NodePgDatabase } from 'drizzle-orm/node-postgres';
-import { mkdir, readdir, readFile, unlink, writeFile } from 'fs/promises';
-import { join } from 'path';
+import { mkdir, readdir, readFile, rename, unlink, writeFile } from 'fs/promises';
+import { basename, join } from 'path';
 
 import { bookCoverDirPath, bookThumbnailPath, findExtractedBookCoverFileName } from '../../common/book-cover-storage';
 import type { RequestUser } from '../../common/types/request-user';
@@ -17,7 +18,7 @@ import { BookReadService } from '../book/book-read.service';
 import { BookMetadataLockService } from '../book-metadata-lock/book-metadata-lock.service';
 import { FileWriteService } from '../file-write/file-write.service';
 import { LibraryService } from '../library/library.service';
-import { generateThumbnail, imageExt } from '../metadata/lib/cover';
+import { generateThumbnail, imageExt, normalizeProgressiveJpeg } from '../metadata/lib/cover';
 import {
   COVER_CUSTOM_FILE_PREFIX,
   COVER_PROXY_MAX_IMAGE_BYTES,
@@ -240,14 +241,42 @@ export class CoverService {
   }
 
   private async saveCustomCover(bookId: number, buffer: Buffer): Promise<void> {
+    let coverBytes: Buffer;
+    let thumb: Buffer;
+    try {
+      coverBytes = await normalizeProgressiveJpeg(buffer);
+      thumb = await generateThumbnail(coverBytes);
+    } catch {
+      throw new BadRequestException('Invalid image file');
+    }
+
     const dir = bookCoverDirPath(this.appDataPath, bookId);
     await mkdir(dir, { recursive: true });
-    await this.deleteFilesByPrefix(dir, COVER_CUSTOM_FILE_PREFIX);
 
-    const ext = imageExt(buffer);
-    await writeFile(join(dir, `${COVER_CUSTOM_FILE_PREFIX}${ext}`), buffer);
-    const thumb = await generateThumbnail(buffer);
-    await writeFile(bookThumbnailPath(this.appDataPath, bookId), thumb);
+    const ext = imageExt(coverBytes);
+    const coverPath = join(dir, `${COVER_CUSTOM_FILE_PREFIX}${ext}`);
+    const thumbnailPath = bookThumbnailPath(this.appDataPath, bookId);
+    const tempId = randomUUID();
+    const tempCoverPath = join(dir, `.cover-upload-${tempId}.${ext}.tmp`);
+    const tempThumbnailPath = join(dir, `.cover-upload-${tempId}.thumbnail.tmp`);
+    let tempCoverExists = false;
+    let tempThumbnailExists = false;
+
+    try {
+      await writeFile(tempCoverPath, coverBytes);
+      tempCoverExists = true;
+      await writeFile(tempThumbnailPath, thumb);
+      tempThumbnailExists = true;
+
+      await rename(tempThumbnailPath, thumbnailPath);
+      tempThumbnailExists = false;
+      await rename(tempCoverPath, coverPath);
+      tempCoverExists = false;
+      await this.deleteFilesByPrefix(dir, COVER_CUSTOM_FILE_PREFIX, basename(coverPath));
+    } finally {
+      const cleanupPaths = [...(tempCoverExists ? [tempCoverPath] : []), ...(tempThumbnailExists ? [tempThumbnailPath] : [])];
+      await Promise.all(cleanupPaths.map((path) => this.cleanupTemporaryCoverFile(bookId, path)));
+    }
   }
 
   private async findExtractedCover(bookId: number): Promise<string | null> {
@@ -395,10 +424,10 @@ export class CoverService {
     }
   }
 
-  private async deleteFilesByPrefix(path: string, prefix: string): Promise<void> {
+  private async deleteFilesByPrefix(path: string, prefix: string, exceptFileName?: string): Promise<void> {
     const files = await this.readDirIfExists(path);
     for (const fileName of files) {
-      if (!fileName.startsWith(prefix)) continue;
+      if (!fileName.startsWith(prefix) || fileName === exceptFileName) continue;
       await this.removeFileIfPresent(join(path, fileName));
     }
   }
@@ -412,6 +441,16 @@ export class CoverService {
     }
   }
 
+  private async cleanupTemporaryCoverFile(bookId: number, path: string): Promise<void> {
+    try {
+      await this.removeFileIfPresent(path);
+    } catch (error) {
+      this.logger.warn(
+        `[cover.upload_cleanup] [fail] bookId=${bookId} errorClass=${errorClass(error)} error="${sanitizeErrorMessage(error)}" - temporary cover upload file cleanup failed`,
+      );
+    }
+  }
+
   private isRedirectStatus(status: number): boolean {
     return REDIRECT_STATUS_CODES.has(status);
   }

server/src/modules/metadata/lib/cover-normalization.test.ts

@@ -0,0 +1,70 @@
+import sharp from 'sharp';
+
+import { normalizeProgressiveJpeg } from './cover';
+
+describe('normalizeProgressiveJpeg', () => {
+  async function makeJpeg(progressive: boolean, orientation?: number): Promise<Buffer> {
+    let image = sharp({
+      create: {
+        width: 20,
+        height: 30,
+        channels: 3,
+        background: '#785028',
+      },
+    });
+    if (orientation) image = image.withMetadata({ orientation });
+    return image.jpeg({ quality: 90, progressive }).toBuffer();
+  }
+
+  it('re-encodes progressive JPEGs as baseline while preserving metadata', async () => {
+    const source = await makeJpeg(true, 6);
+
+    const result = await normalizeProgressiveJpeg(source);
+
+    expect(result).not.toBe(source);
+    expect(result).not.toEqual(source);
+    const metadata = await sharp(result).metadata();
+    expect(metadata).toMatchObject({
+      format: 'jpeg',
+      isProgressive: false,
+      orientation: 6,
+      width: 20,
+      height: 30,
+    });
+  });
+
+  it('returns baseline JPEG bytes unchanged', async () => {
+    const source = await makeJpeg(false);
+
+    const result = await normalizeProgressiveJpeg(source);
+
+    expect(result).toBe(source);
+  });
+
+  it.each([
+    [
+      'PNG',
+      async () =>
+        sharp({ create: { width: 2, height: 2, channels: 4, background: '#123456' } })
+          .png()
+          .toBuffer(),
+    ],
+    [
+      'WebP',
+      async () =>
+        sharp({ create: { width: 2, height: 2, channels: 3, background: '#123456' } })
+          .webp()
+          .toBuffer(),
+    ],
+  ])('returns %s bytes unchanged', async (_format, createImage) => {
+    const source = await createImage();
+
+    const result = await normalizeProgressiveJpeg(source);
+
+    expect(result).toBe(source);
+  });
+
+  it('rejects invalid image bytes', async () => {
+    await expect(normalizeProgressiveJpeg(Buffer.from('not-an-image'))).rejects.toThrow();
+  });
+});

server/src/modules/metadata/lib/cover.ts

@@ -39,6 +39,13 @@ export async function generateThumbnail(bytes: Buffer): Promise<Buffer> {
   return sharp(bytes).resize(THUMBNAIL_WIDTH, THUMBNAIL_HEIGHT, { fit: 'inside', withoutEnlargement: true }).jpeg({ quality: 90 }).toBuffer();
 }
 
+export async function normalizeProgressiveJpeg(bytes: Buffer): Promise<Buffer> {
+  const metadata = await sharp(bytes, { failOn: 'error' }).metadata();
+  if (metadata.format !== 'jpeg' || !metadata.isProgressive) return bytes;
+
+  return sharp(bytes, { failOn: 'error' }).keepMetadata().jpeg({ quality: 90, progressive: false }).toBuffer();
+}
+
 /**
  * Extract cover bytes from a book file based on its format.
  * Returns null if no cover can be extracted.