fix(server): file sending and cache control (#5829)

* fix: file sending

* fix: tests

server/src/domain/asset/asset.service.spec.ts

@@ -16,7 +16,7 @@ import {
 } from '@test';
 import { when } from 'jest-when';
 import { Readable } from 'stream';
-import { ImmichFileResponse } from '../domain.util';
+import { CacheControl, ImmichFileResponse } from '../domain.util';
 import { JobName } from '../job';
 import {
   AssetStats,
@@ -482,7 +482,7 @@ describe(AssetService.name, () => {
         new ImmichFileResponse({
           path: '/original/path.jpg',
           contentType: 'image/jpeg',
-          cacheControl: false,
+          cacheControl: CacheControl.NONE,
         }),
       );
     });

server/src/domain/asset/asset.service.ts

@@ -8,7 +8,7 @@ import sanitize from 'sanitize-filename';
 import { AccessCore, Permission } from '../access';
 import { AuthDto } from '../auth';
 import { mimeTypes } from '../domain.constant';
-import { HumanReadableSize, ImmichFileResponse, usePagination } from '../domain.util';
+import { CacheControl, HumanReadableSize, ImmichFileResponse, usePagination } from '../domain.util';
 import { IAssetDeletionJob, ISidecarWriteJob, JOBS_ASSET_PAGINATION_SIZE, JobName } from '../job';
 import {
   ClientEvent,
@@ -290,7 +290,7 @@ export class AssetService {
     return new ImmichFileResponse({
       path: asset.originalPath,
       contentType: mimeTypes.lookup(asset.originalPath),
-      cacheControl: false,
+      cacheControl: CacheControl.NONE,
     });
   }
 

server/src/domain/domain.util.ts

@@ -16,10 +16,16 @@ import { CronJob } from 'cron';
 import { basename, extname } from 'node:path';
 import sanitize from 'sanitize-filename';
 
+export enum CacheControl {
+  PRIVATE_WITH_CACHE = 'private_with_cache',
+  PRIVATE_WITHOUT_CACHE = 'private_without_cache',
+  NONE = 'none',
+}
+
 export class ImmichFileResponse {
   public readonly path!: string;
   public readonly contentType!: string;
-  public readonly cacheControl!: boolean;
+  public readonly cacheControl!: CacheControl;
 
   constructor(response: ImmichFileResponse) {
     Object.assign(this, response);

server/src/domain/person/person.service.spec.ts

@@ -18,7 +18,7 @@ import {
   personStub,
 } from '@test';
 import { BulkIdErrorReason } from '../asset';
-import { ImmichFileResponse } from '../domain.util';
+import { CacheControl, ImmichFileResponse } from '../domain.util';
 import { JobName } from '../job';
 import {
   IAssetRepository,
@@ -208,7 +208,7 @@ describe(PersonService.name, () => {
         new ImmichFileResponse({
           path: '/path/to/thumbnail.jpg',
           contentType: 'image/jpeg',
-          cacheControl: true,
+          cacheControl: CacheControl.PRIVATE_WITHOUT_CACHE,
         }),
       );
       expect(accessMock.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1']));

server/src/domain/person/person.service.ts

@@ -6,7 +6,7 @@ import { AccessCore, Permission } from '../access';
 import { AssetResponseDto, BulkIdErrorReason, BulkIdResponseDto, mapAsset } from '../asset';
 import { AuthDto } from '../auth';
 import { mimeTypes } from '../domain.constant';
-import { ImmichFileResponse, usePagination } from '../domain.util';
+import { CacheControl, ImmichFileResponse, usePagination } from '../domain.util';
 import { IBaseJob, IEntityJob, JOBS_ASSET_PAGINATION_SIZE, JobName } from '../job';
 import { FACE_THUMBNAIL_SIZE } from '../media';
 import {
@@ -183,7 +183,7 @@ export class PersonService {
     return new ImmichFileResponse({
       path: person.thumbnailPath,
       contentType: mimeTypes.lookup(person.thumbnailPath),
-      cacheControl: true,
+      cacheControl: CacheControl.PRIVATE_WITHOUT_CACHE,
     });
   }
 

server/src/domain/user/user.service.spec.ts

@@ -17,7 +17,7 @@ import {
   userStub,
 } from '@test';
 import { when } from 'jest-when';
-import { ImmichFileResponse } from '../domain.util';
+import { CacheControl, ImmichFileResponse } from '../domain.util';
 import { JobName } from '../job';
 import {
   IAlbumRepository,
@@ -396,7 +396,7 @@ describe(UserService.name, () => {
         new ImmichFileResponse({
           path: '/path/to/profile.jpg',
           contentType: 'image/jpeg',
-          cacheControl: false,
+          cacheControl: CacheControl.NONE,
         }),
       );
 

server/src/domain/user/user.service.ts

@@ -3,7 +3,7 @@ import { ImmichLogger } from '@app/infra/logger';
 import { BadRequestException, ForbiddenException, Inject, Injectable, NotFoundException } from '@nestjs/common';
 import { randomBytes } from 'crypto';
 import { AuthDto } from '../auth';
-import { ImmichFileResponse } from '../domain.util';
+import { CacheControl, ImmichFileResponse } from '../domain.util';
 import { IEntityJob, JobName } from '../job';
 import {
   IAlbumRepository,
@@ -109,7 +109,7 @@ export class UserService {
     return new ImmichFileResponse({
       path: user.profileImagePath,
       contentType: 'image/jpeg',
-      cacheControl: false,
+      cacheControl: CacheControl.NONE,
     });
   }
 

server/src/immich/api-v1/asset/asset.controller.ts

@@ -5,18 +5,20 @@ import {
   Get,
   HttpCode,
   HttpStatus,
+  Next,
   Param,
   ParseFilePipe,
   Post,
   Query,
-  Response,
+  Res,
   UploadedFiles,
   UseInterceptors,
   ValidationPipe,
 } from '@nestjs/common';
 import { ApiBody, ApiConsumes, ApiHeader, ApiOperation, ApiTags } from '@nestjs/swagger';
-import { Response as Res } from 'express';
+import { NextFunction, Response } from 'express';
 import { Auth, Authenticated, FileResponse, SharedLinkRoute } from '../../app.guard';
+import { sendFile } from '../../app.utils';
 import { UUIDParamDto } from '../../controllers/dto/uuid-param.dto';
 import { FileUploadInterceptor, ImmichFile, Route, mapToUploadFile } from '../../interceptors';
 import FileNotEmptyValidator from '../validation/file-not-empty-validator';
@@ -58,7 +60,7 @@ export class AssetController {
     @Auth() auth: AuthDto,
     @UploadedFiles(new ParseFilePipe({ validators: [new FileNotEmptyValidator(['assetData'])] })) files: UploadFiles,
     @Body(new ValidationPipe({ transform: true })) dto: CreateAssetDto,
-    @Response({ passthrough: true }) res: Res,
+    @Res({ passthrough: true }) res: Response,
   ): Promise<AssetFileUploadResponseDto> {
     const file = mapToUploadFile(files.assetData[0]);
     const _livePhotoFile = files.livePhotoData?.[0];
@@ -84,23 +86,27 @@ export class AssetController {
   @SharedLinkRoute()
   @Get('/file/:id')
   @FileResponse()
-  serveFile(
+  async serveFile(
+    @Res() res: Response,
+    @Next() next: NextFunction,
     @Auth() auth: AuthDto,
     @Param() { id }: UUIDParamDto,
     @Query(new ValidationPipe({ transform: true })) dto: ServeFileDto,
   ) {
-    return this.assetService.serveFile(auth, id, dto);
+    await sendFile(res, next, () => this.assetService.serveFile(auth, id, dto));
   }
 
   @SharedLinkRoute()
   @Get('/thumbnail/:id')
   @FileResponse()
-  getAssetThumbnail(
+  async getAssetThumbnail(
+    @Res() res: Response,
+    @Next() next: NextFunction,
     @Auth() auth: AuthDto,
     @Param() { id }: UUIDParamDto,
     @Query(new ValidationPipe({ transform: true })) dto: GetAssetThumbnailDto,
   ) {
-    return this.assetService.serveThumbnail(auth, id, dto);
+    await sendFile(res, next, () => this.assetService.serveThumbnail(auth, id, dto));
   }
 
   @Get('/curated-objects')

server/src/immich/api-v1/asset/asset.service.ts

@@ -2,6 +2,7 @@ import {
   AccessCore,
   AssetResponseDto,
   AuthDto,
+  CacheControl,
   getLivePhotoMotionFilename,
   IAccessRepository,
   IJobRepository,
@@ -147,7 +148,11 @@ export class AssetService {
 
     const filepath = this.getThumbnailPath(asset, dto.format);
 
-    return new ImmichFileResponse({ path: filepath, contentType: mimeTypes.lookup(filepath), cacheControl: true });
+    return new ImmichFileResponse({
+      path: filepath,
+      contentType: mimeTypes.lookup(filepath),
+      cacheControl: CacheControl.PRIVATE_WITH_CACHE,
+    });
   }
 
   public async serveFile(auth: AuthDto, assetId: string, dto: ServeFileDto): Promise<ImmichFileResponse> {
@@ -166,7 +171,11 @@ export class AssetService {
         ? this.getServePath(asset, dto, allowOriginalFile)
         : asset.encodedVideoPath || asset.originalPath;
 
-    return new ImmichFileResponse({ path: filepath, contentType: mimeTypes.lookup(filepath), cacheControl: true });
+    return new ImmichFileResponse({
+      path: filepath,
+      contentType: mimeTypes.lookup(filepath),
+      cacheControl: CacheControl.PRIVATE_WITH_CACHE,
+    });
   }
 
   async getAssetSearchTerm(auth: AuthDto): Promise<string[]> {

server/src/immich/app.module.ts

@@ -32,7 +32,7 @@ import {
   TagController,
   UserController,
 } from './controllers';
-import { ErrorInterceptor, FileServeInterceptor, FileUploadInterceptor } from './interceptors';
+import { ErrorInterceptor, FileUploadInterceptor } from './interceptors';
 
 @Module({
   imports: [
@@ -66,7 +66,6 @@ import { ErrorInterceptor, FileServeInterceptor, FileUploadInterceptor } from '.
   ],
   providers: [
     { provide: APP_INTERCEPTOR, useClass: ErrorInterceptor },
-    { provide: APP_INTERCEPTOR, useClass: FileServeInterceptor },
     { provide: APP_GUARD, useClass: AppGuard },
     { provide: IAssetRepository, useClass: AssetRepository },
     AppService,

server/src/immich/app.utils.ts

@@ -1,11 +1,15 @@
 import {
+  CacheControl,
   IMMICH_ACCESS_COOKIE,
   IMMICH_API_KEY_HEADER,
   IMMICH_API_KEY_NAME,
+  ImmichFileResponse,
   ImmichReadStream,
+  isConnectionAborted,
   serverVersion,
 } from '@app/domain';
-import { INestApplication, StreamableFile } from '@nestjs/common';
+import { ImmichLogger } from '@app/infra/logger';
+import { HttpException, INestApplication, StreamableFile } from '@nestjs/common';
 import {
   DocumentBuilder,
   OpenAPIObject,
@@ -13,8 +17,11 @@ import {
   SwaggerDocumentOptions,
   SwaggerModule,
 } from '@nestjs/swagger';
+import { NextFunction, Response } from 'express';
 import { writeFileSync } from 'fs';
-import path from 'path';
+import { access, constants } from 'fs/promises';
+import path, { isAbsolute } from 'path';
+import { promisify } from 'util';
 
 import { applyDecorators, UsePipes, ValidationPipe } from '@nestjs/common';
 import { Metadata } from './app.guard';
@@ -30,6 +37,57 @@ export function UseValidation() {
   );
 }
 
+type SendFile = Parameters<Response['sendFile']>;
+type SendFileOptions = SendFile[1];
+
+const logger = new ImmichLogger('SendFile');
+
+export const sendFile = async (
+  res: Response,
+  next: NextFunction,
+  handler: () => Promise<ImmichFileResponse>,
+): Promise<void> => {
+  const _sendFile = (path: string, options: SendFileOptions) =>
+    promisify<string, SendFileOptions>(res.sendFile).bind(res)(path, options);
+
+  try {
+    const file = await handler();
+    switch (file.cacheControl) {
+      case CacheControl.PRIVATE_WITH_CACHE:
+        res.set('Cache-Control', 'private, max-age=86400, no-transform');
+        break;
+
+      case CacheControl.PRIVATE_WITHOUT_CACHE:
+        res.set('Cache-Control', 'private, no-cache, no-transform');
+        break;
+    }
+
+    res.header('Content-Type', file.contentType);
+
+    const options: SendFileOptions = { dotfiles: 'allow' };
+    if (!isAbsolute(file.path)) {
+      options.root = process.cwd();
+    }
+
+    await access(file.path, constants.R_OK);
+
+    return _sendFile(file.path, options);
+  } catch (error: Error | any) {
+    // ignore client-closed connection
+    if (isConnectionAborted(error)) {
+      return;
+    }
+
+    // log non-http errors
+    if (error instanceof HttpException === false) {
+      logger.error(`Unable to send file: ${error.name}`, error.stack);
+    }
+
+    res.header('Cache-Control', 'none');
+    next(error);
+  }
+};
+
 export const asStreamableFile = ({ stream, type, length }: ImmichReadStream) => {
   return new StreamableFile(stream, { type, length });
 };

server/src/immich/controllers/asset.controller.ts

@@ -12,7 +12,6 @@ import {
   BulkIdsDto,
   DownloadInfoDto,
   DownloadResponseDto,
-  ImmichFileResponse,
   MapMarkerDto,
   MapMarkerResponseDto,
   MemoryLaneDto,
@@ -32,16 +31,19 @@ import {
   Get,
   HttpCode,
   HttpStatus,
+  Next,
   Param,
   Post,
   Put,
   Query,
+  Res,
   StreamableFile,
 } from '@nestjs/common';
 import { ApiTags } from '@nestjs/swagger';
+import { NextFunction, Response } from 'express';
 import { DeviceIdDto } from '../api-v1/asset/dto/device-id.dto';
 import { Auth, Authenticated, FileResponse, SharedLinkRoute } from '../app.guard';
-import { UseValidation, asStreamableFile } from '../app.utils';
+import { UseValidation, asStreamableFile, sendFile } from '../app.utils';
 import { Route } from '../interceptors';
 import { UUIDParamDto } from './dto/uuid-param.dto';
 
@@ -98,8 +100,13 @@ export class AssetController {
   @Post('download/:id')
   @HttpCode(HttpStatus.OK)
   @FileResponse()
-  downloadFile(@Auth() auth: AuthDto, @Param() { id }: UUIDParamDto): Promise<ImmichFileResponse> {
-    return this.service.downloadFile(auth, id);
+  async downloadFile(
+    @Res() res: Response,
+    @Next() next: NextFunction,
+    @Auth() auth: AuthDto,
+    @Param() { id }: UUIDParamDto,
+  ) {
+    await sendFile(res, next, () => this.service.downloadFile(auth, id));
   }
 
   /**

server/src/immich/controllers/person.controller.ts

@@ -12,10 +12,11 @@ import {
   PersonStatisticsResponseDto,
   PersonUpdateDto,
 } from '@app/domain';
-import { Body, Controller, Get, Param, Post, Put, Query } from '@nestjs/common';
+import { Body, Controller, Get, Next, Param, Post, Put, Query, Res } from '@nestjs/common';
 import { ApiTags } from '@nestjs/swagger';
+import { NextFunction, Response } from 'express';
 import { Auth, Authenticated, FileResponse } from '../app.guard';
-import { UseValidation } from '../app.utils';
+import { UseValidation, sendFile } from '../app.utils';
 import { UUIDParamDto } from './dto/uuid-param.dto';
 
 @ApiTags('Person')
@@ -70,8 +71,13 @@ export class PersonController {
 
   @Get(':id/thumbnail')
   @FileResponse()
-  getPersonThumbnail(@Auth() auth: AuthDto, @Param() { id }: UUIDParamDto) {
-    return this.service.getThumbnail(auth, id);
+  async getPersonThumbnail(
+    @Res() res: Response,
+    @Next() next: NextFunction,
+    @Auth() auth: AuthDto,
+    @Param() { id }: UUIDParamDto,
+  ) {
+    await sendFile(res, next, () => this.service.getThumbnail(auth, id));
   }
 
   @Get(':id/assets')

server/src/immich/controllers/user.controller.ts

@@ -3,7 +3,6 @@ import {
   CreateUserDto as CreateDto,
   CreateProfileImageDto,
   CreateProfileImageResponseDto,
-  ImmichFileResponse,
   UpdateUserDto as UpdateDto,
   UserResponseDto,
   UserService,
@@ -13,19 +12,21 @@ import {
   Controller,
   Delete,
   Get,
-  Header,
   HttpCode,
   HttpStatus,
+  Next,
   Param,
   Post,
   Put,
   Query,
+  Res,
   UploadedFile,
   UseInterceptors,
 } from '@nestjs/common';
 import { ApiBody, ApiConsumes, ApiTags } from '@nestjs/swagger';
+import { NextFunction, Response } from 'express';
 import { AdminRoute, Auth, Authenticated, FileResponse } from '../app.guard';
-import { UseValidation } from '../app.utils';
+import { UseValidation, sendFile } from '../app.utils';
 import { FileUploadInterceptor, Route } from '../interceptors';
 import { UUIDParamDto } from './dto/uuid-param.dto';
 
@@ -93,9 +94,8 @@ export class UserController {
   }
 
   @Get('profile-image/:id')
-  @Header('Cache-Control', 'private, no-cache, no-transform')
   @FileResponse()
-  getProfileImage(@Param() { id }: UUIDParamDto): Promise<ImmichFileResponse> {
-    return this.service.getProfileImage(id);
+  async getProfileImage(@Res() res: Response, @Next() next: NextFunction, @Param() { id }: UUIDParamDto) {
+    await sendFile(res, next, () => this.service.getProfileImage(id));
   }
 }

server/src/immich/interceptors/error.interceptor.ts

@@ -22,7 +22,7 @@ export class ErrorInterceptor implements NestInterceptor {
           if (error instanceof HttpException === false) {
             const errorMessage = routeToErrorMessage(context.getHandler().name);
             if (!isConnectionAborted(error)) {
-              this.logger.error(errorMessage, error, error?.errors);
+              this.logger.error(errorMessage, error, error?.errors, error?.stack);
             }
             return new InternalServerErrorException(errorMessage);
           } else {

server/src/immich/interceptors/file-serve.interceptor.ts

@@ -1,56 +0,0 @@
-import { ImmichFileResponse, isConnectionAborted } from '@app/domain';
-import { ImmichLogger } from '@app/infra/logger';
-import { CallHandler, ExecutionContext, NestInterceptor } from '@nestjs/common';
-import { Response } from 'express';
-import { access, constants } from 'fs/promises';
-import { isAbsolute } from 'path';
-import { Observable, mergeMap } from 'rxjs';
-import { promisify } from 'util';
-
-type SendFile = Parameters<Response['sendFile']>;
-type SendFileOptions = SendFile[1];
-
-export class FileServeInterceptor implements NestInterceptor {
-  private logger = new ImmichLogger(FileServeInterceptor.name);
-
-  intercept(context: ExecutionContext, next: CallHandler<any>): Observable<any> {
-    const http = context.switchToHttp();
-    const res = http.getResponse<Response>();
-
-    const sendFile = (path: string, options: SendFileOptions) =>
-      promisify<string, SendFileOptions>(res.sendFile).bind(res)(path, options);
-
-    return next.handle().pipe(
-      mergeMap(async (file) => {
-        if (file instanceof ImmichFileResponse === false) {
-          return file;
-        }
-
-        try {
-          if (file.cacheControl) {
-            res.set('Cache-Control', 'private, max-age=86400, no-transform');
-          }
-
-          res.header('Content-Type', file.contentType);
-
-          const options: SendFileOptions = { dotfiles: 'allow' };
-          if (!isAbsolute(file.path)) {
-            options.root = process.cwd();
-          }
-
-          await access(file.path, constants.R_OK);
-
-          return sendFile(file.path, options);
-        } catch (error: Error | any) {
-          res.header('Cache-Control', 'none');
-
-          if (!isConnectionAborted(error)) {
-            this.logger.error(`Unable to send file: ${error.name}`, error.stack);
-          }
-          // throwing closes the connection and prevents `Error: write EPIPE`
-          throw error;
-        }
-      }),
-    );
-  }
-}

server/src/immich/interceptors/index.ts

@@ -1,3 +1,2 @@
 export * from './error.interceptor';
-export * from './file-serve.interceptor';
 export * from './file-upload.interceptor';