fix(nextcloud-files): WR-04/05/06, IN-01 Ersetzen und Abschluss von Uploads abgesichert
- WR-04: die Entity-Tag-Pruefung beim Ersetzen laeuft im Zusammenbau unmittelbar vor dem MOVE (gemessen gegen Nextcloud 34: If-Match auf dem MOVE wird gegen .file geprueft und ergibt immer 412, ist also nicht nutzbar); weicht der Tag ab -> changedMeanwhile - WR-05: completeUpload merkt den Zustand synchron vor dem ersten await; ein zweiter oder gleichzeitiger Abschluss liefert das gemerkte Ergebnis (auch denselben Fehler) und baut nie zweimal zusammen; nur Fehler vor dem MOVE duerfen erneut versucht werden - WR-06: 412 mit replaceEtag (PUT oder Zusammenbau) ist changedMeanwhile, nicht nameTaken - IN-01: replaceEtag muss die Form eines Entity-Tags haben (sonst 400) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -8,6 +8,7 @@ import {
|
||||
IsNotEmpty,
|
||||
IsOptional,
|
||||
IsString,
|
||||
Matches,
|
||||
Max,
|
||||
MaxLength,
|
||||
Min,
|
||||
@@ -26,6 +27,13 @@ import {
|
||||
/** Hoechster Unix-Zeitstempel (Sekunden), den eine Aenderungszeit haben darf (Jahr 2100). */
|
||||
export const MAX_MTIME_SECONDS = 4102444800;
|
||||
|
||||
/**
|
||||
* Form eines Entity-Tags (IN-01): optional `W/`, optional in Anfuehrungszeichen, sonst nur
|
||||
* sichtbare ASCII-Zeichen. Der Wert geht als `If-Match`-Kopfzeile an Nextcloud; CR, LF oder
|
||||
* andere Steuerzeichen liessen den Aufruf sonst scheitern und als "nicht erreichbar" enden.
|
||||
*/
|
||||
export const ETAG_PATTERN = /^(W\/)?"?[\x21\x23-\x7e]{1,128}"?$/;
|
||||
|
||||
export class StartUploadDto {
|
||||
@IsString()
|
||||
@IsNotEmpty()
|
||||
@@ -40,6 +48,7 @@ export class StartUploadDto {
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
@MaxLength(200)
|
||||
@Matches(ETAG_PATTERN)
|
||||
replaceEtag?: string;
|
||||
}
|
||||
|
||||
@@ -62,6 +71,7 @@ export class CompleteUploadDto {
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
@MaxLength(200)
|
||||
@Matches(ETAG_PATTERN)
|
||||
replaceEtag?: string;
|
||||
}
|
||||
|
||||
@@ -88,6 +98,7 @@ export class UploadQueryDto {
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
@MaxLength(200)
|
||||
@Matches(ETAG_PATTERN)
|
||||
replaceEtag?: string;
|
||||
}
|
||||
|
||||
|
||||
@@ -278,6 +278,24 @@ describe('NextcloudFilesTransferService — putSingle / putChunk', () => {
|
||||
expect(body).toMatchObject({ code: 'nameTaken', existing: { etag: '"e9"' } });
|
||||
});
|
||||
|
||||
it('putSingle mit replaceEtag: 412 -> 409 changedMeanwhile, nicht nameTaken (WR-06)', async () => {
|
||||
const { service, calls } = setup((req) =>
|
||||
req.method === 'PUT' ? { status: 412 } : { status: 207, text: statXml('e10') },
|
||||
);
|
||||
const { status, body } = parts(
|
||||
await caught(
|
||||
service.putSingle('t1', 'u1', rawReq(5), {
|
||||
path: '/Projekte/a.txt',
|
||||
size: 5,
|
||||
replaceEtag: '"e9"',
|
||||
}),
|
||||
),
|
||||
);
|
||||
expect(status).toBe(409);
|
||||
expect(body).toMatchObject({ code: 'changedMeanwhile', existing: { etag: '"e10"' } });
|
||||
expect(calls[0].headers['if-match']).toBe('"e9"');
|
||||
});
|
||||
|
||||
it('putChunk: Stueck 3 -> .../uploads/anna/<id>/00003 mit Destination und OC-Total-Length', async () => {
|
||||
const { service, calls } = setup();
|
||||
const req = rawReq(CHUNK);
|
||||
@@ -562,6 +580,113 @@ describe('NextcloudFilesTransferService — Zusammenbau', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('NextcloudFilesTransferService — Abschluss ist idempotent, Ersetzen geprueft (WR-04/05/06)', () => {
|
||||
const dto = { path: '/Projekte/a.txt', size: 30000000 };
|
||||
|
||||
it('zwei gleichzeitige Abschluesse mit replaceEtag: ein PROPFIND, ein MOVE, beide ohne Fehler', async () => {
|
||||
let releaseMove: (() => void) | undefined;
|
||||
const { service, calls } = setup((req) => {
|
||||
if (req.method === 'PROPFIND') return { status: 207, text: statXml('e1') };
|
||||
return new Promise<Reply>((resolve) => {
|
||||
releaseMove = () => resolve({ status: 204 });
|
||||
});
|
||||
});
|
||||
const first = service.completeUpload('t1', 'u1', UP, { ...dto, replaceEtag: '"e1"' });
|
||||
const second = service.completeUpload('t1', 'u1', UP, { ...dto, replaceEtag: '"e1"' });
|
||||
await new Promise((r) => setTimeout(r, 5));
|
||||
releaseMove?.();
|
||||
const results = await Promise.all([first, second]);
|
||||
expect(results).toContainEqual({ state: 'done' });
|
||||
expect(results.every((r) => r.state === 'done' || r.state === 'assembling')).toBe(true);
|
||||
expect(calls.map((c) => c.method)).toEqual(['PROPFIND', 'MOVE']);
|
||||
expect(service.uploadState('t1', 'u1', UP)).toEqual({ state: 'done' });
|
||||
// Ein dritter Abschluss danach: das gemerkte Ergebnis, kein neuer Aufruf.
|
||||
expect(await service.completeUpload('t1', 'u1', UP, { ...dto, replaceEtag: '"e1"' })).toEqual({
|
||||
state: 'done',
|
||||
});
|
||||
expect(calls).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('kein If-Match auf dem MOVE (Nextcloud prueft es gegen .file); die Pruefung laeuft unmittelbar davor', async () => {
|
||||
const { service, calls } = setup((req) =>
|
||||
req.method === 'PROPFIND' ? { status: 207, text: statXml('e1') } : { status: 204 },
|
||||
);
|
||||
await service.completeUpload('t1', 'u1', UP, { ...dto, replaceEtag: '"e1"' });
|
||||
const move = calls.find((c) => c.method === 'MOVE');
|
||||
expect(move?.headers['if-match']).toBeUndefined();
|
||||
expect(move?.headers.overwrite).toBe('T');
|
||||
});
|
||||
|
||||
it('ein Fehler nach dem MOVE wird bei einem erneuten Abschluss gleich gemeldet, ohne zweiten MOVE', async () => {
|
||||
const { service, calls } = setup(() => ({ status: 507 }));
|
||||
const e1 = parts(await caught(service.completeUpload('t1', 'u1', UP, dto)));
|
||||
const e2 = parts(await caught(service.completeUpload('t1', 'u1', UP, dto)));
|
||||
expect(e1).toMatchObject({ status: 507, body: { code: 'quotaExceeded' } });
|
||||
expect(e2).toEqual(e1);
|
||||
expect(calls.filter((c) => c.method === 'MOVE')).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('ein Fehler VOR dem MOVE (Pruefung nicht erreichbar) darf mit einem neuen Abschluss wiederholt werden', async () => {
|
||||
let down = true;
|
||||
const { service, calls } = setup((req) => {
|
||||
if (req.method === 'PROPFIND') {
|
||||
if (down) throw Object.assign(new Error('x'), { code: 'ECONNRESET' });
|
||||
return { status: 207, text: statXml('e1') };
|
||||
}
|
||||
return { status: 204 };
|
||||
});
|
||||
const e = parts(
|
||||
await caught(service.completeUpload('t1', 'u1', UP, { ...dto, replaceEtag: '"e1"' })),
|
||||
);
|
||||
expect(e.body.code).toBe('nextcloudUnavailable');
|
||||
down = false;
|
||||
expect(await service.completeUpload('t1', 'u1', UP, { ...dto, replaceEtag: '"e1"' })).toEqual({
|
||||
state: 'done',
|
||||
});
|
||||
expect(calls.filter((c) => c.method === 'MOVE')).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('MOVE 412 beim Ersetzen: changedMeanwhile statt nameTaken, Upload-Ordner geloescht', async () => {
|
||||
const { service, calls } = setup((req) => {
|
||||
if (req.method === 'MOVE') return { status: 412 };
|
||||
if (req.method === 'PROPFIND') return { status: 207, text: statXml('e1') };
|
||||
return { status: 204 };
|
||||
});
|
||||
const { status, body } = parts(
|
||||
await caught(service.completeUpload('t1', 'u1', UP, { ...dto, replaceEtag: '"e1"' })),
|
||||
);
|
||||
expect(status).toBe(409);
|
||||
expect(body.code).toBe('changedMeanwhile');
|
||||
expect(calls.some((c) => c.method === 'DELETE')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Eingaben: replaceEtag (IN-01)', () => {
|
||||
it('nur sichtbare ASCII-Zeichen in der Form eines Entity-Tags', async () => {
|
||||
const { plainToInstance } = await import('class-transformer');
|
||||
const { validate } = await import('class-validator');
|
||||
const { StartUploadDto, UploadQueryDto, CompleteUploadDto } = await import(
|
||||
'./dto/nextcloud-files-transfer.dto'
|
||||
);
|
||||
const ok = ['"1645b02b08e573cfed4dda2749761446"', 'W/"abc"', 'abc123'];
|
||||
const bad = ['"a\r\nX-Evil: 1"', 'a b', '"ä"', '', 'x'.repeat(140)];
|
||||
for (const etag of ok) {
|
||||
const errs = await validate(
|
||||
plainToInstance(StartUploadDto, { path: '/a.txt', size: 1, replaceEtag: etag }),
|
||||
);
|
||||
expect(errs, etag).toHaveLength(0);
|
||||
}
|
||||
for (const etag of bad) {
|
||||
for (const cls of [StartUploadDto, CompleteUploadDto, UploadQueryDto]) {
|
||||
const errs = await validate(
|
||||
plainToInstance(cls as never, { path: '/a.txt', size: 1, replaceEtag: etag }) as object,
|
||||
);
|
||||
expect(errs.length, `${cls.name} ${JSON.stringify(etag)}`).toBeGreaterThan(0);
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('NextcloudFilesTransferService — Herunterladen', () => {
|
||||
const FILE = {
|
||||
status: 200,
|
||||
|
||||
@@ -1,12 +1,12 @@
|
||||
import { randomUUID } from 'node:crypto';
|
||||
import type { Readable } from 'node:stream';
|
||||
import { Inject, Injectable } from '@nestjs/common';
|
||||
import { HttpException, Inject, Injectable } from '@nestjs/common';
|
||||
import { NEXTCLOUD_FILES_CHUNK_SIZE, NEXTCLOUD_FILES_MAX_CHUNKS } from '@tessera/shared';
|
||||
import type { Response } from 'express';
|
||||
import { NextcloudCallGate } from './nextcloud-call-gate';
|
||||
import * as dav from './nextcloud-dav';
|
||||
import * as transfer from './nextcloud-dav-transfer';
|
||||
import { type NcSession, ncErrorDefault } from './nextcloud-files.types';
|
||||
import { type NcSession, ncError, ncErrorDefault } from './nextcloud-files.types';
|
||||
import { NextcloudFilesAccountService } from './nextcloud-files-account.service';
|
||||
import {
|
||||
type NcResult,
|
||||
@@ -92,6 +92,11 @@ interface AssemblyRecord {
|
||||
state: 'assembling' | 'done' | 'failed';
|
||||
code?: string;
|
||||
message?: string;
|
||||
/** HTTP-Status und Zusatzfelder des Fehlers, damit ein wiederholter Abschluss ihn gleich meldet. */
|
||||
status?: number;
|
||||
extra?: Record<string, unknown>;
|
||||
/** Der Fehler kam, bevor der MOVE an Nextcloud ging: ein neuer Abschluss darf es erneut versuchen. */
|
||||
retryable?: boolean;
|
||||
finishedAt?: number;
|
||||
}
|
||||
|
||||
@@ -302,10 +307,18 @@ export class NextcloudFilesTransferService {
|
||||
guard.settle();
|
||||
}
|
||||
if (!NextcloudFilesTransferService.ok(result)) {
|
||||
const existing =
|
||||
result.ok && result.status === 412 ? await this.existingOf(session, segments) : undefined;
|
||||
if (result.ok && result.status === 412) {
|
||||
const existing = await this.existingOf(session, segments);
|
||||
// WR-06: mit `replaceEtag` heisst 412 "die Version hat sich inzwischen geaendert" (If-Match
|
||||
// passt nicht mehr), nicht "Name vergeben" — sonst fragte die Oberflaeche erneut nach
|
||||
// "Ersetzen" und ersetzte dann eine Version, die der Benutzer nie gesehen hat.
|
||||
if (query.replaceEtag) {
|
||||
throw ncErrorDefault('changedMeanwhile', existing ? { existing } : undefined);
|
||||
}
|
||||
return this.fail(tenantId, userId, result, existing);
|
||||
}
|
||||
return this.fail(tenantId, userId, result);
|
||||
}
|
||||
return { path: pathOf(segments), size: length };
|
||||
}
|
||||
|
||||
@@ -370,6 +383,14 @@ export class NextcloudFilesTransferService {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Abschluss eines Uploads in Stuecken. IDEMPOTENT (WR-05): Der Zustand wird synchron, vor
|
||||
* dem ersten `await`, unter Mandant, Benutzer und Upload-Kennung gemerkt. Ein zweiter oder
|
||||
* gleichzeitiger Abschluss (Wiederholung nach einem Netzfehler, zweiter Tab) liefert das
|
||||
* gemerkte Ergebnis — `assembling`, `done` oder denselben Fehler — und baut nie ein zweites
|
||||
* Mal zusammen. Nur ein Fehler, der VOR dem MOVE entstand (Nextcloud hat also nichts
|
||||
* veraendert), darf mit einem neuen Abschluss erneut versucht werden.
|
||||
*/
|
||||
async completeUpload(
|
||||
tenantId: string,
|
||||
userId: string,
|
||||
@@ -382,50 +403,41 @@ export class NextcloudFilesTransferService {
|
||||
const mtime = NextcloudFilesTransferService.mtimeOf(dto.mtime);
|
||||
const session = await this.session(tenantId, userId);
|
||||
|
||||
// Ab hier kein `await` bis der Zustand gemerkt ist.
|
||||
this.prune();
|
||||
const key = this.key(tenantId, userId, uploadId);
|
||||
const known = this.assemblies.get(key);
|
||||
if (known?.state === 'assembling') return { state: 'assembling' };
|
||||
if (known?.state === 'done') return { state: 'done' };
|
||||
|
||||
// Ersetzen: unmittelbar vor dem Zusammenbau noch einmal pruefen, ob noch die Version
|
||||
// da ist, die der Benutzer ersetzen wollte.
|
||||
if (dto.replaceEtag) {
|
||||
const probe = await dav.stat(this.transport, this.gate, session, segments);
|
||||
if (!probe.ok) return this.fail(tenantId, userId, probe);
|
||||
if (probe.status === 404 || (NextcloudFilesTransferService.ok(probe) && !probe.entry)) {
|
||||
throw ncErrorDefault('changedMeanwhile');
|
||||
}
|
||||
if (!NextcloudFilesTransferService.ok(probe)) return this.fail(tenantId, userId, probe);
|
||||
if (probe.entry?.etag !== dto.replaceEtag) {
|
||||
throw ncErrorDefault(
|
||||
'changedMeanwhile',
|
||||
probe.entry ? { existing: viewOf(probe.entry) } : undefined,
|
||||
);
|
||||
}
|
||||
}
|
||||
if (known && !(known.state === 'failed' && known.retryable)) return replay(known);
|
||||
|
||||
const record: AssemblyRecord = { state: 'assembling' };
|
||||
this.assemblies.set(key, record);
|
||||
const progress = { moveSent: false };
|
||||
|
||||
// Der Zusammenbau gehoert nicht zur Anfrage: er laeuft weiter, auch wenn der Browser
|
||||
// aufgibt, und haelt sein Ergebnis in `record` fest.
|
||||
const job = this.assemble(tenantId, userId, session, uploadId, segments, {
|
||||
const job = this.assemble(tenantId, userId, session, uploadId, segments, progress, {
|
||||
totalLength: total,
|
||||
mtime,
|
||||
replace: Boolean(dto.replaceEtag),
|
||||
replaceEtag: dto.replaceEtag,
|
||||
}).then(
|
||||
() => {
|
||||
record.state = 'done';
|
||||
record.finishedAt = Date.now();
|
||||
},
|
||||
(err: unknown) => {
|
||||
const body = (err as { getResponse?: () => unknown }).getResponse?.() as
|
||||
| { code?: string; message?: string }
|
||||
| undefined;
|
||||
const body =
|
||||
err instanceof HttpException
|
||||
? (err.getResponse() as { code?: string; message?: string } & Record<string, unknown>)
|
||||
: undefined;
|
||||
record.state = 'failed';
|
||||
record.code = body?.code ?? 'nextcloudError';
|
||||
record.message = body?.message ?? ncErrorDefault('nextcloudError').message;
|
||||
record.status = err instanceof HttpException ? err.getStatus() : 502;
|
||||
if (body) {
|
||||
const { code: _c, message: _m, ...extra } = body;
|
||||
record.extra = extra;
|
||||
}
|
||||
record.retryable = !progress.moveSent;
|
||||
record.finishedAt = Date.now();
|
||||
throw err;
|
||||
},
|
||||
@@ -452,21 +464,56 @@ export class NextcloudFilesTransferService {
|
||||
session: NcSession,
|
||||
uploadId: string,
|
||||
segments: readonly string[],
|
||||
opts: { totalLength: number; mtime?: number; replace: boolean },
|
||||
progress: { moveSent: boolean },
|
||||
opts: { totalLength: number; mtime?: number; replaceEtag?: string },
|
||||
): Promise<void> {
|
||||
const { replaceEtag } = opts;
|
||||
if (replaceEtag) {
|
||||
// Ersetzen (WR-04): unmittelbar vor dem MOVE pruefen, ob noch genau die Version da ist,
|
||||
// die der Benutzer ersetzen wollte. Ein `If-Match` auf dem MOVE geht NICHT: Nextcloud
|
||||
// wertet es beim Zusammenbau gegen die Quelle `.file` aus, nicht gegen das Ziel
|
||||
// (gemessen gegen Nextcloud 34: jedes If-Match, auch das richtige, ergibt 412). Es bleibt
|
||||
// nur das kurze Fenster zwischen dieser Pruefung und dem MOVE.
|
||||
const probe = await dav.stat(this.transport, this.gate, session, segments);
|
||||
if (!probe.ok) return this.fail(tenantId, userId, probe);
|
||||
if (probe.status === 404 || (NextcloudFilesTransferService.ok(probe) && !probe.entry)) {
|
||||
throw ncErrorDefault('changedMeanwhile');
|
||||
}
|
||||
if (!NextcloudFilesTransferService.ok(probe)) return this.fail(tenantId, userId, probe);
|
||||
if (probe.entry?.etag !== replaceEtag) {
|
||||
throw ncErrorDefault(
|
||||
'changedMeanwhile',
|
||||
probe.entry ? { existing: viewOf(probe.entry) } : undefined,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
progress.moveSent = true;
|
||||
const result = await transfer.uploadAssemble(
|
||||
this.transport,
|
||||
this.gate,
|
||||
session,
|
||||
uploadId,
|
||||
segments,
|
||||
opts,
|
||||
{ totalLength: opts.totalLength, mtime: opts.mtime, replace: Boolean(replaceEtag) },
|
||||
);
|
||||
// Gesperrter Ursprung oder schon toter Zugang: der MOVE ging gar nicht erst raus.
|
||||
if (
|
||||
!result.ok &&
|
||||
(result.kind === 'paused' ||
|
||||
(result.kind === 'credential-dead' && result.status === undefined))
|
||||
) {
|
||||
progress.moveSent = false;
|
||||
}
|
||||
if (NextcloudFilesTransferService.ok(result)) return;
|
||||
if (result.ok && result.status === 412) {
|
||||
// Das Ziel ist inzwischen vergeben (gemessen: der Upload-Ordner BLEIBT) -> aufraeumen.
|
||||
const existing = await this.existingOf(session, segments);
|
||||
await transfer.uploadAbort(this.transport, this.gate, session, uploadId).catch(() => {});
|
||||
// WR-06: beim Ersetzen heisst 412 "inzwischen geaendert", nicht "Name vergeben".
|
||||
if (replaceEtag) {
|
||||
throw ncErrorDefault('changedMeanwhile', existing ? { existing } : undefined);
|
||||
}
|
||||
return this.fail(tenantId, userId, result, existing);
|
||||
}
|
||||
return this.fail(tenantId, userId, result);
|
||||
@@ -591,3 +638,16 @@ export class NextcloudFilesTransferService {
|
||||
function viewOf(entry: NcEntry): ExistingView {
|
||||
return { etag: entry.etag, size: entry.size, mtime: entry.mtime };
|
||||
}
|
||||
|
||||
/** Gemerktes Ergebnis eines Abschlusses erneut melden (WR-05). */
|
||||
function replay(known: AssemblyRecord): { state: 'done' | 'assembling' } {
|
||||
if (known.state === 'done') return { state: 'done' };
|
||||
if (known.state === 'assembling') return { state: 'assembling' };
|
||||
const code = (known.code ?? 'nextcloudError') as Parameters<typeof ncError>[0];
|
||||
throw ncError(
|
||||
code,
|
||||
known.status ?? 502,
|
||||
known.message ?? ncErrorDefault('nextcloudError').message,
|
||||
known.extra,
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user