diff --git a/server/src/controllers/asset-upload.controller.spec.ts b/server/src/controllers/asset-upload.controller.spec.ts index 11840f9846..e836bfcb1a 100644 --- a/server/src/controllers/asset-upload.controller.spec.ts +++ b/server/src/controllers/asset-upload.controller.spec.ts @@ -112,11 +112,7 @@ describe(AssetUploadController.name, () => { .send(buffer); expect(status).toBe(400); - expect(body).toEqual( - expect.objectContaining({ - message: expect.arrayContaining([expect.stringContaining('uploadComplete')]), - }), - ); + expect(body).toEqual(expect.objectContaining({ message: 'Expected valid upload-complete header' })); }); it('should require Upload-Length header', async () => { @@ -125,18 +121,23 @@ describe(AssetUploadController.name, () => { .set('Upload-Draft-Interop-Version', '8') .set('X-Immich-Asset-Data', makeAssetData()) .set('Repr-Digest', checksum) - .set('Upload-Complete', '?1') + .set('Upload-Complete', '?0') .send(buffer); expect(status).toBe(400); - expect(body).toEqual( - expect.objectContaining({ - message: expect.arrayContaining([ - 'uploadLength must be an integer number', - 'uploadLength must not be less than 0', - ]), - }), - ); + expect(body).toEqual(expect.objectContaining({ message: 'Missing upload-length header' })); + }); + + it('should infer upload length from content length if complete upload', async () => { + const { status } = await request(ctx.getHttpServer()) + .post('/upload') + .set('Upload-Draft-Interop-Version', '8') + .set('X-Immich-Asset-Data', makeAssetData()) + .set('Repr-Digest', checksum) + .set('Upload-Complete', '?1') + .send(buffer); + + expect(status).toBe(201); }); it('should reject invalid Repr-Digest format', async () => { @@ -229,15 +230,17 @@ describe(AssetUploadController.name, () => { }); it('should accept Upload-Incomplete header for version 3', async () => { - const { status } = await request(ctx.getHttpServer()) + const { body, status } = await request(ctx.getHttpServer()) .post('/upload') .set('Upload-Draft-Interop-Version', '3') .set('X-Immich-Asset-Data', makeAssetData()) .set('Repr-Digest', checksum) .set('Upload-Incomplete', '?0') + .set('Upload-Complete', '?1') .set('Upload-Length', '1024') .send(buffer); + expect(body).toEqual({}); expect(status).not.toBe(400); }); @@ -252,11 +255,7 @@ describe(AssetUploadController.name, () => { .send(buffer); expect(status).toBe(400); - expect(body).toEqual( - expect.objectContaining({ - message: expect.arrayContaining([expect.stringContaining('uploadComplete')]), - }), - ); + expect(body).toEqual(expect.objectContaining({ message: 'Expected valid upload-complete header' })); }); it('should validate Upload-Length is a non-negative integer', async () => { @@ -327,11 +326,7 @@ describe(AssetUploadController.name, () => { .send(Buffer.from('test')); expect(status).toBe(400); - expect(body).toEqual( - expect.objectContaining({ - message: expect.arrayContaining([expect.stringContaining('uploadComplete')]), - }), - ); + expect(body).toEqual(expect.objectContaining({ message: 'Expected valid upload-complete header' })); }); it('should validate UUID parameter', async () => { diff --git a/server/src/controllers/asset-upload.controller.ts b/server/src/controllers/asset-upload.controller.ts index 96bc392b8f..e1de5f83fe 100644 --- a/server/src/controllers/asset-upload.controller.ts +++ b/server/src/controllers/asset-upload.controller.ts @@ -1,13 +1,7 @@ import { Controller, Delete, Head, HttpCode, HttpStatus, Options, Param, Patch, Post, Req, Res } from '@nestjs/common'; import { ApiHeader, ApiOkResponse, ApiTags } from '@nestjs/swagger'; import { Request, Response } from 'express'; -import { - GetUploadStatusDto, - ResumeUploadDto, - StartUploadDto, - UploadHeader, - UploadOkDto, -} from 'src/dtos/asset-upload.dto'; +import { GetUploadStatusDto, Header, ResumeUploadDto, StartUploadDto, UploadOkDto } from 'src/dtos/asset-upload.dto'; import { AuthDto } from 'src/dtos/auth.dto'; import { ImmichHeader, Permission } from 'src/enum'; import { Auth, Authenticated } from 'src/middleware/auth.guard'; @@ -16,20 +10,20 @@ import { validateSyncOrReject } from 'src/utils/request'; import { UUIDParamDto } from 'src/validation'; const apiInteropVersion = { - name: UploadHeader.InteropVersion, + name: Header.InteropVersion, description: `Indicates the version of the RUFH protocol supported by the client.`, required: true, }; const apiUploadComplete = { - name: UploadHeader.UploadComplete, + name: Header.UploadComplete, description: 'Structured boolean indicating whether this request completes the file. Use Upload-Incomplete instead for version <= 3.', required: true, }; const apiContentLength = { - name: UploadHeader.ContentLength, + name: Header.ContentLength, description: 'Non-negative size of the request body in bytes.', required: true, }; @@ -60,7 +54,7 @@ export class AssetUploadController { 'device-asset-id="abc123", device-id="phone1", filename="photo.jpg", file-created-at="2024-01-01T00:00:00Z", file-modified-at="2024-01-01T00:00:00Z"', }) @ApiHeader({ - name: UploadHeader.ReprDigest, + name: Header.ReprDigest, description: 'RFC 9651 structured dictionary containing an `sha` (bytesequence) checksum used to detect duplicate files and validate data integrity.', required: true, @@ -77,7 +71,7 @@ export class AssetUploadController { @Patch(':id') @Authenticated({ sharedLink: true, permission: Permission.AssetUpload }) @ApiHeader({ - name: UploadHeader.UploadOffset, + name: Header.UploadOffset, description: 'Non-negative byte offset indicating the starting position of the data in the request body within the entire file.', required: true, diff --git a/server/src/dtos/asset-upload.dto.ts b/server/src/dtos/asset-upload.dto.ts index c49f62d061..0442005088 100644 --- a/server/src/dtos/asset-upload.dto.ts +++ b/server/src/dtos/asset-upload.dto.ts @@ -1,11 +1,22 @@ import { BadRequestException } from '@nestjs/common'; import { ApiProperty } from '@nestjs/swagger'; import { Expose, plainToInstance, Transform, Type } from 'class-transformer'; -import { Equals, IsEmpty, IsEnum, IsInt, IsNotEmpty, IsString, Min, ValidateIf, ValidateNested } from 'class-validator'; +import { Equals, IsInt, IsNotEmpty, IsString, Min, ValidateIf, ValidateNested } from 'class-validator'; import { ImmichHeader } from 'src/enum'; import { Optional, ValidateBoolean, ValidateDate } from 'src/validation'; import { parseDictionary } from 'structured-headers'; +export enum Header { + ContentLength = 'content-length', + ContentType = 'content-type', + InteropVersion = 'upload-draft-interop-version', + ReprDigest = 'repr-digest', + UploadComplete = 'upload-complete', + UploadIncomplete = 'upload-incomplete', + UploadLength = 'upload-length', + UploadOffset = 'upload-offset', +} + export class UploadAssetDataDto { @IsNotEmpty() @IsString() @@ -39,24 +50,8 @@ export class UploadAssetDataDto { iCloudId!: string; } -export enum StructuredBoolean { - False = '?0', - True = '?1', -} - -export enum UploadHeader { - ContentLength = 'content-length', - ContentType = 'content-type', - InteropVersion = 'upload-draft-interop-version', - ReprDigest = 'repr-digest', - UploadComplete = 'upload-complete', - UploadIncomplete = 'upload-incomplete', - UploadLength = 'upload-length', - UploadOffset = 'upload-offset', -} - class BaseRufhHeadersDto { - @Expose({ name: UploadHeader.InteropVersion }) + @Expose({ name: Header.InteropVersion }) @Min(3) @IsInt() @Type(() => Number) @@ -64,28 +59,15 @@ class BaseRufhHeadersDto { } export class BaseUploadHeadersDto extends BaseRufhHeadersDto { - @Expose({ name: UploadHeader.ContentLength }) + @Expose({ name: Header.ContentLength }) @Min(0) @IsInt() @Type(() => Number) contentLength!: number; - @Expose({ name: UploadHeader.UploadComplete }) - @ValidateIf((o) => o.version === null || o.version > 3) - @IsEnum(StructuredBoolean) - uploadComplete!: StructuredBoolean; - - @Expose({ name: UploadHeader.UploadIncomplete }) - @ValidateIf((o) => o.version !== null && o.version <= 3) - @IsEnum(StructuredBoolean) - uploadIncomplete!: StructuredBoolean; - - get isComplete(): boolean { - if (this.version <= 3) { - return this.uploadIncomplete === StructuredBoolean.False; - } - return this.uploadComplete === StructuredBoolean.True; - } + @Expose() + @Transform(({ obj }) => isUploadComplete(obj)) + uploadComplete!: boolean; } export class StartUploadDto extends BaseUploadHeadersDto { @@ -115,66 +97,81 @@ export class StartUploadDto extends BaseUploadHeadersDto { }) assetData!: UploadAssetDataDto; - @Expose({ name: UploadHeader.ReprDigest }) + @Expose({ name: Header.ReprDigest }) @Transform(({ value }) => { if (!value) { - throw new BadRequestException(`Missing ${UploadHeader.ReprDigest} header`); + throw new BadRequestException(`Missing ${Header.ReprDigest} header`); } const checksum = parseDictionary(value).get('sha')?.[0]; if (checksum instanceof ArrayBuffer && checksum.byteLength === 20) { return Buffer.from(checksum); } - throw new BadRequestException(`Invalid ${UploadHeader.ReprDigest} header`); + throw new BadRequestException(`Invalid ${Header.ReprDigest} header`); }) checksum!: Buffer; - @Expose({ name: UploadHeader.UploadLength }) + @Expose() @Min(0) @IsInt() - @Transform(({ obj, value }) => Number(value === undefined ? obj['x-upload-length'] : value)) - uploadLength!: number; + @Transform(({ obj }) => { + const uploadLength = obj[Header.UploadLength]; + if (uploadLength != undefined) { + return Number(uploadLength); + } - @Expose({ name: UploadHeader.UploadOffset }) - @IsEmpty() - uploadOffset?: string; + const contentLength = obj[Header.ContentLength]; + if (contentLength != undefined && isUploadComplete(obj)) { + return Number(contentLength); + } + throw new BadRequestException(`Missing ${Header.UploadLength} header`); + }) + uploadLength!: number; } export class ResumeUploadDto extends BaseUploadHeadersDto { - @Expose({ name: UploadHeader.ContentType }) + @Expose({ name: Header.ContentType }) @ValidateIf((o) => o.version && o.version >= 6) @Equals('application/partial-upload') contentType!: string; - @Expose({ name: UploadHeader.UploadLength }) + @Expose({ name: Header.UploadLength }) @Min(0) @IsInt() @Type(() => Number) @Optional() uploadLength?: number; - @Expose({ name: UploadHeader.UploadOffset }) + @Expose({ name: Header.UploadOffset }) @Min(0) @IsInt() @Type(() => Number) uploadOffset!: number; } -export class GetUploadStatusDto extends BaseRufhHeadersDto { - @Expose({ name: UploadHeader.UploadComplete }) - @IsEmpty() - uploadComplete?: string; - - @Expose({ name: UploadHeader.UploadIncomplete }) - @IsEmpty() - uploadIncomplete?: string; - - @Expose({ name: UploadHeader.UploadOffset }) - @IsEmpty() - uploadOffset?: string; -} +export class GetUploadStatusDto extends BaseRufhHeadersDto {} export class UploadOkDto { @ApiProperty() id!: string; } + +const STRUCTURED_TRUE = '?1'; +const STRUCTURED_FALSE = '?0'; + +function isUploadComplete(obj: any): boolean { + const uploadComplete = obj[Header.UploadComplete]; + if (uploadComplete === STRUCTURED_TRUE) { + return true; + } else if (uploadComplete === STRUCTURED_FALSE) { + return false; + } + + const uploadIncomplete = obj[Header.UploadIncomplete]; + if (uploadIncomplete === STRUCTURED_TRUE) { + return false; + } else if (uploadIncomplete === STRUCTURED_FALSE) { + return true; + } + throw new BadRequestException(`Expected valid ${Header.UploadComplete} header`); +} diff --git a/server/src/services/asset-upload.service.spec.ts b/server/src/services/asset-upload.service.spec.ts index 423a321eb5..f2c146755a 100644 --- a/server/src/services/asset-upload.service.spec.ts +++ b/server/src/services/asset-upload.service.spec.ts @@ -1,5 +1,4 @@ import { BadRequestException, InternalServerErrorException } from '@nestjs/common'; -import { StructuredBoolean } from 'src/dtos/asset-upload.dto'; import { AssetMetadataKey, AssetStatus, AssetType, AssetVisibility, JobName, JobStatus } from 'src/enum'; import { AssetUploadService } from 'src/services/asset-upload.service'; import { ASSET_CHECKSUM_CONSTRAINT } from 'src/utils/database'; @@ -28,8 +27,7 @@ describe(AssetUploadService.name, () => { }, checksum: Buffer.from('checksum'), uploadLength: 1024, - uploadComplete: StructuredBoolean.True, - uploadIncomplete: StructuredBoolean.False, + uploadComplete: true, contentLength: 1024, isComplete: true, version: 8, diff --git a/server/src/services/asset-upload.service.ts b/server/src/services/asset-upload.service.ts index 5b3e274ee7..ec702c364b 100644 --- a/server/src/services/asset-upload.service.ts +++ b/server/src/services/asset-upload.service.ts @@ -54,7 +54,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 { uploadComplete, assetData, uploadLength, contentLength, version } = dto; const { backup } = await this.getConfig({ withCache: true }); const asset = await this.onStart(auth, dto); @@ -72,7 +72,7 @@ export class AssetUploadService extends BaseService { return; } - if (isComplete && uploadLength !== contentLength) { + if (uploadComplete && uploadLength !== contentLength) { return this.sendInconsistentLength(res); } @@ -85,14 +85,14 @@ export class AssetUploadService extends BaseService { await this.databaseRepository.withUuidLock(asset.id, async () => { let checksumBuffer: Buffer | undefined; const writeStream = this.pipe(req, asset.path, contentLength); - if (isComplete) { + if (uploadComplete) { 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) { + this.setCompleteHeader(res, dto.version, uploadComplete); + if (!uploadComplete) { res.status(201).set('Location', location).setHeader('Upload-Limit', this.getUploadLimits(backup)).send(); return; } @@ -107,7 +107,7 @@ export class AssetUploadService extends BaseService { resumeUpload(auth: AuthDto, req: Readable, res: Response, id: string, dto: ResumeUploadDto): Promise { this.logger.verboseFn(() => `Resuming upload for ${id}: ${JSON.stringify(dto)}`); - const { isComplete, uploadLength, uploadOffset, contentLength, version } = dto; + const { uploadComplete, uploadLength, uploadOffset, contentLength, version } = dto; this.setCompleteHeader(res, version, false); this.addRequest(id, req); return this.databaseRepository.withUuidLock(id, async () => { @@ -137,15 +137,15 @@ export class AssetUploadService extends BaseService { return; } - if (contentLength === 0 && !isComplete) { + if (contentLength === 0 && !uploadComplete) { res.status(204).setHeader('Upload-Offset', expectedOffset.toString()).send(); return; } const writeStream = this.pipe(req, path, contentLength); await new Promise((resolve, reject) => writeStream.on('close', resolve).on('error', reject)); - this.setCompleteHeader(res, version, isComplete); - if (!isComplete) { + this.setCompleteHeader(res, version, uploadComplete); + if (!uploadComplete) { try { const offset = await this.getCurrentOffset(path); res.status(204).setHeader('Upload-Offset', offset.toString()).send();