From 838b8e9126cd21324f08b71e3345e025008b0ff9 Mon Sep 17 00:00:00 2001 From: mertalev <101130780+mertalev@users.noreply.github.com> Date: Fri, 10 Oct 2025 19:26:22 -0400 Subject: [PATCH] set `max-age` limit --- e2e/src/api/specs/asset-upload.e2e-spec.ts | 4 +- server/src/config.ts | 16 ++--- .../asset-upload.controller.spec.ts | 9 --- .../controllers/asset-upload.controller.ts | 21 ++----- server/src/dtos/system-config.dto.ts | 29 +++++---- server/src/services/asset-upload.service.ts | 62 ++++++++++++------- server/src/services/job.service.ts | 2 +- .../services/system-config.service.spec.ts | 8 +-- 8 files changed, 71 insertions(+), 80 deletions(-) diff --git a/e2e/src/api/specs/asset-upload.e2e-spec.ts b/e2e/src/api/specs/asset-upload.e2e-spec.ts index f12f1bca74..a2f70dccac 100644 --- a/e2e/src/api/specs/asset-upload.e2e-spec.ts +++ b/e2e/src/api/specs/asset-upload.e2e-spec.ts @@ -974,7 +974,7 @@ describe('/upload', () => { expect(status).toBe(204); expect(headers['upload-offset']).toBe('512'); expect(headers['upload-complete']).toBe('?0'); - expect(headers['upload-limit']).toEqual('min-size=0'); + expect(headers['upload-limit']).toEqual('min-size=0, max-age=259200'); expect(headers['cache-control']).toBe('no-store'); }); @@ -993,7 +993,7 @@ describe('/upload', () => { const { status, headers } = await request(app).options('/upload'); expect(status).toBe(204); - expect(headers['upload-limit']).toEqual('min-size=0'); + expect(headers['upload-limit']).toEqual('min-size=0, max-age=259200'); }); }); }); diff --git a/server/src/config.ts b/server/src/config.ts index 34d5499bc7..011503cd6f 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -22,6 +22,9 @@ export interface SystemConfig { cronExpression: string; keepLastAmount: number; }; + upload: { + maxAgeHours: number; + }; }; ffmpeg: { crf: number; @@ -140,10 +143,7 @@ export interface SystemConfig { clusterNewFaces: boolean; generateMemories: boolean; syncQuotaUsage: boolean; - removeStaleUploads: { - enabled: boolean; - hoursAgo: number; - }; + removeStaleUploads: boolean; }; trash: { enabled: boolean; @@ -202,6 +202,9 @@ export const defaults = Object.freeze({ cronExpression: CronExpression.EVERY_DAY_AT_2AM, keepLastAmount: 14, }, + upload: { + maxAgeHours: 72, + }, }, ffmpeg: { crf: 23, @@ -345,10 +348,7 @@ export const defaults = Object.freeze({ syncQuotaUsage: true, missingThumbnails: true, clusterNewFaces: true, - removeStaleUploads: { - enabled: true, - hoursAgo: 72, - }, + removeStaleUploads: true, }, trash: { enabled: true, diff --git a/server/src/controllers/asset-upload.controller.spec.ts b/server/src/controllers/asset-upload.controller.spec.ts index db4a4f81e1..11840f9846 100644 --- a/server/src/controllers/asset-upload.controller.spec.ts +++ b/server/src/controllers/asset-upload.controller.spec.ts @@ -447,13 +447,4 @@ describe(AssetUploadController.name, () => { expect(status).toBe(400); }); }); - - describe('OPTIONS /upload', () => { - it('should return 204 with upload limits', async () => { - const { status, headers } = await request(ctx.getHttpServer()).options('/upload'); - - expect(status).toBe(204); - expect(headers['upload-limit']).toBe('min-size=0'); - }); - }); }); diff --git a/server/src/controllers/asset-upload.controller.ts b/server/src/controllers/asset-upload.controller.ts index 58cc604fa2..e0aeba80f2 100644 --- a/server/src/controllers/asset-upload.controller.ts +++ b/server/src/controllers/asset-upload.controller.ts @@ -1,17 +1,4 @@ -import { - Controller, - Delete, - Head, - Header, - HttpCode, - HttpStatus, - Options, - Param, - Patch, - Post, - Req, - Res, -} from '@nestjs/common'; +import { Controller, Delete, Head, Options, Param, Patch, Post, Req, Res } from '@nestjs/common'; import { ApiHeader, ApiOkResponse, ApiTags } from '@nestjs/swagger'; import { Request, Response } from 'express'; import { @@ -120,7 +107,7 @@ export class AssetUploadController { } @Options() - @HttpCode(HttpStatus.NO_CONTENT) - @Header('Upload-Limit', 'min-size=0') - getUploadOptions() {} + getUploadOptions(@Res() res: Response) { + return this.service.getUploadOptions(res); + } } diff --git a/server/src/dtos/system-config.dto.ts b/server/src/dtos/system-config.dto.ts index fdaae4e7b5..5911c00013 100644 --- a/server/src/dtos/system-config.dto.ts +++ b/server/src/dtos/system-config.dto.ts @@ -55,11 +55,23 @@ export class DatabaseBackupConfig { keepLastAmount!: number; } +export class UploadBackupConfig { + @IsInt() + @IsPositive() + @IsNotEmpty() + maxAgeHours!: number; +} + export class SystemConfigBackupsDto { @Type(() => DatabaseBackupConfig) @ValidateNested() @IsObject() database!: DatabaseBackupConfig; + + @Type(() => UploadBackupConfig) + @ValidateNested() + @IsObject() + upload!: UploadBackupConfig; } export class SystemConfigFFmpegDto { @@ -337,17 +349,6 @@ class SystemConfigNewVersionCheckDto { enabled!: boolean; } -class SystemConfigRemovePartialUploadsDto { - @ValidateBoolean() - enabled!: boolean; - - @IsInt() - @Min(1) - @Type(() => Number) - @ApiProperty({ type: 'integer' }) - hoursAgo!: number; -} - class SystemConfigNightlyTasksDto { @IsDateStringFormat('HH:mm', { message: 'startTime must be in HH:mm format' }) startTime!: string; @@ -367,10 +368,8 @@ class SystemConfigNightlyTasksDto { @ValidateBoolean() syncQuotaUsage!: boolean; - @Type(() => SystemConfigRemovePartialUploadsDto) - @ValidateNested() - @IsObject() - removeStaleUploads!: SystemConfigRemovePartialUploadsDto; + @ValidateBoolean() + removeStaleUploads!: boolean; } class SystemConfigOAuthDto { diff --git a/server/src/services/asset-upload.service.ts b/server/src/services/asset-upload.service.ts index ef1930d027..ffceea9345 100644 --- a/server/src/services/asset-upload.service.ts +++ b/server/src/services/asset-upload.service.ts @@ -4,6 +4,7 @@ import { DateTime } from 'luxon'; import { createHash } from 'node:crypto'; import { extname, join } from 'node:path'; import { Readable } from 'node:stream'; +import { SystemConfig } from 'src/config'; import { JOBS_ASSET_PAGINATION_SIZE } from 'src/constants'; import { StorageCore } from 'src/cores/storage.core'; import { OnEvent, OnJob } from 'src/decorators'; @@ -54,6 +55,7 @@ export class AssetUploadService extends BaseService { async startUpload(auth: AuthDto, req: Readable, res: Response, dto: StartUploadDto): Promise { this.logger.verboseFn(() => `Starting upload: ${JSON.stringify(dto)}`); const { isComplete, assetData, uploadLength, contentLength, version } = dto; + const { backup } = await this.getConfig({ withCache: true }); const asset = await this.onStart(auth, dto); if (asset.isDuplicate) { @@ -63,7 +65,7 @@ export class AssetUploadService extends BaseService { const location = `/api/upload/${asset.id}`; if (version <= MAX_RUFH_INTEROP_VERSION) { - this.sendInterimResponse(res, location, version); + this.sendInterimResponse(res, location, version, this.getUploadLimits(backup)); } // this is a 5xx to indicate the client should do offset retrieval and resume res.status(500).send('Incomplete asset already exists'); @@ -76,29 +78,31 @@ export class AssetUploadService extends BaseService { const location = `/api/upload/${asset.id}`; if (version <= MAX_RUFH_INTEROP_VERSION) { - this.sendInterimResponse(res, location, version); + this.sendInterimResponse(res, location, version, this.getUploadLimits(backup)); } this.addRequest(asset.id, req); - let checksumBuffer: Buffer | undefined; - const writeStream = this.pipe(req, asset.path, contentLength); - if (isComplete) { - const hash = createHash('sha1'); - req.on('data', (data: Buffer) => hash.update(data)); - writeStream.on('finish', () => (checksumBuffer = hash.digest())); - } - await new Promise((resolve, reject) => writeStream.on('close', resolve).on('error', reject)); - this.setCompleteHeader(res, dto.version, isComplete); - if (!isComplete) { - res.status(201).set('Location', location).setHeader('Upload-Limit', 'min-size=0').send(); - return; - } - if (dto.checksum.compare(checksumBuffer!) !== 0) { - return await this.sendChecksumMismatch(res, asset.id, asset.path); - } + await this.databaseRepository.withUuidLock(asset.id, async () => { + let checksumBuffer: Buffer | undefined; + const writeStream = this.pipe(req, asset.path, contentLength); + if (isComplete) { + const hash = createHash('sha1'); + req.on('data', (data: Buffer) => hash.update(data)); + writeStream.on('finish', () => (checksumBuffer = hash.digest())); + } + await new Promise((resolve, reject) => writeStream.on('close', resolve).on('error', reject)); + this.setCompleteHeader(res, dto.version, isComplete); + if (!isComplete) { + res.status(201).set('Location', location).setHeader('Upload-Limit', this.getUploadLimits(backup)).send(); + return; + } + if (dto.checksum.compare(checksumBuffer!) !== 0) { + return await this.sendChecksumMismatch(res, asset.id, asset.path); + } - await this.onComplete({ id: asset.id, path: asset.path, fileModifiedAt: assetData.fileModifiedAt }); - res.status(200).send({ id: asset.id }); + await this.onComplete({ id: asset.id, path: asset.path, fileModifiedAt: assetData.fileModifiedAt }); + res.status(200).send({ id: asset.id }); + }); } resumeUpload(auth: AuthDto, req: Readable, res: Response, id: string, dto: ResumeUploadDto): Promise { @@ -180,6 +184,7 @@ export class AssetUploadService extends BaseService { async getUploadStatus(auth: AuthDto, res: Response, id: string, { version }: GetUploadStatusDto): Promise { this.logger.verboseFn(() => `Getting upload status for ${id} with version ${version}`); + const { backup } = await this.getConfig({ withCache: true }); this.abortExistingRequest(id); return this.databaseRepository.withUuidLock(id, async () => { const asset = await this.assetRepository.getCompletionMetadata(id, auth.user.id); @@ -194,15 +199,20 @@ export class AssetUploadService extends BaseService { .status(204) .setHeader('Upload-Offset', offset.toString()) .setHeader('Cache-Control', 'no-store') - .setHeader('Upload-Limit', 'min-size=0') + .setHeader('Upload-Limit', this.getUploadLimits(backup)) .send(); }); } + async getUploadOptions(res: Response): Promise { + const { backup } = await this.getConfig({ withCache: true }); + res.status(204).setHeader('Upload-Limit', this.getUploadLimits(backup)).send(); + } + @OnJob({ name: JobName.PartialAssetCleanupQueueAll, queue: QueueName.BackgroundTask }) async removeStaleUploads(): Promise { const config = await this.getConfig({ withCache: false }); - const createdBefore = DateTime.now().minus({ hours: config.nightlyTasks.removeStaleUploads.hoursAgo }).toJSDate(); + const createdBefore = DateTime.now().minus({ hours: config.backup.upload.maxAgeHours }).toJSDate(); let jobs: JobItem[] = []; const assets = this.assetJobRepository.streamForPartialAssetCleanupJob(createdBefore); for await (const asset of assets) { @@ -353,13 +363,13 @@ export class AssetUploadService extends BaseService { return writeStream; } - private sendInterimResponse({ socket }: Response, location: string, interopVersion: number): void { + private sendInterimResponse({ socket }: Response, location: string, interopVersion: number, limits: string): void { if (socket && !socket.destroyed) { // Express doesn't understand interim responses, so write directly to socket socket.write( 'HTTP/1.1 104 Upload Resumption Supported\r\n' + `Location: ${location}\r\n` + - 'Upload-Limit: min-size=0\r\n' + + `Upload-Limit: ${limits}\r\n` + `Upload-Draft-Interop-Version: ${interopVersion}\r\n\r\n`, ); } @@ -428,4 +438,8 @@ export class AssetUploadService extends BaseService { res.setHeader('Upload-Incomplete', isComplete ? '?0' : '?1'); } } + + private getUploadLimits({ upload }: SystemConfig['backup']) { + return `min-size=0, max-age=${upload.maxAgeHours * 3600}`; + } } diff --git a/server/src/services/job.service.ts b/server/src/services/job.service.ts index 728a5e107f..757de370bd 100644 --- a/server/src/services/job.service.ts +++ b/server/src/services/job.service.ts @@ -303,7 +303,7 @@ export class JobService extends BaseService { jobs.push({ name: JobName.FacialRecognitionQueueAll, data: { force: false, nightly: true } }); } - if (config.nightlyTasks.removeStaleUploads.enabled) { + if (config.nightlyTasks.removeStaleUploads) { jobs.push({ name: JobName.PartialAssetCleanupQueueAll }); } diff --git a/server/src/services/system-config.service.spec.ts b/server/src/services/system-config.service.spec.ts index 807ee50213..d3f5e88c2f 100644 --- a/server/src/services/system-config.service.spec.ts +++ b/server/src/services/system-config.service.spec.ts @@ -47,6 +47,9 @@ const updatedConfig = Object.freeze({ cronExpression: '0 02 * * *', keepLastAmount: 14, }, + upload: { + maxAgeHours: 72, + }, }, ffmpeg: { crf: 30, @@ -123,10 +126,7 @@ const updatedConfig = Object.freeze({ missingThumbnails: true, generateMemories: true, syncQuotaUsage: true, - removeStaleUploads: { - enabled: true, - hoursAgo: 72, - }, + removeStaleUploads: true, }, reverseGeocoding: { enabled: true,