diff --git a/DeepDrftAPI/Controllers/TrackController.cs b/DeepDrftAPI/Controllers/TrackController.cs index 015c11f..a277cbb 100644 --- a/DeepDrftAPI/Controllers/TrackController.cs +++ b/DeepDrftAPI/Controllers/TrackController.cs @@ -47,24 +47,25 @@ public class TrackController : ControllerBase _logger = logger; } - // Streams an uploaded audio body to a freshly-named staging file on the data disk, preserving the - // validated extension (the processor router selects by extension and reads from disk; .tmp would be - // rejected). Staging lives under UploadStagingDirectory, never Path.GetTempPath() — on the Linux - // host /tmp is a small tmpfs that cannot hold a large WAV. Returns the staging path; the caller - // owns deletion in a finally block. - private async Task StageUploadAsync( - IFormFile audioFile, string uploadExtension, CancellationToken cancellationToken) - { - var stagingPath = Path.Combine( - _stagingDirectory.Path, Guid.NewGuid().ToString("N") + uploadExtension); + // Builds a unique staging file path on the data disk with the validated extension. The caller MUST + // assign this to the local that its finally block guards BEFORE calling StageUploadAsync — that + // way a mid-copy abort (OperationCanceledException, IO error) still triggers deletion of the + // partially-written file. Staging lives under UploadStagingDirectory, never Path.GetTempPath() — + // on the Linux host /tmp is a small tmpfs that cannot hold a large WAV. + private string BuildStagingPath(string uploadExtension) => + Path.Combine(_stagingDirectory.Path, Guid.NewGuid().ToString("N") + uploadExtension); + // Streams an uploaded audio body to the pre-allocated staging path. The caller owns the path and + // must delete it in a finally block; separating path generation from the copy ensures the finally + // guard fires even when CopyToAsync throws before returning. + private async Task StageUploadAsync( + IFormFile audioFile, string stagingPath, CancellationToken cancellationToken) + { await using var stagingStream = new FileStream( stagingPath, FileMode.CreateNew, FileAccess.Write, FileShare.None, bufferSize: 81920, useAsync: true); await using var uploadStream = audioFile.OpenReadStream(); await uploadStream.CopyToAsync(stagingStream, cancellationToken); - - return stagingPath; } // Best-effort removal of a staging file. Logs and swallows — a stranded staging file is a @@ -359,10 +360,12 @@ public class TrackController : ControllerBase var resolvedTrackNumber = trackNumber is > 0 ? trackNumber.Value : 1; - string? stagingPath = null; + // Build the staging path before the copy so the finally block can delete the partial file + // even if CopyToAsync throws mid-stream (client cancellation, disk-full, IO error). + var stagingPath = BuildStagingPath(uploadExtension); try { - stagingPath = await StageUploadAsync(audioFile, uploadExtension, cancellationToken); + await StageUploadAsync(audioFile, stagingPath, cancellationToken); var result = await _unifiedService.UploadAsync( stagingPath, @@ -411,10 +414,7 @@ public class TrackController : ControllerBase } finally { - if (stagingPath is not null) - { - DeleteStagingFile(stagingPath); - } + DeleteStagingFile(stagingPath); } } @@ -590,10 +590,12 @@ public class TrackController : ControllerBase return BadRequest("Uploaded file must have a .wav, .mp3, or .flac extension"); } - string? stagingPath = null; + // Build the staging path before the copy so the finally block can delete the partial file + // even if CopyToAsync throws mid-stream (client cancellation, disk-full, IO error). + var stagingPath = BuildStagingPath(uploadExtension); try { - stagingPath = await StageUploadAsync(audioFile, uploadExtension, cancellationToken); + await StageUploadAsync(audioFile, stagingPath, cancellationToken); var result = await _unifiedService.ReplaceAudioAsync(id, stagingPath, cancellationToken); if (result.Success) @@ -618,10 +620,7 @@ public class TrackController : ControllerBase } finally { - if (stagingPath is not null) - { - DeleteStagingFile(stagingPath); - } + DeleteStagingFile(stagingPath); } } diff --git a/DeepDrftTests/UploadStagingPathTests.cs b/DeepDrftTests/UploadStagingPathTests.cs index b55135b..02c593c 100644 --- a/DeepDrftTests/UploadStagingPathTests.cs +++ b/DeepDrftTests/UploadStagingPathTests.cs @@ -51,6 +51,12 @@ public class UploadStagingPathTests var resolved = Startup.ResolveStagingPath(configuredPath: null, vaultPath); var systemTemp = Path.GetFullPath(Path.GetTempPath()); + // Note: because vaultPath is relative, Path.GetFullPath resolves it against the CWD, which is + // never the system temp directory. The StartsWith guard therefore catches the case where + // ResolveStagingPath mistakenly uses Path.GetTempPath() directly, rather than proving the + // absolute production path never overlaps with /tmp on any machine. The EndsWith assertion + // is the load-bearing check: it verifies the output is rooted under the vault tree, not + // under a hard-coded temp location. Assert.That(resolved.StartsWith(systemTemp, StringComparison.Ordinal), Is.False, "The default staging directory must never live under the system temp mount"); Assert.That(resolved, Does.EndWith(Path.Combine("Database", "Vaults", "staging")),