From 246b078fec5a58158a33b63ff72ffd0691740d93 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Tue, 13 Oct 2026 20:32:17 +0200 Subject: [PATCH] =?UTF-8?q?fix(server):=20destination=20des=20uploads=20fr?= =?UTF-8?q?agment=C3=A9s=20et=20gestion=20des=20erreurs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - init fragmenté : lit target/path dans la query (corps en repli) (#1) - erreurs client interrompu / disque plein / fichier partiel absent mappées en 400/507/404 sans webhook d'échec pour les interruptions (#2) - nettoyage des sessions expirées et des .part orphelins sécurisé, log des unhandledRejection (#2) - plafond de sessions porté à 256 pour l'usage multi-fenêtres (#3) Co-Authored-By: Claude Sonnet 5.5 --- src/app.ts | 7 +++++- src/config.ts | 8 +++---- src/error-handler.ts | 8 ++++++- src/errors.ts | 14 +++++++++++ src/index.ts | 5 ++++ src/lib/storage.ts | 38 ++++++++++++++++++++++++++---- src/lib/upload-sessions.ts | 3 ++- src/routes/upload-chunked.ts | 42 ++++++++++++++++++++++----------- src/routes/upload.ts | 2 +- tests/config.test.ts | 6 ++--- tests/helpers.ts | 2 +- tests/stream-errors.test.ts | 45 ++++++++++++++++++++++++++++++++++++ tests/upload-chunked.test.ts | 34 ++++++++++++++++++++++++++- 13 files changed, 183 insertions(+), 31 deletions(-) create mode 100644 tests/stream-errors.test.ts diff --git a/src/app.ts b/src/app.ts index a5fa991..47a7e87 100644 --- a/src/app.ts +++ b/src/app.ts @@ -43,7 +43,12 @@ export async function buildApp( }); const uploadSessions = createUploadSessionStore((session) => { - void removeFile(session.partPath); + removeFile(session.partPath).catch((error: unknown) => { + app.log.warn( + { err: error }, + "Impossible de supprimer le fichier partiel d'une session expirée", + ); + }); }); const notifier = createNotifier(config.webhooks, app.log); diff --git a/src/config.ts b/src/config.ts index d93f644..f2b6eff 100644 --- a/src/config.ts +++ b/src/config.ts @@ -204,7 +204,7 @@ function parseMimePolicy(value: unknown, label = 'allowedMimeTypes'): MimePolicy if (!valid) { throw new ConfigError( `${label} : entrée invalide « ${entry} » ` + - '(formes acceptées : "type/sous-type", "type/*", "*/*", "*")', + '(formes acceptées : "type/sous-type", "type/*", "*/*", "*")', ); } } @@ -231,7 +231,7 @@ function parseSessionKey(value: unknown): Buffer { if (key.length !== SESSION_KEY_BYTES) { throw new ConfigError( `session.key doit être ${String(SESSION_KEY_BYTES)} octets encodés en base64 ` + - `(décodé : ${String(key.length)} octets). Générer avec : openssl rand -base64 32`, + `(décodé : ${String(key.length)} octets). Générer avec : openssl rand -base64 32`, ); } return key; @@ -257,7 +257,7 @@ function parseSession(value: unknown): SessionConfig { key: parseSessionKey(raw.key), maxAgeSeconds: Math.round( optionalPositiveNumber(raw.maxAgeHours, DEFAULTS.sessionMaxAgeHours, 'session.maxAgeHours') * - 3600, + 3600, ), }; } @@ -385,7 +385,7 @@ function parseTargets(value: unknown, defaultMimePolicy: MimePolicy): DropTarget allowedMimeTypes: raw.allowedMimeTypes === undefined || raw.allowedMimeTypes === null ? defaultMimePolicy - : parseMimePolicy(raw.allowedMimeTypes, `targets[${String(index)}].allowedMimeTypes`) + : parseMimePolicy(raw.allowedMimeTypes, `targets[${String(index)}].allowedMimeTypes`), }; }; diff --git a/src/error-handler.ts b/src/error-handler.ts index bedd3b4..29e25f9 100644 --- a/src/error-handler.ts +++ b/src/error-handler.ts @@ -1,6 +1,6 @@ import type { FastifyError, FastifyInstance, FastifyRequest } from 'fastify'; -import { AppError } from './errors.js'; +import { AppError, ClientAbortedError } from './errors.js'; import { getSessionUser } from './session.js'; interface ErrorBody { @@ -74,6 +74,12 @@ export function registerErrorHandler(app: FastifyInstance): void { const { status, body } = resolveError(error, request); reply.status(status).send(body); + // Annulation/pause côté client : comportement normal, pas un échec à signaler. + if (error instanceof ClientAbortedError) { + request.log.debug({ err: error }, 'Requête interrompue par le client'); + return; + } + const path = pathOf(request); if (status >= 400 && isUploadPath(path)) { app.notifier.uploadFailed({ diff --git a/src/errors.ts b/src/errors.ts index b4ef790..199dc04 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -63,3 +63,17 @@ export class TooManyUploadsError extends AppError { super(message, 429); } } + +/** Le client a interrompu la requête (annulation, pause, coupure réseau) avant la fin du corps. */ +export class ClientAbortedError extends AppError { + public constructor(message = 'Requête interrompue par le client') { + super(message, 400); + } +} + +/** Plus d'espace disque disponible sur la cible de dépôt. */ +export class InsufficientStorageError extends AppError { + public constructor(message = 'Espace de stockage insuffisant sur le serveur') { + super(message, 507); + } +} diff --git a/src/index.ts b/src/index.ts index 54853b0..2f490be 100644 --- a/src/index.ts +++ b/src/index.ts @@ -37,6 +37,11 @@ async function main(): Promise { void shutdown('SIGTERM'); }); + // Filet de sécurité : une promesse rejetée sans handler ne doit pas passer inaperçue. + process.on('unhandledRejection', (reason: unknown) => { + app.log.error({ err: reason }, 'Promesse rejetée non gérée'); + }); + await app.listen({ port: config.port, host: config.host }); } diff --git a/src/lib/storage.ts b/src/lib/storage.ts index c744c51..a39e43a 100644 --- a/src/lib/storage.ts +++ b/src/lib/storage.ts @@ -5,7 +5,14 @@ import { randomUUID } from 'node:crypto'; import { Transform, type Readable } from 'node:stream'; import { pipeline } from 'node:stream/promises'; -import { PayloadTooLargeError, ValidationError } from '../errors.js'; +import { + AppError, + ClientAbortedError, + InsufficientStorageError, + PayloadTooLargeError, + UploadSessionNotFoundError, + ValidationError, +} from '../errors.js'; import { extensionForMime } from './media-types.js'; /** Métadonnées d'un fichier enregistré sur le disque. */ @@ -39,7 +46,6 @@ export interface SaveFileParams { * à la racine de `targetDir`. */ placement?: FilePlacement; - } const MAX_BASE_LENGTH = 60; @@ -74,8 +80,20 @@ export function buildStoredName(originalName: string, mimeType: string): string return `${sanitizeBase(originalName)}-${Date.now()}-${randomUUID().slice(0, 8)}${ext}`; } -function normalizeStreamError(error: unknown): Error { - if (error instanceof PayloadTooLargeError) { +const CLIENT_ABORT_CODES: ReadonlySet = new Set([ + 'ECONNRESET', + 'ECONNABORTED', + 'ERR_STREAM_PREMATURE_CLOSE', + 'FST_ERR_CTP_INVALID_CONTENT_LENGTH', +]); + +/** + * Convertit une erreur de flux/disque en `AppError` explicite : dépassement de + * taille (413), requête interrompue par le client (400), disque plein (507). + * Le reste est renvoyé tel quel. + */ +export function normalizeStreamError(error: unknown): Error { + if (error instanceof AppError) { return error; } const code = @@ -83,6 +101,12 @@ function normalizeStreamError(error: unknown): Error { if (code === 'FST_REQ_FILE_TOO_LARGE') { return new PayloadTooLargeError(); } + if (typeof code === 'string' && CLIENT_ABORT_CODES.has(code)) { + return new ClientAbortedError(); + } + if (code === 'ENOSPC') { + return new InsufficientStorageError(); + } return error instanceof Error ? error : new Error("Échec de l'écriture du fichier"); } @@ -163,7 +187,7 @@ export async function saveFile(params: SaveFileParams): Promise { await pipeline( source, limiter, - createWriteStream(destination, { highWaterMark: WRITE_HIGH_WATER_MARK ,mode:0o777}), + createWriteStream(destination, { highWaterMark: WRITE_HIGH_WATER_MARK, mode: 0o777 }), ); } catch (error) { await rm(destination, { force: true }); @@ -219,6 +243,10 @@ export async function writeChunkAt(params: { }), ); } catch (error) { + // Fichier partiel supprimé entre-temps (upload annulé ou session expirée). + if ((error as { code?: unknown }).code === 'ENOENT') { + throw new UploadSessionNotFoundError(); + } throw normalizeStreamError(error); } return written; diff --git a/src/lib/upload-sessions.ts b/src/lib/upload-sessions.ts index fc0b0c5..c23452d 100644 --- a/src/lib/upload-sessions.ts +++ b/src/lib/upload-sessions.ts @@ -25,7 +25,8 @@ export type NewUploadSession = Omit => { const body = (request.body ?? {}) as InitBody; - const target = resolveTarget(cfg.targets, body.target); - const placement = resolvePlacement(target, body.path); + const query = (request.query ?? {}) as InitQuery; + // Le client web transmet `target` et `path` dans la query (comme `POST /upload`) ; + // le corps JSON reste accepté. + const target = resolveTarget(cfg.targets, query.target ?? body.target); + const placement = resolvePlacement(target, query.path ?? body.path); const mimeType = nonEmptyString(body.type, 'application/octet-stream'); const totalSize = positiveInt(body.size, 'size'); @@ -105,17 +114,24 @@ export const chunkedUploadRoutes: FastifyPluginCallback = (app, _options, done) const partPath = destination + PART_SUFFIX; await createEmptyFile(partPath); - const session = app.uploadSessions.create({ - target, - originalName, - storedName, - mimeType, - partPath, - finalPath: destination, - totalSize, - chunkSize, - chunkCount, - }); + let session: UploadSession; + try { + session = app.uploadSessions.create({ + target, + originalName, + storedName, + mimeType, + partPath, + finalPath: destination, + totalSize, + chunkSize, + chunkCount, + }); + } catch (error) { + // Trop de sessions : ne pas laisser de fichier partiel orphelin. + await removeFile(partPath); + throw error; + } reply.status(201); return { id: session.id, chunkSize, chunkCount }; diff --git a/src/routes/upload.ts b/src/routes/upload.ts index edea859..3d8c182 100644 --- a/src/routes/upload.ts +++ b/src/routes/upload.ts @@ -93,7 +93,7 @@ export const uploadRoutes: FastifyPluginCallback = (app, _options, done) => { mimeType: part.mimetype, targetDir: target.dir, maxFileSizeBytes: app.appConfig.maxFileSizeBytes, - ...(placement !== undefined ? { placement } : {}) + ...(placement !== undefined ? { placement } : {}), }); app.notifier.uploadCompleted({ diff --git a/tests/config.test.ts b/tests/config.test.ts index 056c43b..e862579 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -31,7 +31,7 @@ describe('parseConfig', () => { publicUrl: 'http://localhost:3000', maxFileSizeBytes: 512 * 1024 * 1024, logLevel: 'info', - chunkPoolSize: 30 + chunkPoolSize: 30, }); expect(config.targets).toEqual([ { @@ -39,7 +39,7 @@ describe('parseConfig', () => { label: 'photos', dir: './uploads/photos', allowDirectories: false, - allowedMimeTypes: DEFAULT_ALLOWED_MIME + allowedMimeTypes: DEFAULT_ALLOWED_MIME, }, ]); expect(config.targets[0]?.allowedMimeTypes).toBe(DEFAULT_ALLOWED_MIME); @@ -96,7 +96,7 @@ describe('parseConfig', () => { label: 'X', dir: './x', allowDirectories: false, - allowedMimeTypes: DEFAULT_ALLOWED_MIME + allowedMimeTypes: DEFAULT_ALLOWED_MIME, }); expect( parseConfig({ ...BASE, targets: [{ name: 'x', dir: './x', allowDirectories: true }] }) diff --git a/tests/helpers.ts b/tests/helpers.ts index 9771d92..048a702 100644 --- a/tests/helpers.ts +++ b/tests/helpers.ts @@ -54,7 +54,7 @@ export function makeConfig(overrides: ConfigOverrides = {}): AppConfig { label: target.label ?? target.name, dir: target.dir, allowDirectories: target.allowDirectories ?? false, - allowedMimeTypes: target.allowedMimeTypes ?? allowedMimeTypes + allowedMimeTypes: target.allowedMimeTypes ?? allowedMimeTypes, })); return { diff --git a/tests/stream-errors.test.ts b/tests/stream-errors.test.ts new file mode 100644 index 0000000..386a827 --- /dev/null +++ b/tests/stream-errors.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from 'vitest'; + +import { + ClientAbortedError, + InsufficientStorageError, + PayloadTooLargeError, + ValidationError, +} from '../src/errors.js'; +import { normalizeStreamError } from '../src/lib/storage.js'; + +function withCode(code: string): Error { + return Object.assign(new Error('boom'), { code }); +} + +describe('normalizeStreamError', () => { + it('conserve les AppError telles quelles', () => { + const error = new ValidationError('x'); + expect(normalizeStreamError(error)).toBe(error); + }); + + it('convertit le dépassement multipart en 413', () => { + expect(normalizeStreamError(withCode('FST_REQ_FILE_TOO_LARGE'))).toBeInstanceOf( + PayloadTooLargeError, + ); + }); + + it.each(['ECONNRESET', 'ECONNABORTED', 'ERR_STREAM_PREMATURE_CLOSE'])( + 'convertit %s (client parti) en ClientAbortedError', + (code) => { + expect(normalizeStreamError(withCode(code))).toBeInstanceOf(ClientAbortedError); + }, + ); + + it('convertit ENOSPC en InsufficientStorageError (507)', () => { + const result = normalizeStreamError(withCode('ENOSPC')); + expect(result).toBeInstanceOf(InsufficientStorageError); + expect((result as InsufficientStorageError).statusCode).toBe(507); + }); + + it('laisse passer les autres erreurs et enveloppe les valeurs non-Error', () => { + const other = withCode('EACCES'); + expect(normalizeStreamError(other)).toBe(other); + expect(normalizeStreamError('texte')).toBeInstanceOf(Error); + }); +}); diff --git a/tests/upload-chunked.test.ts b/tests/upload-chunked.test.ts index 4ea1660..77c3daf 100644 --- a/tests/upload-chunked.test.ts +++ b/tests/upload-chunked.test.ts @@ -1,4 +1,4 @@ -import { readFile, readdir } from 'node:fs/promises'; +import { readFile, readdir, rm } from 'node:fs/promises'; import { join } from 'node:path'; import { randomBytes } from 'node:crypto'; @@ -142,6 +142,38 @@ describe('upload fragmenté', () => { expect(await readdir(join(dir, 'a', 'b'))).toEqual(['clip.mp4']); }); + it('init lit target et path dans la query (comme le client web)', async () => { + const data = randomBytes(1000); + const query = new URLSearchParams({ target: 'tree', path: 'dossier/clip.mp4' }); + const res = await fetch(`${baseUrl}/upload/init?${query.toString()}`, { + method: 'POST', + headers: { 'content-type': 'application/json', cookie }, + body: JSON.stringify({ name: 'clip.mp4', size: data.length, type: 'video/mp4' }), + }); + expect(res.status).toBe(201); + const { id } = (await res.json()) as { id: string }; + expect(await putChunk(id, 0, data)).toBe(200); + const done = await complete(id); + expect(done.body).toMatchObject({ target: 'tree', storedName: 'dossier/clip.mp4' }); + expect(await readFile(join(dir, 'dossier', 'clip.mp4'))).toEqual(data); + }); + + it('init refuse une cible inconnue passée dans la query (400)', async () => { + const res = await fetch(`${baseUrl}/upload/init?target=inconnue`, { + method: 'POST', + headers: { 'content-type': 'application/json', cookie }, + body: JSON.stringify({ name: 'clip.mp4', size: 3, type: 'video/mp4' }), + }); + expect(res.status).toBe(400); + }); + + it('PUT renvoie 404 (et non 500) si le fichier partiel a disparu', async () => { + const { body } = await init({ size: MIB, chunkSize: MIB }); + const [part] = await readdir(dir); + await rm(join(dir, part!)); + expect(await putChunk(body.id!, 0, Buffer.alloc(10))).toBe(404); + }); + it('init refuse une taille au-dessus de la limite (413)', async () => { expect((await init({ size: 9 * MIB })).status).toBe(413); }); -- 2.45.2