From 9153ad5ed6b4994eb885fcad250a53cad68cb9df Mon Sep 17 00:00:00 2001 From: Tomas Rutkauskas Date: Mon, 9 Nov 2026 10:05:55 +0200 Subject: [PATCH] Fix SaveFileAsync keeping old bytes when it overwrites a longer file SaveFileAsync(Uri, string), which every SaveFileAsync and SaveFilesAsync overload goes through, opened the target with FileInfo.OpenWrite. That is FileMode.OpenOrCreate, which does not truncate, so a result saved over a longer file kept that file's trailing bytes: a 146,734-byte PDF saved over a 400,000-byte file stayed 400,000 bytes. It now opens the file with FileMode.Create, with the same access and sharing as before. SaveFilesAsync(outputDirectory) also creates the folder when it is missing, instead of throwing DirectoryNotFoundException. An empty path still means the current directory. Adds ConvertApi.Tests (NUnit), the repository's first test project. The tests serve result files from a fake IConvertApiHttpClient, so they need no network and no token. The build workflow runs them before the pack. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/build.yml | 15 ++- CHANGELOG.md | 13 +++ ConvertApi.Tests/ConvertApi.Tests.csproj | 27 +++++ ConvertApi.Tests/SaveFileTests.cs | 134 +++++++++++++++++++++++ ConvertApi.sln | 6 + ConvertApi/ConvertApiExtension.cs | 22 +++- RELEASING.md | 2 +- 7 files changed, 212 insertions(+), 7 deletions(-) create mode 100644 ConvertApi.Tests/ConvertApi.Tests.csproj create mode 100644 ConvertApi.Tests/SaveFileTests.cs diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e91ef78..c180355 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -4,10 +4,11 @@ run-name: >- || format('Building {0}', github.ref_name) }} # Trunk-based: main is the only long-lived branch. Every pull request and every push to main builds the -# whole solution (the library and all Examples) and packs the library, but nothing is published here: -# this is a public NuGet package, and NuGet versions are permanent, so a release is a deliberate act - -# publishing a GitHub Release v{semver}, which runs publish.yml. There is no test suite, so a clean build -# and pack is the check that gates the merge (the required status check is this job's name, `build`). +# whole solution (the library, its unit tests and all Examples), runs the unit tests and packs the library, +# but nothing is published here: this is a public NuGet package, and NuGet versions are permanent, so a +# release is a deliberate act - publishing a GitHub Release v{semver}, which runs publish.yml. A clean build, +# a passing test run and a pack are the check that gates the merge (the required status check is this job's +# name, `build`). on: pull_request: branches: [main] @@ -41,9 +42,13 @@ jobs: - name: Restore run: dotnet restore ConvertApi.sln - - name: Build the library and the examples + - name: Build the library, the tests and the examples run: dotnet build ConvertApi.sln -c Release --no-restore -p:ContinuousIntegrationBuild=true + # ConvertApi.Tests serves results from a fake HTTP client, so the tests need no network and no API token. + - name: Test + run: dotnet test ConvertApi.Tests/ConvertApi.Tests.csproj -c Release --no-build + # A pre-release version that sorts below any real release and names the run, so a package # downloaded from this run can be tried out but never mistaken for a published one. - name: Pack diff --git a/CHANGELOG.md b/CHANGELOG.md index 60f1b07..54f8c66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,19 @@ All notable changes to this project will be documented in this file. The format is based on Keep a Changelog and this project adheres to Semantic Versioning. +## [Unreleased] + +### Fixed +- `SaveFileAsync` and `SaveFilesAsync` replace an existing file instead of writing over its start. They + opened it with `FileInfo.OpenWrite`, which does not truncate, so a result saved over a longer file kept + that file's trailing bytes and came out corrupt: a 146,734-byte PDF saved over a 400,000-byte file stayed + 400,000 bytes. Every earlier version has this bug. + +### Changed +- `SaveFilesAsync(outputDirectory)` creates the folder if it does not exist, instead of throwing + `DirectoryNotFoundException`. An empty path still means the current directory. +- Unit tests (`ConvertApi.Tests`, NUnit) run in the `build` check of every pull request. + ## [3.2.2] — 2026-10-26 ### Changed diff --git a/ConvertApi.Tests/ConvertApi.Tests.csproj b/ConvertApi.Tests/ConvertApi.Tests.csproj new file mode 100644 index 0000000..32b4ccc --- /dev/null +++ b/ConvertApi.Tests/ConvertApi.Tests.csproj @@ -0,0 +1,27 @@ + + + + net10.0 + ConvertApiDotNet.Tests + enable + enable + false + true + + + + + + + + + + + + + + + + + + diff --git a/ConvertApi.Tests/SaveFileTests.cs b/ConvertApi.Tests/SaveFileTests.cs new file mode 100644 index 0000000..b9420a5 --- /dev/null +++ b/ConvertApi.Tests/SaveFileTests.cs @@ -0,0 +1,134 @@ +using System.Net; +using ConvertApiDotNet.Interface; +using ConvertApiDotNet.Model; + +namespace ConvertApiDotNet.Tests; + +// SaveFileAsync and SaveFilesAsync download through the static ConvertApi.GetClient(). Each test installs a +// fake client that serves the result files from memory, so the tests make no network calls, and they must not +// run in parallel with other tests that install a client. +[NonParallelizable] +public class SaveFileTests +{ + private string _dir = ""; + + [SetUp] + public void CreateTempDirectory() + { + _dir = Path.Combine(Path.GetTempPath(), "ConvertApi.Tests-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(_dir); + } + + [TearDown] + public void DeleteTempDirectory() => Directory.Delete(_dir, recursive: true); + + [Test] + public async Task SaveFileAsync_ReplacesALongerExistingFile() + { + // The sizes from the report: a 146,734-byte PDF saved over a 400,000-byte file left 400,000 bytes. + var path = Path.Combine(_dir, "document.pdf"); + File.WriteAllBytes(path, Bytes(400_000, seed: 1)); + var content = Bytes(146_734, seed: 2); + var file = Serve(("document.pdf", content)).Single(); + + var saved = await file.SaveFileAsync(path); + + using (Assert.EnterMultipleScope()) + { + Assert.That(saved.FullName, Is.EqualTo(Path.GetFullPath(path))); + Assert.That(saved.Length, Is.EqualTo(content.Length)); + Assert.That(File.ReadAllBytes(path), Is.EqualTo(content)); + } + } + + [Test] + public async Task SaveFilesAsync_ReplacesLongerExistingFiles() + { + var first = Bytes(1_000, seed: 3); + var second = Bytes(2_000, seed: 4); + File.WriteAllBytes(Path.Combine(_dir, "document.jpg"), Bytes(50_000, seed: 5)); + File.WriteAllBytes(Path.Combine(_dir, "document-2.jpg"), Bytes(50_000, seed: 6)); + var response = new ConvertApiResponse { Files = Serve(("document.jpg", first), ("document-2.jpg", second)) }; + + var saved = await response.SaveFilesAsync(_dir); + + using (Assert.EnterMultipleScope()) + { + Assert.That(saved.Select(f => f.Name), Is.EqualTo(new[] { "document.jpg", "document-2.jpg" })); + Assert.That(File.ReadAllBytes(Path.Combine(_dir, "document.jpg")), Is.EqualTo(first)); + Assert.That(File.ReadAllBytes(Path.Combine(_dir, "document-2.jpg")), Is.EqualTo(second)); + } + } + + [Test] + public async Task SaveFilesAsync_CreatesAMissingFolder() + { + var output = Path.Combine(_dir, "output", "nested"); + var content = Bytes(1_000, seed: 7); + var response = new ConvertApiResponse { Files = Serve(("document.pdf", content)) }; + + await response.SaveFilesAsync(output); + + Assert.That(File.ReadAllBytes(Path.Combine(output, "document.pdf")), Is.EqualTo(content)); + } + + [Test] + public async Task SaveFilesAsync_EmptyFolderMeansTheCurrentDirectory() + { + var content = Bytes(1_000, seed: 8); + var response = new ConvertApiResponse { Files = Serve(("document.pdf", content)) }; + + var previous = Environment.CurrentDirectory; + Environment.CurrentDirectory = _dir; + try + { + await response.SaveFilesAsync(""); + } + finally + { + Environment.CurrentDirectory = previous; + } + + Assert.That(File.ReadAllBytes(Path.Combine(_dir, "document.pdf")), Is.EqualTo(content)); + } + + private static byte[] Bytes(int length, int seed) + { + var bytes = new byte[length]; + new Random(seed).NextBytes(bytes); + return bytes; + } + + // Installs a client that serves these files, and returns them as a conversion response lists them. + private static ConvertApiFile[] Serve(params (string Name, byte[] Content)[] files) + { + var served = new Dictionary(); + var results = new List(); + foreach (var (name, content) in files) + { + var url = new Uri("https://v2.convertapi.com/d/test/" + name); + served[url] = content; + results.Add(new ConvertApiFile { FileName = name, FileSize = content.Length, Url = url }); + } + + // This constructor is the SDK's only way to replace the client that ConvertApi.GetClient() returns. + _ = new ConvertApi("test-token", new FakeClient(served)); + return results.ToArray(); + } + + private sealed class FakeClient(Dictionary files) : IConvertApiHttpClient + { + public HttpClient Client { get; } = new(new FileServer(files)); + } + + private sealed class FileServer(Dictionary files) : HttpMessageHandler + { + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + if (request.Method != HttpMethod.Get || !files.TryGetValue(request.RequestUri!, out var content)) + throw new InvalidOperationException($"Unexpected request: {request.Method} {request.RequestUri}"); + + return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { Content = new ByteArrayContent(content) }); + } + } +} diff --git a/ConvertApi.sln b/ConvertApi.sln index f41db12..7f8a346 100644 --- a/ConvertApi.sln +++ b/ConvertApi.sln @@ -59,6 +59,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "UploadOnceReuseTwice", "Exa EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ConverterCatalogDemo", "Examples\ConverterCatalogDemo\ConverterCatalogDemo.csproj", "{DFEC8F03-E140-488A-B457-59715605F5CA}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ConvertApi.Tests", "ConvertApi.Tests\ConvertApi.Tests.csproj", "{2DBDE523-FC5A-4662-A99D-94835F760E20}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -153,6 +155,10 @@ Global {DFEC8F03-E140-488A-B457-59715605F5CA}.Debug|Any CPU.Build.0 = Debug|Any CPU {DFEC8F03-E140-488A-B457-59715605F5CA}.Release|Any CPU.ActiveCfg = Release|Any CPU {DFEC8F03-E140-488A-B457-59715605F5CA}.Release|Any CPU.Build.0 = Release|Any CPU + {2DBDE523-FC5A-4662-A99D-94835F760E20}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {2DBDE523-FC5A-4662-A99D-94835F760E20}.Debug|Any CPU.Build.0 = Debug|Any CPU + {2DBDE523-FC5A-4662-A99D-94835F760E20}.Release|Any CPU.ActiveCfg = Release|Any CPU + {2DBDE523-FC5A-4662-A99D-94835F760E20}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/ConvertApi/ConvertApiExtension.cs b/ConvertApi/ConvertApiExtension.cs index 584ad49..92bc164 100644 --- a/ConvertApi/ConvertApiExtension.cs +++ b/ConvertApi/ConvertApiExtension.cs @@ -33,7 +33,9 @@ private static async Task SaveFileAsync(Uri url, string fileName) var fileInfo = new FileInfo(fileName); using (var readFile = await AsStreamAsync(url)) { - using (var fileStream = fileInfo.OpenWrite()) + // FileMode.Create truncates an existing file. FileInfo.OpenWrite (FileMode.OpenOrCreate) did not, + // so a result saved over a longer file kept that file's trailing bytes and came out corrupt. + using (var fileStream = new FileStream(fileInfo.FullName, FileMode.Create, FileAccess.Write, FileShare.None)) await readFile.CopyToAsync(fileStream); } @@ -41,11 +43,18 @@ private static async Task SaveFileAsync(Uri url, string fileName) } #region File Task Methods + /// + /// Save the first result file as , replacing the file if it exists. + /// public static async Task SaveFileAsync(this ConvertApiResponse response, string fileName) { return await response.Files[0].SaveFileAsync(fileName); } + /// + /// Save every result file into under its own name, replacing files + /// that exist. The folder is created if it does not exist. + /// public static async Task> SaveFilesAsync(this ConvertApiResponse response, string outputDirectory) { return await response.Files.SaveFilesAsync(outputDirectory); @@ -56,13 +65,24 @@ public static async Task FileStreamAsync(this ConvertApiFile processedFi return await AsStreamAsync(processedFile.Url); } + /// + /// Save the file as , replacing the file if it exists. + /// public static async Task SaveFileAsync(this ConvertApiFile processedFile, string fileName) { return await SaveFileAsync(processedFile.Url, fileName); } + /// + /// Save the files into , each under its own name, replacing files that + /// exist. The folder is created if it does not exist. + /// public static async Task> SaveFilesAsync(this IEnumerable processedFile, string outputDirectory) { + // An empty path still means the current directory, as Path.Combine("", name) below always did. + if (!string.IsNullOrEmpty(outputDirectory)) + Directory.CreateDirectory(outputDirectory); + var list = new List(); foreach (var file in processedFile) { diff --git a/RELEASING.md b/RELEASING.md index e73c592..47c0ceb 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -2,7 +2,7 @@ `main` is the only long-lived branch. Changes reach it through pull requests, merge commits only, and every pull request and push to `main` must pass the `build` check (`.github/workflows/build.yml`: restore, -build the library and every example, pack). Merging publishes nothing. +build the library, its unit tests and every example, run the unit tests, pack). Merging publishes nothing. A release is a **GitHub Release** whose tag is the version, published by `tomasr78`: