From 9e3d9a7770dba87d3b4224d74f13ca2f883a93d8 Mon Sep 17 00:00:00 2001 From: Daniel Dietzler <36593685+danieldietzler@users.noreply.github.com> Date: Tue, 25 Aug 2026 19:57:48 +0200 Subject: [PATCH] fix: thumbnail generation for specific SVGs (#30908) --- e2e/src/specs/server/api/oauth.e2e-spec.ts | 15 + packages/e2e-auth-server/auth-server.ts | 9 + server/Dockerfile | 4 +- server/bin/start.sh | 1 - .../src/repositories/media.repository.spec.ts | 359 +----------------- server/src/repositories/oauth.repository.ts | 5 +- server/src/services/auth.service.spec.ts | 10 +- server/src/services/auth.service.ts | 2 +- 8 files changed, 48 insertions(+), 357 deletions(-) diff --git a/e2e/src/specs/server/api/oauth.e2e-spec.ts b/e2e/src/specs/server/api/oauth.e2e-spec.ts index 5bb569d5d..ac37ce2a6 100644 --- a/e2e/src/specs/server/api/oauth.e2e-spec.ts +++ b/e2e/src/specs/server/api/oauth.e2e-spec.ts @@ -286,6 +286,21 @@ describe(`/oauth`, () => { }); }); + it('should set the profile picture from a picture claim with an embedded image', async () => { + const callbackParams = await loginWithOAuth(OAuthUser.WITH_EMBEDDED_PROFILE_PICTURE); + const { status, body } = await request(app).post('/oauth/callback').send(callbackParams); + expect(status).toBe(201); + expect(body).toMatchObject({ + accessToken: expect.any(String), + userId: expect.any(String), + userEmail: 'oauth-with-embedded-profile-picture@immich.app', + profileImagePath: expect.any(String), + }); + + const user = await getMyUser({ headers: asBearerAuth(body.accessToken) }); + expect(user.profileImagePath.length).toBeGreaterThan(0); + }); + it('should work with RS256 signed user profiles', async () => { await setupOAuth(admin.accessToken, { enabled: true, diff --git a/packages/e2e-auth-server/auth-server.ts b/packages/e2e-auth-server/auth-server.ts index 15aaa71c1..f8c67e32d 100644 --- a/packages/e2e-auth-server/auth-server.ts +++ b/packages/e2e-auth-server/auth-server.ts @@ -21,6 +21,7 @@ export enum OAuthUser { WITH_QUOTA = 'with-quota', WITH_USERNAME = 'with-username', WITH_ROLE = 'with-role', + WITH_EMBEDDED_PROFILE_PICTURE = 'with-embedded-profile-picture', } const claims = [ @@ -49,6 +50,13 @@ const claims = [ email_verified: true, immich_role: 'admin', }, + { + sub: OAuthUser.WITH_EMBEDDED_PROFILE_PICTURE, + email: 'oauth-with-embedded-profile-picture@immich.app', + email_verified: true, + picture: + 'data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSI2NHB4IiBoZWlnaHQ9IjY0cHgiIHZpZXdCb3g9IjAgMCA2NCA2NCIgdmVyc2lvbj0iMS4xIj48cmVjdCBmaWxsPSIjMzc3YjM3IiBjeD0iMzIiIGN5PSIzMiIgd2lkdGg9IjY0IiBoZWlnaHQ9IjY0IiByPSIzMiIvPjx0ZXh0IHg9IjUwJSIgeT0iNTAlIiBzdHlsZT0iY29sb3I6ICNmZmY7IGxpbmUtaGVpZ2h0OiAxOyBmb250LWZhbWlseTogJ1JlZEhhdFRleHQnLCdPdmVycGFzcycsb3ZlcnBhc3MsaGVsdmV0aWNhLGFyaWFsLHNhbnMtc2VyaWY7ICIgZmlsbD0iI2ZmZiIgYWxpZ25tZW50LWJhc2VsaW5lPSJtaWRkbGUiIGRvbWluYW50LWJhc2VsaW5lPSJtaWRkbGUiIHRleHQtYW5jaG9yPSJtaWRkbGUiIGZvbnQtc2l6ZT0iMjgiIGZvbnQtd2VpZ2h0PSI0MDAiIGR5PSIuMWVtIj5BQTwvdGV4dD48L3N2Zz4=', + }, ]; const privateKey = await importPKCS8(PRIVATE_KEY_PEM, 'RS256', { @@ -124,6 +132,7 @@ const setup = async () => { 'immich_quota', 'immich_username', 'immich_role', + 'picture', ], }, features: { diff --git a/server/Dockerfile b/server/Dockerfile index 95f842e65..23b6c7584 100644 --- a/server/Dockerfile +++ b/server/Dockerfile @@ -1,4 +1,4 @@ -FROM ghcr.io/immich-app/base-server-dev:202608111116@sha256:f53e6bc2aa6e4c7b043e6ec1740448af4ba69e2090411daa7fa1df288bc0131a AS builder +FROM ghcr.io/immich-app/base-server-dev:202608251107@sha256:648929186198b6679e944292807625bd21c2c811d246898da6da4d54470ab35b AS builder ENV COREPACK_ENABLE_DOWNLOAD_PROMPT=0 \ CI=1 \ COREPACK_HOME=/tmp \ @@ -86,7 +86,7 @@ RUN --mount=type=cache,id=pnpm,target=/buildcache/pnpm-store \ --mount=type=cache,id=mise-tools-${TARGETPLATFORM},target=/buildcache/mise \ mise //:plugins -FROM ghcr.io/immich-app/base-server-prod:202608111116@sha256:15083a121ada9d243c3e717d84c2819a028f214c1f4bf3b39d0b63a299e994b3 +FROM ghcr.io/immich-app/base-server-prod:202608251107@sha256:7b7b04e96a20a1e542f289890d972ae8e587e8b7196097208f69ad91dba5d144 WORKDIR /usr/src/app ENV NODE_ENV=production \ diff --git a/server/bin/start.sh b/server/bin/start.sh index 15f441195..97d515fbb 100755 --- a/server/bin/start.sh +++ b/server/bin/start.sh @@ -21,7 +21,6 @@ if [ -f "$lib_path" ]; then else echo "skipping libmimalloc - path not found $lib_path" fi -export LD_LIBRARY_PATH="$LD_LIBRARY_PATH:/usr/lib/jellyfin-ffmpeg/lib" SERVER_HOME="$(readlink -f "$(dirname "$0")/..")" read_file_and_export() { diff --git a/server/src/repositories/media.repository.spec.ts b/server/src/repositories/media.repository.spec.ts index 08475b440..116f8af59 100644 --- a/server/src/repositories/media.repository.spec.ts +++ b/server/src/repositories/media.repository.spec.ts @@ -1,12 +1,11 @@ +import { mkdtempDisposableSync, statSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; import sharp from 'sharp'; -import { AssetFace } from 'src/database'; import { AssetEditAction, MirrorAxis } from 'src/dtos/editing.dto'; -import { AssetOcrResponseDto } from 'src/dtos/ocr.dto'; -import { SourceType } from 'src/enum'; +import { Colorspace, ImageFormat } from 'src/enum'; import { LoggingRepository } from 'src/repositories/logging.repository'; -import { BoundingBox } from 'src/repositories/machine-learning.repository'; import { MediaRepository } from 'src/repositories/media.repository'; -import { checkFaceVisibility, checkOcrVisibility } from 'src/utils/editor'; import { automock } from 'test/utils'; const getPixelColor = async (buffer: Buffer, x: number, y: number) => { @@ -325,343 +324,21 @@ describe(MediaRepository.name, () => { }); }); - describe('checkFaceVisibility', () => { - const baseFace: AssetFace = { - id: 'face-1', - assetId: 'asset-1', - personGroupId: 'person-1', - boundingBoxX1: 100, - boundingBoxY1: 100, - boundingBoxX2: 200, - boundingBoxY2: 200, - imageWidth: 1000, - imageHeight: 800, - sourceType: SourceType.MachineLearning, - isVisible: true, - updatedAt: new Date(), - deletedAt: null, - updateId: '', - }; + describe('generateThumbnail', () => { + it('should process random Authentik thumbnail image', async () => { + const response = await fetch( + 'data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSI2NHB4IiBoZWlnaHQ9IjY0cHgiIHZpZXdCb3g9IjAgMCA2NCA2NCIgdmVyc2lvbj0iMS4xIj48cmVjdCBmaWxsPSIjMzc3YjM3IiBjeD0iMzIiIGN5PSIzMiIgd2lkdGg9IjY0IiBoZWlnaHQ9IjY0IiByPSIzMiIvPjx0ZXh0IHg9IjUwJSIgeT0iNTAlIiBzdHlsZT0iY29sb3I6ICNmZmY7IGxpbmUtaGVpZ2h0OiAxOyBmb250LWZhbWlseTogJ1JlZEhhdFRleHQnLCdPdmVycGFzcycsb3ZlcnBhc3MsaGVsdmV0aWNhLGFyaWFsLHNhbnMtc2VyaWY7ICIgZmlsbD0iI2ZmZiIgYWxpZ25tZW50LWJhc2VsaW5lPSJtaWRkbGUiIGRvbWluYW50LWJhc2VsaW5lPSJtaWRkbGUiIHRleHQtYW5jaG9yPSJtaWRkbGUiIGZvbnQtc2l6ZT0iMjgiIGZvbnQtd2VpZ2h0PSI0MDAiIGR5PSIuMWVtIj5BQTwvdGV4dD48L3N2Zz4=', + ); + const buffer = Buffer.from(await response.arrayBuffer()); + const dir = mkdtempDisposableSync(join(tmpdir(), 'media-repository-')); + const file = join(dir.path, 'test.webp'); + await sut.generateThumbnail( + buffer, + { colorspace: Colorspace.P3, quality: 80, format: ImageFormat.Webp, processInvalidImages: false }, + file, + ); - const assetDimensions = { width: 1000, height: 800 }; - - describe('with no crop edit', () => { - it('should return only currently invisible faces when no crop is provided', () => { - const visibleFace = { ...baseFace, id: 'face-visible', isVisible: true }; - const invisibleFace = { ...baseFace, id: 'face-invisible', isVisible: false }; - const faces = [visibleFace, invisibleFace]; - const result = checkFaceVisibility(faces, assetDimensions); - - expect(result.visible).toEqual([invisibleFace]); - expect(result.hidden).toEqual([]); - }); - - it('should return empty arrays when all faces are already visible and no crop is provided', () => { - const faces = [baseFace]; - const result = checkFaceVisibility(faces, assetDimensions); - - expect(result.visible).toEqual([]); - expect(result.hidden).toEqual([]); - }); - - it('should return all faces when all are invisible and no crop is provided', () => { - const face1 = { ...baseFace, id: 'face-1', isVisible: false }; - const face2 = { ...baseFace, id: 'face-2', isVisible: false }; - const faces = [face1, face2]; - const result = checkFaceVisibility(faces, assetDimensions); - - expect(result.visible).toEqual([face1, face2]); - expect(result.hidden).toEqual([]); - }); - }); - - describe('with crop edit', () => { - it('should mark face as visible when fully inside crop area', () => { - const crop: BoundingBox = { x1: 0, y1: 0, x2: 500, y2: 400 }; - const faces = [baseFace]; - const result = checkFaceVisibility(faces, assetDimensions, crop); - - expect(result.visible).toEqual(faces); - expect(result.hidden).toEqual([]); - }); - - it('should mark face as visible when more than 50% inside crop area', () => { - const crop: BoundingBox = { x1: 150, y1: 150, x2: 650, y2: 550 }; - // Face at (100,100)-(200,200), crop starts at (150,150) - // Overlap: (150,150)-(200,200) = 50x50 = 2500 - // Face area: 100x100 = 10000 - // Overlap percentage: 25% - should be hidden - const faces = [baseFace]; - const result = checkFaceVisibility(faces, assetDimensions, crop); - - expect(result.visible).toEqual([]); - expect(result.hidden).toEqual(faces); - }); - - it('should mark face as hidden when less than 50% inside crop area', () => { - const crop: BoundingBox = { x1: 250, y1: 250, x2: 750, y2: 650 }; - // Face completely outside crop area - const faces = [baseFace]; - const result = checkFaceVisibility(faces, assetDimensions, crop); - - expect(result.visible).toEqual([]); - expect(result.hidden).toEqual(faces); - }); - - it('should mark face as hidden when completely outside crop area', () => { - const crop: BoundingBox = { x1: 500, y1: 500, x2: 700, y2: 700 }; - const faces = [baseFace]; - const result = checkFaceVisibility(faces, assetDimensions, crop); - - expect(result.visible).toEqual([]); - expect(result.hidden).toEqual(faces); - }); - - it('should handle multiple faces with mixed visibility', () => { - const crop: BoundingBox = { x1: 0, y1: 0, x2: 300, y2: 300 }; - const faceInside: AssetFace = { - ...baseFace, - id: 'face-inside', - boundingBoxX1: 50, - boundingBoxY1: 50, - boundingBoxX2: 150, - boundingBoxY2: 150, - }; - const faceOutside: AssetFace = { - ...baseFace, - id: 'face-outside', - boundingBoxX1: 400, - boundingBoxY1: 400, - boundingBoxX2: 500, - boundingBoxY2: 500, - }; - const faces = [faceInside, faceOutside]; - const result = checkFaceVisibility(faces, assetDimensions, crop); - - expect(result.visible).toEqual([faceInside]); - expect(result.hidden).toEqual([faceOutside]); - }); - - it('should handle face at exactly 50% overlap threshold', () => { - // Face at (0,0)-(100,100), crop at (50,0)-(150,100) - // Overlap: (50,0)-(100,100) = 50x100 = 5000 - // Face area: 100x100 = 10000 - // Overlap percentage: 50% - exactly at threshold, should be visible - const faceAtEdge: AssetFace = { - ...baseFace, - id: 'face-edge', - boundingBoxX1: 0, - boundingBoxY1: 0, - boundingBoxX2: 100, - boundingBoxY2: 100, - }; - const crop: BoundingBox = { x1: 50, y1: 0, x2: 150, y2: 100 }; - const faces = [faceAtEdge]; - const result = checkFaceVisibility(faces, assetDimensions, crop); - - expect(result.visible).toEqual([faceAtEdge]); - expect(result.hidden).toEqual([]); - }); - }); - - describe('with scaled dimensions', () => { - it('should handle faces when asset dimensions differ from face image dimensions', () => { - // Face stored at 1000x800 resolution, but displaying at 500x400 - const scaledDimensions = { width: 500, height: 400 }; - const crop: BoundingBox = { x1: 0, y1: 0, x2: 250, y2: 200 }; - // Face at (100,100)-(200,200) on 1000x800 - // Scaled to 500x400: (50,50)-(100,100) - // Crop at (0,0)-(250,200) - face is fully inside - const faces = [baseFace]; - const result = checkFaceVisibility(faces, scaledDimensions, crop); - - expect(result.visible).toEqual(faces); - expect(result.hidden).toEqual([]); - }); - }); - }); - - describe('checkOcrVisibility', () => { - const baseOcr: AssetOcrResponseDto & { isVisible: boolean } = { - id: 'ocr-1', - assetId: 'asset-1', - x1: 0.1, - y1: 0.1, - x2: 0.2, - y2: 0.1, - x3: 0.2, - y3: 0.2, - x4: 0.1, - y4: 0.2, - boxScore: 0.9, - textScore: 0.85, - text: 'Test OCR', - isVisible: false, - }; - - const assetDimensions = { width: 1000, height: 800 }; - - describe('with no crop edit', () => { - it('should return only currently invisible OCR items when no crop is provided', () => { - const visibleOcr = { ...baseOcr, id: 'ocr-visible', isVisible: true }; - const invisibleOcr = { ...baseOcr, id: 'ocr-invisible', isVisible: false }; - const ocrs = [visibleOcr, invisibleOcr]; - const result = checkOcrVisibility(ocrs, assetDimensions); - - expect(result.visible).toEqual([invisibleOcr]); - expect(result.hidden).toEqual([]); - }); - - it('should return empty arrays when all OCR items are already visible and no crop is provided', () => { - const visibleOcr = { ...baseOcr, isVisible: true }; - const ocrs = [visibleOcr]; - const result = checkOcrVisibility(ocrs, assetDimensions); - - expect(result.visible).toEqual([]); - expect(result.hidden).toEqual([]); - }); - - it('should return all OCR items when all are invisible and no crop is provided', () => { - const ocr1 = { ...baseOcr, id: 'ocr-1', isVisible: false }; - const ocr2 = { ...baseOcr, id: 'ocr-2', isVisible: false }; - const ocrs = [ocr1, ocr2]; - const result = checkOcrVisibility(ocrs, assetDimensions); - - expect(result.visible).toEqual([ocr1, ocr2]); - expect(result.hidden).toEqual([]); - }); - }); - - describe('with crop edit', () => { - it('should mark OCR as visible when fully inside crop area', () => { - const crop: BoundingBox = { x1: 0, y1: 0, x2: 500, y2: 400 }; - // OCR box: (0.1,0.1)-(0.2,0.2) on 1000x800 = (100,80)-(200,160) - // Crop: (0,0)-(500,400) - OCR fully inside - const ocrs = [baseOcr]; - const result = checkOcrVisibility(ocrs, assetDimensions, crop); - - expect(result.visible).toEqual(ocrs); - expect(result.hidden).toEqual([]); - }); - - it('should mark OCR as hidden when completely outside crop area', () => { - const crop: BoundingBox = { x1: 500, y1: 500, x2: 700, y2: 700 }; - // OCR box: (100,80)-(200,160) - completely outside crop - const ocrs = [baseOcr]; - const result = checkOcrVisibility(ocrs, assetDimensions, crop); - - expect(result.visible).toEqual([]); - expect(result.hidden).toEqual(ocrs); - }); - - it('should mark OCR as hidden when less than 50% inside crop area', () => { - const crop: BoundingBox = { x1: 150, y1: 120, x2: 650, y2: 520 }; - // OCR box: (100,80)-(200,160) - // Crop: (150,120)-(650,520) - // Overlap: (150,120)-(200,160) = 50x40 = 2000 - // OCR area: 100x80 = 8000 - // Overlap percentage: 25% - should be hidden - const ocrs = [baseOcr]; - const result = checkOcrVisibility(ocrs, assetDimensions, crop); - - expect(result.visible).toEqual([]); - expect(result.hidden).toEqual(ocrs); - }); - - it('should handle multiple OCR items with mixed visibility', () => { - const crop: BoundingBox = { x1: 0, y1: 0, x2: 300, y2: 300 }; - const ocrInside = { - ...baseOcr, - id: 'ocr-inside', - }; - const ocrOutside = { - ...baseOcr, - id: 'ocr-outside', - x1: 0.5, - y1: 0.5, - x2: 0.6, - y2: 0.5, - x3: 0.6, - y3: 0.6, - x4: 0.5, - y4: 0.6, - }; - const ocrs = [ocrInside, ocrOutside]; - const result = checkOcrVisibility(ocrs, assetDimensions, crop); - - expect(result.visible).toEqual([ocrInside]); - expect(result.hidden).toEqual([ocrOutside]); - }); - - it('should handle OCR boxes with rotated/skewed polygons', () => { - // OCR with a rotated bounding box (not axis-aligned) - const rotatedOcr = { - ...baseOcr, - id: 'ocr-rotated', - x1: 0.15, - y1: 0.1, - x2: 0.25, - y2: 0.15, - x3: 0.2, - y3: 0.25, - x4: 0.1, - y4: 0.2, - }; - const crop: BoundingBox = { x1: 0, y1: 0, x2: 300, y2: 300 }; - const ocrs = [rotatedOcr]; - const result = checkOcrVisibility(ocrs, assetDimensions, crop); - - expect(result.visible).toEqual([rotatedOcr]); - expect(result.hidden).toEqual([]); - }); - }); - - describe('visibility is only affected by crop (not rotate or mirror)', () => { - it('should keep all OCR items visible when there is no crop regardless of other transforms', () => { - // Rotate and mirror edits don't affect visibility - only crop does - // The visibility functions only take an optional crop parameter - const ocrs = [baseOcr]; - - // Without any crop, all OCR items remain visible - const result = checkOcrVisibility(ocrs, assetDimensions); - - expect(result.visible).toEqual(ocrs); - expect(result.hidden).toEqual([]); - }); - - it('should only consider crop for visibility calculation', () => { - // Even if the image will be rotated/mirrored, visibility is determined - // solely by whether the OCR box overlaps with the crop area - const crop: BoundingBox = { x1: 0, y1: 0, x2: 300, y2: 300 }; - - const ocrInsideCrop = { - ...baseOcr, - id: 'ocr-inside', - // OCR at (0.1,0.1)-(0.2,0.2) = (100,80)-(200,160) on 1000x800, inside crop - }; - - const ocrOutsideCrop = { - ...baseOcr, - id: 'ocr-outside', - x1: 0.5, - y1: 0.5, - x2: 0.6, - y2: 0.5, - x3: 0.6, - y3: 0.6, - x4: 0.5, - y4: 0.6, - // OCR at (500,400)-(600,480) on 1000x800, outside crop - }; - - const ocrs = [ocrInsideCrop, ocrOutsideCrop]; - const result = checkOcrVisibility(ocrs, assetDimensions, crop); - - // OCR inside crop area is visible, OCR outside is hidden - // This is true regardless of any subsequent rotate/mirror operations - expect(result.visible).toEqual([ocrInsideCrop]); - expect(result.hidden).toEqual([ocrOutsideCrop]); - }); + expect(statSync(file).blksize).toBeGreaterThan(0); }); }); }); diff --git a/server/src/repositories/oauth.repository.ts b/server/src/repositories/oauth.repository.ts index 8aa6cfee9..bbf7ef00b 100644 --- a/server/src/repositories/oauth.repository.ts +++ b/server/src/repositories/oauth.repository.ts @@ -135,10 +135,7 @@ export class OAuthRepository { throw new Error(`Failed to fetch picture: ${response.statusText}`); } - return { - data: await response.arrayBuffer(), - contentType: response.headers.get('content-type'), - }; + return response.arrayBuffer(); } private jwksClients: Map = new Map(); // useful for caching and performnce diff --git a/server/src/services/auth.service.spec.ts b/server/src/services/auth.service.spec.ts index 48dcf5a50..6a7469344 100644 --- a/server/src/services/auth.service.spec.ts +++ b/server/src/services/auth.service.spec.ts @@ -995,10 +995,7 @@ describe(AuthService.name, () => { mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ profile }); mocks.user.getByOAuthId.mockResolvedValue(user); mocks.crypto.randomUUID.mockReturnValue(fileId); - mocks.oauth.getProfilePicture.mockResolvedValue({ - contentType: 'image/jpeg', - data: pictureBytes.buffer, - }); + mocks.oauth.getProfilePicture.mockResolvedValue(pictureBytes.buffer); mocks.user.update.mockResolvedValue(user); mocks.session.create.mockResolvedValue(SessionFactory.create()); @@ -1027,10 +1024,7 @@ describe(AuthService.name, () => { mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthEnabled); mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ profile }); mocks.user.getByOAuthId.mockResolvedValue(user); - mocks.oauth.getProfilePicture.mockResolvedValue({ - contentType: 'text/html', - data: new Uint8Array([1, 2, 3, 4, 5]).buffer, - }); + mocks.oauth.getProfilePicture.mockResolvedValue(new Uint8Array([1, 2, 3, 4, 5]).buffer); mocks.media.generateThumbnail.mockRejectedValue(new Error('not an image')); mocks.session.create.mockResolvedValue(SessionFactory.create()); diff --git a/server/src/services/auth.service.ts b/server/src/services/auth.service.ts index 560381921..3229ae2b0 100644 --- a/server/src/services/auth.service.ts +++ b/server/src/services/auth.service.ts @@ -378,7 +378,7 @@ export class AuthService extends BaseService { private async syncProfilePicture(user: UserAdmin, url: string) { try { const oldPath = user.profileImagePath; - const { data } = await this.oauthRepository.getProfilePicture(url); + const data = await this.oauthRepository.getProfilePicture(url); const config = await this.getConfig({ withCache: true }); const profileImagePath = await generateProfileImage(