diff --git a/Storage/Extensions/HttpClientProgress.cs b/Storage/Extensions/HttpClientProgress.cs index ddb57fd..4d423e2 100644 --- a/Storage/Extensions/HttpClientProgress.cs +++ b/Storage/Extensions/HttpClientProgress.cs @@ -20,6 +20,13 @@ namespace Supabase.Storage.Extensions /// internal static class HttpClientProgress { + /// + /// Buffer size for stream copies, matching the BCL's default + /// size (80 KB). Kept just under the 85,000-byte Large Object Heap threshold so the buffer + /// stays on the gen-0 heap. This is set to the default expected value. + /// + private const int CopyBufferSize = 81920; + public static async Task DownloadDataAsync( this HttpClient client, Uri uri, @@ -49,7 +56,8 @@ public static async Task DownloadDataAsync( using ( var response = await client.SendAsync( message, - HttpCompletionOption.ResponseHeadersRead + HttpCompletionOption.ResponseHeadersRead, + cancellationToken ) ) { @@ -76,10 +84,9 @@ public static async Task DownloadDataAsync( var contentLength = response.Content.Headers.ContentLength; using (var download = await response.Content.ReadAsStreamAsync()) { - // no progress... no contentLength... very sad if (progress is null || !contentLength.HasValue) { - await download.CopyToAsync(destination); + await download.CopyToAsync(destination, CopyBufferSize, cancellationToken); return destination; } @@ -89,7 +96,7 @@ public static async Task DownloadDataAsync( ); await download.CopyToAsync( destination, - 81920, + CopyBufferSize, progressWrapper, cancellationToken ); diff --git a/Storage/Interfaces/IStorageFileApi.cs b/Storage/Interfaces/IStorageFileApi.cs index 5cb3a17..9410d89 100644 --- a/Storage/Interfaces/IStorageFileApi.cs +++ b/Storage/Interfaces/IStorageFileApi.cs @@ -20,33 +20,38 @@ Task CreateSignedUrl( int expiresIn, DownloadOptions? options = null ); - Task Download(string supabasePath, EventHandler? onProgress = null); + Task Download(string supabasePath, EventHandler? onProgress = null, CancellationToken cancellationToken = default); Task Download( string supabasePath, TransformOptions? transformOptions = null, - EventHandler? onProgress = null + EventHandler? onProgress = null, + CancellationToken cancellationToken = default ); Task Download( string supabasePath, string localPath, - EventHandler? onProgress = null + EventHandler? onProgress = null, + CancellationToken cancellationToken = default ); Task Download( string supabasePath, string localPath, TransformOptions? transformOptions = null, - EventHandler? onProgress = null + EventHandler? onProgress = null, + CancellationToken cancellationToken = default ); Task DownloadPublicFile( string supabasePath, TransformOptions? transformOptions = null, - EventHandler? onProgress = null + EventHandler? onProgress = null, + CancellationToken cancellationToken = default ); Task DownloadPublicFile( string supabasePath, string localPath, TransformOptions? transformOptions = null, - EventHandler? onProgress = null + EventHandler? onProgress = null, + CancellationToken cancellationToken = default ); string GetPublicUrl( string path, diff --git a/Storage/StorageFileApi.cs b/Storage/StorageFileApi.cs index 4a66222..ed568f3 100644 --- a/Storage/StorageFileApi.cs +++ b/Storage/StorageFileApi.cs @@ -513,19 +513,21 @@ await Helpers.MakeRequest( /// /// /// + /// /// public Task Download( string supabasePath, string localPath, TransformOptions? transformOptions = null, - EventHandler? onProgress = null + EventHandler? onProgress = null, + CancellationToken cancellationToken = default ) { var url = transformOptions != null ? $"{Url}/render/image/authenticated/{GetFinalPath(supabasePath)}" : $"{Url}/object/{GetFinalPath(supabasePath)}"; - return DownloadFile(url, localPath, transformOptions, onProgress); + return DownloadFile(url, localPath, transformOptions, onProgress, cancellationToken); } /// @@ -534,12 +536,14 @@ public Task Download( /// /// /// + /// /// public Task Download( string supabasePath, string localPath, - EventHandler? onProgress = null - ) => Download(supabasePath, localPath, null, onProgress: onProgress); + EventHandler? onProgress = null, + CancellationToken cancellationToken = default + ) => Download(supabasePath, localPath, null, onProgress: onProgress, cancellationToken); /// /// Downloads a byte array from a private bucket to be used programmatically. For public buckets @@ -547,15 +551,17 @@ public Task Download( /// /// /// + /// /// public Task Download( string supabasePath, TransformOptions? transformOptions = null, - EventHandler? onProgress = null + EventHandler? onProgress = null, + CancellationToken cancellationToken = default ) { var url = $"{Url}/object/{GetFinalPath(supabasePath)}"; - return DownloadBytes(url, transformOptions, onProgress); + return DownloadBytes(url, transformOptions, onProgress, cancellationToken); } /// @@ -563,9 +569,10 @@ public Task Download( /// /// /// + /// /// - public Task Download(string supabasePath, EventHandler? onProgress = null) => - Download(supabasePath, transformOptions: null, onProgress: onProgress); + public Task Download(string supabasePath, EventHandler? onProgress = null, CancellationToken cancellationToken = default) => + Download(supabasePath, transformOptions: null, onProgress: onProgress, cancellationToken); /// /// Downloads a public file to the filesystem. This method DOES NOT VERIFY that the file is actually public. @@ -574,16 +581,18 @@ public Task Download(string supabasePath, EventHandler? onProgres /// /// /// + /// /// public Task DownloadPublicFile( string supabasePath, string localPath, TransformOptions? transformOptions = null, - EventHandler? onProgress = null + EventHandler? onProgress = null, + CancellationToken cancellationToken = default ) { var url = GetPublicUrl(supabasePath, transformOptions); - return DownloadFile(url, localPath, transformOptions, onProgress); + return DownloadFile(url, localPath, transformOptions, onProgress, cancellationToken); } /// @@ -592,15 +601,17 @@ public Task DownloadPublicFile( /// /// /// + /// /// public Task DownloadPublicFile( string supabasePath, TransformOptions? transformOptions = null, - EventHandler? onProgress = null + EventHandler? onProgress = null, + CancellationToken cancellationToken = default ) { var url = GetPublicUrl(supabasePath, transformOptions); - return DownloadBytes(url, transformOptions, onProgress); + return DownloadBytes(url, transformOptions, onProgress, cancellationToken); } /// @@ -835,7 +846,8 @@ private async Task DownloadFile( string url, string localPath, TransformOptions? transformOptions = null, - EventHandler? onProgress = null + EventHandler? onProgress = null, + CancellationToken cancellationToken = default ) { var builder = new UriBuilder(url); @@ -850,7 +862,8 @@ private async Task DownloadFile( var stream = await Helpers.HttpDownloadClient!.DownloadDataAsync( builder.Uri, Headers, - progress + progress, + cancellationToken ); using var fileStream = new FileStream( @@ -867,7 +880,8 @@ private async Task DownloadFile( private async Task DownloadBytes( string url, TransformOptions? transformOptions = null, - EventHandler? onProgress = null + EventHandler? onProgress = null, + CancellationToken cancellationToken = default ) { var builder = new UriBuilder(url); @@ -882,7 +896,8 @@ private async Task DownloadBytes( var stream = await Helpers.HttpDownloadClient!.DownloadDataAsync( builder.Uri, Headers, - progress + progress, + cancellationToken ); return stream.ToArray(); diff --git a/StorageTests/Files/StorageFileDownloadCancellationTests.cs b/StorageTests/Files/StorageFileDownloadCancellationTests.cs new file mode 100644 index 0000000..9635450 --- /dev/null +++ b/StorageTests/Files/StorageFileDownloadCancellationTests.cs @@ -0,0 +1,122 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Net; +using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; +using FluentAssertions; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Supabase.Storage; + +namespace StorageTests.Files; + +/// +/// Contract tests pinning that the file API threads the caller's all +/// the way through the download pipeline: onto the HTTP request itself, and onto the response-body copy +/// on the chunked (no Content-Length) path that the progress-reporting branch never exercises. +/// A stub stands in for the download client, and each test observes the +/// token *at the point under test* rather than relying on the thrown exception — the body copy honoured +/// the token before these fixes too, so a bare "throws when cancelled" assertion cannot tell the fix +/// apart from the pre-existing behaviour. +/// +[TestClass] +[TestCategory("Contract")] +public class StorageFileDownloadCancellationTests +{ + private const string Bucket = "bucket"; + + private Client client = null!; + private HttpClient? downloadClient; + + [TestInitialize] + public void TestInitialize() => + this.client = new Client("http://localhost/storage/v1", new Dictionary + { + { "Authorization", "Bearer test-key" } + }); + + [TestCleanup] + public void TestCleanup() => this.downloadClient?.Dispose(); + + [TestMethod] + public async Task Download_ShouldForwardTheTokenToTheRequest() + { + using var cts = new CancellationTokenSource(); + var requestSawLiveToken = false; + this.UseHandler(new StubHandler((_, token) => + { + cts.Cancel(); + requestSawLiveToken = token.IsCancellationRequested; + token.ThrowIfCancellationRequested(); + return EmptyOk(); + })); + var act = () => this.client.From(Bucket).Download("a.bin", onProgress: null, cts.Token); + await act.Should().ThrowAsync(); + requestSawLiveToken.Should().BeTrue( + "the request must carry the caller's token so a stalled connection can be cancelled before any body arrives"); + } + + [TestMethod] + public async Task Download_ShouldHonorCancellation_GivenResponseWithoutContentLength() + { + using var cts = new CancellationTokenSource(); + var bodyCopySawLiveToken = false; + var body = new TokenObservingStream(token => + { + cts.Cancel(); + bodyCopySawLiveToken = token.IsCancellationRequested; + token.ThrowIfCancellationRequested(); + }); + this.UseHandler(new StubHandler((_, _) => + new HttpResponseMessage(HttpStatusCode.OK) { Content = new StreamContent(body) })); + var act = () => this.client.From(Bucket).Download("a.bin", onProgress: null, cts.Token); + await act.Should().ThrowAsync(); + bodyCopySawLiveToken.Should().BeTrue( + "the body copy must carry the token even when the server omits Content-Length and no progress handler is supplied"); + } + + private void UseHandler(HttpMessageHandler handler) => + Supabase.Storage.Helpers.HttpDownloadClient = this.downloadClient = new HttpClient(handler); + + private static HttpResponseMessage EmptyOk() => + new(HttpStatusCode.OK) { Content = new ByteArrayContent(Array.Empty()) }; + + private sealed class StubHandler(Func respond) + : HttpMessageHandler + { + protected override Task SendAsync( + HttpRequestMessage request, CancellationToken cancellationToken) => + Task.FromResult(respond(request, cancellationToken)); + } + + /// + /// A non-seekable response body (so the response carries no Content-Length) that reports the + /// token handed to each read back to the test before signalling end-of-stream. + /// + private sealed class TokenObservingStream(Action onRead) : Stream + { + public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + onRead(cancellationToken); + return Task.FromResult(0); + } + + public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) + { + onRead(cancellationToken); + return new ValueTask(0); + } + + public override bool CanRead => true; + public override bool CanSeek => false; + public override bool CanWrite => false; + public override long Length => throw new NotSupportedException(); + public override long Position { get => throw new NotSupportedException(); set => throw new NotSupportedException(); } + public override void Flush() { } + public override int Read(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + public override void SetLength(long value) => throw new NotSupportedException(); + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + } +} diff --git a/StorageTests/Files/StorageFileTests.cs b/StorageTests/Files/StorageFileTests.cs index 532d2ef..44ea248 100644 --- a/StorageTests/Files/StorageFileTests.cs +++ b/StorageTests/Files/StorageFileTests.cs @@ -192,6 +192,21 @@ public async Task Download_ShouldWriteFileToDisk() await this.bucket.Remove(new List { name }); } + [TestMethod] + public async Task Download_ShouldCancelAndLeaveNoFile_GivenCancelledToken() + { + using var cts = new CancellationTokenSource(); + await cts.CancelAsync(); + var name = $"{Guid.NewGuid()}.png"; + var imagePath = Path.Combine(BasePath(), "Assets", "supabase-csharp.png"); + await this.bucket.Upload(imagePath, name); + var downloadPath = Path.Combine(BasePath(), name); + var act = () => this.bucket.Download(name, downloadPath, null, cts.Token); + await act.Should().ThrowAsync(); + File.Exists(downloadPath).Should().BeFalse(); + await this.bucket.Remove(new List { name }); + } + [TestMethod] public async Task Download_ShouldReturnTheStoredBytes() {