fix: always delete staging file on mid-copy abort

Build the staging path before the copy in both UploadTrack and ReplaceAudio so the finally block deletes it on cancellation or IO error, not only on success.
This commit is contained in:
daniel-c-harvey
2026-06-19 17:36:06 -04:00
parent 37cf19c405
commit d7071fdbc2
2 changed files with 29 additions and 24 deletions
+23 -24
View File
@@ -47,24 +47,25 @@ public class TrackController : ControllerBase
_logger = logger; _logger = logger;
} }
// Streams an uploaded audio body to a freshly-named staging file on the data disk, preserving the // Builds a unique staging file path on the data disk with the validated extension. The caller MUST
// validated extension (the processor router selects by extension and reads from disk; .tmp would be // assign this to the local that its finally block guards BEFORE calling StageUploadAsync — that
// rejected). Staging lives under UploadStagingDirectory, never Path.GetTempPath() — on the Linux // way a mid-copy abort (OperationCanceledException, IO error) still triggers deletion of the
// host /tmp is a small tmpfs that cannot hold a large WAV. Returns the staging path; the caller // partially-written file. Staging lives under UploadStagingDirectory, never Path.GetTempPath() —
// owns deletion in a finally block. // on the Linux host /tmp is a small tmpfs that cannot hold a large WAV.
private async Task<string> StageUploadAsync( private string BuildStagingPath(string uploadExtension) =>
IFormFile audioFile, string uploadExtension, CancellationToken cancellationToken) Path.Combine(_stagingDirectory.Path, Guid.NewGuid().ToString("N") + uploadExtension);
{
var stagingPath = 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( await using var stagingStream = new FileStream(
stagingPath, FileMode.CreateNew, FileAccess.Write, FileShare.None, stagingPath, FileMode.CreateNew, FileAccess.Write, FileShare.None,
bufferSize: 81920, useAsync: true); bufferSize: 81920, useAsync: true);
await using var uploadStream = audioFile.OpenReadStream(); await using var uploadStream = audioFile.OpenReadStream();
await uploadStream.CopyToAsync(stagingStream, cancellationToken); await uploadStream.CopyToAsync(stagingStream, cancellationToken);
return stagingPath;
} }
// Best-effort removal of a staging file. Logs and swallows — a stranded staging file is a // 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; 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 try
{ {
stagingPath = await StageUploadAsync(audioFile, uploadExtension, cancellationToken); await StageUploadAsync(audioFile, stagingPath, cancellationToken);
var result = await _unifiedService.UploadAsync( var result = await _unifiedService.UploadAsync(
stagingPath, stagingPath,
@@ -411,10 +414,7 @@ public class TrackController : ControllerBase
} }
finally 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"); 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 try
{ {
stagingPath = await StageUploadAsync(audioFile, uploadExtension, cancellationToken); await StageUploadAsync(audioFile, stagingPath, cancellationToken);
var result = await _unifiedService.ReplaceAudioAsync(id, stagingPath, cancellationToken); var result = await _unifiedService.ReplaceAudioAsync(id, stagingPath, cancellationToken);
if (result.Success) if (result.Success)
@@ -618,10 +620,7 @@ public class TrackController : ControllerBase
} }
finally finally
{ {
if (stagingPath is not null) DeleteStagingFile(stagingPath);
{
DeleteStagingFile(stagingPath);
}
} }
} }
+6
View File
@@ -51,6 +51,12 @@ public class UploadStagingPathTests
var resolved = Startup.ResolveStagingPath(configuredPath: null, vaultPath); var resolved = Startup.ResolveStagingPath(configuredPath: null, vaultPath);
var systemTemp = Path.GetFullPath(Path.GetTempPath()); 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, Assert.That(resolved.StartsWith(systemTemp, StringComparison.Ordinal), Is.False,
"The default staging directory must never live under the system temp mount"); "The default staging directory must never live under the system temp mount");
Assert.That(resolved, Does.EndWith(Path.Combine("Database", "Vaults", "staging")), Assert.That(resolved, Does.EndWith(Path.Combine("Database", "Vaults", "staging")),