From fb2c29f4bb4c4a31f01c23c151ff86592d1cff4b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 10:39:53 +0000 Subject: [PATCH 01/23] Ignore drive-relative XDG overrides on Windows Fixes #139 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- Essentials.Tests/UserDirectoriesTests.cs | 28 ++++++++++++++++++++++++ Essentials/UserDirectories.cs | 6 +++-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/Essentials.Tests/UserDirectoriesTests.cs b/Essentials.Tests/UserDirectoriesTests.cs index 63bdf7a..8e046cd 100644 --- a/Essentials.Tests/UserDirectoriesTests.cs +++ b/Essentials.Tests/UserDirectoriesTests.cs @@ -56,6 +56,34 @@ public void StateAndCache_Ignore_Relative_Overrides() Assert.AreEqual(Path.Join(UserDirectories.GetHomeDirectory(), ".cache"), UserDirectories.GetCacheHome())); } + [TestMethod] + [TestCategory("OS-Specific")] + public void Homes_Ignore_Drive_Relative_Overrides_On_Windows() + { + if (!OperatingSystem.IsWindows()) + { + Assert.Inconclusive("Drive-relative paths only exist on Windows."); + } + + // Both are rooted but not fully qualified: "\x" is relative to the current drive, "C:x" to the + // current directory on drive C. + foreach (string relative in new[] { @"\x", "C:x" }) + { + WithVariable(UserDirectories.ConfigHomeVariable, relative, () => + Assert.AreEqual(Path.Join(UserDirectories.GetHomeDirectory(), ".config"), UserDirectories.GetConfigHome())); + WithVariable(UserDirectories.DataHomeVariable, relative, () => + Assert.AreEqual(Path.Join(UserDirectories.GetHomeDirectory(), ".local", "share"), UserDirectories.GetDataHome())); + } + } + + [TestMethod] + public void Homes_Honour_A_Fully_Qualified_Override() + { + string overridden = Path.GetFullPath(Path.Join(Path.GetTempPath(), "xdg_config_" + Guid.NewGuid().ToString("N")[..8])); + WithVariable(UserDirectories.ConfigHomeVariable, overridden, () => + Assert.AreEqual(overridden, UserDirectories.GetConfigHome())); + } + [TestMethod] public void Application_Directories_Reject_A_Blank_Name() { diff --git a/Essentials/UserDirectories.cs b/Essentials/UserDirectories.cs index 369a9ab..a87803b 100644 --- a/Essentials/UserDirectories.cs +++ b/Essentials/UserDirectories.cs @@ -137,8 +137,10 @@ private static string ResolveBase(string variable, string relativeDefault) { string? overridden = Environment.GetEnvironmentVariable(variable); - // The specification says a value that is not an absolute path must be ignored. - return !string.IsNullOrWhiteSpace(overridden) && Path.IsPathRooted(overridden) + // The specification says a value that is not an absolute path must be ignored. IsPathRooted is not + // enough: on Windows it also accepts "\cfg" and "C:cfg", which resolve against the current drive or + // the current directory on a drive. + return !string.IsNullOrWhiteSpace(overridden) && Path.IsPathFullyQualified(overridden) ? overridden! : Path.Combine(GetHomeDirectory(), relativeDefault); } From 30ccfc689a97cce90b4efe1f7da60da5a4c2217c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 10:39:53 +0000 Subject: [PATCH 02/23] Escape COM0, LPT0 and the superscript device names in persistence keys Fixes #146 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- Essentials.Tests/PersistenceNamingTests.cs | 6 +++++- Essentials/PersistenceProviderUtilities.cs | 11 ++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/Essentials.Tests/PersistenceNamingTests.cs b/Essentials.Tests/PersistenceNamingTests.cs index 299a001..ddbe784 100644 --- a/Essentials.Tests/PersistenceNamingTests.cs +++ b/Essentials.Tests/PersistenceNamingTests.cs @@ -57,7 +57,11 @@ public void SafeFileName_Roundtrips_Through_Decoding() [TestMethod] public void SafeFileName_Escapes_Windows_Reserved_Device_Names() { - foreach (string reserved in new[] { "CON", "PRN", "AUX", "NUL", "COM1", "LPT9", "con", "NuL" }) + foreach (string reserved in new[] + { + "CON", "PRN", "AUX", "NUL", "COM1", "LPT9", "con", "NuL", + "COM0", "LPT0", "COM\u00B9", "COM\u00B2", "COM\u00B3", "LPT\u00B9", "LPT\u00B2", "LPT\u00B3", "com\u00B9.json", + }) { string encoded = PersistenceProviderUtilities.GetSafeFileName(reserved); diff --git a/Essentials/PersistenceProviderUtilities.cs b/Essentials/PersistenceProviderUtilities.cs index 7c7eaff..fa0d1be 100644 --- a/Essentials/PersistenceProviderUtilities.cs +++ b/Essentials/PersistenceProviderUtilities.cs @@ -18,12 +18,17 @@ public static class PersistenceProviderUtilities /// private static readonly char[] ReservedCharacters = ['<', '>', ':', '"', '/', '\\', '|', '?', '*', '%']; - /// Device names Windows reserves regardless of extension. + /// + /// Device names Windows reserves regardless of extension. Windows counts the superscript digits + /// ¹, ² and ³ as digits in a device name, so COM¹ is a serial port just as COM1 is. + /// private static readonly string[] ReservedDeviceNames = [ "CON", "PRN", "AUX", "NUL", - "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", - "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", + "COM0", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", + "COM\u00B9", "COM\u00B2", "COM\u00B3", + "LPT0", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", + "LPT\u00B9", "LPT\u00B2", "LPT\u00B3", ]; /// From 7294bc3a88d71ea6a5d1218bf54b546f16d6b9c9 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 10:39:53 +0000 Subject: [PATCH 03/23] Compute the hex bound in long so large inputs cannot wrap it negative Fixes #150 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- .../HexEncodingProvider.cs | 13 +++++++++++-- Essentials.Tests/EncodingProviderTests.cs | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/Essentials.EncodingProviders.Hex/HexEncodingProvider.cs b/Essentials.EncodingProviders.Hex/HexEncodingProvider.cs index faa76dd..2751500 100644 --- a/Essentials.EncodingProviders.Hex/HexEncodingProvider.cs +++ b/Essentials.EncodingProviders.Hex/HexEncodingProvider.cs @@ -22,7 +22,15 @@ public class HexEncodingProvider : IEncodingProvider private const string HexDigits = "0123456789ABCDEF"; /// - public int GetMaxEncodedLength(int sourceLength) => sourceLength * 2; + /// The encoded length of bytes exceeds . + public int GetMaxEncodedLength(int sourceLength) + { + // Computed in long: doubling an input of 1 GiB or more would wrap to a negative bound. + long bound = 2L * sourceLength; + return bound <= int.MaxValue + ? (int)bound + : throw new ArgumentOutOfRangeException(nameof(sourceLength), sourceLength, "The encoded length exceeds the largest possible buffer."); + } /// public int GetMaxDecodedLength(int encodedLength) => encodedLength / 2; @@ -32,7 +40,8 @@ public bool TryEncode(ReadOnlySpan data, Span destination, out int b { bytesWritten = 0; - if (destination.Length < data.Length * 2) + // Compared in long so an input of 1 GiB or more cannot wrap the requirement negative and pass. + if (destination.Length < 2L * data.Length) { return false; } diff --git a/Essentials.Tests/EncodingProviderTests.cs b/Essentials.Tests/EncodingProviderTests.cs index 531ef42..71c4352 100644 --- a/Essentials.Tests/EncodingProviderTests.cs +++ b/Essentials.Tests/EncodingProviderTests.cs @@ -9,6 +9,8 @@ namespace ktsu.Essentials.Tests; using System.Threading; using System.Threading.Tasks; using ktsu.Essentials; +using ktsu.Essentials.EncodingProviders.Hex; +using ktsu.Essentials.ObfuscationProviders.Hex; using Microsoft.Extensions.DependencyInjection; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -48,6 +50,23 @@ public void Encoding_OverlappingBuffers_MatchesOutOfPlace(IEncodingProvider enco } } + [TestMethod] + [DynamicData(nameof(EncodingProviders))] + public void GetMaxEncodedLength_Throws_When_The_Bound_Does_Not_Fit_An_Int(IEncodingProvider encoder, string providerName) + => Assert.ThrowsExactly(() => encoder.GetMaxEncodedLength(int.MaxValue), + $"{providerName} must not return a wrapped, negative bound"); + + [TestMethod] + public void Hex_Bound_Is_Exact_Up_To_The_Largest_Encodable_Input() + { + HexEncodingProvider hex = new(); + Assert.AreEqual(int.MaxValue - 1, hex.GetMaxEncodedLength(int.MaxValue / 2)); + Assert.ThrowsExactly(() => hex.GetMaxEncodedLength((int.MaxValue / 2) + 1)); + + HexObfuscationProvider obfuscator = new(); + Assert.ThrowsExactly(() => obfuscator.GetMaxObfuscatedLength(int.MaxValue)); + } + [TestMethod] [DynamicData(nameof(EncodingProviders))] public void Encoding_Roundtrip_Stream(IEncodingProvider encoder, string providerName) From b28316324aca93374f56833a165de3d6736902ee Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 10:39:53 +0000 Subject: [PATCH 04/23] Report System.Text.Json's InvalidOperationException as a failed serialize or deserialize Fixes #161 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- .../JsonSerializationProvider.cs | 10 +++++- .../JsonSerializationOptionsTests.cs | 34 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/Essentials.SerializationProviders.Json/JsonSerializationProvider.cs b/Essentials.SerializationProviders.Json/JsonSerializationProvider.cs index d531988..ea02790 100644 --- a/Essentials.SerializationProviders.Json/JsonSerializationProvider.cs +++ b/Essentials.SerializationProviders.Json/JsonSerializationProvider.cs @@ -77,8 +77,10 @@ public bool TrySerialize(object obj, TextWriter writer) { return false; } - catch (ObjectDisposedException) + catch (InvalidOperationException) { + // System.Text.Json reports a type it cannot map (two members with one JSON name, say) this way, + // and a disposed writer's ObjectDisposedException derives from it. return false; } } @@ -112,5 +114,11 @@ public bool TrySerialize(object obj, TextWriter writer) { return default; } + catch (InvalidOperationException) + { + // A type System.Text.Json cannot map, such as one whose constructor parameters do not bind to + // its properties. + return default; + } } } diff --git a/Essentials.Tests/JsonSerializationOptionsTests.cs b/Essentials.Tests/JsonSerializationOptionsTests.cs index 4798069..b152a4b 100644 --- a/Essentials.Tests/JsonSerializationOptionsTests.cs +++ b/Essentials.Tests/JsonSerializationOptionsTests.cs @@ -86,6 +86,40 @@ public void Registration_With_Options_Is_The_One_Resolved() Assert.AreEqual("{\"displayName\":\"x\",\"theme\":0}", Serialize(contract, new Settings { DisplayName = "x" })); } + [TestMethod] + public void A_Type_With_Colliding_Names_Fails_Without_Throwing() + { + ISerializationProvider provider = new JsonSerializationProvider(); + using StringWriter writer = new(); + + Assert.IsFalse(provider.TrySerialize(new Colliding { A = 1, B = 2 }, writer)); + Assert.AreEqual(string.Empty, writer.ToString(), "nothing reaches the writer when serialization fails"); + Assert.IsNull(provider.Deserialize("{\"x\":1}"u8)); + Assert.ThrowsExactly(() => provider.Serialize(new Colliding())); + } + + [TestMethod] + public void A_Constructor_That_Does_Not_Bind_Fails_Without_Throwing() + { + ISerializationProvider provider = new JsonSerializationProvider(); + Assert.IsNull(provider.Deserialize("{\"Value\":1}"u8)); + } + + public sealed class Colliding + { + [JsonPropertyName("x")] + public int A { get; set; } + + [JsonPropertyName("x")] + public int B { get; set; } + } + + // The only constructor's parameter is not named after a property, so it cannot be bound. + public sealed class Unbindable(int p) + { + public int Value { get; } = p; + } + private static string Serialize(ISerializationProvider provider, object value) { using StringWriter writer = new(); From 8e7aa2f2b7b3c1194f54a3ae84951a0578cb87ca Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 10:39:54 +0000 Subject: [PATCH 05/23] Make the file-backed persistence providers safe under concurrency, cancellation and unreadable records FileSystem and Temp now share one FileRecordStore instead of two copies. - A record that is there but cannot be parsed throws instead of reading as missing, so RetrieveOrCreateAsync never writes a default over it. Fixes #82 - Cancellation part way through surfaces as OperationCanceledException. Fixes #121 - A remove landing between a reader's existence check and its read reports the key missing. Fixes #123 - Deletes retry the sharing violation a reader or replace raises. Fixes #143 - RetrieveOrCreateAsync publishes its new instance only if nothing appeared meanwhile (CreateNew, since a non-overwriting File.Move is check-then-rename on Unix), on every provider including InMemory, ConfigHome and DataHome. Fixes #144 - ClearAsync removes staging files an interrupted write abandoned. Fixes #145 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- CLAUDE.md | 4 +- .../ConfigHomePersistenceProvider.cs | 4 + .../DataHomePersistenceProvider.cs | 4 + ...als.PersistenceProviders.FileSystem.csproj | 2 +- .../FileSystemPersistenceProvider.cs | 173 ++-------- .../InMemoryPersistenceProvider.cs | 33 ++ ...ssentials.PersistenceProviders.Temp.csproj | 2 +- .../TempPersistenceProvider.cs | 181 +++-------- .../PersistenceRobustnessTests.cs | 267 ++++++++++++++++ Shared/AtomicFile.cs | 122 ++++++- Shared/FileRecordStore.cs | 299 ++++++++++++++++++ Shared/PersistenceDefaults.cs | 56 ---- 12 files changed, 791 insertions(+), 356 deletions(-) create mode 100644 Essentials.Tests/PersistenceRobustnessTests.cs create mode 100644 Shared/FileRecordStore.cs delete mode 100644 Shared/PersistenceDefaults.cs diff --git a/CLAUDE.md b/CLAUDE.md index 69ab6e6..665a503 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -49,8 +49,8 @@ This is a .NET library (`ktsu.Essentials`) providing high-performance interfaces - `Shared/SpecialFunctions.cs` - Incomplete gamma, incomplete beta, error function, normal quantile and cancellation-free `ExpM1`/`Log1P`, linked into the Normal, LogNormal, Exponential, Binomial, Poisson and Geometric providers rather than placed in the interfaces-only package - `Shared/DistributionArguments.cs` - Parameter validation shared by every distribution provider, linked in the same way - `Shared/ProviderRegistration.cs` - DI registration for providers whose `Add…Provider` overloads take arguments (Xor, Caesar, BitRotate and every distribution): a call replaces an earlier registration of the same concrete type in place, and `AddEssentials()` keeps a registration made before it, whether as the concrete type, the interface or both, moved to its default's position, rather than overwriting it; a concrete-only one gains an interface registration forwarding to it. Linked into those providers and `Essentials.All`. See issues #132 and #156 -- `Shared/PersistenceDefaults.cs` - The `IPersistenceProvider` default `RetrieveOrCreateAsync` body, linked into the FileSystem and Temp providers, which declare that member themselves and so hide the default -- `Shared/AtomicFile.cs` - The FileSystem and Temp providers' write and read paths: each store stages its data in a uniquely named `.tmp` beside the target and publishes it with an overwriting move, retrying briefly while Windows holds the target open, so concurrent writers can't collide and a reader never sees the key missing; reads retry only the Windows sharing violation a replace in progress raises. Linked into both providers like `PersistenceDefaults.cs` +- `Shared/FileRecordStore.cs` - Everything the FileSystem and Temp persistence providers do, which differ only in where their directory is; linked into both, and ConfigHome and DataHome delegate to FileSystem. Cancellation passes through as itself, every other failure is a `PersistenceProviderException`. A record that is there but deserializes to null throws rather than reading as missing, so `RetrieveOrCreateAsync` never writes a default over a file it could not parse (#82). `RetrieveOrCreateAsync` publishes its new instance only if nothing has appeared meanwhile (#144); `ClearAsync` also removes staging files older than a minute, which an interrupted write left behind (#145) +- `Shared/AtomicFile.cs` - The file operations under it: each store stages its data in a uniquely named `.tmp` beside the target and publishes it with an overwriting move, so concurrent writers can't collide and a reader never sees the key missing; reads and deletes retry the sharing violation a reader or a replace in progress raises (#143). `TryCreateAsync` opens the target with `FileMode.CreateNew` rather than staging and moving, because a non-overwriting `File.Move` is a check then a rename on Unix and loses a record published in between. Off Windows, .NET emulates `FileShare.None` with an advisory lock whose refusal carries a raw `EWOULDBLOCK` (11 on Linux, 35 on macOS) as its `HResult`, which counts as a sharing violation there - `Essentials/ISerializationProvider.cs` - Object serialization/deserialization interface - `Shared/Utf8Bom.cs` - Strips a leading UTF-8 byte order mark, linked into the four serialization providers so each byte `Deserialize` overload reads `File.ReadAllBytes` output from Notepad or Visual Studio the same way its text overloads do - `Essentials/ISerializationOptions.cs` - Configurable serialization options (naming, inclusion, boxing policies) diff --git a/Essentials.PersistenceProviders.ConfigHome/ConfigHomePersistenceProvider.cs b/Essentials.PersistenceProviders.ConfigHome/ConfigHomePersistenceProvider.cs index 5ad77f1..f10744a 100644 --- a/Essentials.PersistenceProviders.ConfigHome/ConfigHomePersistenceProvider.cs +++ b/Essentials.PersistenceProviders.ConfigHome/ConfigHomePersistenceProvider.cs @@ -54,6 +54,10 @@ public Task StoreAsync(TKey key, T obj, CancellationToken cancellationToken = public Task RetrieveAsync(TKey key, CancellationToken cancellationToken = default) => _inner.RetrieveAsync(key, cancellationToken); + /// + public Task RetrieveOrCreateAsync(TKey key, CancellationToken cancellationToken = default) where T : new() + => _inner.RetrieveOrCreateAsync(key, cancellationToken); + /// public Task ExistsAsync(TKey key, CancellationToken cancellationToken = default) => _inner.ExistsAsync(key, cancellationToken); diff --git a/Essentials.PersistenceProviders.DataHome/DataHomePersistenceProvider.cs b/Essentials.PersistenceProviders.DataHome/DataHomePersistenceProvider.cs index 2c92a5a..fc96671 100644 --- a/Essentials.PersistenceProviders.DataHome/DataHomePersistenceProvider.cs +++ b/Essentials.PersistenceProviders.DataHome/DataHomePersistenceProvider.cs @@ -54,6 +54,10 @@ public Task StoreAsync(TKey key, T obj, CancellationToken cancellationToken = public Task RetrieveAsync(TKey key, CancellationToken cancellationToken = default) => _inner.RetrieveAsync(key, cancellationToken); + /// + public Task RetrieveOrCreateAsync(TKey key, CancellationToken cancellationToken = default) where T : new() + => _inner.RetrieveOrCreateAsync(key, cancellationToken); + /// public Task ExistsAsync(TKey key, CancellationToken cancellationToken = default) => _inner.ExistsAsync(key, cancellationToken); diff --git a/Essentials.PersistenceProviders.FileSystem/Essentials.PersistenceProviders.FileSystem.csproj b/Essentials.PersistenceProviders.FileSystem/Essentials.PersistenceProviders.FileSystem.csproj index 4e5bf75..ac9dde5 100644 --- a/Essentials.PersistenceProviders.FileSystem/Essentials.PersistenceProviders.FileSystem.csproj +++ b/Essentials.PersistenceProviders.FileSystem/Essentials.PersistenceProviders.FileSystem.csproj @@ -15,7 +15,7 @@ - + diff --git a/Essentials.PersistenceProviders.FileSystem/FileSystemPersistenceProvider.cs b/Essentials.PersistenceProviders.FileSystem/FileSystemPersistenceProvider.cs index a3b5b18..f8fcbe6 100644 --- a/Essentials.PersistenceProviders.FileSystem/FileSystemPersistenceProvider.cs +++ b/Essentials.PersistenceProviders.FileSystem/FileSystemPersistenceProvider.cs @@ -5,8 +5,6 @@ namespace ktsu.Essentials.PersistenceProviders.FileSystem; using ktsu.Essentials; using System; using System.Collections.Generic; -using System.IO; -using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -27,13 +25,16 @@ public sealed class FileSystemPersistenceProvider( string baseDirectory) : IPersistenceProvider where TKey : notnull { - private readonly IFileSystemProvider _fileSystemProvider = Ensure.NotNull(fileSystemProvider); - private readonly ISerializationProvider _serializationProvider = Ensure.NotNull(serializationProvider); + private readonly FileRecordStore _records = new( + Ensure.NotNull(fileSystemProvider), + Ensure.NotNull(serializationProvider), + Ensure.NotNull(baseDirectory), + "file system"); /// /// Gets the directory this provider stores objects in. /// - public string BaseDirectory { get; } = Ensure.NotNull(baseDirectory); + public string BaseDirectory { get; } = baseDirectory; /// public string ProviderName => "FileSystem"; @@ -42,75 +43,37 @@ public sealed class FileSystemPersistenceProvider( public bool IsPersistent => true; /// - public async Task StoreAsync(TKey key, T obj, CancellationToken cancellationToken = default) + public Task StoreAsync(TKey key, T obj, CancellationToken cancellationToken = default) { #pragma warning disable KTSU0003 // Ensure.NotNull requires class constraint, but TKey is notnull ArgumentNullException.ThrowIfNull(key); #pragma warning restore KTSU0003 - - if (obj is null) - { - await RemoveAsync(key, cancellationToken).ConfigureAwait(false); - return; - } - - cancellationToken.ThrowIfCancellationRequested(); - - try - { - string filePath = GetFilePath(key); - string serializedData = await _serializationProvider.SerializeAsync(obj, cancellationToken).ConfigureAwait(false); - - // Ensure directory exists - string? directory = _fileSystemProvider.Path.GetDirectoryName(filePath); - if (!string.IsNullOrEmpty(directory)) - { - _fileSystemProvider.Directory.CreateDirectory(directory); - } - - await AtomicFile.WriteAllTextAsync(_fileSystemProvider, filePath, serializedData, cancellationToken).ConfigureAwait(false); - } - catch (Exception ex) - { - throw new PersistenceProviderException($"Failed to store object with key '{key}' to file system", ex); - } + return _records.StoreAsync(key, obj, cancellationToken); } /// - public async Task RetrieveAsync(TKey key, CancellationToken cancellationToken = default) + /// The record is there but cannot be read as , or reading it failed. + public Task RetrieveAsync(TKey key, CancellationToken cancellationToken = default) { #pragma warning disable KTSU0003 // Ensure.NotNull requires class constraint, but TKey is notnull ArgumentNullException.ThrowIfNull(key); #pragma warning restore KTSU0003 - cancellationToken.ThrowIfCancellationRequested(); - - try - { - string filePath = GetFilePath(key); - - if (!_fileSystemProvider.File.Exists(filePath)) - { - return default; - } - - string serializedData = await AtomicFile.ReadAllTextAsync(_fileSystemProvider, filePath, cancellationToken).ConfigureAwait(false); - - if (string.IsNullOrEmpty(serializedData)) - { - return default; - } - - return await _serializationProvider.DeserializeAsync(serializedData, cancellationToken).ConfigureAwait(false); - } - catch (Exception ex) - { - throw new PersistenceProviderException($"Failed to retrieve object with key '{key}' from file system", ex); - } + return _records.RetrieveAsync(key, cancellationToken); } /// + /// + /// The new instance is stored only if no record has appeared under the key in the meantime; if one has, it is + /// returned instead. A record that is there but cannot be read is left untouched and throws. + /// + /// The record is there but cannot be read as , or reading or storing it failed. public Task RetrieveOrCreateAsync(TKey key, CancellationToken cancellationToken = default) where T : new() - => PersistenceDefaults.RetrieveOrCreateAsync(this, key, cancellationToken); + { +#pragma warning disable KTSU0003 // Ensure.NotNull requires class constraint, but TKey is notnull + ArgumentNullException.ThrowIfNull(key); +#pragma warning restore KTSU0003 + return _records.RetrieveOrCreateAsync(key, cancellationToken); + } /// public Task ExistsAsync(TKey key, CancellationToken cancellationToken = default) @@ -119,10 +82,7 @@ public Task ExistsAsync(TKey key, CancellationToken cancellationToken = de ArgumentNullException.ThrowIfNull(key); #pragma warning restore KTSU0003 cancellationToken.ThrowIfCancellationRequested(); - - string filePath = GetFilePath(key); - bool exists = _fileSystemProvider.File.Exists(filePath); - return Task.FromResult(exists); + return Task.FromResult(_records.Exists(key)); } /// @@ -131,94 +91,21 @@ public Task RemoveAsync(TKey key, CancellationToken cancellationToken = de #pragma warning disable KTSU0003 // Ensure.NotNull requires class constraint, but TKey is notnull ArgumentNullException.ThrowIfNull(key); #pragma warning restore KTSU0003 - cancellationToken.ThrowIfCancellationRequested(); - - try - { - string filePath = GetFilePath(key); - - if (!_fileSystemProvider.File.Exists(filePath)) - { - return Task.FromResult(false); - } - - _fileSystemProvider.File.Delete(filePath); - return Task.FromResult(true); - } - catch (Exception ex) - { - throw new PersistenceProviderException($"Failed to remove object with key '{key}' from file system", ex); - } + return _records.RemoveAsync(key, cancellationToken); } /// public Task> GetAllKeysAsync(CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); - - try - { - if (!_fileSystemProvider.Directory.Exists(BaseDirectory)) - { - return Task.FromResult(Enumerable.Empty()); - } - - string[] files = _fileSystemProvider.Directory.GetFiles(BaseDirectory, "*" + _serializationProvider.FileExtension, SearchOption.TopDirectoryOnly); - List keys = []; - foreach (string file in files) - { - string? name = _fileSystemProvider.Path.GetFileNameWithoutExtension(file); - if (string.IsNullOrEmpty(name)) - { - continue; - } - - // Names are percent-encoded, so the original key is recovered exactly. Truncated - // names cannot be decoded and are skipped rather than reported as a wrong key. - string? decoded = PersistenceProviderUtilities.GetKeyFromFileName(name!); - if (decoded is not null && PersistenceProviderUtilities.TryConvertToKey(decoded, out TKey key)) - { - keys.Add(key); - } - } - - return Task.FromResult>(keys); - } - catch (Exception ex) - { - throw new PersistenceProviderException("Failed to retrieve all keys from file system", ex); - } + return Task.FromResult(_records.GetAllKeys()); } /// - public Task ClearAsync(CancellationToken cancellationToken = default) - { - cancellationToken.ThrowIfCancellationRequested(); - - try - { - if (!_fileSystemProvider.Directory.Exists(BaseDirectory)) - { - return Task.CompletedTask; - } - - string[] files = _fileSystemProvider.Directory.GetFiles(BaseDirectory, "*" + _serializationProvider.FileExtension, SearchOption.TopDirectoryOnly); - foreach (string file in files) - { - _fileSystemProvider.File.Delete(file); - } + /// + /// Also removes staging files that writes interrupted by a crash or power loss left behind, each of which + /// holds a whole record. Staging files younger than a minute belong to writes still in progress and are kept. + /// + public Task ClearAsync(CancellationToken cancellationToken = default) => _records.ClearAsync(cancellationToken); - return Task.CompletedTask; - } - catch (Exception ex) - { - throw new PersistenceProviderException("Failed to clear all objects from file system", ex); - } - } - - private string GetFilePath(TKey key) - { - string fileName = PersistenceProviderUtilities.GetSafeFileName(PersistenceProviderUtilities.FormatKey(key)) + _serializationProvider.FileExtension; - return _fileSystemProvider.Path.Combine(BaseDirectory, fileName); - } } diff --git a/Essentials.PersistenceProviders.InMemory/InMemoryPersistenceProvider.cs b/Essentials.PersistenceProviders.InMemory/InMemoryPersistenceProvider.cs index b163e91..12e9f0f 100644 --- a/Essentials.PersistenceProviders.InMemory/InMemoryPersistenceProvider.cs +++ b/Essentials.PersistenceProviders.InMemory/InMemoryPersistenceProvider.cs @@ -74,6 +74,39 @@ public Task StoreAsync(TKey key, T obj, CancellationToken cancellationToken = return Task.FromResult(default); } + /// + /// Retrieves an object using the specified key, or creates and stores a new instance if not found. + /// + /// + /// Creation is atomic: callers racing on one missing key, or racing a , all get + /// the instance left stored, and a value stored in the meantime is never replaced by a new one. + /// + /// The type of object to retrieve or create. + /// The unique key that identifies the stored object. + /// A cancellation token to cancel the operation. + /// The stored object, or the new instance now stored under . + public Task RetrieveOrCreateAsync(TKey key, CancellationToken cancellationToken = default) where T : new() + { + cancellationToken.ThrowIfCancellationRequested(); + + while (true) + { + object? stored = store.GetOrAdd(key, static _ => new T()); + if (stored is T typed) + { + return Task.FromResult(typed); + } + + // Something of another type is stored, which RetrieveAsync reports as missing, so it is replaced as a + // missing key would be created; but only if it is still what was seen, so a concurrent store wins. + T created = new(); + if (store.TryUpdate(key, created, stored)) + { + return Task.FromResult(created); + } + } + } + /// /// Checks whether an object with the specified key exists in storage. /// diff --git a/Essentials.PersistenceProviders.Temp/Essentials.PersistenceProviders.Temp.csproj b/Essentials.PersistenceProviders.Temp/Essentials.PersistenceProviders.Temp.csproj index 4e5bf75..ac9dde5 100644 --- a/Essentials.PersistenceProviders.Temp/Essentials.PersistenceProviders.Temp.csproj +++ b/Essentials.PersistenceProviders.Temp/Essentials.PersistenceProviders.Temp.csproj @@ -15,7 +15,7 @@ - + diff --git a/Essentials.PersistenceProviders.Temp/TempPersistenceProvider.cs b/Essentials.PersistenceProviders.Temp/TempPersistenceProvider.cs index d668b15..9a1cbd0 100644 --- a/Essentials.PersistenceProviders.Temp/TempPersistenceProvider.cs +++ b/Essentials.PersistenceProviders.Temp/TempPersistenceProvider.cs @@ -6,8 +6,6 @@ namespace ktsu.Essentials.PersistenceProviders.Temp; using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; -using System.IO; -using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -29,8 +27,16 @@ public sealed class TempPersistenceProvider( where TKey : notnull { private readonly IFileSystemProvider _fileSystemProvider = Ensure.NotNull(fileSystemProvider); - private readonly ISerializationProvider _serializationProvider = Ensure.NotNull(serializationProvider); - private readonly string _tempDirectory = CreateTempDirectory(fileSystemProvider, applicationName); + private readonly FileRecordStore _records = new( + Ensure.NotNull(fileSystemProvider), + Ensure.NotNull(serializationProvider), + CreateTempDirectory(fileSystemProvider, applicationName), + "temp directory"); + + /// + /// Gets the directory this provider stores objects in: a fresh subdirectory of the system temporary directory. + /// + public string BaseDirectory => _records.Directory; /// public string ProviderName => "Temp"; @@ -39,75 +45,37 @@ public sealed class TempPersistenceProvider( public bool IsPersistent => false; // Temp files can be cleaned up by the system /// - public async Task StoreAsync(TKey key, T obj, CancellationToken cancellationToken = default) + public Task StoreAsync(TKey key, T obj, CancellationToken cancellationToken = default) { #pragma warning disable KTSU0003 // Ensure.NotNull requires class constraint, but TKey is notnull ArgumentNullException.ThrowIfNull(key); #pragma warning restore KTSU0003 - - if (obj is null) - { - await RemoveAsync(key, cancellationToken).ConfigureAwait(false); - return; - } - - cancellationToken.ThrowIfCancellationRequested(); - - try - { - string filePath = GetFilePath(key); - string serializedData = await _serializationProvider.SerializeAsync(obj, cancellationToken).ConfigureAwait(false); - - // Ensure directory exists - string? directory = _fileSystemProvider.Path.GetDirectoryName(filePath); - if (!string.IsNullOrEmpty(directory)) - { - _fileSystemProvider.Directory.CreateDirectory(directory); - } - - await AtomicFile.WriteAllTextAsync(_fileSystemProvider, filePath, serializedData, cancellationToken).ConfigureAwait(false); - } - catch (Exception ex) - { - throw new PersistenceProviderException($"Failed to store object with key '{key}' to temp directory", ex); - } + return _records.StoreAsync(key, obj, cancellationToken); } /// - public async Task RetrieveAsync(TKey key, CancellationToken cancellationToken = default) + /// The record is there but cannot be read as , or reading it failed. + public Task RetrieveAsync(TKey key, CancellationToken cancellationToken = default) { #pragma warning disable KTSU0003 // Ensure.NotNull requires class constraint, but TKey is notnull ArgumentNullException.ThrowIfNull(key); #pragma warning restore KTSU0003 - cancellationToken.ThrowIfCancellationRequested(); - - try - { - string filePath = GetFilePath(key); - - if (!_fileSystemProvider.File.Exists(filePath)) - { - return default; - } - - string serializedData = await AtomicFile.ReadAllTextAsync(_fileSystemProvider, filePath, cancellationToken).ConfigureAwait(false); - - if (string.IsNullOrEmpty(serializedData)) - { - return default; - } - - return await _serializationProvider.DeserializeAsync(serializedData, cancellationToken).ConfigureAwait(false); - } - catch (Exception ex) - { - throw new PersistenceProviderException($"Failed to retrieve object with key '{key}' from temp directory", ex); - } + return _records.RetrieveAsync(key, cancellationToken); } /// + /// + /// The new instance is stored only if no record has appeared under the key in the meantime; if one has, it is + /// returned instead. A record that is there but cannot be read is left untouched and throws. + /// + /// The record is there but cannot be read as , or reading or storing it failed. public Task RetrieveOrCreateAsync(TKey key, CancellationToken cancellationToken = default) where T : new() - => PersistenceDefaults.RetrieveOrCreateAsync(this, key, cancellationToken); + { +#pragma warning disable KTSU0003 // Ensure.NotNull requires class constraint, but TKey is notnull + ArgumentNullException.ThrowIfNull(key); +#pragma warning restore KTSU0003 + return _records.RetrieveOrCreateAsync(key, cancellationToken); + } /// public Task ExistsAsync(TKey key, CancellationToken cancellationToken = default) @@ -116,10 +84,7 @@ public Task ExistsAsync(TKey key, CancellationToken cancellationToken = de ArgumentNullException.ThrowIfNull(key); #pragma warning restore KTSU0003 cancellationToken.ThrowIfCancellationRequested(); - - string filePath = GetFilePath(key); - bool exists = _fileSystemProvider.File.Exists(filePath); - return Task.FromResult(exists); + return Task.FromResult(_records.Exists(key)); } /// @@ -128,90 +93,22 @@ public Task RemoveAsync(TKey key, CancellationToken cancellationToken = de #pragma warning disable KTSU0003 // Ensure.NotNull requires class constraint, but TKey is notnull ArgumentNullException.ThrowIfNull(key); #pragma warning restore KTSU0003 - cancellationToken.ThrowIfCancellationRequested(); - - try - { - string filePath = GetFilePath(key); - - if (!_fileSystemProvider.File.Exists(filePath)) - { - return Task.FromResult(false); - } - - _fileSystemProvider.File.Delete(filePath); - return Task.FromResult(true); - } - catch (Exception ex) - { - throw new PersistenceProviderException($"Failed to remove object with key '{key}' from temp directory", ex); - } + return _records.RemoveAsync(key, cancellationToken); } /// public Task> GetAllKeysAsync(CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); - - try - { - if (!_fileSystemProvider.Directory.Exists(_tempDirectory)) - { - return Task.FromResult(Enumerable.Empty()); - } - - string[] files = _fileSystemProvider.Directory.GetFiles(_tempDirectory, "*" + _serializationProvider.FileExtension, SearchOption.TopDirectoryOnly); - List keys = []; - foreach (string file in files) - { - string? name = _fileSystemProvider.Path.GetFileNameWithoutExtension(file); - if (string.IsNullOrEmpty(name)) - { - continue; - } - - // Names are percent-encoded, so the original key is recovered exactly. Truncated - // names cannot be decoded and are skipped rather than reported as a wrong key. - string? decoded = PersistenceProviderUtilities.GetKeyFromFileName(name!); - if (decoded is not null && PersistenceProviderUtilities.TryConvertToKey(decoded, out TKey key)) - { - keys.Add(key); - } - } - - return Task.FromResult>(keys); - } - catch (Exception ex) - { - throw new PersistenceProviderException("Failed to retrieve all keys from temp directory", ex); - } + return Task.FromResult(_records.GetAllKeys()); } /// - public Task ClearAsync(CancellationToken cancellationToken = default) - { - cancellationToken.ThrowIfCancellationRequested(); - - try - { - if (!_fileSystemProvider.Directory.Exists(_tempDirectory)) - { - return Task.CompletedTask; - } - - string[] files = _fileSystemProvider.Directory.GetFiles(_tempDirectory, "*" + _serializationProvider.FileExtension, SearchOption.TopDirectoryOnly); - foreach (string file in files) - { - _fileSystemProvider.File.Delete(file); - } - - return Task.CompletedTask; - } - catch (Exception ex) - { - throw new PersistenceProviderException("Failed to clear all objects from temp directory", ex); - } - } + /// + /// Also removes staging files that writes interrupted by a crash or power loss left behind, each of which + /// holds a whole record. Staging files younger than a minute belong to writes still in progress and are kept. + /// + public Task ClearAsync(CancellationToken cancellationToken = default) => _records.ClearAsync(cancellationToken); /// /// Releases resources. Does not automatically clean up the temporary directory. @@ -228,9 +125,9 @@ public void CleanupDirectory() { try { - if (_fileSystemProvider.Directory.Exists(_tempDirectory)) + if (_fileSystemProvider.Directory.Exists(_records.Directory)) { - _fileSystemProvider.Directory.Delete(_tempDirectory, recursive: true); + _fileSystemProvider.Directory.Delete(_records.Directory, recursive: true); } } catch (Exception) @@ -240,12 +137,6 @@ public void CleanupDirectory() } } - private string GetFilePath(TKey key) - { - string fileName = PersistenceProviderUtilities.GetSafeFileName(PersistenceProviderUtilities.FormatKey(key)) + _serializationProvider.FileExtension; - return _fileSystemProvider.Path.Combine(_tempDirectory, fileName); - } - private static string CreateTempDirectory(IFileSystemProvider fileSystemProvider, string? applicationName) { string tempPath = fileSystemProvider.Path.GetTempPath(); diff --git a/Essentials.Tests/PersistenceRobustnessTests.cs b/Essentials.Tests/PersistenceRobustnessTests.cs new file mode 100644 index 0000000..b24378c --- /dev/null +++ b/Essentials.Tests/PersistenceRobustnessTests.cs @@ -0,0 +1,267 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Essentials.Tests; + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using ktsu.Essentials; +using ktsu.Essentials.FileSystemProviders.Native; +using ktsu.Essentials.PersistenceProviders.FileSystem; +using ktsu.Essentials.PersistenceProviders.InMemory; +using ktsu.Essentials.PersistenceProviders.Temp; +using ktsu.Essentials.SerializationProviders.Json; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// The file-backed providers under the conditions a real application meets: a record it cannot parse, a call +/// cancelled part way, and other tasks reading, writing and removing the same keys. +/// +[TestClass] +public sealed class PersistenceRobustnessTests +{ + private readonly List directories = []; + private readonly List> temps = []; + + public TestContext TestContext { get; set; } = null!; + + [TestCleanup] + public void Cleanup() + { + foreach (TempPersistenceProvider temp in temps) + { + temp.CleanupDirectory(); + temp.Dispose(); + } + + foreach (string directory in directories.Where(Directory.Exists)) + { + Directory.Delete(directory, recursive: true); + } + } + + [TestMethod] + public async Task A_Record_That_Cannot_Be_Parsed_Throws_And_Is_Never_Overwritten() + { + // The serializer reports a parse failure as null, which RetrieveOrCreateAsync took for a missing key and + // replaced with new T(), destroying a hand-edited config over a trailing comma (issue #82). + foreach ((IPersistenceProvider persistence, string directory) in FileBackedProviders(new JsonSerializationProvider())) + { + string name = persistence.ProviderName; + string path = Path.Join(directory, "settings.json"); + const string malformed = "{\"Name\":\"prod\",}"; + await File.WriteAllTextAsync(path, malformed, TestContext.CancellationToken).ConfigureAwait(false); + + await Assert.ThrowsExactlyAsync( + () => persistence.RetrieveOrCreateAsync("settings", TestContext.CancellationToken), + $"{name} took an unreadable record for a missing one").ConfigureAwait(false); + await Assert.ThrowsExactlyAsync( + () => persistence.RetrieveAsync("settings", TestContext.CancellationToken), + $"{name} reported an unreadable record as missing").ConfigureAwait(false); + + Assert.AreEqual(malformed, await File.ReadAllTextAsync(path, TestContext.CancellationToken).ConfigureAwait(false), $"{name} changed the unreadable record"); + } + } + + [TestMethod] + public async Task A_Missing_Record_Is_Still_Created() + { + foreach ((IPersistenceProvider persistence, string _) in FileBackedProviders(new JsonSerializationProvider())) + { + Settings created = await persistence.RetrieveOrCreateAsync("fresh", TestContext.CancellationToken).ConfigureAwait(false); + Assert.AreEqual("default", created.Name); + + Settings? stored = await persistence.RetrieveAsync("fresh", TestContext.CancellationToken).ConfigureAwait(false); + Assert.IsNotNull(stored, $"{persistence.ProviderName} did not store the new instance"); + } + } + + [TestMethod] + public async Task Cancellation_Part_Way_Through_Surfaces_As_Cancellation() + { + // Cancellation that arrived during an operation was wrapped in a PersistenceProviderException, so + // callers' cancellation handling saw a storage failure (issue #121). + CancellingSerializer serializer = new(); + + foreach ((IPersistenceProvider persistence, string _) in FileBackedProviders(serializer)) + { + string name = persistence.ProviderName; + + using (CancellationTokenSource cancellation = serializer.CancelOnSerialize(new())) + { + await Assert.ThrowsAsync( + () => persistence.StoreAsync("k", new Settings(), cancellation.Token), + $"{name} wrapped a cancelled store").ConfigureAwait(false); + } + + serializer.Stop(); + await persistence.StoreAsync("k", new Settings(), TestContext.CancellationToken).ConfigureAwait(false); + + using (CancellationTokenSource cancellation = serializer.CancelOnDeserialize(new())) + { + await Assert.ThrowsAsync( + () => persistence.RetrieveAsync("k", cancellation.Token), + $"{name} wrapped a cancelled retrieve").ConfigureAwait(false); + } + + serializer.Stop(); + } + } + + [TestMethod] + public async Task Readers_Writers_And_Removers_On_One_Key_Never_Throw() + { + // A remove landing between a reader's existence check and its read made the read throw instead of + // reporting the key missing (issue #123), and on Windows a delete under a reader or a replace failed + // at once (issue #143). + foreach ((IPersistenceProvider persistence, string _) in FileBackedProviders(new JsonSerializationProvider())) + { + const int rounds = 200; + Task[] work = + [ + Repeat(rounds, i => persistence.StoreAsync("k", new Settings { Name = $"writer {i}" }, TestContext.CancellationToken)), + Repeat(rounds, _ => persistence.RemoveAsync("k", TestContext.CancellationToken)), + Repeat(rounds, _ => persistence.RetrieveAsync("k", TestContext.CancellationToken)), + Repeat(rounds, _ => persistence.RetrieveOrCreateAsync("k", TestContext.CancellationToken)), + ]; + + await Task.WhenAll(work).ConfigureAwait(false); + await persistence.ClearAsync(TestContext.CancellationToken).ConfigureAwait(false); + Assert.IsFalse(await persistence.ExistsAsync("k", TestContext.CancellationToken).ConfigureAwait(false), $"{persistence.ProviderName} did not clear"); + } + } + + [TestMethod] + public async Task RetrieveOrCreate_Never_Replaces_A_Record_Stored_Concurrently() + { + // The create step stored new T() unconditionally, so a record stored between its look and its store was + // replaced by defaults (issue #144). + List> providers = [.. FileBackedProviders(new JsonSerializationProvider()).Select(p => p.Persistence)]; + providers.Add(new InMemoryPersistenceProvider()); + + foreach (IPersistenceProvider persistence in providers) + { + for (int round = 0; round < 100; round++) + { + await persistence.RemoveAsync("cfg", TestContext.CancellationToken).ConfigureAwait(false); + + Task bootstrap = Task.Run(() => persistence.RetrieveOrCreateAsync("cfg", TestContext.CancellationToken), TestContext.CancellationToken); + Task import = Task.Run(() => persistence.StoreAsync("cfg", new Settings { Name = "imported" }, TestContext.CancellationToken), TestContext.CancellationToken); + await Task.WhenAll(bootstrap, import).ConfigureAwait(false); + + Settings? stored = await persistence.RetrieveAsync("cfg", TestContext.CancellationToken).ConfigureAwait(false); + Assert.AreEqual("imported", stored?.Name, $"{persistence.ProviderName} replaced a concurrent store with a default in round {round}"); + } + } + } + + [TestMethod] + public async Task InMemory_RetrieveOrCreate_Gives_Every_Racing_Caller_The_Stored_Instance() + { + InMemoryPersistenceProvider persistence = new(); + Task[] callers = [.. Enumerable.Range(0, 32).Select(_ => Task.Run( + () => persistence.RetrieveOrCreateAsync("shared", TestContext.CancellationToken), + TestContext.CancellationToken))]; + Settings[] results = await Task.WhenAll(callers).ConfigureAwait(false); + + Settings? stored = await persistence.RetrieveAsync("shared", TestContext.CancellationToken).ConfigureAwait(false); + Assert.IsTrue(results.All(r => ReferenceEquals(r, stored)), "a caller got an instance that is not the stored one"); + } + + [TestMethod] + public async Task Clear_Removes_Abandoned_Staging_Files_And_Nothing_Else() + { + // A write interrupted by a crash left its ".tmp", holding a whole record, and ClearAsync left it + // there (issue #145). + foreach ((IPersistenceProvider persistence, string directory) in FileBackedProviders(new JsonSerializationProvider())) + { + string name = persistence.ProviderName; + await persistence.StoreAsync("k", new Settings(), TestContext.CancellationToken).ConfigureAwait(false); + + string abandoned = Path.Join(directory, $"{Guid.NewGuid():N}.tmp"); + string inFlight = Path.Join(directory, $"{Guid.NewGuid():N}.tmp"); + string unrelated = Path.Join(directory, "notes.tmp"); + foreach (string path in new[] { abandoned, inFlight, unrelated }) + { + await File.WriteAllTextAsync(path, "{\"Name\":\"secret\"}", TestContext.CancellationToken).ConfigureAwait(false); + } + + File.SetLastWriteTimeUtc(abandoned, DateTime.UtcNow.AddHours(-1)); + File.SetLastWriteTimeUtc(unrelated, DateTime.UtcNow.AddHours(-1)); + + await persistence.ClearAsync(TestContext.CancellationToken).ConfigureAwait(false); + + Assert.IsFalse(File.Exists(abandoned), $"{name} kept an abandoned staging file"); + Assert.IsTrue(File.Exists(inFlight), $"{name} deleted a staging file a write may still be using"); + Assert.IsTrue(File.Exists(unrelated), $"{name} deleted a file it did not write"); + Assert.IsFalse(await persistence.ExistsAsync("k", TestContext.CancellationToken).ConfigureAwait(false)); + } + } + + private static Task Repeat(int count, Func body) => Task.Run(async () => + { + for (int i = 0; i < count; i++) + { + await body(i).ConfigureAwait(false); + } + }); + + private IEnumerable<(IPersistenceProvider Persistence, string Directory)> FileBackedProviders(ISerializationProvider serializer) + { + NativeFileSystemProvider fs = new(); + + string fileSystemDir = Path.Join(Path.GetTempPath(), "PersistenceRobustness_" + Guid.NewGuid().ToString("N")[..8]); + directories.Add(fileSystemDir); + Directory.CreateDirectory(fileSystemDir); + yield return (new FileSystemPersistenceProvider(fs, serializer, fileSystemDir), fileSystemDir); + + TempPersistenceProvider temp = new(fs, serializer, "PersistenceRobustness_" + Guid.NewGuid().ToString("N")[..8]); + temps.Add(temp); + yield return (temp, temp.BaseDirectory); + } + + public sealed class Settings + { + public string Name { get; set; } = "default"; + } + + /// A serializer whose work is where the caller's token fires, as a slow one's would. + private sealed class CancellingSerializer : ISerializationProvider + { + private readonly JsonSerializationProvider inner = new(); + private CancellationTokenSource? onSerialize; + private CancellationTokenSource? onDeserialize; + + public string FileExtension => inner.FileExtension; + + public CancellationTokenSource CancelOnSerialize(CancellationTokenSource source) => onSerialize = source; + + public CancellationTokenSource CancelOnDeserialize(CancellationTokenSource source) => onDeserialize = source; + + public void Stop() => onSerialize = onDeserialize = null; + + public bool TrySerialize(object obj, TextWriter writer) + { + Fire(onSerialize); + return inner.TrySerialize(obj, writer); + } + + public T? Deserialize(ReadOnlySpan data) + { + Fire(onDeserialize); + return inner.Deserialize(data); + } + + private static void Fire(CancellationTokenSource? source) + { + if (source is not null) + { + source.Cancel(); + source.Token.ThrowIfCancellationRequested(); + } + } + } +} diff --git a/Shared/AtomicFile.cs b/Shared/AtomicFile.cs index 848c795..42519f2 100644 --- a/Shared/AtomicFile.cs +++ b/Shared/AtomicFile.cs @@ -5,6 +5,8 @@ namespace ktsu.Essentials; using System; using System.IO; using System.IO.Abstractions; +using System.Runtime.InteropServices; +using System.Text; using System.Threading; using System.Threading.Tasks; @@ -20,12 +22,25 @@ namespace ktsu.Essentials; /// internal static class AtomicFile { + /// The extension of a staging file. + internal const string StagingExtension = ".tmp"; + private const int MaxAttempts = 50; + // What File.WriteAllText writes, so a record created here reads back exactly as a replaced one does. + private static readonly UTF8Encoding Utf8NoBom = new(encoderShouldEmitUTF8Identifier: false); + // Win32 ERROR_SHARING_VIOLATION and ERROR_LOCK_VIOLATION, carried in the low word of IOException.HResult. private const int SharingViolation = 32; private const int LockViolation = 33; + // Off Windows, .NET emulates FileShare.None with an advisory lock, and a second open of a file held that way + // fails with EWOULDBLOCK, carried as the raw errno: 11 on Linux, 35 on macOS. + private const int UnixWouldBlockLinux = 11; + private const int UnixWouldBlockMac = 35; + + private static readonly bool IsWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); + /// /// Writes to , replacing any existing file atomically. /// @@ -36,13 +51,7 @@ internal static class AtomicFile /// A task that completes once the new content is in place. internal static async Task WriteAllTextAsync(IFileSystem fileSystem, string filePath, string content, CancellationToken cancellationToken) { - // Same directory as the target, so the move is a rename on one volume. The name is not derived from - // the target's, because PersistenceProviderUtilities budgets a key's filename for only the extension - // and ".tmp", and a longer suffix on a long key would exceed the 255-byte name limit. The .tmp - // extension keeps the staging file out of the providers' "*" key listings. - string? directory = fileSystem.Path.GetDirectoryName(filePath); - string tempFileName = $"{Guid.NewGuid():N}.tmp"; - string tempFilePath = string.IsNullOrEmpty(directory) ? tempFileName : fileSystem.Path.Combine(directory, tempFileName); + string tempFilePath = StagingPathFor(fileSystem, filePath); try { @@ -102,6 +111,100 @@ internal static async Task ReadAllTextAsync(IFileSystem fileSystem, stri return await fileSystem.File.ReadAllTextAsync(filePath, cancellationToken).ConfigureAwait(false); } + /// + /// Writes to only if no file is there yet, so a record + /// another writer published first is never replaced. + /// + /// The filesystem to write through. + /// The file to create. + /// The text to write. + /// A token to cancel the operation. + /// if this call created the file; if one was already there. + internal static async Task TryCreateAsync(IFileSystem fileSystem, string filePath, string content, CancellationToken cancellationToken) + { + // Not staged and moved like a replace: a move that does not overwrite is a check followed by a rename on + // Unix, so a writer publishing in between is silently replaced. Opening with CreateNew is the one create + // that is atomic on every platform. Readers can see the file empty until the content is written, which + // FileRecordStore waits out; on Windows the share mode makes them wait here instead. + Stream stream; + try + { + stream = fileSystem.File.Open(filePath, FileMode.CreateNew, FileAccess.Write, FileShare.None); + } + catch (IOException ex) when (ex.GetType() == typeof(IOException)) + { + // The file already exists, or did until a remover took it just now; either way this call did not + // create it, and the caller looks again. A missing directory and the like are subclasses and surface. + return false; + } + + byte[] bytes = Utf8NoBom.GetBytes(content); + await using (stream.ConfigureAwait(false)) + { + await stream.WriteAsync(bytes, cancellationToken).ConfigureAwait(false); + await stream.FlushAsync(cancellationToken).ConfigureAwait(false); + } + + return true; + } + + /// + /// Deletes , waiting out a concurrent reader or replace holding it open. + /// + /// The filesystem to delete through. + /// The file to delete. + /// A token to cancel the operation. + /// if the file was there to delete; if it was already gone. + internal static async Task DeleteAsync(IFileSystem fileSystem, string filePath, CancellationToken cancellationToken) + { + for (int attempt = 1; ; attempt++) + { + if (!fileSystem.File.Exists(filePath)) + { + return false; + } + + try + { + fileSystem.File.Delete(filePath); + return true; + } + catch (DirectoryNotFoundException) + { + return false; + } + catch (Exception ex) when (attempt < MaxAttempts && ex is IOException or UnauthorizedAccessException) + { + // Windows refuses to delete a file a reader has open without delete sharing, as ReadAllText + // opens it, and a replace holds the target for an instant. The same moment as above is enough. + await Task.Delay(Backoff(attempt), cancellationToken).ConfigureAwait(false); + } + } + } + + /// + /// Whether is the name of a staging file this class writes: a GUID in + /// N format with a .tmp extension. No key encodes to that shape with a serializer's extension, + /// so a file so named was left behind by a write that never finished. + /// + /// The file name, without its directory. + /// for a staging file's name. + internal static bool IsStagingFileName(string fileName) + => fileName.Length == 36 + && fileName.EndsWith(StagingExtension, StringComparison.OrdinalIgnoreCase) + && Guid.TryParseExact(fileName[..32], "N", out _); + + private static string StagingPathFor(IFileSystem fileSystem, string filePath) + { + // Same directory as the target, so the move is a rename on one volume. The name is not derived from + // the target's, because PersistenceProviderUtilities budgets a key's filename for only the extension + // and ".tmp", and a longer suffix on a long key would exceed the 255-byte name limit. The .tmp + // extension keeps the staging file out of the providers' "*" key listings. + string? directory = fileSystem.Path.GetDirectoryName(filePath); + string tempFileName = $"{Guid.NewGuid():N}{StagingExtension}"; + return string.IsNullOrEmpty(directory) ? tempFileName : fileSystem.Path.Combine(directory, tempFileName); + } + #if NETSTANDARD2_1 private static void Replace(IFileSystem fileSystem, string tempFilePath, string filePath) { @@ -122,7 +225,10 @@ private static void Replace(IFileSystem fileSystem, string tempFilePath, string => fileSystem.File.Move(tempFilePath, filePath, overwrite: true); #endif - private static bool IsSharingViolation(IOException ex) => (ex.HResult & 0xFFFF) is SharingViolation or LockViolation; + private static bool IsSharingViolation(IOException ex) + => IsWindows + ? (ex.HResult & 0xFFFF) is SharingViolation or LockViolation + : ex.HResult is UnixWouldBlockLinux or UnixWouldBlockMac; private static int Backoff(int attempt) => Math.Min(attempt * 2, 20); } diff --git a/Shared/FileRecordStore.cs b/Shared/FileRecordStore.cs new file mode 100644 index 0000000..5438c3c --- /dev/null +++ b/Shared/FileRecordStore.cs @@ -0,0 +1,299 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Essentials; + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; + +/// +/// One record per file in a directory, each named after its key and holding the serialized object: the whole +/// of what the FileSystem and Temp persistence providers do, which differ only in where the directory is. +/// +/// +/// Linked into both providers rather than placed in the interfaces package, following AtomicFile. +/// Every operation lets cancellation through as itself rather than wrapping it (issue #121), and wraps every +/// other failure in a naming location. +/// +/// The type used to identify stored objects. +/// The file system to store through. +/// The serializer records are written and read with. +/// The directory records are stored in. +/// How failure messages name the store, such as "file system". +internal sealed class FileRecordStore(IFileSystemProvider fileSystem, ISerializationProvider serializer, string directory, string location) + where TKey : notnull +{ + /// + /// How old a staging file has to be before treats it as abandoned. A write in + /// progress stages its file for milliseconds, and deleting it would fail that write. + /// + private static readonly TimeSpan AbandonedStagingAge = TimeSpan.FromMinutes(1); + + /// How many times an empty record is read before it is believed. + private const int EmptyRecordAttempts = 10; + + /// Gets the directory records are stored in. + internal string Directory { get; } = directory; + + /// Stores under , or removes the key when it is null. + /// The type of object to store. + /// The key to store under. + /// The object to store. + /// A token to cancel the operation. + /// A task that completes once the record is in place. + internal async Task StoreAsync(TKey key, T obj, CancellationToken cancellationToken) + { + if (obj is null) + { + await RemoveAsync(key, cancellationToken).ConfigureAwait(false); + return; + } + + cancellationToken.ThrowIfCancellationRequested(); + + try + { + string filePath = GetFilePath(key); + string serializedData = await serializer.SerializeAsync(obj, cancellationToken).ConfigureAwait(false); + EnsureDirectory(); + await AtomicFile.WriteAllTextAsync(fileSystem, filePath, serializedData, cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + throw new PersistenceProviderException($"Failed to store object with key '{key}' to {location}", ex); + } + } + + /// Retrieves the object stored under . + /// The type of object to retrieve. + /// The key to read. + /// A token to cancel the operation. + /// The stored object, or when nothing is stored under the key. + /// + /// The record could not be read, including when it is there but cannot be deserialized as . + /// + internal async Task RetrieveAsync(TKey key, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + + try + { + string? serializedData = await ReadAsync(key, cancellationToken).ConfigureAwait(false); + return string.IsNullOrEmpty(serializedData) + ? default + : await DeserializeAsync(key, serializedData!, cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) when (ex is not OperationCanceledException and not PersistenceProviderException) + { + throw new PersistenceProviderException($"Failed to retrieve object with key '{key}' from {location}", ex); + } + } + + /// + /// Retrieves the object stored under , or creates, stores and returns a new one. + /// + /// + /// The new record is published only if no record has appeared in the meantime (issue #144): when another + /// writer gets there first, its record is the one returned and the new instance is discarded. A record that is + /// there but cannot be read is never replaced (issue #82); that throws instead, leaving the file as it was. + /// + /// The type of object to retrieve or create. + /// The key to read or create. + /// A token to cancel the operation. + /// The stored object, or the new instance now stored under . + internal async Task RetrieveOrCreateAsync(TKey key, CancellationToken cancellationToken) + where T : new() + { + cancellationToken.ThrowIfCancellationRequested(); + + try + { + string filePath = GetFilePath(key); + + // Each pass either returns or sees the record removed between looking and creating, which only a + // concurrent remover can do, so a handful of passes is far more than any real interleaving needs. + for (int attempt = 1; ; attempt++) + { + string? serializedData = await ReadAsync(key, cancellationToken).ConfigureAwait(false); + if (!string.IsNullOrEmpty(serializedData)) + { + return (await DeserializeAsync(key, serializedData!, cancellationToken).ConfigureAwait(false))!; + } + + T newInstance = new(); + string newData = await serializer.SerializeAsync(newInstance!, cancellationToken).ConfigureAwait(false); + EnsureDirectory(); + + if (serializedData is not null || attempt >= 10) + { + // An empty file holds no record to keep, so it is replaced as a missing one would be created. + await AtomicFile.WriteAllTextAsync(fileSystem, filePath, newData, cancellationToken).ConfigureAwait(false); + return newInstance; + } + + if (await AtomicFile.TryCreateAsync(fileSystem, filePath, newData, cancellationToken).ConfigureAwait(false)) + { + return newInstance; + } + } + } + catch (Exception ex) when (ex is not OperationCanceledException and not PersistenceProviderException) + { + throw new PersistenceProviderException($"Failed to retrieve or create object with key '{key}' in {location}", ex); + } + } + + /// Whether a record is stored under . + /// The key to look for. + /// if a record is stored under the key. + internal bool Exists(TKey key) => fileSystem.File.Exists(GetFilePath(key)); + + /// Removes the record stored under . + /// The key to remove. + /// A token to cancel the operation. + /// if there was a record to remove. + internal async Task RemoveAsync(TKey key, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + + try + { + return await AtomicFile.DeleteAsync(fileSystem, GetFilePath(key), cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + throw new PersistenceProviderException($"Failed to remove object with key '{key}' from {location}", ex); + } + } + + /// Lists the keys of every record stored. + /// The keys, in no particular order. + internal IEnumerable GetAllKeys() + { + try + { + if (!fileSystem.Directory.Exists(Directory)) + { + return []; + } + + List keys = []; + foreach (string file in fileSystem.Directory.GetFiles(Directory, "*" + serializer.FileExtension, SearchOption.TopDirectoryOnly)) + { + string? name = fileSystem.Path.GetFileNameWithoutExtension(file); + if (string.IsNullOrEmpty(name)) + { + continue; + } + + // Names are percent-encoded, so the original key is recovered exactly. Truncated + // names cannot be decoded and are skipped rather than reported as a wrong key. + string? decoded = PersistenceProviderUtilities.GetKeyFromFileName(name!); + if (decoded is not null && PersistenceProviderUtilities.TryConvertToKey(decoded, out TKey key)) + { + keys.Add(key); + } + } + + return keys; + } + catch (Exception ex) + { + throw new PersistenceProviderException($"Failed to retrieve all keys from {location}", ex); + } + } + + /// + /// Removes every record, and every staging file a write abandoned when the process stopped mid-write + /// (issue #145), since each of those holds a whole record too. + /// + /// A token to cancel the operation. + /// A task that completes once the records are gone. + internal async Task ClearAsync(CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + + try + { + if (!fileSystem.Directory.Exists(Directory)) + { + return; + } + + foreach (string file in fileSystem.Directory.GetFiles(Directory, "*" + serializer.FileExtension, SearchOption.TopDirectoryOnly)) + { + await AtomicFile.DeleteAsync(fileSystem, file, cancellationToken).ConfigureAwait(false); + } + + DateTime abandonedBefore = DateTime.UtcNow - AbandonedStagingAge; + IEnumerable abandoned = fileSystem.Directory + .GetFiles(Directory, "*" + AtomicFile.StagingExtension, SearchOption.TopDirectoryOnly) + .Where(file => AtomicFile.IsStagingFileName(fileSystem.Path.GetFileName(file)) + && fileSystem.File.GetLastWriteTimeUtc(file) < abandonedBefore); + foreach (string file in abandoned) + { + await AtomicFile.DeleteAsync(fileSystem, file, cancellationToken).ConfigureAwait(false); + } + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + throw new PersistenceProviderException($"Failed to clear all objects from {location}", ex); + } + } + + /// + /// Reads the record stored under , or returns when there is none, + /// including one a concurrent remover deleted between looking and reading (issue #123). + /// + private async Task ReadAsync(TKey key, CancellationToken cancellationToken) + { + string filePath = GetFilePath(key); + if (!fileSystem.File.Exists(filePath)) + { + return null; + } + + try + { + // A record being created is empty until its creator has written it (AtomicFile.TryCreateAsync), so + // an empty file is given a moment to fill before it is taken for an empty record. + for (int attempt = 1; ; attempt++) + { + string content = await AtomicFile.ReadAllTextAsync(fileSystem, filePath, cancellationToken).ConfigureAwait(false); + if (content.Length > 0 || attempt >= EmptyRecordAttempts) + { + return content; + } + + await Task.Delay(attempt, cancellationToken).ConfigureAwait(false); + } + } + catch (Exception ex) when (ex is FileNotFoundException or DirectoryNotFoundException) + { + return null; + } + } + + private async Task DeserializeAsync(TKey key, string serializedData, CancellationToken cancellationToken) + { + T? value = await serializer.DeserializeAsync(serializedData, cancellationToken).ConfigureAwait(false); + + // The serializers report content they cannot parse as default rather than throwing, which for a + // reference type is indistinguishable from a missing key. Content is there, so it is a failure: returning + // null would invite RetrieveOrCreateAsync, or the caller, to write a fresh default over the record. + return value is null + ? throw new PersistenceProviderException($"The object stored with key '{key}' in {location} could not be read as {typeof(T).Name}") + : value; + } + + private void EnsureDirectory() => fileSystem.Directory.CreateDirectory(Directory); + + private string GetFilePath(TKey key) + { + string fileName = PersistenceProviderUtilities.GetSafeFileName(PersistenceProviderUtilities.FormatKey(key)) + serializer.FileExtension; + return fileSystem.Path.Combine(Directory, fileName); + } +} diff --git a/Shared/PersistenceDefaults.cs b/Shared/PersistenceDefaults.cs deleted file mode 100644 index d351538..0000000 --- a/Shared/PersistenceDefaults.cs +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright (c) 2023-2026 ktsu-dev contributors - -namespace ktsu.Essentials; - -using System.Threading; -using System.Threading.Tasks; - -/// -/// The interface default bodies a persistence provider has to repeat when it declares the member itself. -/// -/// -/// Linked into the FileSystem and Temp providers rather than placed in the interfaces package, following -/// DistributionArguments. Both declare a public RetrieveOrCreateAsync of their own, which -/// hides the default body, so the documented behaviour has to be -/// restated; restating it once here keeps the two from drifting apart again. -/// -internal static class PersistenceDefaults -{ - /// - /// Retrieves the object stored under , or creates, stores and returns a new one. - /// - /// The type used to identify stored objects. - /// The type of object to retrieve or create. - /// The provider to read from and store into. - /// The key of the object. - /// A token to cancel the operation. - /// The stored object, or the new instance now stored under . - internal static async Task RetrieveOrCreateAsync(IPersistenceProvider provider, TKey key, CancellationToken cancellationToken) - where TKey : notnull - where T : new() - { - // A non-nullable value type comes back from RetrieveAsync as default(T) when the key is missing, - // which is never null, so presence has to be asked separately rather than read off the value. - if (default(T) is not null) - { - if (await provider.ExistsAsync(key, cancellationToken).ConfigureAwait(false)) - { - return (await provider.RetrieveAsync(key, cancellationToken).ConfigureAwait(false))!; - } - } - else - { - T? existing = await provider.RetrieveAsync(key, cancellationToken).ConfigureAwait(false); - if (existing is not null) - { - return existing; - } - } - - // Stored before it is returned, as the interface documents: callers bootstrap a record with this, - // and one that was never written comes back as a fresh default on every call. - T newInstance = new(); - await provider.StoreAsync(key, newInstance, cancellationToken).ConfigureAwait(false); - return newInstance; - } -} From f61dc5cf91dda9e7be2befb1ab2a24f5de89c8d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 10:47:57 +0000 Subject: [PATCH 06/23] Skip whitespace in Base64 input on every target framework net8 and later skipped it while earlier targets rejected it. Fixes #140 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- .../Base64EncodingProvider.cs | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/Essentials.EncodingProviders.Base64/Base64EncodingProvider.cs b/Essentials.EncodingProviders.Base64/Base64EncodingProvider.cs index c844872..354f74d 100644 --- a/Essentials.EncodingProviders.Base64/Base64EncodingProvider.cs +++ b/Essentials.EncodingProviders.Base64/Base64EncodingProvider.cs @@ -20,6 +20,11 @@ namespace ktsu.Essentials.EncodingProviders.Base64; /// public class Base64EncodingProvider : IEncodingProvider { +#if !NET8_0_OR_GREATER + // The whitespace .NET 8's decoder skips. + private static ReadOnlySpan Whitespace => " \t\r\n"u8; +#endif + /// public int GetMaxEncodedLength(int sourceLength) => SysBase64.GetMaxEncodedToUtf8Length(sourceLength); @@ -94,8 +99,37 @@ public bool TryEncode(Stream data, Stream destination) } /// + /// + /// ASCII whitespace (space, tab, carriage return and line feed) is skipped wherever it appears, so line-wrapped + /// MIME or PEM-style input decodes. .NET 8's decoder skips it itself; earlier targets strip it first, so every + /// target framework decodes the same input the same way. + /// public bool TryDecode(ReadOnlySpan encodedData, Span destination, out int bytesWritten) { +#if !NET8_0_OR_GREATER + if (encodedData.IndexOfAny(Whitespace) >= 0) + { + byte[] stripped = ArrayPool.Shared.Rent(encodedData.Length); + try + { + int length = 0; + foreach (byte b in encodedData) + { + if (Whitespace.IndexOf(b) < 0) + { + stripped[length++] = b; + } + } + + return TryDecode(stripped.AsSpan(0, length), destination, out bytesWritten); + } + finally + { + ArrayPool.Shared.Return(stripped, clearArray: true); + } + } +#endif + OperationStatus status = SysBase64.DecodeFromUtf8(encodedData, destination, out int consumed, out bytesWritten); if (status == OperationStatus.Done && consumed == encodedData.Length) From 882fcc176fb0b16274f108c508feb799ba5e4898 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 10:47:58 +0000 Subject: [PATCH 07/23] Keep the triangular distribution finite when its width overflows Fixes #142 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- .../TriangularDistributionProvider.cs | 62 ++++++++++++++----- Essentials.Tests/DistributionProviderTests.cs | 33 ++++++++++ 2 files changed, 80 insertions(+), 15 deletions(-) diff --git a/Essentials.DistributionProviders.Triangular/TriangularDistributionProvider.cs b/Essentials.DistributionProviders.Triangular/TriangularDistributionProvider.cs index 7b8681c..a95ddc4 100644 --- a/Essentials.DistributionProviders.Triangular/TriangularDistributionProvider.cs +++ b/Essentials.DistributionProviders.Triangular/TriangularDistributionProvider.cs @@ -16,6 +16,10 @@ namespace ktsu.Essentials.DistributionProviders.Triangular; /// public sealed class TriangularDistributionProvider : IContinuousDistribution { + // The widths are held at half size when the full range overflows a double, as the uniform distribution's + // are (issue #142): halving is exact, and every ratio below is the same at either size. + private readonly bool halved; + private readonly double scale; private readonly double width; private readonly double lowerWidth; private readonly double upperWidth; @@ -40,9 +44,11 @@ public TriangularDistributionProvider(double minimum, double mode, double maximu Minimum = minimum; Mode = mode; Maximum = maximum; - width = maximum - minimum; - lowerWidth = mode - minimum; - upperWidth = maximum - mode; + halved = double.IsInfinity(maximum - minimum); + scale = halved ? 0.5 : 1.0; + width = Scaled(maximum) - Scaled(minimum); + lowerWidth = Scaled(mode) - Scaled(minimum); + upperWidth = Scaled(maximum) - Scaled(mode); modeProbability = lowerWidth / width; } @@ -62,7 +68,9 @@ public TriangularDistributionProvider(double minimum, double mode, double maximu /// Measured from the lower bound rather than as the average of the three raw parameters, so bounds /// near ± cannot overflow the sum. /// - public double Mean => Minimum + ((lowerWidth + width) / 3.0); + public double Mean => halved + ? (Minimum / 3.0) + (Mode / 3.0) + (Maximum / 3.0) + : Minimum + ((lowerWidth + width) / 3.0); /// /// @@ -73,7 +81,7 @@ public TriangularDistributionProvider(double minimum, double mode, double maximu /// shape of the triangle, not on where it sits. /// public double Variance - => ((lowerWidth * lowerWidth) + (upperWidth * upperWidth) + (lowerWidth * upperWidth)) / 18.0; + => ((lowerWidth * lowerWidth) + (upperWidth * upperWidth) + (lowerWidth * upperWidth)) / 18.0 / (scale * scale); /// public double Pdf(double value) @@ -87,13 +95,13 @@ public double Pdf(double value) // of the denominator. Below the mode is safe unguarded: if the lower side is empty then the mode // is the lower bound and no value in range is below it. Above needs the guard, because the mode // itself reaches it, and where the mode is the upper bound that leaves 0/0 rather than the peak. - double peak = 2.0 / width; + double peak = 2.0 * scale / width; if (value < Mode) { - return peak * (value - Minimum) / lowerWidth; + return peak * (Scaled(value) - Scaled(Minimum)) / lowerWidth; } - return upperWidth > 0.0 ? peak * (Maximum - value) / upperWidth : peak; + return upperWidth > 0.0 ? peak * (Scaled(Maximum) - Scaled(value)) / upperWidth : peak; } /// @@ -113,8 +121,8 @@ public double Cdf(double value) // zero-width side out of the denominator: a mode sitting on a bound leaves that side empty, and // no value can fall strictly inside it. return value <= Mode - ? (value - Minimum) * (value - Minimum) / (width * lowerWidth) - : 1.0 - ((Maximum - value) * (Maximum - value) / (width * upperWidth)); + ? LowerTail(value) + : 1.0 - UpperTail(value); } /// @@ -142,8 +150,8 @@ public double SurvivalFunction(double value) // The same branch test as Cdf, which keeps a zero-width side out of the denominator. return value <= Mode - ? 1.0 - ((value - Minimum) * (value - Minimum) / (width * lowerWidth)) - : (Maximum - value) * (Maximum - value) / (width * upperWidth); + ? 1.0 - LowerTail(value) + : UpperTail(value); } /// @@ -151,8 +159,32 @@ public double Quantile(double probability) { DistributionArguments.QuantileProbability(probability, nameof(probability)); - return probability < modeProbability - ? Minimum + Math.Sqrt(probability * width * lowerWidth) - : Maximum - Math.Sqrt((1.0 - probability) * width * upperWidth); + // Each root is taken of its two factors separately, since their product overflows for wide bounds, and + // the offset from the bound is added once per halving rather than doubled, which could overflow too. + if (probability < modeProbability) + { + double offset = Math.Sqrt(probability * width) * Math.Sqrt(lowerWidth); + return halved ? Minimum + offset + offset : Minimum + offset; + } + + double fromTop = Math.Sqrt((1.0 - probability) * width) * Math.Sqrt(upperWidth); + return halved ? Maximum - fromTop - fromTop : Maximum - fromTop; + } + + // The CDF below the mode, (v - min)² / (width × lower width), as a product of two ratios no larger than one, + // so neither the square nor the product of widths can overflow. + private double LowerTail(double value) + { + double fromBottom = Scaled(value) - Scaled(Minimum); + return fromBottom / width * (fromBottom / lowerWidth); } + + // The survival function above the mode, (max - v)² / (width × upper width), in the same form. + private double UpperTail(double value) + { + double fromTop = Scaled(Maximum) - Scaled(value); + return fromTop / width * (fromTop / upperWidth); + } + + private double Scaled(double value) => value * scale; } diff --git a/Essentials.Tests/DistributionProviderTests.cs b/Essentials.Tests/DistributionProviderTests.cs index 29c7887..4984d73 100644 --- a/Essentials.Tests/DistributionProviderTests.cs +++ b/Essentials.Tests/DistributionProviderTests.cs @@ -586,6 +586,39 @@ public void Triangular_Moments_Do_Not_Depend_On_Where_The_Triangle_Sits() Assert.AreEqual(double.MaxValue * 0.75, huge.Mean, double.MaxValue * 1e-15); } + [TestMethod] + public void Triangular_Handles_Bounds_Whose_Width_Overflows() + { + // max - min overflowed to infinity, which made the mean infinite, the CDF NaN and every sample + // -Infinity, outside the bounds (issue #142). + IContinuousDistribution symmetric = new TriangularDistributionProvider(-1e308, 0.0, 1e308); + Assert.AreEqual(0.0, symmetric.Mean, 1e293); + Assert.AreEqual(0.5, symmetric.Cdf(0.0), Tight); + Assert.AreEqual(0.5, symmetric.SurvivalFunction(0.0), Tight); + Assert.AreEqual(0.0, symmetric.Median, 1e293); + Assert.AreEqual(1e-308, symmetric.Pdf(0.0), 1e-320); + + IRandomProvider random = SeededRandom(); + foreach (double sample in symmetric.Sample(random, 1000)) + { + Assert.IsTrue(sample is >= -1e308 and <= 1e308, $"sample {sample} is outside the bounds"); + } + + IContinuousDistribution skewed = new TriangularDistributionProvider(-1e308, -5e307, 1e308); + double previous = 0.0; + for (int i = 1; i < 20; i++) + { + double value = (i - 10) / 10.0 * 1e308; + double cdf = skewed.Cdf(value); + Assert.IsTrue(cdf >= previous, $"the CDF fell at {value}"); + Assert.AreEqual(1.0, cdf + skewed.SurvivalFunction(value), 1e-12, $"the survival function does not complement the CDF at {value}"); + Assert.AreEqual(value, skewed.Quantile(cdf), 1e295, $"the quantile does not invert the CDF at {value}"); + previous = cdf; + } + + Assert.AreEqual(0.25, skewed.Cdf(-5e307), 1e-12, "the mode carries the lower side's share of the mass"); + } + [TestMethod] public void Bernoulli_Matches_Its_Closed_Form() { From 5b659d6ac1fc862eb2321365cf9bc35a14652879 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 10:47:58 +0000 Subject: [PATCH 08/23] Keep a category default registered before AddEssentials() the default Fixes #149 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- Essentials.All/ServiceCollectionExtensions.cs | 12 +++--- Essentials.Tests/DefaultProviderTests.cs | 42 +++++++++++++++++++ 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/Essentials.All/ServiceCollectionExtensions.cs b/Essentials.All/ServiceCollectionExtensions.cs index 1ef76ed..400b866 100644 --- a/Essentials.All/ServiceCollectionExtensions.cs +++ b/Essentials.All/ServiceCollectionExtensions.cs @@ -132,7 +132,7 @@ public static IServiceCollection AddCompressionProviders(this IServiceCollection #endif // Registered last so that it is the provider an ICompressionProvider resolves to. - return services.AddGzipCompressionProvider(); + return services.AddDefaultOrKeep(static s => s.AddGzipCompressionProvider()); } /// @@ -184,7 +184,7 @@ public static IServiceCollection AddRandomProviders(this IServiceCollection serv .AddXoshiroRandomProvider() .AddPcgRandomProvider() // Registered last so that it is the provider an IRandomProvider resolves to. - .AddCryptoRandomProvider(); + .AddDefaultOrKeep(static s => s.AddCryptoRandomProvider()); } /// @@ -199,7 +199,7 @@ public static IServiceCollection AddEncodingProviders(this IServiceCollection se return services .AddHexEncodingProvider() // Registered last so that it is the provider an IEncodingProvider resolves to. - .AddBase64EncodingProvider(); + .AddDefaultOrKeep(static s => s.AddBase64EncodingProvider()); } /// @@ -239,7 +239,7 @@ public static IServiceCollection AddHashProviders(this IServiceCollection servic .AddXxHash3HashProvider() .AddXxHash128HashProvider() // Registered last so that it is the provider an IHashProvider resolves to. - .AddSHA256HashProvider(); + .AddDefaultOrKeep(static s => s.AddSHA256HashProvider()); } /// @@ -255,7 +255,7 @@ public static IServiceCollection AddKeyedHashProviders(this IServiceCollection s .AddHmacSha384KeyedHashProvider() .AddHmacSha512KeyedHashProvider() // Registered last so that it is the provider an IKeyedHashProvider resolves to. - .AddHmacSha256KeyedHashProvider(); + .AddDefaultOrKeep(static s => s.AddHmacSha256KeyedHashProvider()); } /// @@ -296,7 +296,7 @@ public static IServiceCollection AddSerializationProviders(this IServiceCollecti .AddTomlSerializationProvider() // Registered last so that it is the provider an ISerializationProvider resolves to, which // also makes it the format the file-backed persistence providers write. - .AddJsonSerializationProvider(); + .AddDefaultOrKeep(static s => s.AddJsonSerializationProvider()); } /// diff --git a/Essentials.Tests/DefaultProviderTests.cs b/Essentials.Tests/DefaultProviderTests.cs index 3d67b38..c78d47f 100644 --- a/Essentials.Tests/DefaultProviderTests.cs +++ b/Essentials.Tests/DefaultProviderTests.cs @@ -2,9 +2,11 @@ namespace ktsu.Essentials.Tests; +using System; using System.Collections.Generic; using System.IO; using System.Linq; +using System.Text.Json; using System.Threading.Tasks; using ktsu.Essentials; using ktsu.Essentials.All; @@ -53,6 +55,46 @@ public void AddEssentials_Resolves_The_Documented_Default_Per_Category() Assert.IsInstanceOfType(serviceProvider.GetRequiredService()); } + [TestMethod] + public void A_Default_Registered_Before_AddEssentials_Stays_The_Default() + { + // The default's own registration added nothing the second time, so whichever provider AddEssentials + // registered before it went in last and became what the interface resolved to: JSON became TOML, the + // CSPRNG became PCG (issue #149). + static void AssertDefault(Action registerFirst) + where TService : class + { + ServiceCollection services = new(); + registerFirst(services); + services.AddEssentials(); + using ServiceProvider serviceProvider = services.BuildServiceProvider(); + Assert.IsInstanceOfType(serviceProvider.GetRequiredService(), $"registering {typeof(TDefault).Name} first changed the {typeof(TService).Name} default"); + AssertDistinct(serviceProvider.GetServices()); + } + + AssertDefault(static s => s.AddJsonSerializationProvider()); + AssertDefault(static s => s.AddCryptoRandomProvider()); + AssertDefault(static s => s.AddGzipCompressionProvider()); + AssertDefault(static s => s.AddSHA256HashProvider()); + AssertDefault(static s => s.AddBase64EncodingProvider()); + AssertDefault(static s => s.AddHmacSha256KeyedHashProvider()); + } + + [TestMethod] + public void Json_Options_Registered_Before_AddEssentials_Are_The_Ones_Used() + { + ServiceCollection services = new(); + services.AddJsonSerializationProvider(new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }); + services.AddEssentials(); + using ServiceProvider serviceProvider = services.BuildServiceProvider(); + + ISerializationProvider serializer = serviceProvider.GetRequiredService(); + Assert.IsInstanceOfType(serializer); + using StringWriter writer = new(); + Assert.IsTrue(serializer.TrySerialize(new { DisplayName = "x" }, writer)); + Assert.AreEqual("{\"displayName\":\"x\"}", writer.ToString()); + } + [TestMethod] public void AddEssentials_Lists_Each_Implementation_Once() { From 9f26f7b4f860366b987751ef8386514fe621df3f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 10:47:58 +0000 Subject: [PATCH 09/23] Measure cache lifetimes on a monotonic clock, sweep expired entries, and make GetOrAdd atomic - Expiry uses Stopwatch timestamps, so a wall-clock step changes nothing. Fixes #162 - Sets sweep expired entries now and then. Fixes #79 - GetOrAdd adds only if nothing live is cached. Fixes #85 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- .../InMemoryCacheProvider.cs | 148 ++++++++++++++++-- Essentials.Tests/CacheProviderTests.cs | 107 +++++++++++++ 2 files changed, 240 insertions(+), 15 deletions(-) diff --git a/Essentials.CacheProviders.InMemory/InMemoryCacheProvider.cs b/Essentials.CacheProviders.InMemory/InMemoryCacheProvider.cs index 867b4ba..27accc1 100644 --- a/Essentials.CacheProviders.InMemory/InMemoryCacheProvider.cs +++ b/Essentials.CacheProviders.InMemory/InMemoryCacheProvider.cs @@ -6,15 +6,62 @@ namespace ktsu.Essentials.CacheProviders.InMemory; using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Diagnostics; +using System.Threading; /// /// An in-memory cache provider that stores key-value pairs with optional expiration support. /// +/// +/// +/// A time-to-live is measured on a monotonic clock, not the wall clock, so a clock correction, a manual change +/// or a virtual machine resuming can neither keep an entry past its lifetime nor expire one early. +/// +/// +/// Expired entries are swept away as entries are set, so a cache of short-lived keys that are seldom read again +/// does not grow without bound. A sweep visits every entry, and runs once a second at most or once as many +/// entries have been set as the cache holds, whichever comes first, so its cost is spread across those sets. +/// +/// /// The type of the cache key. /// The type of the cached value. public class InMemoryCacheProvider : ICacheProvider where TKey : notnull { + /// The fewest sets between two sweeps, so a small cache is not swept on every set. + private const int SweepMinimumSets = 1024; + + /// The expiration of an entry that never expires. + private const long Never = long.MaxValue; + private readonly ConcurrentDictionary cache = new(); + private readonly Func clock; + private readonly long sweepInterval; + private long lastSweep; + private int setsSinceSweep; + private int sweeping; + + /// + /// Initializes a new instance of the class. + /// + public InMemoryCacheProvider() + : this(Stopwatch.GetTimestamp) + { + } + + /// + /// Initializes a new instance over a clock other than , so tests can move + /// time on rather than sleep. + /// + /// Returns the current time in ticks, never going backwards. + internal InMemoryCacheProvider(Func clock) + { + this.clock = clock; + sweepInterval = Stopwatch.Frequency; + lastSweep = clock(); + } + + /// Gets how many entries the cache holds, expired or not. + internal int Count => cache.Count; /// /// Tries to get a cached value by key. @@ -26,7 +73,7 @@ public bool TryGet(TKey key, out TValue? value) { if (cache.TryGetValue(key, out CacheEntry? entry)) { - if (entry.Expiration is null || entry.Expiration > DateTime.UtcNow) + if (!entry.IsExpired(clock())) { value = entry.Value; return true; @@ -47,8 +94,50 @@ public bool TryGet(TKey key, out TValue? value) /// The cache key. /// The value to cache. /// The optional time-to-live for the cached entry. If null, the entry does not expire. - public void Set(TKey key, TValue value, TimeSpan? expiration = null) => - cache[key] = new CacheEntry(value, ComputeExpiration(expiration)); + public void Set(TKey key, TValue value, TimeSpan? expiration = null) + { + long now = clock(); + cache[key] = new CacheEntry(value, ComputeExpiration(expiration, now)); + SweepIfDue(now); + } + + /// + /// Gets the value cached under , or adds and returns the one makes. + /// + /// + /// Adds only if nothing live is cached by the time the factory returns: a value set while the factory ran is + /// kept and returned, and callers racing on one missing key all get the one value left cached. The factory + /// may therefore run and have its result discarded, as with . + /// + /// The cache key. + /// Makes the value to add when none is cached. + /// The optional time-to-live for an added entry. If null, the entry does not expire. + /// The cached value. + public TValue GetOrAdd(TKey key, Func factory, TimeSpan? expiration = null) + { + Ensure.NotNull(factory); + + while (true) + { + if (cache.TryGetValue(key, out CacheEntry? observed) && !observed.IsExpired(clock())) + { + return observed.Value; + } + + TValue value = factory(key); + long now = clock(); + CacheEntry created = new(value, ComputeExpiration(expiration, now)); + + // Replace only what was seen to be missing or expired; anything else means another writer got there + // first, and the loop returns its value. + bool stored = observed is null ? cache.TryAdd(key, created) : cache.TryUpdate(key, created, observed); + if (stored) + { + SweepIfDue(now); + return value; + } + } + } /// /// Removes a cached value by key. @@ -59,7 +148,7 @@ public void Set(TKey key, TValue value, TimeSpan? expiration = null) => /// reports false, matching , which treats it as absent. /// public bool Remove(TKey key) => - cache.TryRemove(key, out CacheEntry? entry) && (entry.Expiration is null || entry.Expiration > DateTime.UtcNow); + cache.TryRemove(key, out CacheEntry? entry) && !entry.IsExpired(clock()); /// /// Clears all entries from the cache. @@ -67,24 +156,52 @@ public bool Remove(TKey key) => public void Clear() => cache.Clear(); /// - /// Converts a time-to-live into an absolute expiration time, saturating rather than overflowing. - /// A time-to-live that reaches past never expires, and one that - /// reaches before has already expired. + /// Converts a time-to-live into an expiration on the clock, saturating rather than overflowing. A time-to-live + /// too long to represent never expires, and one reaching before the clock's start has already expired. /// - private static DateTime? ComputeExpiration(TimeSpan? expiration) + private static long ComputeExpiration(TimeSpan? expiration, long now) { if (expiration is null) { - return null; + return Never; } - DateTime now = DateTime.UtcNow; - if (expiration.Value > DateTime.MaxValue - now) + // In double, which holds every TimeSpan's tick count to within a part in 2^53 and cannot overflow here. + double ticks = expiration.Value.Ticks * ((double)Stopwatch.Frequency / TimeSpan.TicksPerSecond); + double at = now + ticks; + if (at >= Never) { - return null; + return Never; } - return expiration.Value < DateTime.MinValue - now ? DateTime.MinValue : now + expiration.Value; + return at <= long.MinValue ? long.MinValue : (long)at; + } + + private void SweepIfDue(long now) + { + int sets = Interlocked.Increment(ref setsSinceSweep); + bool due = sets >= Math.Max(SweepMinimumSets, cache.Count) || now - Interlocked.Read(ref lastSweep) >= sweepInterval; + if (!due || Interlocked.CompareExchange(ref sweeping, 1, 0) != 0) + { + return; + } + + try + { + Interlocked.Exchange(ref setsSinceSweep, 0); + Interlocked.Exchange(ref lastSweep, now); + foreach (KeyValuePair pair in cache) + { + if (pair.Value.IsExpired(now)) + { + RemoveEntry(pair.Key, pair.Value); + } + } + } + finally + { + Interlocked.Exchange(ref sweeping, 0); + } } private void RemoveEntry(TKey key, CacheEntry entry) @@ -97,9 +214,10 @@ private void RemoveEntry(TKey key, CacheEntry entry) #endif } - private sealed class CacheEntry(TValue value, DateTime? expiration) + private sealed class CacheEntry(TValue value, long expiration) { public TValue Value { get; } = value; - public DateTime? Expiration { get; } = expiration; + + public bool IsExpired(long now) => expiration <= now; } } diff --git a/Essentials.Tests/CacheProviderTests.cs b/Essentials.Tests/CacheProviderTests.cs index b2262ab..c8e97b8 100644 --- a/Essentials.Tests/CacheProviderTests.cs +++ b/Essentials.Tests/CacheProviderTests.cs @@ -4,6 +4,8 @@ namespace ktsu.Essentials.Tests; using System; using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; using System.Threading; using System.Threading.Tasks; using ktsu.Essentials; @@ -256,6 +258,111 @@ public void Cache_Set_With_MinValue_Expiration_Has_Already_Expired() Assert.IsFalse(cache.TryGet("key", out _), "An entry with the most negative time-to-live has already expired"); } + [TestMethod] + public void Cache_Expiry_Follows_Elapsed_Time_Not_The_Wall_Clock() + { + // The time-to-live was turned into a DateTime and checked against DateTime.UtcNow, so a clock step + // stretched or cut every entry's lifetime (issue #162). Only the clock the cache is given moves here. + long now = 0; + InMemoryCacheProvider cache = new(() => now); + + cache.Set("token", 1, TimeSpan.FromSeconds(30)); + now += Stopwatch.Frequency * 29; + Assert.IsTrue(cache.TryGet("token", out _), "an entry is live until its time-to-live has elapsed"); + + now += Stopwatch.Frequency * 2; + Assert.IsFalse(cache.TryGet("token", out _), "an entry expires once its time-to-live has elapsed"); + } + + [TestMethod] + public void Cache_Sweeps_Expired_Entries_That_Are_Never_Read_Again() + { + // An expired entry was removed only when its own key was read, so short-lived keys that were never + // read again stayed for the life of the process (issue #79). + long now = 0; + InMemoryCacheProvider cache = new(() => now); + + for (int i = 0; i < 10_000; i++) + { + cache.Set(i, i, TimeSpan.FromMilliseconds(1)); + } + + cache.Set(-1, -1); + now += Stopwatch.Frequency * 2; + cache.Set(-2, -2); + + Assert.AreEqual(2, cache.Count, "only the two entries that do not expire should be left"); + Assert.IsTrue(cache.TryGet(-1, out _)); + } + + [TestMethod] + public void Cache_Sweep_Keeps_Live_Entries() + { + long now = 0; + InMemoryCacheProvider cache = new(() => now); + + for (int i = 0; i < 5_000; i++) + { + cache.Set(i, i, TimeSpan.FromHours(1)); + } + + now += Stopwatch.Frequency * 2; + cache.Set(-1, -1); + + Assert.AreEqual(5_001, cache.Count); + } + + [TestMethod] + public void Cache_GetOrAdd_Keeps_A_Value_Set_While_The_Factory_Ran() + { + // GetOrAdd checked, ran the factory and then set unconditionally, so a value another writer set in + // between was overwritten (issue #85). + ICacheProvider cache = new InMemoryCacheProvider(); + + string result = cache.GetOrAdd("k", key => + { + cache.Set(key, "fresh"); + return "factory"; + }); + + Assert.AreEqual("fresh", result); + Assert.IsTrue(cache.TryGet("k", out string? stored)); + Assert.AreEqual("fresh", stored); + } + + [TestMethod] + public async Task Cache_GetOrAdd_Gives_Racing_Callers_The_Cached_Instance() + { + ICacheProvider cache = new InMemoryCacheProvider(); + using Barrier barrier = new(2); + + Task[] callers = [.. Enumerable.Range(0, 2).Select(_ => Task.Run(() => cache.GetOrAdd("k", _ => + { + barrier.SignalAndWait(TestContext.CancellationToken); + return new object(); + }), TestContext.CancellationToken))]; + object[] results = await Task.WhenAll(callers).ConfigureAwait(false); + + (bool found, object? cached) = await cache.TryGetAsync("k", TestContext.CancellationToken).ConfigureAwait(false); + Assert.IsTrue(found); + Assert.AreSame(cached, results[0], "the first caller holds an instance that is not cached"); + Assert.AreSame(cached, results[1], "the second caller holds an instance that is not cached"); + } + + [TestMethod] + public void Cache_GetOrAdd_Replaces_An_Expired_Entry() + { + long now = 0; + InMemoryCacheProvider cache = new(() => now); + + cache.Set("k", 1, TimeSpan.FromSeconds(1)); + now += Stopwatch.Frequency * 2; + + Assert.AreEqual(2, cache.GetOrAdd("k", _ => 2)); + Assert.IsTrue(cache.TryGet("k", out int value)); + Assert.AreEqual(2, value); + } + /// /// A key that runs an action when it is hashed for the second time after being armed, which lets a /// test interleave work between the lookups a single cache call makes. From 93636dea606a6f84c7e63d58f6da60cf29b3fc3b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 10:48:11 +0000 Subject: [PATCH 10/23] Test that line-wrapped Base64 decodes Refs #140 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- Essentials.Tests/EncodingProviderTests.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Essentials.Tests/EncodingProviderTests.cs b/Essentials.Tests/EncodingProviderTests.cs index 71c4352..da07153 100644 --- a/Essentials.Tests/EncodingProviderTests.cs +++ b/Essentials.Tests/EncodingProviderTests.cs @@ -9,6 +9,7 @@ namespace ktsu.Essentials.Tests; using System.Threading; using System.Threading.Tasks; using ktsu.Essentials; +using ktsu.Essentials.EncodingProviders.Base64; using ktsu.Essentials.EncodingProviders.Hex; using ktsu.Essentials.ObfuscationProviders.Hex; using Microsoft.Extensions.DependencyInjection; @@ -56,6 +57,22 @@ public void GetMaxEncodedLength_Throws_When_The_Bound_Does_Not_Fit_An_Int(IEncod => Assert.ThrowsExactly(() => encoder.GetMaxEncodedLength(int.MaxValue), $"{providerName} must not return a wrapped, negative bound"); + [TestMethod] + public void Base64_Decodes_Line_Wrapped_Input() + { + // net8 and later skipped whitespace and earlier targets rejected it, so one package decoded the same + // input differently by target framework (issue #140). Every target now skips it. + IEncodingProvider base64 = new Base64EncodingProvider(); + foreach (string wrapped in new[] { "SGVs\r\nbG8=", "SGVs\nbG8=", " SGVs bG8= ", "SGVs\tbG8\r\n=" }) + { + byte[] decoded = base64.Decode(Encoding.ASCII.GetBytes(wrapped)); + Assert.AreEqual("Hello", Encoding.ASCII.GetString(decoded), $"'{wrapped}' should decode"); + } + + Assert.IsFalse(base64.TryDecode("SGVs*bG8="u8, new byte[16], out int written), "other characters are still invalid"); + Assert.AreEqual(0, written); + } + [TestMethod] public void Hex_Bound_Is_Exact_Up_To_The_Largest_Encodable_Input() { From 5deb5cb207a0096d065f20e392a3973c352edc64 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 10:54:10 +0000 Subject: [PATCH 11/23] Report unreadable streams as failures in the HMAC and MD5/SHA stream paths HmacKeyedHashCore's stream TryHash caught only IOException, so a disposed or write-only stream threw ObjectDisposedException or NotSupportedException out of a Try method. The MD5 and SHA stream paths had the same gap. Both now report false, matching the non-cryptographic providers. Fixes #74 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- .../MD5HashProvider.cs | 3 +- .../SHA1HashProvider.cs | 3 +- .../SHA256HashProvider.cs | 3 +- .../SHA384HashProvider.cs | 3 +- .../SHA512HashProvider.cs | 3 +- Essentials.Tests/UnreadableStreamHashTests.cs | 32 +++++++++++++++++++ Shared/HmacKeyedHashCore.cs | 4 ++- 7 files changed, 45 insertions(+), 6 deletions(-) diff --git a/Essentials.HashProviders.MD5/MD5HashProvider.cs b/Essentials.HashProviders.MD5/MD5HashProvider.cs index 6ddd847..e3185d9 100644 --- a/Essentials.HashProviders.MD5/MD5HashProvider.cs +++ b/Essentials.HashProviders.MD5/MD5HashProvider.cs @@ -94,8 +94,9 @@ public bool TryHash(Stream data, Span destination, out int bytesWritten) bytesWritten = 0; return false; } - catch (IOException) + catch (Exception ex) when (ex is IOException or ObjectDisposedException or NotSupportedException) { + // A closed or unreadable stream is a failure to report, not a crash (issue #74). bytesWritten = 0; return false; } diff --git a/Essentials.HashProviders.SHA1/SHA1HashProvider.cs b/Essentials.HashProviders.SHA1/SHA1HashProvider.cs index 51ff011..b2a914b 100644 --- a/Essentials.HashProviders.SHA1/SHA1HashProvider.cs +++ b/Essentials.HashProviders.SHA1/SHA1HashProvider.cs @@ -94,8 +94,9 @@ public bool TryHash(Stream data, Span destination, out int bytesWritten) bytesWritten = 0; return false; } - catch (IOException) + catch (Exception ex) when (ex is IOException or ObjectDisposedException or NotSupportedException) { + // A closed or unreadable stream is a failure to report, not a crash (issue #74). bytesWritten = 0; return false; } diff --git a/Essentials.HashProviders.SHA256/SHA256HashProvider.cs b/Essentials.HashProviders.SHA256/SHA256HashProvider.cs index 120337e..72d63b1 100644 --- a/Essentials.HashProviders.SHA256/SHA256HashProvider.cs +++ b/Essentials.HashProviders.SHA256/SHA256HashProvider.cs @@ -92,8 +92,9 @@ public bool TryHash(Stream data, Span destination, out int bytesWritten) bytesWritten = 0; return false; } - catch (IOException) + catch (Exception ex) when (ex is IOException or ObjectDisposedException or NotSupportedException) { + // A closed or unreadable stream is a failure to report, not a crash (issue #74). bytesWritten = 0; return false; } diff --git a/Essentials.HashProviders.SHA384/SHA384HashProvider.cs b/Essentials.HashProviders.SHA384/SHA384HashProvider.cs index e1a9538..89fb171 100644 --- a/Essentials.HashProviders.SHA384/SHA384HashProvider.cs +++ b/Essentials.HashProviders.SHA384/SHA384HashProvider.cs @@ -92,8 +92,9 @@ public bool TryHash(Stream data, Span destination, out int bytesWritten) bytesWritten = 0; return false; } - catch (IOException) + catch (Exception ex) when (ex is IOException or ObjectDisposedException or NotSupportedException) { + // A closed or unreadable stream is a failure to report, not a crash (issue #74). bytesWritten = 0; return false; } diff --git a/Essentials.HashProviders.SHA512/SHA512HashProvider.cs b/Essentials.HashProviders.SHA512/SHA512HashProvider.cs index acd9e0f..ab78122 100644 --- a/Essentials.HashProviders.SHA512/SHA512HashProvider.cs +++ b/Essentials.HashProviders.SHA512/SHA512HashProvider.cs @@ -92,8 +92,9 @@ public bool TryHash(Stream data, Span destination, out int bytesWritten) bytesWritten = 0; return false; } - catch (IOException) + catch (Exception ex) when (ex is IOException or ObjectDisposedException or NotSupportedException) { + // A closed or unreadable stream is a failure to report, not a crash (issue #74). bytesWritten = 0; return false; } diff --git a/Essentials.Tests/UnreadableStreamHashTests.cs b/Essentials.Tests/UnreadableStreamHashTests.cs index 4356ccf..6bfdb55 100644 --- a/Essentials.Tests/UnreadableStreamHashTests.cs +++ b/Essentials.Tests/UnreadableStreamHashTests.cs @@ -56,6 +56,38 @@ private static FileStream CreateWriteOnlyStream(string path) => private static string TempPath() => Path.Join(Path.GetTempPath(), $"{nameof(UnreadableStreamHashTests)}_{Guid.NewGuid():N}.bin"); + [TestMethod] + [DynamicData(nameof(HashProviders))] + public void TryHash_Returns_False_For_An_Unreadable_Stream(IHashProvider provider, string providerName) + { + byte[] destination = new byte[provider.HashLengthBytes]; + + using MemoryStream disposed = CreateDisposedStream(); + Assert.IsFalse(provider.TryHash(disposed, destination, out _), $"{providerName} should report a disposed stream as a failure"); + Assert.ThrowsExactly(() => provider.Hash(disposed)); + + using FileStream writeOnly = CreateWriteOnlyStream(TempPath()); + Assert.IsFalse(provider.TryHash(writeOnly, destination, out _), $"{providerName} should report a write-only stream as a failure"); + Assert.ThrowsExactly(() => provider.Hash(writeOnly)); + } + + [TestMethod] + [DynamicData(nameof(KeyedHashProviders))] + public void Keyed_TryHash_Returns_False_For_An_Unreadable_Stream(IKeyedHashProvider provider, string providerName) + { + // The HMAC stream path caught only IOException, so a closed stream threw ObjectDisposedException out of a + // Try method (issue #74). + byte[] destination = new byte[provider.HashLengthBytes]; + + using MemoryStream disposed = CreateDisposedStream(); + Assert.IsFalse(provider.TryHash(Key, disposed, destination, out _), $"{providerName} should report a disposed stream as a failure"); + Assert.ThrowsExactly(() => provider.Hash(Key, disposed)); + + using FileStream writeOnly = CreateWriteOnlyStream(TempPath()); + Assert.IsFalse(provider.TryHash(Key, writeOnly, destination, out _), $"{providerName} should report a write-only stream as a failure"); + Assert.ThrowsExactly(() => provider.Hash(Key, writeOnly)); + } + [TestMethod] [DynamicData(nameof(HashProviders))] public async Task TryHashAsync_Returns_False_For_A_Disposed_Stream(IHashProvider provider, string providerName) diff --git a/Shared/HmacKeyedHashCore.cs b/Shared/HmacKeyedHashCore.cs index b4930bf..0aa7adc 100644 --- a/Shared/HmacKeyedHashCore.cs +++ b/Shared/HmacKeyedHashCore.cs @@ -119,8 +119,10 @@ internal static bool TryHash(HashAlgorithmName algorithm, int hashLengthBytes, R bytesWritten = 0; return false; } - catch (IOException) + catch (Exception ex) when (ex is IOException or ObjectDisposedException or NotSupportedException) { + // A closed stream, or one that cannot be read, is a failure to report rather than a crash, as it is + // for every other hash provider (issue #74). bytesWritten = 0; return false; } From f28c038e67e1909ac96b3d9fc2e8d4889f962167 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 11:00:07 +0000 Subject: [PATCH 12/23] Report unusable streams as failures in every stream Try* primitive Compression, AES and the encoders let the NotSupportedException the BCL raises for a write-only source or a read-only destination escape a Try method, and the self-contained obfuscators caught nothing at all. All of them now return false, and the self-allocating wrappers throw their documented InvalidOperationException. Fixes #86 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- .../BrotliCompressionProvider.cs | 16 +++ .../DeflateCompressionProvider.cs | 16 +++ .../GzipCompressionProvider.cs | 16 +++ .../ZLibCompressionProvider.cs | 16 +++ .../Base64EncodingProvider.cs | 16 +++ .../HexEncodingProvider.cs | 16 +++ .../AesEncryptionProvider.cs | 16 +++ .../BitRotateObfuscationProvider.cs | 38 +++-- .../CaesarObfuscationProvider.cs | 38 +++-- .../CompositeObfuscationProvider.cs | 42 ++++-- .../ReverseObfuscationProvider.cs | 21 ++- .../XorObfuscationProvider.cs | 23 ++- Essentials.Tests/UnusableStreamTests.cs | 135 ++++++++++++++++++ 13 files changed, 364 insertions(+), 45 deletions(-) create mode 100644 Essentials.Tests/UnusableStreamTests.cs diff --git a/Essentials.CompressionProviders.Brotli/BrotliCompressionProvider.cs b/Essentials.CompressionProviders.Brotli/BrotliCompressionProvider.cs index 8e07705..b9d12ff 100644 --- a/Essentials.CompressionProviders.Brotli/BrotliCompressionProvider.cs +++ b/Essentials.CompressionProviders.Brotli/BrotliCompressionProvider.cs @@ -102,6 +102,10 @@ public bool TryCompress(Stream data, Stream destination) { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -153,6 +157,10 @@ public async Task TryCompressAsync(Stream data, Stream destination, Cancel { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -223,6 +231,10 @@ public bool TryDecompress(Stream compressedData, Stream destination) { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -275,5 +287,9 @@ public async Task TryDecompressAsync(Stream compressedData, Stream destina { return false; } + catch (NotSupportedException) + { + return false; + } } } diff --git a/Essentials.CompressionProviders.Deflate/DeflateCompressionProvider.cs b/Essentials.CompressionProviders.Deflate/DeflateCompressionProvider.cs index cc0a3f6..fbbf0a1 100644 --- a/Essentials.CompressionProviders.Deflate/DeflateCompressionProvider.cs +++ b/Essentials.CompressionProviders.Deflate/DeflateCompressionProvider.cs @@ -102,6 +102,10 @@ public bool TryCompress(Stream data, Stream destination) { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -153,6 +157,10 @@ public async Task TryCompressAsync(Stream data, Stream destination, Cancel { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -223,6 +231,10 @@ public bool TryDecompress(Stream compressedData, Stream destination) { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -275,5 +287,9 @@ public async Task TryDecompressAsync(Stream compressedData, Stream destina { return false; } + catch (NotSupportedException) + { + return false; + } } } diff --git a/Essentials.CompressionProviders.Gzip/GzipCompressionProvider.cs b/Essentials.CompressionProviders.Gzip/GzipCompressionProvider.cs index 11300fe..c2ede50 100644 --- a/Essentials.CompressionProviders.Gzip/GzipCompressionProvider.cs +++ b/Essentials.CompressionProviders.Gzip/GzipCompressionProvider.cs @@ -102,6 +102,10 @@ public bool TryCompress(Stream data, Stream destination) { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -154,6 +158,10 @@ public async Task TryCompressAsync(Stream data, Stream destination, Cancel { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -224,6 +232,10 @@ public bool TryDecompress(Stream compressedData, Stream destination) { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -276,5 +288,9 @@ public async Task TryDecompressAsync(Stream compressedData, Stream destina { return false; } + catch (NotSupportedException) + { + return false; + } } } diff --git a/Essentials.CompressionProviders.ZLib/ZLibCompressionProvider.cs b/Essentials.CompressionProviders.ZLib/ZLibCompressionProvider.cs index 59a39b9..3cb3c87 100644 --- a/Essentials.CompressionProviders.ZLib/ZLibCompressionProvider.cs +++ b/Essentials.CompressionProviders.ZLib/ZLibCompressionProvider.cs @@ -102,6 +102,10 @@ public bool TryCompress(Stream data, Stream destination) { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -153,6 +157,10 @@ public async Task TryCompressAsync(Stream data, Stream destination, Cancel { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -223,6 +231,10 @@ public bool TryDecompress(Stream compressedData, Stream destination) { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -275,5 +287,9 @@ public async Task TryDecompressAsync(Stream compressedData, Stream destina { return false; } + catch (NotSupportedException) + { + return false; + } } } diff --git a/Essentials.EncodingProviders.Base64/Base64EncodingProvider.cs b/Essentials.EncodingProviders.Base64/Base64EncodingProvider.cs index 354f74d..0606db3 100644 --- a/Essentials.EncodingProviders.Base64/Base64EncodingProvider.cs +++ b/Essentials.EncodingProviders.Base64/Base64EncodingProvider.cs @@ -96,6 +96,10 @@ public bool TryEncode(Stream data, Stream destination) { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -177,6 +181,10 @@ public bool TryDecode(Stream encodedData, Stream destination) { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -220,6 +228,10 @@ public async Task TryEncodeAsync(Stream data, Stream destination, Cancella { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -254,6 +266,10 @@ public async Task TryDecodeAsync(Stream encodedData, Stream destination, C { return false; } + catch (NotSupportedException) + { + return false; + } } /// diff --git a/Essentials.EncodingProviders.Hex/HexEncodingProvider.cs b/Essentials.EncodingProviders.Hex/HexEncodingProvider.cs index 2751500..a4bd0a4 100644 --- a/Essentials.EncodingProviders.Hex/HexEncodingProvider.cs +++ b/Essentials.EncodingProviders.Hex/HexEncodingProvider.cs @@ -98,6 +98,10 @@ public bool TryEncode(Stream data, Stream destination) { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -162,6 +166,10 @@ public bool TryDecode(Stream encodedData, Stream destination) { return false; } + catch (NotSupportedException) + { + return false; + } } private static bool TryParseNibble(byte character, out int value) @@ -223,6 +231,10 @@ public async Task TryEncodeAsync(Stream data, Stream destination, Cancella { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -283,6 +295,10 @@ public async Task TryDecodeAsync(Stream encodedData, Stream destination, C { return false; } + catch (NotSupportedException) + { + return false; + } } /// diff --git a/Essentials.EncryptionProviders.Aes/AesEncryptionProvider.cs b/Essentials.EncryptionProviders.Aes/AesEncryptionProvider.cs index 0f4def2..8221586 100644 --- a/Essentials.EncryptionProviders.Aes/AesEncryptionProvider.cs +++ b/Essentials.EncryptionProviders.Aes/AesEncryptionProvider.cs @@ -149,6 +149,10 @@ public bool TryEncrypt(Stream data, ReadOnlySpan key, ReadOnlySpan i { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -205,6 +209,10 @@ public async Task TryEncryptAsync(Stream data, ReadOnlyMemory key, R { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -297,6 +305,10 @@ public bool TryDecrypt(Stream data, ReadOnlySpan key, ReadOnlySpan i { return false; } + catch (NotSupportedException) + { + return false; + } } /// @@ -354,6 +366,10 @@ public async Task TryDecryptAsync(Stream data, ReadOnlyMemory key, R { return false; } + catch (NotSupportedException) + { + return false; + } } /// diff --git a/Essentials.ObfuscationProviders.BitRotate/BitRotateObfuscationProvider.cs b/Essentials.ObfuscationProviders.BitRotate/BitRotateObfuscationProvider.cs index c79e64d..1196bf2 100644 --- a/Essentials.ObfuscationProviders.BitRotate/BitRotateObfuscationProvider.cs +++ b/Essentials.ObfuscationProviders.BitRotate/BitRotateObfuscationProvider.cs @@ -68,13 +68,22 @@ public bool TryObfuscate(Stream data, Stream destination) return false; } - int b; - while ((b = data.ReadByte()) >= 0) + try { - destination.WriteByte(RotateLeft((byte)b, _bits)); - } + int b; + while ((b = data.ReadByte()) >= 0) + { + destination.WriteByte(RotateLeft((byte)b, _bits)); + } - return true; + return true; + } + catch (Exception ex) when (ex is IOException or ObjectDisposedException or NotSupportedException) + { + // A stream that cannot be read or written is a failure to report, as the encoder-backed + // obfuscators already do (issue #86). + return false; + } } /// @@ -106,12 +115,21 @@ public bool TryDeobfuscate(Stream obfuscatedData, Stream destination) return false; } - int b; - while ((b = obfuscatedData.ReadByte()) >= 0) + try { - destination.WriteByte(RotateRight((byte)b, _bits)); - } + int b; + while ((b = obfuscatedData.ReadByte()) >= 0) + { + destination.WriteByte(RotateRight((byte)b, _bits)); + } - return true; + return true; + } + catch (Exception ex) when (ex is IOException or ObjectDisposedException or NotSupportedException) + { + // A stream that cannot be read or written is a failure to report, as the encoder-backed + // obfuscators already do (issue #86). + return false; + } } } diff --git a/Essentials.ObfuscationProviders.Caesar/CaesarObfuscationProvider.cs b/Essentials.ObfuscationProviders.Caesar/CaesarObfuscationProvider.cs index 365c48b..4b5c8ae 100644 --- a/Essentials.ObfuscationProviders.Caesar/CaesarObfuscationProvider.cs +++ b/Essentials.ObfuscationProviders.Caesar/CaesarObfuscationProvider.cs @@ -49,13 +49,22 @@ public bool TryObfuscate(Stream data, Stream destination) return false; } - int b; - while ((b = data.ReadByte()) >= 0) + try { - destination.WriteByte((byte)(b + _shift)); - } + int b; + while ((b = data.ReadByte()) >= 0) + { + destination.WriteByte((byte)(b + _shift)); + } - return true; + return true; + } + catch (Exception ex) when (ex is IOException or ObjectDisposedException or NotSupportedException) + { + // A stream that cannot be read or written is a failure to report, as the encoder-backed + // obfuscators already do (issue #86). + return false; + } } /// @@ -87,12 +96,21 @@ public bool TryDeobfuscate(Stream obfuscatedData, Stream destination) return false; } - int b; - while ((b = obfuscatedData.ReadByte()) >= 0) + try { - destination.WriteByte((byte)(b - _shift)); - } + int b; + while ((b = obfuscatedData.ReadByte()) >= 0) + { + destination.WriteByte((byte)(b - _shift)); + } - return true; + return true; + } + catch (Exception ex) when (ex is IOException or ObjectDisposedException or NotSupportedException) + { + // A stream that cannot be read or written is a failure to report, as the encoder-backed + // obfuscators already do (issue #86). + return false; + } } } diff --git a/Essentials.ObfuscationProviders.Composite/CompositeObfuscationProvider.cs b/Essentials.ObfuscationProviders.Composite/CompositeObfuscationProvider.cs index e9e5401..696fce7 100644 --- a/Essentials.ObfuscationProviders.Composite/CompositeObfuscationProvider.cs +++ b/Essentials.ObfuscationProviders.Composite/CompositeObfuscationProvider.cs @@ -89,15 +89,24 @@ public bool TryObfuscate(Stream data, Stream destination) return false; } - using MemoryStream buffer = new(); - data.CopyTo(buffer); - if (!TryRunForward(buffer.ToArray(), out byte[] current)) + try { + using MemoryStream buffer = new(); + data.CopyTo(buffer); + if (!TryRunForward(buffer.ToArray(), out byte[] current)) + { + return false; + } + + destination.Write(current, 0, current.Length); + return true; + } + catch (Exception ex) when (ex is IOException or ObjectDisposedException or NotSupportedException) + { + // A stream that cannot be read or written is a failure to report, as the encoder-backed + // obfuscators already do (issue #86). return false; } - - destination.Write(current, 0, current.Length); - return true; } /// @@ -128,15 +137,24 @@ public bool TryDeobfuscate(Stream obfuscatedData, Stream destination) return false; } - using MemoryStream buffer = new(); - obfuscatedData.CopyTo(buffer); - if (!TryRunReverse(buffer.ToArray(), out byte[] current)) + try { + using MemoryStream buffer = new(); + obfuscatedData.CopyTo(buffer); + if (!TryRunReverse(buffer.ToArray(), out byte[] current)) + { + return false; + } + + destination.Write(current, 0, current.Length); + return true; + } + catch (Exception ex) when (ex is IOException or ObjectDisposedException or NotSupportedException) + { + // A stream that cannot be read or written is a failure to report, as the encoder-backed + // obfuscators already do (issue #86). return false; } - - destination.Write(current, 0, current.Length); - return true; } /// diff --git a/Essentials.ObfuscationProviders.Reverse/ReverseObfuscationProvider.cs b/Essentials.ObfuscationProviders.Reverse/ReverseObfuscationProvider.cs index 42877c3..9117d08 100644 --- a/Essentials.ObfuscationProviders.Reverse/ReverseObfuscationProvider.cs +++ b/Essentials.ObfuscationProviders.Reverse/ReverseObfuscationProvider.cs @@ -45,12 +45,21 @@ public bool TryObfuscate(Stream data, Stream destination) return false; } - using MemoryStream buffer = new(); - data.CopyTo(buffer); - byte[] bytes = buffer.ToArray(); - Array.Reverse(bytes); - destination.Write(bytes, 0, bytes.Length); - return true; + try + { + using MemoryStream buffer = new(); + data.CopyTo(buffer); + byte[] bytes = buffer.ToArray(); + Array.Reverse(bytes); + destination.Write(bytes, 0, bytes.Length); + return true; + } + catch (Exception ex) when (ex is IOException or ObjectDisposedException or NotSupportedException) + { + // A stream that cannot be read or written is a failure to report, as the encoder-backed + // obfuscators already do (issue #86). + return false; + } } /// diff --git a/Essentials.ObfuscationProviders.Xor/XorObfuscationProvider.cs b/Essentials.ObfuscationProviders.Xor/XorObfuscationProvider.cs index a11940e..8185ad9 100644 --- a/Essentials.ObfuscationProviders.Xor/XorObfuscationProvider.cs +++ b/Essentials.ObfuscationProviders.Xor/XorObfuscationProvider.cs @@ -66,15 +66,24 @@ public bool TryObfuscate(Stream data, Stream destination) return false; } - int b; - long i = 0; - while ((b = data.ReadByte()) >= 0) + try { - destination.WriteByte((byte)(b ^ _key[(int)(i % _key.Length)])); - i++; + int b; + long i = 0; + while ((b = data.ReadByte()) >= 0) + { + destination.WriteByte((byte)(b ^ _key[(int)(i % _key.Length)])); + i++; + } + + return true; + } + catch (Exception ex) when (ex is IOException or ObjectDisposedException or NotSupportedException) + { + // A stream that cannot be read or written is a failure to report, as the encoder-backed + // obfuscators already do (issue #86). + return false; } - - return true; } /// diff --git a/Essentials.Tests/UnusableStreamTests.cs b/Essentials.Tests/UnusableStreamTests.cs new file mode 100644 index 0000000..f06122e --- /dev/null +++ b/Essentials.Tests/UnusableStreamTests.cs @@ -0,0 +1,135 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Essentials.Tests; + +using System; +using System.Collections.Generic; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using ktsu.Essentials; +using ktsu.Essentials.All; +using ktsu.Essentials.ObfuscationProviders.Composite; +using ktsu.Essentials.ObfuscationProviders.Reverse; +using ktsu.Essentials.ObfuscationProviders.Xor; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Every stream Try* primitive in the compression, encryption, encoding and obfuscation providers reports a +/// source it cannot read, or a destination it cannot write, as rather than letting the +/// the BCL raises escape (issue #86), and the self-allocating wrapper turns that +/// into its documented . +/// +[TestClass] +public sealed class UnusableStreamTests +{ + private static readonly byte[] Payload = [.. System.Text.Encoding.UTF8.GetBytes("The quick brown fox jumps over the lazy dog, twice over.")]; + + public TestContext TestContext { get; set; } = null!; + + public static IEnumerable Transforms + { + get + { + ServiceCollection services = new(); + services.AddCompressionProviders().AddEncryptionProviders().AddEncodingProviders().AddObfuscationProviders(); + using ServiceProvider provider = services.BuildServiceProvider(); + + foreach (ICompressionProvider compression in provider.GetServices()) + { + string name = compression.GetType().Name; + yield return [new StreamTransform($"{name} compress", compression.TryCompress, compression.TryCompressAsync, compression.Compress, Payload)]; + yield return [new StreamTransform($"{name} decompress", compression.TryDecompress, compression.TryDecompressAsync, compression.Decompress, compression.Compress(Payload))]; + } + + foreach (IEncryptionProvider encryption in provider.GetServices()) + { + string name = encryption.GetType().Name; + byte[] key = encryption.GenerateKey(); + byte[] iv = encryption.GenerateIV(); + yield return [new StreamTransform( + $"{name} encrypt", + (s, d) => encryption.TryEncrypt(s, key, iv, d), + (s, d, ct) => encryption.TryEncryptAsync(s, key, iv, d, ct), + s => encryption.Encrypt(s, key, iv), + Payload)]; + yield return [new StreamTransform( + $"{name} decrypt", + (s, d) => encryption.TryDecrypt(s, key, iv, d), + (s, d, ct) => encryption.TryDecryptAsync(s, key, iv, d, ct), + s => encryption.Decrypt(s, key, iv), + encryption.Encrypt(Payload, key, iv))]; + } + + foreach (IEncodingProvider encoding in provider.GetServices()) + { + string name = encoding.GetType().Name; + yield return [new StreamTransform($"{name} encode", encoding.TryEncode, encoding.TryEncodeAsync, encoding.Encode, Payload)]; + yield return [new StreamTransform($"{name} decode", encoding.TryDecode, encoding.TryDecodeAsync, encoding.Decode, encoding.Encode(Payload))]; + } + + List obfuscators = [.. provider.GetServices()]; + obfuscators.Add(new CompositeObfuscationProvider([new XorObfuscationProvider(), new ReverseObfuscationProvider()])); + foreach (IObfuscationProvider obfuscation in obfuscators) + { + string name = obfuscation.GetType().Name; + yield return [new StreamTransform($"{name} obfuscate", obfuscation.TryObfuscate, obfuscation.TryObfuscateAsync, obfuscation.Obfuscate, Payload)]; + yield return [new StreamTransform($"{name} deobfuscate", obfuscation.TryDeobfuscate, obfuscation.TryDeobfuscateAsync, obfuscation.Deobfuscate, obfuscation.Obfuscate(Payload))]; + } + } + } + + [TestMethod] + [DynamicData(nameof(Transforms))] + public async Task A_Source_That_Cannot_Be_Read_Is_Reported_As_A_Failure(StreamTransform transform) + { + ArgumentNullException.ThrowIfNull(transform); + + string path = Path.Join(Path.GetTempPath(), $"{nameof(UnusableStreamTests)}_{Guid.NewGuid():N}.bin"); + using FileStream source = new(path, FileMode.Create, FileAccess.Write, FileShare.None, 4096, FileOptions.DeleteOnClose); + await source.WriteAsync(transform.Input, TestContext.CancellationToken).ConfigureAwait(false); + source.Position = 0; + + using MemoryStream destination = new(); + Assert.IsFalse(transform.Try(source, destination), $"{transform} read a write-only source"); + Assert.IsFalse(await transform.TryAsync(source, destination, TestContext.CancellationToken).ConfigureAwait(false), $"{transform} read a write-only source asynchronously"); + Assert.ThrowsExactly(() => transform.Allocate(source), $"{transform} did not wrap the failure"); + } + + [TestMethod] + [DynamicData(nameof(Transforms))] + public async Task A_Destination_That_Cannot_Be_Written_Is_Reported_As_A_Failure(StreamTransform transform) + { + ArgumentNullException.ThrowIfNull(transform); + + using MemoryStream destination = new(new byte[4096], writable: false); + + using (MemoryStream source = new(transform.Input.ToArray())) + { + Assert.IsFalse(transform.Try(source, destination), $"{transform} wrote to a read-only destination"); + } + + using (MemoryStream source = new(transform.Input.ToArray())) + { + Assert.IsFalse(await transform.TryAsync(source, destination, TestContext.CancellationToken).ConfigureAwait(false), $"{transform} wrote to a read-only destination asynchronously"); + } + } + + /// One provider's stream primitives in one direction, and an input it accepts. + /// The provider and direction, shown as the test case's name. + /// The synchronous stream primitive. + /// The asynchronous stream primitive. + /// The self-allocating stream wrapper. + /// Bytes the transform accepts. + public sealed record StreamTransform( + string Name, + Func Try, + Func> TryAsync, + Func Allocate, + ReadOnlyMemory Input) + { + /// + public override string ToString() => Name; + } +} From 56096407efa25c4c24964d6fc7d8d10cdbf964da Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 11:05:22 +0000 Subject: [PATCH 13/23] Keep zero-weight outcomes out of categorical draws and weighted Choose Weights such as [1e308, 1e308, 0] summed to infinity, so every share was zero and the last outcome took every draw although its weight was zero. The weights are now summed divided by the largest when the plain total overflows. The categorical CDF is pinned to one from the last outcome with weight onward, so a trailing zero weight no longer inherits the rounding gap below one. Fixes #78 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- .../CategoricalDistributionProvider.cs | 32 +++++++++++++-- Essentials.Tests/DistributionProviderTests.cs | 39 +++++++++++++++++++ Essentials.Tests/RandomProviderTests.cs | 23 +++++++++++ Essentials/IRandomProvider.cs | 4 +- Essentials/RandomHelpers.cs | 33 +++++++++++++--- 5 files changed, 120 insertions(+), 11 deletions(-) diff --git a/Essentials.DistributionProviders.Categorical/CategoricalDistributionProvider.cs b/Essentials.DistributionProviders.Categorical/CategoricalDistributionProvider.cs index f0c81b8..4504e14 100644 --- a/Essentials.DistributionProviders.Categorical/CategoricalDistributionProvider.cs +++ b/Essentials.DistributionProviders.Categorical/CategoricalDistributionProvider.cs @@ -47,6 +47,8 @@ public CategoricalDistributionProvider(IReadOnlyList weights) } double total = 0.0; + double largest = 0.0; + int lastNonZero = 0; for (int i = 0; i < weights.Count; i++) { double weight = weights[i]; @@ -56,6 +58,11 @@ public CategoricalDistributionProvider(IReadOnlyList weights) } total += weight; + largest = Math.Max(largest, weight); + if (weight > 0.0) + { + lastNonZero = i; + } } if (total <= 0.0) @@ -63,6 +70,19 @@ public CategoricalDistributionProvider(IReadOnlyList weights) throw new ArgumentException("At least one weight must be greater than zero.", nameof(weights)); } + // Every weight is finite but the sum need not be, and every share of an infinite total is zero (issue #78). + // Divided by the largest, the weights sum to at most their count. + double divisor = 1.0; + if (double.IsInfinity(total)) + { + divisor = largest; + total = 0.0; + for (int i = 0; i < weights.Count; i++) + { + total += weights[i] / divisor; + } + } + probabilities = new double[weights.Count]; cumulative = new double[weights.Count]; double running = 0.0; @@ -70,7 +90,7 @@ public CategoricalDistributionProvider(IReadOnlyList weights) double secondMoment = 0.0; for (int i = 0; i < weights.Count; i++) { - double probability = weights[i] / total; + double probability = weights[i] / divisor / total; probabilities[i] = probability; running += probability; cumulative[i] = running; @@ -78,9 +98,13 @@ public CategoricalDistributionProvider(IReadOnlyList weights) secondMoment += (double)i * i * probability; } - // The running total lands a rounding step either side of one; pinning the last entry keeps the - // CDF from ever reporting a value below one at the top of the support. - cumulative[^1] = 1.0; + // The running total lands a rounding step either side of one. Pinning it from the last outcome that can + // occur keeps the CDF from reporting a value below one at the top of the support, and gives no mass to + // trailing outcomes weighted zero, which pinning only the last entry handed the rounding gap (issue #78). + for (int i = lastNonZero; i < cumulative.Length; i++) + { + cumulative[i] = 1.0; + } upper = new double[weights.Count]; double above = 0.0; diff --git a/Essentials.Tests/DistributionProviderTests.cs b/Essentials.Tests/DistributionProviderTests.cs index 4984d73..32e894d 100644 --- a/Essentials.Tests/DistributionProviderTests.cs +++ b/Essentials.Tests/DistributionProviderTests.cs @@ -946,6 +946,45 @@ public void Categorical_Never_Draws_A_Zero_Weighted_Outcome() } } + [TestMethod] + public void Categorical_Splits_Weights_Whose_Total_Overflows() + { + // The weights summed to infinity, so every mass was zero and the pinned last entry took every draw, + // though its weight was zero (issue #78). + CategoricalDistributionProvider distribution = new([1e308, 1e308, 0.0]); + + Assert.AreEqual(0.5, distribution.Pmf(0), Tight); + Assert.AreEqual(0.5, distribution.Pmf(1), Tight); + Assert.AreEqual(0.0, distribution.Pmf(2)); + Assert.AreEqual(1.0, distribution.Pmf(0) + distribution.Pmf(1) + distribution.Pmf(2), Tight); + Assert.AreEqual(0.5, distribution.Mean, Tight); + + IDiscreteDistribution discrete = distribution; + IRandomProvider random = SeededRandom(); + int first = 0; + for (int i = 0; i < 20000; i++) + { + int sample = discrete.Sample(random); + Assert.AreNotEqual(2, sample); + first += sample == 0 ? 1 : 0; + } + + Assert.IsTrue(Math.Abs(first - 10000) < 400, $"drew the first outcome {first} times out of 20000"); + } + + [TestMethod] + public void Categorical_Gives_A_Trailing_Zero_Weight_None_Of_The_Rounding_Gap() + { + // These weights sum to a CDF of 0.9999999999999998 at outcome 6, and the last entry was pinned to one, so + // the quantile just below one named outcome 7, whose weight is zero (issue #78). + IDiscreteDistribution distribution = new CategoricalDistributionProvider( + [0.79, 0.37, 0.58, 0.01, 0.05, 0.18, 0.96, 0.0]); + + Assert.AreEqual(1.0, distribution.Cdf(6)); + Assert.AreEqual(6, distribution.Quantile(1.0 - Math.Pow(2, -53))); + Assert.AreEqual(6, distribution.Quantile(1.0)); + } + // ---- argument validation ------------------------------------------------------------------- [TestMethod] diff --git a/Essentials.Tests/RandomProviderTests.cs b/Essentials.Tests/RandomProviderTests.cs index fa77fe1..1d9ee66 100644 --- a/Essentials.Tests/RandomProviderTests.cs +++ b/Essentials.Tests/RandomProviderTests.cs @@ -294,6 +294,29 @@ public void Choose_Never_Returns_A_Zero_Weighted_Element(IRandomProvider random, Assert.IsTrue(Math.Abs(often - (SampleCount * 0.9)) < SampleCount * 0.02, $"{name} chose 'often' {often} times out of {SampleCount}"); } + [TestMethod] + [DynamicData(nameof(RandomProviders))] + public void Choose_Splits_Weights_Whose_Total_Overflows(IRandomProvider random, string name) + { + // The weights summed to infinity, every share of it was zero, and the fallback took the last element + // every time (issue #78). + int[] items = [0, 1, 2]; + double[] weights = [1e308, 1e308, 0.0]; + + int first = 0; + for (int i = 0; i < SampleCount; i++) + { + int chosen = random.Choose(items, weights); + Assert.AreNotEqual(2, chosen, name); + if (chosen == 0) + { + first++; + } + } + + Assert.IsTrue(Math.Abs(first - (SampleCount * 0.5)) < SampleCount * 0.02, $"{name} chose the first element {first} times out of {SampleCount}"); + } + [TestMethod] [DynamicData(nameof(RandomProviders))] public void Choose_Rejects_Malformed_Weights(IRandomProvider random, string name) diff --git a/Essentials/IRandomProvider.cs b/Essentials/IRandomProvider.cs index c61b7c4..05efe29 100644 --- a/Essentials/IRandomProvider.cs +++ b/Essentials/IRandomProvider.cs @@ -284,12 +284,12 @@ public T Choose(IReadOnlyList items, IReadOnlyList weights) throw new ArgumentException($"Expected {items.Count} weights to match the collection, but got {weights.Count}.", nameof(weights)); } - double total = RandomHelpers.SumWeights(weights, nameof(weights)); + double total = RandomHelpers.SumWeights(weights, nameof(weights), out double divisor); double target = NextDouble() * total; double cumulative = 0.0; for (int i = 0; i < items.Count; i++) { - cumulative += weights[i]; + cumulative += weights[i] / divisor; if (target < cumulative) { return items[i]; diff --git a/Essentials/RandomHelpers.cs b/Essentials/RandomHelpers.cs index b4ed213..596a67b 100644 --- a/Essentials/RandomHelpers.cs +++ b/Essentials/RandomHelpers.cs @@ -131,13 +131,21 @@ internal static void EnsureProbability(double value, string name) /// /// Adds up a weight list, rejecting negative, non-finite, and all-zero weights. /// + /// + /// Each weight is finite but their sum need not be: [1e308, 1e308] adds up to infinity, and every share + /// of an infinite total is zero (issue #78). When that happens the weights are summed again divided by the + /// largest, which cannot overflow, and reports what to divide each weight by so its + /// share is measured against the same total. Otherwise the divisor is one and the weights are used as given. + /// /// The weights to total. /// The parameter name to report. - /// The sum of the weights. + /// What every weight is divided by before it is compared with the returned total. + /// The sum of the weights, each divided by . /// A weight is negative or not finite, or they sum to zero. - internal static double SumWeights(IReadOnlyList weights, string name) + internal static double SumWeights(IReadOnlyList weights, string name, out double divisor) { double total = 0.0; + double largest = 0.0; for (int i = 0; i < weights.Count; i++) { double weight = weights[i]; @@ -147,11 +155,26 @@ internal static double SumWeights(IReadOnlyList weights, string name) } total += weight; + largest = Math.Max(largest, weight); + } + + if (total <= 0.0) + { + throw new ArgumentException("At least one weight must be greater than zero.", name); + } + + divisor = 1.0; + if (double.IsInfinity(total)) + { + divisor = largest; + total = 0.0; + for (int i = 0; i < weights.Count; i++) + { + total += weights[i] / divisor; + } } - return total > 0.0 - ? total - : throw new ArgumentException("At least one weight must be greater than zero.", name); + return total; } /// From 11a276c3ed157c43fbc097aa486a7e711d31f792 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 11:10:58 +0000 Subject: [PATCH 14/23] Round trip TOML through Tomlyn's model binding in both directions Serialize wrote snake_case names through Tomlyn while Deserialize used a hand-written mapper that looked for the PascalCase name, skipped lists and nested tables, and threw MissingMethodException for types without a parameterless constructor. Deserialize now binds with Tomlyn's typed ToModel under the same options Serialize writes with, ignores unknown keys, and returns default for a type Tomlyn cannot create. Fixes #69 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- .../TomlSerializationProvider.cs | 91 ++++++++++-------- .../SerializationRoundTripTests.cs | 92 +++++++++++++++++++ 2 files changed, 147 insertions(+), 36 deletions(-) create mode 100644 Essentials.Tests/SerializationRoundTripTests.cs diff --git a/Essentials.SerializationProviders.Toml/TomlSerializationProvider.cs b/Essentials.SerializationProviders.Toml/TomlSerializationProvider.cs index b0593ce..94e0227 100644 --- a/Essentials.SerializationProviders.Toml/TomlSerializationProvider.cs +++ b/Essentials.SerializationProviders.Toml/TomlSerializationProvider.cs @@ -6,20 +6,28 @@ namespace ktsu.Essentials.SerializationProviders.Toml; using System; using System.Collections; using System.Collections.Generic; -using System.Globalization; using System.IO; using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.Serialization; using System.Text; using Tomlyn; -using Tomlyn.Model; /// /// A serialization provider that uses Tomlyn for serializing and deserializing objects. /// +/// +/// Both directions go through Tomlyn's model binding with the same options, so a property is written and read +/// under one name: DisplayName becomes display_name on the way out and is matched by it on the way +/// back, and lists and nested objects round trip. Keys with no matching property are ignored. Deserializing +/// needs a class with a public parameterless constructor; any other type, such as a positional record or a +/// struct, deserializes to . +/// public class TomlSerializationProvider : ISerializationProvider { + /// The options both directions bind with, so names convert the same way in each. + private static readonly TomlModelOptions ModelOptions = new() { IgnoreMissingProperties = true }; + /// /// Gets the conventional file extension for TOML, including the leading dot. /// @@ -47,7 +55,7 @@ public bool TrySerialize(object obj, TextWriter writer) try { - string toml = Tomlyn.Toml.FromModel(obj); + string toml = Tomlyn.Toml.FromModel(obj, ModelOptions); writer.Write(toml); return true; } @@ -78,17 +86,16 @@ public bool TrySerialize(object obj, TextWriter writer) return default; } + Func? bind = ModelBinder.Bind; + if (bind is null) + { + return default; + } + try { string content = Encoding.UTF8.GetString(Utf8Bom.Skip(data)); - TomlTable table = Tomlyn.Toml.ToModel(content); - object? result = ConvertToModel(table); - if (result is T typedResult) - { - return typedResult; - } - - return default; + return bind(content); } catch (TomlException) { @@ -98,6 +105,18 @@ public bool TrySerialize(object obj, TextWriter writer) { return default; } + catch (InvalidOperationException) + { + return default; + } + catch (InvalidCastException) + { + return default; + } + catch (MissingMethodException) + { + return default; + } } /// @@ -218,36 +237,36 @@ private sealed class ReferenceComparer : IEqualityComparer public int GetHashCode(object obj) => RuntimeHelpers.GetHashCode(obj); } - private static T? ConvertToModel(TomlTable table) + private static TModel BindModel(string content) + where TModel : class, new() + => Tomlyn.Toml.ToModel(content, sourcePath: null, ModelOptions); + + /// + /// Tomlyn's typed ToModel for , made once per type, or + /// when is not a class with a public parameterless constructor, which Tomlyn needs to + /// create it. A delegate rather than a reflected call each time, so a binding failure arrives as itself rather + /// than wrapped in a . + /// + private static class ModelBinder { - Type type = typeof(T); - object instance = Activator.CreateInstance(type)!; + internal static readonly Func? Bind = Create(); - foreach (KeyValuePair kvp in table) + private static Func? Create() { - PropertyInfo? property = type.GetProperty(kvp.Key, BindingFlags.Public | BindingFlags.Instance | BindingFlags.IgnoreCase); - if (property is not null && property.CanWrite) + Type type = typeof(T); + if (!type.IsClass || type.IsAbstract || type.GetConstructor(Type.EmptyTypes) is null) { - try - { - object? value = Convert.ChangeType(kvp.Value, property.PropertyType, CultureInfo.InvariantCulture); - property.SetValue(instance, value); - } - catch (InvalidCastException) - { - // Skip properties that can't be converted - } - catch (FormatException) - { - // Skip properties that can't be converted - } - catch (OverflowException) - { - // Skip properties that can't be converted - } + return null; } - } - return (T)instance; + MethodInfo method = typeof(TomlSerializationProvider) + .GetMethod(nameof(BindModel), BindingFlags.NonPublic | BindingFlags.Static)! + .MakeGenericMethod(type); +#if NET5_0_OR_GREATER + return method.CreateDelegate>(); +#else + return (Func)method.CreateDelegate(typeof(Func)); +#endif + } } } diff --git a/Essentials.Tests/SerializationRoundTripTests.cs b/Essentials.Tests/SerializationRoundTripTests.cs new file mode 100644 index 0000000..6276aa3 --- /dev/null +++ b/Essentials.Tests/SerializationRoundTripTests.cs @@ -0,0 +1,92 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Essentials.Tests; + +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.Diagnostics.CodeAnalysis; +using ktsu.Essentials; +using ktsu.Essentials.SerializationProviders.Json; +using ktsu.Essentials.SerializationProviders.NewtonsoftJson; +using ktsu.Essentials.SerializationProviders.Toml; +using ktsu.Essentials.SerializationProviders.Yaml; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// A realistic settings object, with a multi-word property, a list and a nested object, comes back from every +/// serializer as it went in. TOML wrote display_name and read it back looking for DisplayName, and +/// dropped lists and nested tables, so most of such an object was lost on the way back (issue #69). +/// +[TestClass] +public sealed class SerializationRoundTripTests +{ + private static readonly string[] ExpectedTags = ["a", "b"]; + + public static IEnumerable Serializers => + [ + [new JsonSerializationProvider(), nameof(JsonSerializationProvider)], + [new NewtonsoftJsonSerializationProvider(), nameof(NewtonsoftJsonSerializationProvider)], + [new YamlSerializationProvider(), nameof(YamlSerializationProvider)], + [new TomlSerializationProvider(), nameof(TomlSerializationProvider)], + ]; + + [TestMethod] + [DynamicData(nameof(Serializers))] + public void A_Settings_Object_Survives_A_Round_Trip(ISerializationProvider serializer, string name) + { + Settings original = new() + { + DisplayName = "hi", + MaxCount = 5, + Tags = ["a", "b"], + Inner = new() { Value = 3 }, + }; + + string text = serializer.Serialize(original); + Settings? back = serializer.Deserialize(text); + + Assert.IsNotNull(back, $"{name} could not read back what it wrote:\n{text}"); + Assert.AreEqual("hi", back.DisplayName, $"{name} lost a multi-word property:\n{text}"); + Assert.AreEqual(5, back.MaxCount, $"{name} lost a multi-word property:\n{text}"); + CollectionAssert.AreEqual(ExpectedTags, back.Tags, $"{name} lost a list:\n{text}"); + Assert.AreEqual(3, back.Inner?.Value, $"{name} lost a nested object:\n{text}"); + } + + [TestMethod] + public void Toml_Returns_Default_For_A_Type_It_Cannot_Create() + { + // Activator.CreateInstance threw MissingMethodException out of Deserialize for a positional record. + ISerializationProvider serializer = new TomlSerializationProvider(); + + Assert.IsNull(serializer.Deserialize("a = 1")); + Assert.IsNull(serializer.Deserialize("a = 1")); + } + + [TestMethod] + public void Toml_Ignores_Keys_With_No_Property() + { + ISerializationProvider serializer = new TomlSerializationProvider(); + Settings? back = serializer.Deserialize("display_name = \"hi\"\nunknown = 1\n"); + + Assert.AreEqual("hi", back?.DisplayName); + } + + public sealed class Settings + { + public string DisplayName { get; set; } = string.Empty; + + public int MaxCount { get; set; } + + [SuppressMessage("Usage", "CA2227:Collection properties should be read only", Justification = "A settable list is what a settings class written for a serializer usually has, and not every serializer here fills a read-only one.")] + public Collection Tags { get; set; } = []; + + public Inner? Inner { get; set; } + } + + public sealed class Inner + { + public int Value { get; set; } + } + + public sealed record Positional(int A); +} From 5383dd475ea4e0287cff4a9952a7193ba5e56aa0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 11:15:01 +0000 Subject: [PATCH 15/23] Make a log roll all or nothing, and append when it is refused Roll deleted the oldest file and shifted the rest before moving the active one, so while a viewer held the log open without delete sharing every entry deleted another rolled file until no history was left. Each move is now recorded and undone if a later one fails, the oldest file is deleted only after the roll completes, and an entry whose roll is refused is appended past the limit rather than dropped. Fixes #120 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- CLAUDE.md | 5 +- .../FileLoggingOptions.cs | 3 +- .../FileLoggingProvider.cs | 96 ++++++++++++++++--- Essentials.Tests/FileLoggingProviderTests.cs | 38 ++++++++ 4 files changed, 127 insertions(+), 15 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 665a503..142ab58 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -154,7 +154,10 @@ the line, including inside a `///` comment, which is how the doc comment broke w Rolling happens before an entry that would push a non-empty file past `MaxFileSizeBytes`, so no file exceeds the limit unless a single entry does, and such an entry is written whole to a fresh file. `MaxFileCount` -counts the current file, so 1 means a full file is deleted and started again. +counts the current file, so 1 means a full file is deleted and started again. A roll is all or nothing: the active file and every rolled one move under unique `.rolling` +staging names or up a slot, each move recorded and undone in reverse if a later one is refused, and the +oldest is deleted only after the roll is complete. A refused roll (a viewer holding the file without +`FileShare.Delete` on Windows) appends the entry past the limit rather than dropping it. See issue #120. ### New packages and the validation baseline diff --git a/Essentials.LoggingProviders.File/FileLoggingOptions.cs b/Essentials.LoggingProviders.File/FileLoggingOptions.cs index 133110e..54a2d25 100644 --- a/Essentials.LoggingProviders.File/FileLoggingOptions.cs +++ b/Essentials.LoggingProviders.File/FileLoggingOptions.cs @@ -38,7 +38,8 @@ public class FileLoggingOptions /// /// /// A file is never left empty to honour the limit: an entry larger than the limit on its own is - /// written whole to a fresh file. + /// written whole to a fresh file. A file can also grow past the limit while another program holds it + /// open without delete sharing, which stops it being rolled; entries are appended until it can be. /// public long MaxFileSizeBytes { get; set; } = DefaultMaxFileSizeBytes; diff --git a/Essentials.LoggingProviders.File/FileLoggingProvider.cs b/Essentials.LoggingProviders.File/FileLoggingProvider.cs index d87dca3..69d98c3 100644 --- a/Essentials.LoggingProviders.File/FileLoggingProvider.cs +++ b/Essentials.LoggingProviders.File/FileLoggingProvider.cs @@ -3,6 +3,7 @@ namespace ktsu.Essentials.LoggingProviders.File; using System; +using System.Collections.Generic; using System.Globalization; using System.IO; using System.Text; @@ -29,7 +30,9 @@ namespace ktsu.Essentials.LoggingProviders.File; /// /// /// Logging never throws for an I/O failure. An entry that cannot be written, because the disk is -/// full or the file is locked, is dropped and counted in . +/// full or the file is locked, is dropped and counted in . A roll that +/// cannot be done, because another program holds a file without delete sharing, leaves every file +/// where it was, and the entry is appended to the active file past the size limit until a roll succeeds. /// /// public class FileLoggingProvider : ILoggingProvider @@ -231,9 +234,11 @@ private void Write(string entry) { fileSystem.Directory.CreateDirectory(Directory); long length = fileSystem.File.Exists(FilePath) ? fileSystem.FileInfo.New(FilePath).Length : 0; + // A roll that cannot be done, because another program holds the file, leaves every file as it was, + // and the entry is appended past the limit rather than dropped: the write itself still succeeds. if (length > 0 && length + bytes.Length > maxFileSizeBytes) { - Roll(); + _ = TryRoll(); } using Stream stream = fileSystem.FileStream.New(FilePath, FileMode.Append, FileAccess.Write, FileShare.ReadWrite | FileShare.Delete); @@ -250,29 +255,94 @@ private void Write(string entry) } } - private void Roll() + /// + /// Moves the active file into slot 1, shifting the rolled files up a slot and discarding the oldest, all or + /// nothing. Every move is recorded, and a failure undoes them in reverse, so a roll refused part way leaves + /// every file where it was. The oldest file is moved aside rather than deleted, and deleted only once the roll + /// is complete: deleting it first meant each refused roll discarded another file, until a log a viewer held + /// open had lost all its history (issue #120). + /// + /// if the active file was rolled. + private bool TryRoll() { if (maxFileCount == 1) { - fileSystem.File.Delete(FilePath); - return; + try + { + fileSystem.File.Delete(FilePath); + return true; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + return false; + } } - string oldest = GetRolledFilePath(maxFileCount - 1); - if (fileSystem.File.Exists(oldest)) + // Unique names, so a staging file a crash left behind cannot block every later roll. + string activeStaging = StagingPath(); + string? oldestStaging = null; + List<(string From, string To)> moves = []; + + try + { + Move(FilePath, activeStaging); + + string oldest = GetRolledFilePath(maxFileCount - 1); + if (fileSystem.File.Exists(oldest)) + { + oldestStaging = StagingPath(); + Move(oldest, oldestStaging); + } + + for (int index = maxFileCount - 2; index >= 1; index--) + { + string source = GetRolledFilePath(index); + if (fileSystem.File.Exists(source)) + { + Move(source, GetRolledFilePath(index + 1)); + } + } + + Move(activeStaging, GetRolledFilePath(1)); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { - fileSystem.File.Delete(oldest); + for (int i = moves.Count - 1; i >= 0; i--) + { + try + { + fileSystem.File.Move(moves[i].To, moves[i].From); + } + catch (Exception undo) when (undo is IOException or UnauthorizedAccessException) + { + // Nothing more can be done for this file; the rest are still put back. + } + } + + return false; } - for (int index = maxFileCount - 2; index >= 1; index--) + if (oldestStaging is not null) { - string source = GetRolledFilePath(index); - if (fileSystem.File.Exists(source)) + try + { + fileSystem.File.Delete(oldestStaging); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { - fileSystem.File.Move(source, GetRolledFilePath(index + 1)); + // The roll is complete; a discarded file another program holds open is left for it to release. } } - fileSystem.File.Move(FilePath, GetRolledFilePath(1)); + return true; + + void Move(string from, string to) + { + fileSystem.File.Move(from, to); + moves.Add((from, to)); + } } + + private string StagingPath() + => fileSystem.Path.Combine(Directory, string.Format(CultureInfo.InvariantCulture, "{0}.{1:N}.rolling", baseName, Guid.NewGuid())); } diff --git a/Essentials.Tests/FileLoggingProviderTests.cs b/Essentials.Tests/FileLoggingProviderTests.cs index 6467c36..3e875d5 100644 --- a/Essentials.Tests/FileLoggingProviderTests.cs +++ b/Essentials.Tests/FileLoggingProviderTests.cs @@ -11,6 +11,7 @@ namespace ktsu.Essentials.Tests; using ktsu.Essentials.LoggingProviders.Console; using ktsu.Essentials.LoggingProviders.File; using Microsoft.Extensions.DependencyInjection; +using Testably.Abstractions.Testing; using Microsoft.VisualStudio.TestTools.UnitTesting; [TestClass] @@ -112,6 +113,43 @@ public void Rolls_When_The_Next_Entry_Would_Pass_The_Size_Limit() "the kept files hold the most recent entries in order, with none lost between them"); } + [TestMethod] + public void A_Roll_Refused_By_A_Reader_Keeps_Every_File_And_Every_Entry() + { + // The roll deleted the oldest file and shifted the rest before the active file's move failed, so each + // entry logged while a viewer held the log deleted another rolled file (issue #120). + InMemoryFileSystemProvider fileSystem = new(new MockFileSystem(o => o.SimulatingOperatingSystem(SimulationMode.Windows))); + FileLoggingProvider logger = Create(fileSystem, maxFileSizeBytes: 100, maxFileCount: 5); + ILoggingProvider log = logger; + + for (int i = 0; i < 12; i++) + { + log.LogInformation($"entry {i:00} padded out to a length"); + } + + string[] rolled = [.. Enumerable.Range(1, 4).Select(logger.GetRolledFilePath)]; + string[] before = [.. rolled.Select(fileSystem.File.ReadAllText)]; + + using (System.IO.Stream viewer = fileSystem.FileStream.New(logger.FilePath, System.IO.FileMode.Open, System.IO.FileAccess.Read, System.IO.FileShare.ReadWrite)) + { + for (int i = 12; i < 18; i++) + { + log.LogInformation($"entry {i:00} padded out to a length"); + } + + CollectionAssert.AreEqual(before, rolled.Select(fileSystem.File.ReadAllText).ToArray(), "a refused roll moved or deleted rolled files"); + Assert.AreEqual(0L, logger.DroppedEntryCount, "entries were dropped although the file could be appended to"); + Assert.AreEqual(17, EntryNumber(ReadLines(fileSystem, logger.FilePath)[^1])); + } + + log.LogInformation("entry 18 padded out to a length"); + + Assert.AreEqual(18, EntryNumber(ReadLines(fileSystem, logger.FilePath).Single()), "the next entry after the reader closed did not roll"); + StringAssert.Contains(ReadLines(fileSystem, logger.GetRolledFilePath(1))[^1], "entry 17"); + Assert.AreEqual(before[0], fileSystem.File.ReadAllText(logger.GetRolledFilePath(2)), "history did not shift up a slot"); + Assert.IsEmpty(fileSystem.Directory.GetFiles(LogDirectory, "*.rolling"), "a staging file was left behind"); + } + [TestMethod] public void Rolled_Files_Number_Before_The_Extension() { From e0d0e499ef4c701d892d6b510d1c672834c85a88 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 11:18:39 +0000 Subject: [PATCH 16/23] Zero AES key and plaintext copies once each operation is done with them Every AES path copied the caller's key with ToArray and dropped the copy unzeroed, and the span paths did the same with the plaintext and the decrypted result. Each copy is now cleared with CryptographicOperations.ZeroMemory as soon as the transform holds its own, as the HMAC providers already do. A known-answer test checks every path against an independent Aes, since a round trip would not notice a key zeroed too early. Fixes #98 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- .../AesEncryptionProvider.cs | 72 ++++++++++---- Essentials.Tests/AesKnownAnswerTests.cs | 94 +++++++++++++++++++ 2 files changed, 146 insertions(+), 20 deletions(-) create mode 100644 Essentials.Tests/AesKnownAnswerTests.cs diff --git a/Essentials.EncryptionProviders.Aes/AesEncryptionProvider.cs b/Essentials.EncryptionProviders.Aes/AesEncryptionProvider.cs index 8221586..5cae414 100644 --- a/Essentials.EncryptionProviders.Aes/AesEncryptionProvider.cs +++ b/Essentials.EncryptionProviders.Aes/AesEncryptionProvider.cs @@ -84,17 +84,25 @@ public bool TryEncrypt(ReadOnlySpan data, ReadOnlySpan key, ReadOnly try { using System.Security.Cryptography.Aes aes = System.Security.Cryptography.Aes.Create(); - using ICryptoTransform encryptor = aes.CreateEncryptor(key.ToArray(), iv.ToArray()); - byte[] encryptedData = encryptor.TransformFinalBlock(data.ToArray(), 0, data.Length); + using ICryptoTransform encryptor = CreateTransform(aes, key, iv, encrypt: true); + byte[] plaintext = data.ToArray(); + try + { + byte[] encryptedData = encryptor.TransformFinalBlock(plaintext, 0, plaintext.Length); + + if (encryptedData.Length > destination.Length) + { + return false; + } - if (encryptedData.Length > destination.Length) + encryptedData.CopyTo(destination); + bytesWritten = encryptedData.Length; + return true; + } + finally { - return false; + CryptographicOperations.ZeroMemory(plaintext); } - - encryptedData.CopyTo(destination); - bytesWritten = encryptedData.Length; - return true; } catch (ArgumentException) { @@ -128,7 +136,7 @@ public bool TryEncrypt(Stream data, ReadOnlySpan key, ReadOnlySpan i try { using System.Security.Cryptography.Aes aes = System.Security.Cryptography.Aes.Create(); - using ICryptoTransform encryptor = aes.CreateEncryptor(key.ToArray(), iv.ToArray()); + using ICryptoTransform encryptor = CreateTransform(aes, key, iv, encrypt: true); using CryptoStream cryptoStream = new(destination, encryptor, CryptoStreamMode.Write, leaveOpen: true); data.CopyTo(cryptoStream); return true; @@ -184,7 +192,7 @@ public async Task TryEncryptAsync(Stream data, ReadOnlyMemory key, R try { using System.Security.Cryptography.Aes aes = System.Security.Cryptography.Aes.Create(); - using ICryptoTransform encryptor = aes.CreateEncryptor(key.ToArray(), iv.ToArray()); + using ICryptoTransform encryptor = CreateTransform(aes, key.Span, iv.Span, encrypt: true); CryptoStream cryptoStream = new(destination, encryptor, CryptoStreamMode.Write, leaveOpen: true); await using (cryptoStream.ConfigureAwait(false)) { @@ -239,17 +247,23 @@ public bool TryDecrypt(ReadOnlySpan data, ReadOnlySpan key, ReadOnly // length by trimming trailing zeros, because the API could not report how many bytes the // matching encrypt call wrote; that corrupted any ciphertext ending in a zero byte. using System.Security.Cryptography.Aes aes = System.Security.Cryptography.Aes.Create(); - using ICryptoTransform decryptor = aes.CreateDecryptor(key.ToArray(), iv.ToArray()); + using ICryptoTransform decryptor = CreateTransform(aes, key, iv, encrypt: false); byte[] decryptedData = decryptor.TransformFinalBlock(data.ToArray(), 0, data.Length); - - if (decryptedData.Length > destination.Length) + try { - return false; + if (decryptedData.Length > destination.Length) + { + return false; + } + + decryptedData.CopyTo(destination); + bytesWritten = decryptedData.Length; + return true; + } + finally + { + CryptographicOperations.ZeroMemory(decryptedData); } - - decryptedData.CopyTo(destination); - bytesWritten = decryptedData.Length; - return true; } catch (ArgumentException) { @@ -283,7 +297,7 @@ public bool TryDecrypt(Stream data, ReadOnlySpan key, ReadOnlySpan i try { using System.Security.Cryptography.Aes aes = System.Security.Cryptography.Aes.Create(); - using ICryptoTransform decryptor = aes.CreateDecryptor(key.ToArray(), iv.ToArray()); + using ICryptoTransform decryptor = CreateTransform(aes, key, iv, encrypt: false); CountingReadStream source = new(data); using CryptoStream cryptoStream = new(source, decryptor, CryptoStreamMode.Read, leaveOpen: true); cryptoStream.CopyTo(destination); @@ -340,7 +354,7 @@ public async Task TryDecryptAsync(Stream data, ReadOnlyMemory key, R try { using System.Security.Cryptography.Aes aes = System.Security.Cryptography.Aes.Create(); - using ICryptoTransform decryptor = aes.CreateDecryptor(key.ToArray(), iv.ToArray()); + using ICryptoTransform decryptor = CreateTransform(aes, key.Span, iv.Span, encrypt: false); CountingReadStream source = new(data); CryptoStream cryptoStream = new(source, decryptor, CryptoStreamMode.Read, leaveOpen: true); await using (cryptoStream.ConfigureAwait(false)) @@ -426,4 +440,22 @@ private int Count(int read) return read; } } + + /// + /// Creates an encryptor or decryptor over a copy of that is zeroed as soon as the + /// transform holds its own, so the secret does not linger in an unreachable heap array until the collector + /// reuses the memory, as the HMAC providers already ensure (issue #98). + /// + private static ICryptoTransform CreateTransform(System.Security.Cryptography.Aes aes, ReadOnlySpan key, ReadOnlySpan iv, bool encrypt) + { + byte[] keyCopy = key.ToArray(); + try + { + return encrypt ? aes.CreateEncryptor(keyCopy, iv.ToArray()) : aes.CreateDecryptor(keyCopy, iv.ToArray()); + } + finally + { + CryptographicOperations.ZeroMemory(keyCopy); + } + } } diff --git a/Essentials.Tests/AesKnownAnswerTests.cs b/Essentials.Tests/AesKnownAnswerTests.cs new file mode 100644 index 0000000..1e27004 --- /dev/null +++ b/Essentials.Tests/AesKnownAnswerTests.cs @@ -0,0 +1,94 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Essentials.Tests; + +using System.IO; +using System.Linq; +using System.Security.Cryptography; +using System.Threading.Tasks; +using ktsu.Essentials; +using ktsu.Essentials.EncryptionProviders.Aes; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// The provider zeroes its copies of the key and the plaintext once each operation is done with them (issue #98). +/// A round trip cannot show that the zeroing came too early, since encrypting and decrypting under the same +/// zeroed key would still agree, so every path is checked against ciphertext from an independent +/// instead, and the caller's key and plaintext are checked to be untouched. +/// +[TestClass] +public sealed class AesKnownAnswerTests +{ + private static readonly byte[] Key = [.. Enumerable.Range(1, 32).Select(i => (byte)i)]; + private static readonly byte[] Iv = [.. Enumerable.Range(100, 16).Select(i => (byte)i)]; + private static readonly byte[] Plaintext = System.Text.Encoding.UTF8.GetBytes("Seventeen bytes!! and then some more of them"); + + public TestContext TestContext { get; set; } = null!; + + [TestMethod] + public async Task Every_Path_Encrypts_And_Decrypts_Under_The_Callers_Key() + { + IEncryptionProvider provider = new AesEncryptionProvider(); + byte[] key = [.. Key]; + byte[] plaintext = [.. Plaintext]; + byte[] expected = Reference(); + + byte[] destination = new byte[provider.GetMaxEncryptedLength(plaintext.Length)]; + Assert.IsTrue(provider.TryEncrypt(plaintext, key, Iv, destination, out int written)); + CollectionAssert.AreEqual(expected, destination[..written], "span encrypt"); + + using (MemoryStream source = new(plaintext)) + using (MemoryStream sink = new()) + { + Assert.IsTrue(provider.TryEncrypt(source, key, Iv, sink)); + CollectionAssert.AreEqual(expected, sink.ToArray(), "stream encrypt"); + } + + using (MemoryStream source = new(plaintext)) + using (MemoryStream sink = new()) + { + Assert.IsTrue(await provider.TryEncryptAsync(source, key, Iv, sink, TestContext.CancellationToken).ConfigureAwait(false)); + CollectionAssert.AreEqual(expected, sink.ToArray(), "async stream encrypt"); + } + + byte[] decrypted = new byte[expected.Length]; + Assert.IsTrue(provider.TryDecrypt(expected, key, Iv, decrypted, out written)); + CollectionAssert.AreEqual(Plaintext, decrypted[..written], "span decrypt"); + + using (MemoryStream source = new(expected)) + using (MemoryStream sink = new()) + { + Assert.IsTrue(provider.TryDecrypt(source, key, Iv, sink)); + CollectionAssert.AreEqual(Plaintext, sink.ToArray(), "stream decrypt"); + } + + using (MemoryStream source = new(expected)) + using (MemoryStream sink = new()) + { + Assert.IsTrue(await provider.TryDecryptAsync(source, key, Iv, sink, TestContext.CancellationToken).ConfigureAwait(false)); + CollectionAssert.AreEqual(Plaintext, sink.ToArray(), "async stream decrypt"); + } + + CollectionAssert.AreEqual(Key, key, "the caller's key was changed"); + CollectionAssert.AreEqual(Plaintext, plaintext, "the caller's plaintext was changed"); + } + + [TestMethod] + public void An_Undersized_Destination_Still_Fails() + { + IEncryptionProvider provider = new AesEncryptionProvider(); + byte[] ciphertext = Reference(); + + Assert.IsFalse(provider.TryDecrypt(ciphertext, Key, Iv, new byte[Plaintext.Length - 1], out int written)); + Assert.AreEqual(0, written); + Assert.IsFalse(provider.TryEncrypt(Plaintext, Key, Iv, new byte[ciphertext.Length - 1], out written)); + Assert.AreEqual(0, written); + } + + private static byte[] Reference() + { + using Aes aes = Aes.Create(); + aes.Key = Key; + return aes.EncryptCbc(Plaintext, Iv, PaddingMode.PKCS7); + } +} From 056ddd3a756250ac1ad42303b1e63a6b386b5522 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 11:24:25 +0000 Subject: [PATCH 17/23] Honour cancellation while a background job holds the command's pipes After the shell exited, Execute joined its output drains with no token check, so a job the command left running on the pipes made a cancel do nothing until that job ended, and the run was then reported as a success. The drains are now waited for under the token, the two abandon waits share one budget, and the net6.0 and netstandard2.1 async reads are bounded by the token too. Tests pin both #125 and #97. The tree kill that #97 asks for already shipped with #100; the new test covers its subshell case and how promptly the call returns. Fixes #125 Fixes #97 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- .../NativeCommandExecutor.cs | 87 ++++++++++++++----- Essentials.Tests/CommandExecutorTests.cs | 85 ++++++++++++++++++ 2 files changed, 152 insertions(+), 20 deletions(-) diff --git a/Essentials.CommandExecutors.Native/NativeCommandExecutor.cs b/Essentials.CommandExecutors.Native/NativeCommandExecutor.cs index 3b9a19a..53e5490 100644 --- a/Essentials.CommandExecutors.Native/NativeCommandExecutor.cs +++ b/Essentials.CommandExecutors.Native/NativeCommandExecutor.cs @@ -79,8 +79,11 @@ public async Task ExecuteAsync(string command, IReadOnlyDictionar await Task.Run(process.WaitForExit, cancellationToken).ConfigureAwait(false); #endif - string stdout = await stdoutTask.ConfigureAwait(false); - string stderr = await stderrTask.ConfigureAwait(false); + // A background job the command started keeps the pipes open after the shell exits, so the reads + // can outlast the wait above. Where ReadToEndAsync takes no token they are bounded by it here, or + // cancelling would do nothing until that job ended (issue #125). + string stdout = await ReadToEndAsync(stdoutTask, cancellationToken).ConfigureAwait(false); + string stderr = await ReadToEndAsync(stderrTask, cancellationToken).ConfigureAwait(false); return new CommandResult(process.ExitCode, stdout, stderr); } @@ -155,16 +158,26 @@ public CommandResult Execute(string command, IReadOnlyDictionary } } - return new CommandResult(process.ExitCode, stdout.Wait(), stderr.Wait()); + // A background job the command started keeps the pipes open after the shell exits, so the + // drains are waited for under the same token rather than without one, which ignored a cancel + // and then reported the run as a success once that job ended (issue #125). + if (!stdout.Wait(cancellationToken) || !stderr.Wait(cancellationToken)) + { + TryKill(process); + return Cancelled(); + } + + return new CommandResult(process.ExitCode, stdout.Text, stderr.Text); } finally { // The cancelled path abandons the drains rather than waiting for them. They still have to // stop touching the streams before the `using` above disposes the process, so they are - // given a bounded chance to notice the child is gone. On the path that returns a result - // both have already been waited for, and these joins return immediately. - stdout.Abandon(); - stderr.Abandon(); + // given a bounded chance, shared between the two, to notice the child is gone. On the path + // that returns a result both have already been waited for, and these joins return at once. + Stopwatch abandoning = Stopwatch.StartNew(); + stdout.Abandon(DrainAbandonTimeoutMilliseconds); + stderr.Abandon(Math.Max(0, DrainAbandonTimeoutMilliseconds - (int)abandoning.ElapsedMilliseconds)); } } catch (InvalidOperationException ex) @@ -248,6 +261,30 @@ private static void TryKill(Process process) } } + /// + /// Awaits a read of a redirected stream, giving up when is cancelled. + /// + /// The read, started without a token where the platform has no overload taking one. + /// A cancellation token to stop waiting. + /// Everything the stream carried. + /// was cancelled first. + private static async Task ReadToEndAsync(Task read, CancellationToken cancellationToken) + { +#if NET6_0_OR_GREATER + return await read.WaitAsync(cancellationToken).ConfigureAwait(false); +#else + using CancellationTokenSource stop = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + Task first = await Task.WhenAny(read, Task.Delay(Timeout.Infinite, stop.Token)).ConfigureAwait(false); + await stop.CancelAsync().ConfigureAwait(false); + if (first != read) + { + cancellationToken.ThrowIfCancellationRequested(); + } + + return await read.ConfigureAwait(false); +#endif + } + /// /// Builds the result returned when an operation is cancelled. /// @@ -315,15 +352,10 @@ internal static string QuoteArgument(string argument) private sealed class StreamDrain { /// - /// The thread reading the stream. Joining it publishes to the caller. + /// The thread reading the stream. Joining it publishes to the caller. /// private readonly Thread thread; - /// - /// What was read. Written only by , and read only after it has been joined. - /// - private string text = string.Empty; - /// /// Initializes a new instance of the class and starts reading. /// @@ -334,7 +366,7 @@ private StreamDrain(StreamReader reader) { try { - text = reader.ReadToEnd(); + Text = reader.ReadToEnd(); } catch (Exception ex) when (ex is IOException or ObjectDisposedException) { @@ -358,19 +390,34 @@ private StreamDrain(StreamReader reader) public static StreamDrain Start(StreamReader reader) => new(reader); /// - /// Waits for the stream to reach its end and returns everything it carried. + /// Gets everything the stream carried, exactly as the child wrote it. Written only by + /// , so read it only after has returned . + /// + public string Text { get; private set; } = string.Empty; + + /// + /// Waits for the stream to reach its end, checking as it goes. /// - /// The stream's contents, exactly as the child wrote them. - public string Wait() + /// A cancellation token to stop waiting. + /// once the stream has ended; if cancelled first. + public bool Wait(CancellationToken cancellationToken) { - thread.Join(); - return text; + while (!thread.Join(CancellationPollIntervalMilliseconds)) + { + if (cancellationToken.IsCancellationRequested) + { + return false; + } + } + + return true; } /// /// Gives an abandoned drain a bounded chance to finish, so it is not still reading a stream whose /// process is about to be disposed. Whatever it read is discarded. /// - public void Abandon() => thread.Join(DrainAbandonTimeoutMilliseconds); + /// How long to wait at most. + public void Abandon(int timeoutMilliseconds) => thread.Join(timeoutMilliseconds); } } diff --git a/Essentials.Tests/CommandExecutorTests.cs b/Essentials.Tests/CommandExecutorTests.cs index a16c128..011acb8 100644 --- a/Essentials.Tests/CommandExecutorTests.cs +++ b/Essentials.Tests/CommandExecutorTests.cs @@ -292,6 +292,91 @@ public void CommandExecutor_Cancellation_Stops_The_Command(ICommandExecutor exec } } + /// + /// Regression test for issue #125: a background job holding the pipes open after the shell exited made the + /// synchronous path wait for it with no token check and then report success. + /// + /// The executor under test. + /// The name of the executor, for assertion messages. + [TestMethod] + [DynamicData(nameof(CommandExecutors))] + public void CommandExecutor_Cancellation_Is_Honoured_After_The_Shell_Exits(ICommandExecutor executor, string providerName) + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + Assert.Inconclusive("cmd.exe has no plain way to leave a job running on the inherited pipes."); + } + + const string command = "sleep 10 & echo hi"; + TimeSpan limit = TimeSpan.FromSeconds(5); + + Stopwatch syncRun = Stopwatch.StartNew(); + using (CancellationTokenSource cts = new(TimeSpan.FromMilliseconds(500))) + { + CommandResult result = executor.Execute(command, cancellationToken: cts.Token); + Assert.IsFalse(result.Success, $"{providerName} sync reported a cancelled run as a success"); + } + + Assert.IsLessThan(limit, syncRun.Elapsed, $"{providerName} sync waited for the background job"); + + Stopwatch asyncRun = Stopwatch.StartNew(); + using (CancellationTokenSource cts = new(TimeSpan.FromMilliseconds(500))) + { + CommandResult result = executor.ExecuteAsync(command, cancellationToken: cts.Token).Result; + Assert.IsFalse(result.Success, $"{providerName} async reported a cancelled run as a success"); + } + + Assert.IsLessThan(limit, asyncRun.Elapsed, $"{providerName} async waited for the background job"); + } + + /// + /// Regression test for issue #97: cancelling killed only the shell, so work in a subshell carried on to its + /// side effect, and held the pipes open so the call came back a second per stream late. + /// + /// The executor under test. + /// The name of the executor, for assertion messages. + [TestMethod] + [DynamicData(nameof(CommandExecutors))] + public void CommandExecutor_Cancellation_Stops_Work_In_A_Subshell(ICommandExecutor executor, string providerName) + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + Assert.Inconclusive("Subshells are a Unix shell construct; cmd.exe children are covered by CommandExecutor_Cancellation_Stops_The_Command."); + } + + string syncMarker = Path.Join(Path.GetTempPath(), $"ktsu-subshell-sync-{Guid.NewGuid():N}"); + string asyncMarker = Path.Join(Path.GetTempPath(), $"ktsu-subshell-async-{Guid.NewGuid():N}"); + TimeSpan prompt = TimeSpan.FromMilliseconds(1750); + + try + { + Stopwatch syncRun = Stopwatch.StartNew(); + using (CancellationTokenSource cts = new(CancellationDelay)) + { + CommandResult result = executor.Execute($"(sleep 3; touch '{syncMarker}'); true", cancellationToken: cts.Token); + Assert.IsFalse(result.Success, $"{providerName} sync should report the cancellation"); + } + + Assert.IsLessThan(prompt, syncRun.Elapsed, $"{providerName} sync waited out the pipes a surviving subshell held"); + + using (CancellationTokenSource cts = new(CancellationDelay)) + { + CommandResult result = executor.ExecuteAsync($"(sleep 3; touch '{asyncMarker}'); true", cancellationToken: cts.Token).Result; + Assert.IsFalse(result.Success, $"{providerName} async should report the cancellation"); + } + + _ = TestContext.CancellationToken.WaitHandle.WaitOne(TimeSpan.FromSeconds(4)); + + Assert.IsFalse(File.Exists(syncMarker), $"{providerName} sync left the subshell running to its side effect"); + Assert.IsFalse(File.Exists(asyncMarker), $"{providerName} async left the subshell running to its side effect"); + } + finally + { + File.Delete(syncMarker); + File.Delete(asyncMarker); + } + } + /// /// How long the command in waits before /// creating its marker. It has to be far past , with room for a loaded CI From 017feedaa3f452fed29b4dee7311399fdc90fab2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 11:25:54 +0000 Subject: [PATCH 18/23] Check async compression and encryption write the same bytes as the sync paths Only Gzip had a byte-identity test between its async and sync stream paths. The test now runs over Gzip, Deflate, ZLib and Brotli, and an AES test with a fixed key and IV compares TryEncryptAsync with TryEncrypt. Test-only change. Fixes #83 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- Essentials.Tests/AsyncStreamIoTests.cs | 57 ++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 4 deletions(-) diff --git a/Essentials.Tests/AsyncStreamIoTests.cs b/Essentials.Tests/AsyncStreamIoTests.cs index 408a35e..ff6fa3c 100644 --- a/Essentials.Tests/AsyncStreamIoTests.cs +++ b/Essentials.Tests/AsyncStreamIoTests.cs @@ -70,15 +70,28 @@ public async Task GzipTryCompressAsync_UsesOnlyTheAsynchronousStreamMembersAsync Assert.AreNotEqual(0, destination.ToArray().Length, "Expected compressed bytes to be written."); } + public static IEnumerable AllCompressionProviders => + [ + [new GzipCompressionProvider()], + [new DeflateCompressionProvider()], + [new ZLibCompressionProvider()], + [new BrotliCompressionProvider()], + ]; + + /// + /// A round trip only shows that async compression and async decompression agree with each other; data written + /// asynchronously and read back synchronously needs the two compressors to write the same stream (issue #83). + /// + /// The provider under test. [TestMethod] + [DynamicData(nameof(AllCompressionProviders))] [System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1849:Call async methods when in an async method", Justification = "Deliberately exercises the synchronous path to compare its output against the asynchronous path.")] - public async Task GzipAsyncCompression_ProducesTheSameBytesAsTheSynchronousPathAsync() + public async Task AsyncCompression_ProducesTheSameBytesAsTheSynchronousPathAsync(ICompressionProvider provider) { - ICompressionProvider provider = new GzipCompressionProvider(); - byte[] payload = new byte[4096]; + byte[] payload = new byte[100_000]; for (int i = 0; i < payload.Length; i++) { - payload[i] = (byte)(i % 251); + payload[i] = (byte)(i * 7 % 251); } using MemoryStream syncSource = new(payload); @@ -91,6 +104,42 @@ public async Task GzipAsyncCompression_ProducesTheSameBytesAsTheSynchronousPathA .TryCompressAsync(asyncSource, asyncDestination, TestContext.CancellationTokenSource.Token) .ConfigureAwait(false)); + CollectionAssert.AreEqual(syncDestination.ToArray(), asyncDestination.ToArray(), $"{provider.GetType().Name} wrote different bytes asynchronously"); + } + + [TestMethod] + [System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1849:Call async methods when in an async method", Justification = "Deliberately exercises the synchronous path to compare its output against the asynchronous path.")] + public async Task AesAsyncEncryption_ProducesTheSameBytesAsTheSynchronousPathAsync() + { + IEncryptionProvider provider = new AesEncryptionProvider(); + byte[] key = new byte[32]; + byte[] iv = new byte[16]; + for (int i = 0; i < key.Length; i++) + { + key[i] = (byte)(i * 3); + } + + for (int i = 0; i < iv.Length; i++) + { + iv[i] = (byte)(200 - i); + } + + byte[] payload = new byte[10_000]; + for (int i = 0; i < payload.Length; i++) + { + payload[i] = (byte)(i % 253); + } + + using MemoryStream syncSource = new(payload); + using MemoryStream syncDestination = new(); + Assert.IsTrue(provider.TryEncrypt(syncSource, key, iv, syncDestination)); + + using AsyncOnlyStream asyncSource = new(payload); + using AsyncOnlyStream asyncDestination = new(); + Assert.IsTrue(await provider + .TryEncryptAsync(asyncSource, key, iv, asyncDestination, TestContext.CancellationTokenSource.Token) + .ConfigureAwait(false)); + CollectionAssert.AreEqual(syncDestination.ToArray(), asyncDestination.ToArray()); } From 517b2818cab8cc9e48564610ed4092e88168fef6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 11:30:45 +0000 Subject: [PATCH 19/23] Read FNV, Hex and byte-wise obfuscator streams a chunk at a time The FNV hashes, the Hex encoder and the Xor, Caesar and BitRotate obfuscators called ReadByte and WriteByte per byte, which on a stream that does not override them allocates and makes a call per byte: about eleven times as slow, with 32 bytes of garbage per input byte. Each now transforms a pooled chunk at a time, as their async paths already did. Tests compare every stream path with its span path for empty, short and multi-chunk inputs through a short-reading stream. Fixes #91 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- .../HexEncodingProvider.cs | 66 +++++- .../FNV1_32HashProvider.cs | 26 ++- .../FNV1_64HashProvider.cs | 26 ++- .../FNV1a_32HashProvider.cs | 26 ++- .../FNV1a_64HashProvider.cs | 26 ++- .../BitRotateObfuscationProvider.cs | 46 +++- .../CaesarObfuscationProvider.cs | 46 +++- .../XorObfuscationProvider.cs | 29 ++- Essentials.Tests/ChunkedStreamPathTests.cs | 197 ++++++++++++++++++ 9 files changed, 440 insertions(+), 48 deletions(-) create mode 100644 Essentials.Tests/ChunkedStreamPathTests.cs diff --git a/Essentials.EncodingProviders.Hex/HexEncodingProvider.cs b/Essentials.EncodingProviders.Hex/HexEncodingProvider.cs index a4bd0a4..1997807 100644 --- a/Essentials.EncodingProviders.Hex/HexEncodingProvider.cs +++ b/Essentials.EncodingProviders.Hex/HexEncodingProvider.cs @@ -79,13 +79,23 @@ public bool TryEncode(Stream data, Stream destination) return false; } + // A chunk at a time, as the asynchronous path does: the base Stream.ReadByte and WriteByte allocate and + // make a call per byte on any stream that does not override them (issue #91). + byte[] source = new byte[ChunkSize]; + byte[] encoded = new byte[ChunkSize * 2]; + try { - int b; - while ((b = data.ReadByte()) >= 0) + int read; + while ((read = data.Read(source, 0, ChunkSize)) > 0) { - destination.WriteByte((byte)HexDigits[b >> 4]); - destination.WriteByte((byte)HexDigits[b & 0x0F]); + for (int i = 0; i < read; i++) + { + encoded[i * 2] = (byte)HexDigits[source[i] >> 4]; + encoded[(i * 2) + 1] = (byte)HexDigits[source[i] & 0x0F]; + } + + destination.Write(encoded, 0, read * 2); } return true; @@ -137,25 +147,36 @@ public bool TryDecode(Stream encodedData, Stream destination) return false; } + byte[] source = new byte[ChunkSize]; + byte[] decoded = new byte[ChunkSize / 2]; + try { while (true) { - int first = encodedData.ReadByte(); - if (first < 0) + int filled = ReadPairs(encodedData, source); + if (filled == 0) { return true; } - int second = encodedData.ReadByte(); - if (second < 0 - || !TryParseNibble((byte)first, out int high) - || !TryParseNibble((byte)second, out int low)) + if (filled < 0) { + // An odd number of characters: the last byte has no partner, so the input is truncated. return false; } - destination.WriteByte((byte)((high << 4) | low)); + for (int i = 0; i < filled; i += 2) + { + if (!TryParseNibble(source[i], out int high) || !TryParseNibble(source[i + 1], out int low)) + { + return false; + } + + decoded[i / 2] = (byte)((high << 4) | low); + } + + destination.Write(decoded, 0, filled / 2); } } catch (IOException) @@ -333,6 +354,29 @@ private static async Task ReadPairsAsync(Stream source, byte[] buffer, Canc return filled % 2 == 0 ? filled : -1; } + /// + /// The synchronous counterpart of , with the same result. + /// + /// The stream to read. + /// The buffer to fill. + /// How many characters were read, zero at a clean end of stream, or -1 if the stream ended on an unpaired character. + private static int ReadPairs(Stream source, byte[] buffer) + { + int filled = 0; + while (filled < buffer.Length) + { + int read = source.Read(buffer, filled, buffer.Length - filled); + if (read == 0) + { + break; + } + + filled += read; + } + + return filled % 2 == 0 ? filled : -1; + } + /// /// The number of characters transformed per chunk. Even, so a pair is never split. /// diff --git a/Essentials.HashProviders.FNV1_32/FNV1_32HashProvider.cs b/Essentials.HashProviders.FNV1_32/FNV1_32HashProvider.cs index 0be4659..5175b70 100644 --- a/Essentials.HashProviders.FNV1_32/FNV1_32HashProvider.cs +++ b/Essentials.HashProviders.FNV1_32/FNV1_32HashProvider.cs @@ -4,6 +4,7 @@ namespace ktsu.Essentials.HashProviders.FNV1_32; using ktsu.Essentials; using System; +using System.Buffers; using System.IO; /// @@ -14,6 +15,9 @@ public class FNV1_32HashProvider : IHashProvider private const uint FnvOffsetBasis32 = 0x811c9dc5; private const uint FnvPrime32 = 0x01000193; + /// How many bytes the stream path reads at a time. + private const int StreamBufferSize = 81920; + /// /// The length of the FNV-1 32-bit hash in bytes (4 bytes / 32 bits). /// @@ -84,12 +88,24 @@ public bool TryHash(Stream data, Span destination, out int bytesWritten) try { uint hash = FnvOffsetBasis32; - int b; - - while ((b = data.ReadByte()) != -1) + // A chunk at a time: the base Stream.ReadByte allocates and makes a Read call per byte, which on a + // stream that does not override it cost eleven times as long and 32 bytes of garbage per byte (issue #91). + byte[] buffer = ArrayPool.Shared.Rent(StreamBufferSize); + try { - hash *= FnvPrime32; - hash ^= (byte)b; + int read; + while ((read = data.Read(buffer, 0, buffer.Length)) > 0) + { + for (int i = 0; i < read; i++) + { + hash *= FnvPrime32; + hash ^= buffer[i]; + } + } + } + finally + { + ArrayPool.Shared.Return(buffer, clearArray: true); } // Write the hash as little-endian bytes diff --git a/Essentials.HashProviders.FNV1_64/FNV1_64HashProvider.cs b/Essentials.HashProviders.FNV1_64/FNV1_64HashProvider.cs index fa772e3..5d61d96 100644 --- a/Essentials.HashProviders.FNV1_64/FNV1_64HashProvider.cs +++ b/Essentials.HashProviders.FNV1_64/FNV1_64HashProvider.cs @@ -4,6 +4,7 @@ namespace ktsu.Essentials.HashProviders.FNV1_64; using ktsu.Essentials; using System; +using System.Buffers; using System.IO; /// @@ -14,6 +15,9 @@ public class FNV1_64HashProvider : IHashProvider private const ulong FnvOffsetBasis64 = 0xcbf29ce484222325; private const ulong FnvPrime64 = 0x00000100000001b3; + /// How many bytes the stream path reads at a time. + private const int StreamBufferSize = 81920; + /// /// The length of the FNV-1 64-bit hash in bytes (8 bytes / 64 bits). /// @@ -88,12 +92,24 @@ public bool TryHash(Stream data, Span destination, out int bytesWritten) try { ulong hash = FnvOffsetBasis64; - int b; - - while ((b = data.ReadByte()) != -1) + // A chunk at a time: the base Stream.ReadByte allocates and makes a Read call per byte, which on a + // stream that does not override it cost eleven times as long and 32 bytes of garbage per byte (issue #91). + byte[] buffer = ArrayPool.Shared.Rent(StreamBufferSize); + try { - hash *= FnvPrime64; - hash ^= (byte)b; + int read; + while ((read = data.Read(buffer, 0, buffer.Length)) > 0) + { + for (int i = 0; i < read; i++) + { + hash *= FnvPrime64; + hash ^= buffer[i]; + } + } + } + finally + { + ArrayPool.Shared.Return(buffer, clearArray: true); } // Write the hash as little-endian bytes diff --git a/Essentials.HashProviders.FNV1a_32/FNV1a_32HashProvider.cs b/Essentials.HashProviders.FNV1a_32/FNV1a_32HashProvider.cs index ac14263..b5c061d 100644 --- a/Essentials.HashProviders.FNV1a_32/FNV1a_32HashProvider.cs +++ b/Essentials.HashProviders.FNV1a_32/FNV1a_32HashProvider.cs @@ -4,6 +4,7 @@ namespace ktsu.Essentials.HashProviders.FNV1a_32; using ktsu.Essentials; using System; +using System.Buffers; using System.IO; /// @@ -14,6 +15,9 @@ public class FNV1a_32HashProvider : IHashProvider private const uint FnvOffsetBasis32 = 0x811c9dc5; private const uint FnvPrime32 = 0x01000193; + /// How many bytes the stream path reads at a time. + private const int StreamBufferSize = 81920; + /// /// The length of the FNV-1a 32-bit hash in bytes (4 bytes / 32 bits). /// @@ -84,12 +88,24 @@ public bool TryHash(Stream data, Span destination, out int bytesWritten) try { uint hash = FnvOffsetBasis32; - int b; - - while ((b = data.ReadByte()) != -1) + // A chunk at a time: the base Stream.ReadByte allocates and makes a Read call per byte, which on a + // stream that does not override it cost eleven times as long and 32 bytes of garbage per byte (issue #91). + byte[] buffer = ArrayPool.Shared.Rent(StreamBufferSize); + try { - hash ^= (byte)b; - hash *= FnvPrime32; + int read; + while ((read = data.Read(buffer, 0, buffer.Length)) > 0) + { + for (int i = 0; i < read; i++) + { + hash ^= buffer[i]; + hash *= FnvPrime32; + } + } + } + finally + { + ArrayPool.Shared.Return(buffer, clearArray: true); } // Write the hash as little-endian bytes diff --git a/Essentials.HashProviders.FNV1a_64/FNV1a_64HashProvider.cs b/Essentials.HashProviders.FNV1a_64/FNV1a_64HashProvider.cs index 1af2385..499cd73 100644 --- a/Essentials.HashProviders.FNV1a_64/FNV1a_64HashProvider.cs +++ b/Essentials.HashProviders.FNV1a_64/FNV1a_64HashProvider.cs @@ -4,6 +4,7 @@ namespace ktsu.Essentials.HashProviders.FNV1a_64; using ktsu.Essentials; using System; +using System.Buffers; using System.IO; /// @@ -14,6 +15,9 @@ public class FNV1a_64HashProvider : IHashProvider private const ulong FnvOffsetBasis64 = 0xcbf29ce484222325; private const ulong FnvPrime64 = 0x00000100000001b3; + /// How many bytes the stream path reads at a time. + private const int StreamBufferSize = 81920; + /// /// The length of the FNV-1a 64-bit hash in bytes (8 bytes / 64 bits). /// @@ -88,12 +92,24 @@ public bool TryHash(Stream data, Span destination, out int bytesWritten) try { ulong hash = FnvOffsetBasis64; - int b; - - while ((b = data.ReadByte()) != -1) + // A chunk at a time: the base Stream.ReadByte allocates and makes a Read call per byte, which on a + // stream that does not override it cost eleven times as long and 32 bytes of garbage per byte (issue #91). + byte[] buffer = ArrayPool.Shared.Rent(StreamBufferSize); + try { - hash ^= (byte)b; - hash *= FnvPrime64; + int read; + while ((read = data.Read(buffer, 0, buffer.Length)) > 0) + { + for (int i = 0; i < read; i++) + { + hash ^= buffer[i]; + hash *= FnvPrime64; + } + } + } + finally + { + ArrayPool.Shared.Return(buffer, clearArray: true); } // Write the hash as little-endian bytes diff --git a/Essentials.ObfuscationProviders.BitRotate/BitRotateObfuscationProvider.cs b/Essentials.ObfuscationProviders.BitRotate/BitRotateObfuscationProvider.cs index 1196bf2..16dc4d8 100644 --- a/Essentials.ObfuscationProviders.BitRotate/BitRotateObfuscationProvider.cs +++ b/Essentials.ObfuscationProviders.BitRotate/BitRotateObfuscationProvider.cs @@ -4,6 +4,7 @@ namespace ktsu.Essentials.ObfuscationProviders.BitRotate; using ktsu.Essentials; using System; +using System.Buffers; using System.IO; /// @@ -12,6 +13,9 @@ namespace ktsu.Essentials.ObfuscationProviders.BitRotate; /// public class BitRotateObfuscationProvider : IObfuscationProvider { + /// How many bytes the stream paths transform at a time. + private const int StreamBufferSize = 81920; + private readonly int _bits; /// Initializes a new instance rotating by 3 bits. @@ -70,10 +74,25 @@ public bool TryObfuscate(Stream data, Stream destination) try { - int b; - while ((b = data.ReadByte()) >= 0) + // A chunk at a time: the base Stream.ReadByte and WriteByte allocate and make a call per byte on any + // stream that does not override them (issue #91). + byte[] buffer = ArrayPool.Shared.Rent(StreamBufferSize); + try + { + int read; + while ((read = data.Read(buffer, 0, buffer.Length)) > 0) + { + for (int i = 0; i < read; i++) + { + buffer[i] = RotateLeft(buffer[i], _bits); + } + + destination.Write(buffer, 0, read); + } + } + finally { - destination.WriteByte(RotateLeft((byte)b, _bits)); + ArrayPool.Shared.Return(buffer, clearArray: true); } return true; @@ -117,10 +136,25 @@ public bool TryDeobfuscate(Stream obfuscatedData, Stream destination) try { - int b; - while ((b = obfuscatedData.ReadByte()) >= 0) + // A chunk at a time: the base Stream.ReadByte and WriteByte allocate and make a call per byte on any + // stream that does not override them (issue #91). + byte[] buffer = ArrayPool.Shared.Rent(StreamBufferSize); + try + { + int read; + while ((read = obfuscatedData.Read(buffer, 0, buffer.Length)) > 0) + { + for (int i = 0; i < read; i++) + { + buffer[i] = RotateRight(buffer[i], _bits); + } + + destination.Write(buffer, 0, read); + } + } + finally { - destination.WriteByte(RotateRight((byte)b, _bits)); + ArrayPool.Shared.Return(buffer, clearArray: true); } return true; diff --git a/Essentials.ObfuscationProviders.Caesar/CaesarObfuscationProvider.cs b/Essentials.ObfuscationProviders.Caesar/CaesarObfuscationProvider.cs index 4b5c8ae..dc81602 100644 --- a/Essentials.ObfuscationProviders.Caesar/CaesarObfuscationProvider.cs +++ b/Essentials.ObfuscationProviders.Caesar/CaesarObfuscationProvider.cs @@ -3,6 +3,7 @@ namespace ktsu.Essentials.ObfuscationProviders.Caesar; using System; +using System.Buffers; using System.IO; using ktsu.Essentials; @@ -12,6 +13,9 @@ namespace ktsu.Essentials.ObfuscationProviders.Caesar; /// public class CaesarObfuscationProvider(byte shift = 13) : IObfuscationProvider { + /// How many bytes the stream paths transform at a time. + private const int StreamBufferSize = 81920; + private readonly byte _shift = shift; /// @@ -51,10 +55,25 @@ public bool TryObfuscate(Stream data, Stream destination) try { - int b; - while ((b = data.ReadByte()) >= 0) + // A chunk at a time: the base Stream.ReadByte and WriteByte allocate and make a call per byte on any + // stream that does not override them (issue #91). + byte[] buffer = ArrayPool.Shared.Rent(StreamBufferSize); + try + { + int read; + while ((read = data.Read(buffer, 0, buffer.Length)) > 0) + { + for (int i = 0; i < read; i++) + { + buffer[i] = (byte)(buffer[i] + _shift); + } + + destination.Write(buffer, 0, read); + } + } + finally { - destination.WriteByte((byte)(b + _shift)); + ArrayPool.Shared.Return(buffer, clearArray: true); } return true; @@ -98,10 +117,25 @@ public bool TryDeobfuscate(Stream obfuscatedData, Stream destination) try { - int b; - while ((b = obfuscatedData.ReadByte()) >= 0) + // A chunk at a time: the base Stream.ReadByte and WriteByte allocate and make a call per byte on any + // stream that does not override them (issue #91). + byte[] buffer = ArrayPool.Shared.Rent(StreamBufferSize); + try + { + int read; + while ((read = obfuscatedData.Read(buffer, 0, buffer.Length)) > 0) + { + for (int i = 0; i < read; i++) + { + buffer[i] = (byte)(buffer[i] - _shift); + } + + destination.Write(buffer, 0, read); + } + } + finally { - destination.WriteByte((byte)(b - _shift)); + ArrayPool.Shared.Return(buffer, clearArray: true); } return true; diff --git a/Essentials.ObfuscationProviders.Xor/XorObfuscationProvider.cs b/Essentials.ObfuscationProviders.Xor/XorObfuscationProvider.cs index 8185ad9..206c1da 100644 --- a/Essentials.ObfuscationProviders.Xor/XorObfuscationProvider.cs +++ b/Essentials.ObfuscationProviders.Xor/XorObfuscationProvider.cs @@ -3,6 +3,7 @@ namespace ktsu.Essentials.ObfuscationProviders.Xor; using System; +using System.Buffers; using System.IO; using ktsu.Essentials; @@ -12,6 +13,9 @@ namespace ktsu.Essentials.ObfuscationProviders.Xor; /// public class XorObfuscationProvider : IObfuscationProvider { + /// How many bytes the stream paths transform at a time. + private const int StreamBufferSize = 81920; + private readonly byte[] _key; /// Initializes a new instance with the default single-byte key. @@ -68,12 +72,27 @@ public bool TryObfuscate(Stream data, Stream destination) try { - int b; - long i = 0; - while ((b = data.ReadByte()) >= 0) + // A chunk at a time: the base Stream.ReadByte and WriteByte allocate and make a call per byte on any + // stream that does not override them (issue #91). + byte[] buffer = ArrayPool.Shared.Rent(StreamBufferSize); + try + { + long offset = 0; + int read; + while ((read = data.Read(buffer, 0, buffer.Length)) > 0) + { + for (int i = 0; i < read; i++) + { + buffer[i] ^= _key[(int)((offset + i) % _key.Length)]; + } + + destination.Write(buffer, 0, read); + offset += read; + } + } + finally { - destination.WriteByte((byte)(b ^ _key[(int)(i % _key.Length)])); - i++; + ArrayPool.Shared.Return(buffer, clearArray: true); } return true; diff --git a/Essentials.Tests/ChunkedStreamPathTests.cs b/Essentials.Tests/ChunkedStreamPathTests.cs new file mode 100644 index 0000000..133d882 --- /dev/null +++ b/Essentials.Tests/ChunkedStreamPathTests.cs @@ -0,0 +1,197 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Essentials.Tests; + +using System; +using System.Collections.Generic; +using System.IO; +using ktsu.Essentials; +using ktsu.Essentials.EncodingProviders.Hex; +using ktsu.Essentials.HashProviders.FNV1_32; +using ktsu.Essentials.HashProviders.FNV1_64; +using ktsu.Essentials.HashProviders.FNV1a_32; +using ktsu.Essentials.HashProviders.FNV1a_64; +using ktsu.Essentials.ObfuscationProviders.BitRotate; +using ktsu.Essentials.ObfuscationProviders.Caesar; +using ktsu.Essentials.ObfuscationProviders.Xor; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// The FNV, Hex and byte-wise obfuscation stream paths read a chunk at a time rather than calling +/// per byte (issue #91). Their output must be exactly what the span paths give, for +/// an empty stream, for one shorter than a chunk and for several chunks, read through a stream that returns short +/// reads and overrides neither nor . +/// +[TestClass] +public sealed class ChunkedStreamPathTests +{ + public static IEnumerable Lengths => [[0], [1], [1000], [200_001]]; + + public static IEnumerable HashCases() + { + IHashProvider[] providers = [new FNV1_32HashProvider(), new FNV1a_32HashProvider(), new FNV1_64HashProvider(), new FNV1a_64HashProvider()]; + foreach (IHashProvider provider in providers) + { + foreach (object[] length in Lengths) + { + yield return [provider, provider.GetType().Name, length[0]]; + } + } + } + + public static IEnumerable ObfuscationCases() + { + IObfuscationProvider[] providers = + [ + new XorObfuscationProvider([0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77]), + new CaesarObfuscationProvider(200), + new BitRotateObfuscationProvider(5), + ]; + foreach (IObfuscationProvider provider in providers) + { + foreach (object[] length in Lengths) + { + yield return [provider, provider.GetType().Name, length[0]]; + } + } + } + + [TestMethod] + [DynamicData(nameof(HashCases))] + public void Hash_Stream_Path_Matches_The_Span_Path(IHashProvider provider, string name, int length) + { + byte[] payload = Payload(length); + byte[] expected = provider.Hash(payload); + + using ShortReadStream source = new(payload); + byte[] actual = new byte[provider.HashLengthBytes]; + Assert.IsTrue(provider.TryHash(source, actual, out int written), name); + + Assert.AreEqual(provider.HashLengthBytes, written, name); + CollectionAssert.AreEqual(expected, actual, $"{name} hashed {length} streamed bytes differently"); + } + + [TestMethod] + [DynamicData(nameof(Lengths))] + public void Hex_Stream_Paths_Match_The_Span_Paths(int length) + { + IEncodingProvider hex = new HexEncodingProvider(); + byte[] payload = Payload(length); + byte[] encoded = hex.Encode(payload); + + using ShortReadStream source = new(payload); + using ShortReadStream sink = new([]); + Assert.IsTrue(hex.TryEncode(source, sink)); + CollectionAssert.AreEqual(encoded, sink.ToArray(), "streamed encoding differs"); + + using ShortReadStream encodedSource = new(encoded); + using ShortReadStream decodedSink = new([]); + Assert.IsTrue(hex.TryDecode(encodedSource, decodedSink)); + CollectionAssert.AreEqual(payload, decodedSink.ToArray(), "streamed decoding differs"); + } + + [TestMethod] + public void Hex_Stream_Decode_Still_Rejects_Bad_Input() + { + IEncodingProvider hex = new HexEncodingProvider(); + byte[] encoded = hex.Encode(Payload(200_001)); + + byte[] odd = encoded[..^1]; + using (ShortReadStream source = new(odd)) + using (ShortReadStream sink = new([])) + { + Assert.IsFalse(hex.TryDecode(source, sink), "an odd number of characters was accepted"); + } + + byte[] invalid = [.. encoded]; + invalid[150_000] = (byte)'g'; + using (ShortReadStream source = new(invalid)) + using (ShortReadStream sink = new([])) + { + Assert.IsFalse(hex.TryDecode(source, sink), "a character that is not hex was accepted"); + } + } + + [TestMethod] + [DynamicData(nameof(ObfuscationCases))] + public void Obfuscation_Stream_Paths_Match_The_Span_Paths(IObfuscationProvider provider, string name, int length) + { + byte[] payload = Payload(length); + byte[] obfuscated = provider.Obfuscate(payload); + + using ShortReadStream source = new(payload); + using ShortReadStream sink = new([]); + Assert.IsTrue(provider.TryObfuscate(source, sink), name); + CollectionAssert.AreEqual(obfuscated, sink.ToArray(), $"{name} obfuscated {length} streamed bytes differently"); + + using ShortReadStream obfuscatedSource = new(obfuscated); + using ShortReadStream restored = new([]); + Assert.IsTrue(provider.TryDeobfuscate(obfuscatedSource, restored), name); + CollectionAssert.AreEqual(payload, restored.ToArray(), $"{name} deobfuscated {length} streamed bytes differently"); + } + + private static byte[] Payload(int length) + { + byte[] payload = new byte[length]; + for (int i = 0; i < length; i++) + { + payload[i] = (byte)((i * 31) ^ (i >> 7)); + } + + return payload; + } + + /// + /// A stream over a growable buffer whose reads return at most 1,000 bytes, and which leaves + /// and to the base class, as most wrapping + /// streams do. + /// + private sealed class ShortReadStream : Stream + { + private readonly MemoryStream inner = new(); + + public ShortReadStream(byte[] content) + { + inner.Write(content, 0, content.Length); + inner.Position = 0; + } + + public override bool CanRead => true; + + public override bool CanSeek => false; + + public override bool CanWrite => true; + + public override long Length => throw new NotSupportedException(); + + public override long Position + { + get => throw new NotSupportedException(); + set => throw new NotSupportedException(); + } + + public byte[] ToArray() => inner.ToArray(); + + public override int Read(byte[] buffer, int offset, int count) => inner.Read(buffer, offset, Math.Min(count, 1000)); + + public override void Write(byte[] buffer, int offset, int count) => inner.Write(buffer, offset, count); + + public override void Flush() + { + } + + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + + public override void SetLength(long value) => throw new NotSupportedException(); + + protected override void Dispose(bool disposing) + { + if (disposing) + { + inner.Dispose(); + } + + base.Dispose(disposing); + } + } +} From ee3c9d6773372aaed289d06bf4330e77067eacb0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 11:40:48 +0000 Subject: [PATCH 20/23] Report truncated Gzip, ZLib and Brotli input as a failure GZipStream and ZLibStream treat input that runs out as a clean end of stream, so a payload cut off part way decompressed to a shorter one and was reported as a success. Every decompression path now checks the format's trailer against the output, and Brotli goes through BrotliDecoder, which reports an unfinished stream itself. Raw Deflate has no trailer, which its documentation now says. Fixes #80 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- CLAUDE.md | 3 +- .../BrotliCompressionProvider.cs | 112 ++++++- ...entials.CompressionProviders.Brotli.csproj | 1 - .../DeflateCompressionProvider.cs | 6 + ...ssentials.CompressionProviders.Gzip.csproj | 1 + .../GzipCompressionProvider.cs | 13 +- ...ssentials.CompressionProviders.ZLib.csproj | 1 + .../ZLibCompressionProvider.cs | 13 +- Essentials.Tests/TruncatedCompressionTests.cs | 132 ++++++++ Shared/BoundedDecompression.cs | 32 +- Shared/CompressedStreamEnd.cs | 289 ++++++++++++++++++ 11 files changed, 562 insertions(+), 41 deletions(-) create mode 100644 Essentials.Tests/TruncatedCompressionTests.cs create mode 100644 Shared/CompressedStreamEnd.cs diff --git a/CLAUDE.md b/CLAUDE.md index 142ab58..9bdbf4f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -45,7 +45,8 @@ This is a .NET library (`ktsu.Essentials`) providing high-performance interfaces - `Essentials/IContinuousDistribution.cs` - `IDistribution` plus `Pdf`/`LogPdf` - `Essentials/IDiscreteDistribution.cs` - `IDistribution` plus `Pmf`/`LogPmf`, and a default `Quantile` that bisects the CDF. That default is an explicit reimplementation of `IDistribution.Quantile`, which CA1033 misreads as an explicit implementation on a class; the suppression on it says why - `Shared/DeflateBound.cs` - zlib's `deflateBound` plus container overhead, linked into the Deflate, Gzip and ZLib providers as their `GetMaxCompressedLength` -- `Shared/BoundedDecompression.cs` - The span overload of `TryDecompress` for Brotli, Deflate, Gzip and ZLib: reads the decompressing stream straight into the destination and fails once it is full and more output remains, so the destination bounds the work. See issue #122 +- `Shared/BoundedDecompression.cs` - The span overload of `TryDecompress` for Deflate, Gzip and ZLib: reads the decompressing stream straight into the destination and fails once it is full and more output remains, so the destination bounds the work. See issue #122. Brotli uses `BrotliDecoder` directly instead, which reports a truncated stream itself +- `Shared/CompressedStreamEnd.cs` - Gzip's CRC-32 and length trailer and ZLib's Adler-32, checked against the output on every decompression path, because `GZipStream` and `ZLibStream` treat input that runs out as a clean end of stream. Raw Deflate has no trailer, so its truncation cannot be detected. See issue #80 - `Shared/SpecialFunctions.cs` - Incomplete gamma, incomplete beta, error function, normal quantile and cancellation-free `ExpM1`/`Log1P`, linked into the Normal, LogNormal, Exponential, Binomial, Poisson and Geometric providers rather than placed in the interfaces-only package - `Shared/DistributionArguments.cs` - Parameter validation shared by every distribution provider, linked in the same way - `Shared/ProviderRegistration.cs` - DI registration for providers whose `Add…Provider` overloads take arguments (Xor, Caesar, BitRotate and every distribution): a call replaces an earlier registration of the same concrete type in place, and `AddEssentials()` keeps a registration made before it, whether as the concrete type, the interface or both, moved to its default's position, rather than overwriting it; a concrete-only one gains an interface registration forwarding to it. Linked into those providers and `Essentials.All`. See issues #132 and #156 diff --git a/Essentials.CompressionProviders.Brotli/BrotliCompressionProvider.cs b/Essentials.CompressionProviders.Brotli/BrotliCompressionProvider.cs index b9d12ff..9a22c2b 100644 --- a/Essentials.CompressionProviders.Brotli/BrotliCompressionProvider.cs +++ b/Essentials.CompressionProviders.Brotli/BrotliCompressionProvider.cs @@ -4,6 +4,7 @@ namespace ktsu.Essentials.CompressionProviders.Brotli; using ktsu.Essentials; using System; +using System.Buffers; using System.IO; using System.IO.Compression; using System.Threading; @@ -176,11 +177,16 @@ public bool TryDecompress(ReadOnlySpan compressedData, Span destinat try { - return BoundedDecompression.TryDecompress( - compressedData, - destination, - static input => new BrotliStream(input, CompressionMode.Decompress, leaveOpen: true), - out bytesWritten); + // BrotliDecoder writes straight into the destination, so the destination bounds the work (issue #122), + // and it reports whether the final block arrived, which BrotliStream does not: input cut off part way + // read as a clean end and decompressed to a shorter output (issue #80). + if (!BrotliDecoder.TryDecompress(compressedData, destination, out bytesWritten)) + { + bytesWritten = 0; + return false; + } + + return true; } catch (ArgumentException) { @@ -211,9 +217,36 @@ public bool TryDecompress(Stream compressedData, Stream destination) try { - using BrotliStream brotliStream = new(compressedData, CompressionMode.Decompress, leaveOpen: true); - brotliStream.CopyTo(destination); - return true; + BrotliDecoder decoder = new(); + try + { + byte[] input = new byte[BufferSize]; + byte[] output = new byte[BufferSize]; + int offset = 0; + int available = 0; + bool ended = false; + while (true) + { + if (offset == available && !ended) + { + available = compressedData.Read(input, 0, input.Length); + offset = 0; + ended = available == 0; + } + + OperationStatus status = decoder.Decompress(input.AsSpan(offset, available - offset), output, out int consumed, out int written); + offset += consumed; + destination.Write(output, 0, written); + if (IsFinished(status, ended, out bool complete)) + { + return complete; + } + } + } + finally + { + decoder.Dispose(); + } } catch (ArgumentException) { @@ -263,13 +296,36 @@ public async Task TryDecompressAsync(Stream compressedData, Stream destina try { - BrotliStream brotliStream = new(compressedData, CompressionMode.Decompress, leaveOpen: true); - await using (brotliStream.ConfigureAwait(false)) + BrotliDecoder decoder = new(); + try { - await brotliStream.CopyToAsync(destination, 81920, cancellationToken).ConfigureAwait(false); - } + byte[] input = new byte[BufferSize]; + byte[] output = new byte[BufferSize]; + int offset = 0; + int available = 0; + bool ended = false; + while (true) + { + if (offset == available && !ended) + { + available = await compressedData.ReadAsync(input.AsMemory(), cancellationToken).ConfigureAwait(false); + offset = 0; + ended = available == 0; + } - return true; + OperationStatus status = decoder.Decompress(input.AsSpan(offset, available - offset), output, out int consumed, out int written); + offset += consumed; + await destination.WriteAsync(output.AsMemory(0, written), cancellationToken).ConfigureAwait(false); + if (IsFinished(status, ended, out bool complete)) + { + return complete; + } + } + } + finally + { + decoder.Dispose(); + } } catch (ArgumentException) { @@ -292,4 +348,34 @@ public async Task TryDecompressAsync(Stream compressedData, Stream destina return false; } } + + /// How many bytes of input and of output the stream paths work through at a time. + private const int BufferSize = 81920; + + /// + /// Reads one step of a stream loop. Decoding is driven by the decoder rather than + /// because only the decoder says whether the final block arrived: input that ends + /// before it is truncated, which reports as a clean end (issue #80). + /// + /// What the decoder reported. + /// Whether the input has run out. + /// When finished, whether the whole stream was decoded. + /// when the loop is done, with as its answer. + private static bool IsFinished(OperationStatus status, bool ended, out bool complete) + { + switch (status) + { + case OperationStatus.Done: + complete = true; + return true; + case OperationStatus.NeedMoreData when ended: + case OperationStatus.InvalidData: + complete = false; + return true; + default: + // More output to collect, or more input to read. + complete = false; + return false; + } + } } diff --git a/Essentials.CompressionProviders.Brotli/Essentials.CompressionProviders.Brotli.csproj b/Essentials.CompressionProviders.Brotli/Essentials.CompressionProviders.Brotli.csproj index e899806..801ec27 100644 --- a/Essentials.CompressionProviders.Brotli/Essentials.CompressionProviders.Brotli.csproj +++ b/Essentials.CompressionProviders.Brotli/Essentials.CompressionProviders.Brotli.csproj @@ -17,7 +17,6 @@ - diff --git a/Essentials.CompressionProviders.Deflate/DeflateCompressionProvider.cs b/Essentials.CompressionProviders.Deflate/DeflateCompressionProvider.cs index fbbf0a1..a155315 100644 --- a/Essentials.CompressionProviders.Deflate/DeflateCompressionProvider.cs +++ b/Essentials.CompressionProviders.Deflate/DeflateCompressionProvider.cs @@ -12,6 +12,11 @@ namespace ktsu.Essentials.CompressionProviders.Deflate; /// /// A compression provider that uses Deflate for data compression and decompression. /// +/// +/// Raw Deflate has no trailer, so input cut off at a block boundary cannot be told from a complete stream and +/// decompresses to the data before the cut. Use the Gzip, ZLib or Brotli provider where truncation must be +/// detected; each of those reports it as a failure (issue #80). +/// public class DeflateCompressionProvider : ICompressionProvider { /// @@ -180,6 +185,7 @@ public bool TryDecompress(ReadOnlySpan compressedData, Span destinat compressedData, destination, static input => new DeflateStream(input, CompressionMode.Decompress, leaveOpen: true), + isComplete: null, out bytesWritten); } catch (ArgumentException) diff --git a/Essentials.CompressionProviders.Gzip/Essentials.CompressionProviders.Gzip.csproj b/Essentials.CompressionProviders.Gzip/Essentials.CompressionProviders.Gzip.csproj index d77c9b7..7c13511 100644 --- a/Essentials.CompressionProviders.Gzip/Essentials.CompressionProviders.Gzip.csproj +++ b/Essentials.CompressionProviders.Gzip/Essentials.CompressionProviders.Gzip.csproj @@ -15,6 +15,7 @@ + diff --git a/Essentials.CompressionProviders.Gzip/GzipCompressionProvider.cs b/Essentials.CompressionProviders.Gzip/GzipCompressionProvider.cs index c2ede50..fb2a3ee 100644 --- a/Essentials.CompressionProviders.Gzip/GzipCompressionProvider.cs +++ b/Essentials.CompressionProviders.Gzip/GzipCompressionProvider.cs @@ -181,6 +181,7 @@ public bool TryDecompress(ReadOnlySpan compressedData, Span destinat compressedData, destination, static input => new GZipStream(input, CompressionMode.Decompress, leaveOpen: true), + static (output, input) => new CompressedStreamEnd.Gzip().IsComplete(output, input), out bytesWritten); } catch (ArgumentException) @@ -212,9 +213,7 @@ public bool TryDecompress(Stream compressedData, Stream destination) try { - using GZipStream gzipStream = new(compressedData, CompressionMode.Decompress, leaveOpen: true); - gzipStream.CopyTo(destination); - return true; + return new CompressedStreamEnd.Gzip().CopyTo(compressedData, destination, static input => new GZipStream(input, CompressionMode.Decompress, leaveOpen: true)); } catch (ArgumentException) { @@ -264,13 +263,7 @@ public async Task TryDecompressAsync(Stream compressedData, Stream destina try { - GZipStream gzipStream = new(compressedData, CompressionMode.Decompress, leaveOpen: true); - await using (gzipStream.ConfigureAwait(false)) - { - await gzipStream.CopyToAsync(destination, 81920, cancellationToken).ConfigureAwait(false); - } - - return true; + return await new CompressedStreamEnd.Gzip().CopyToAsync(compressedData, destination, static input => new GZipStream(input, CompressionMode.Decompress, leaveOpen: true), cancellationToken).ConfigureAwait(false); } catch (ArgumentException) { diff --git a/Essentials.CompressionProviders.ZLib/Essentials.CompressionProviders.ZLib.csproj b/Essentials.CompressionProviders.ZLib/Essentials.CompressionProviders.ZLib.csproj index 103b884..126e971 100644 --- a/Essentials.CompressionProviders.ZLib/Essentials.CompressionProviders.ZLib.csproj +++ b/Essentials.CompressionProviders.ZLib/Essentials.CompressionProviders.ZLib.csproj @@ -15,6 +15,7 @@ + diff --git a/Essentials.CompressionProviders.ZLib/ZLibCompressionProvider.cs b/Essentials.CompressionProviders.ZLib/ZLibCompressionProvider.cs index 3cb3c87..df01e43 100644 --- a/Essentials.CompressionProviders.ZLib/ZLibCompressionProvider.cs +++ b/Essentials.CompressionProviders.ZLib/ZLibCompressionProvider.cs @@ -180,6 +180,7 @@ public bool TryDecompress(ReadOnlySpan compressedData, Span destinat compressedData, destination, static input => new ZLibStream(input, CompressionMode.Decompress, leaveOpen: true), + static (output, input) => new CompressedStreamEnd.ZLib().IsComplete(output, input), out bytesWritten); } catch (ArgumentException) @@ -211,9 +212,7 @@ public bool TryDecompress(Stream compressedData, Stream destination) try { - using ZLibStream zlibStream = new(compressedData, CompressionMode.Decompress, leaveOpen: true); - zlibStream.CopyTo(destination); - return true; + return new CompressedStreamEnd.ZLib().CopyTo(compressedData, destination, static input => new ZLibStream(input, CompressionMode.Decompress, leaveOpen: true)); } catch (ArgumentException) { @@ -263,13 +262,7 @@ public async Task TryDecompressAsync(Stream compressedData, Stream destina try { - ZLibStream zlibStream = new(compressedData, CompressionMode.Decompress, leaveOpen: true); - await using (zlibStream.ConfigureAwait(false)) - { - await zlibStream.CopyToAsync(destination, 81920, cancellationToken).ConfigureAwait(false); - } - - return true; + return await new CompressedStreamEnd.ZLib().CopyToAsync(compressedData, destination, static input => new ZLibStream(input, CompressionMode.Decompress, leaveOpen: true), cancellationToken).ConfigureAwait(false); } catch (ArgumentException) { diff --git a/Essentials.Tests/TruncatedCompressionTests.cs b/Essentials.Tests/TruncatedCompressionTests.cs new file mode 100644 index 0000000..a65a1a5 --- /dev/null +++ b/Essentials.Tests/TruncatedCompressionTests.cs @@ -0,0 +1,132 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Essentials.Tests; + +using System; +using System.Collections.Generic; +using System.IO; +using System.Threading.Tasks; +using ktsu.Essentials; +using ktsu.Essentials.CompressionProviders.Brotli; +using ktsu.Essentials.CompressionProviders.Gzip; +using ktsu.Essentials.CompressionProviders.ZLib; +using ktsu.Essentials.Tests.Infrastructure; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Compressed data cut off part way is a failure, not a shorter success (issue #80). The framework decompressors +/// treat the end of the input as a clean end of stream, so the providers check the format's own end: Gzip's +/// CRC-32 and size trailer, ZLib's Adler-32 trailer, and Brotli's final block. Raw Deflate carries none of these +/// and is documented as unable to tell, so it is not covered here. +/// +[TestClass] +public sealed class TruncatedCompressionTests +{ + public TestContext TestContext { get; set; } = null!; + + public static IEnumerable Providers => + [ + [new GzipCompressionProvider()], + [new ZLibCompressionProvider()], + [new BrotliCompressionProvider()], + ]; + + [TestMethod] + [DynamicData(nameof(Providers))] + [System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1849:Call async methods when in an async method", Justification = "Deliberately exercises the synchronous stream path alongside the asynchronous one.")] + public async Task Truncated_Input_Is_Reported_As_A_Failure_On_Every_Path(ICompressionProvider provider) + { + string name = provider.GetType().Name; + byte[] payload = Payload(); + byte[] compressed = provider.Compress(payload); + + foreach (int length in Cuts(compressed.Length)) + { + byte[] truncated = compressed[..length]; + + Assert.IsFalse(provider.TryDecompress(truncated, new byte[payload.Length], out int written), $"{name} span accepted {length} of {compressed.Length} bytes"); + Assert.AreEqual(0, written, $"{name} span reported output for {length} bytes"); + + using (MemoryStream source = new(truncated)) + using (MemoryStream sink = new()) + { + Assert.IsFalse(provider.TryDecompress(source, sink), $"{name} stream accepted {length} of {compressed.Length} bytes"); + } + + using (AsyncOnlyStream source = new(truncated)) + using (AsyncOnlyStream sink = new()) + { + Assert.IsFalse(await provider.TryDecompressAsync(source, sink, TestContext.CancellationToken).ConfigureAwait(false), $"{name} async accepted {length} of {compressed.Length} bytes"); + } + + Assert.ThrowsExactly(() => provider.Decompress(truncated), $"{name} Decompress returned short output for {length} bytes"); + } + } + + [TestMethod] + [DynamicData(nameof(Providers))] + [System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1849:Call async methods when in an async method", Justification = "Deliberately exercises the synchronous stream path alongside the asynchronous one.")] + public async Task Complete_Input_Still_Decompresses_On_Every_Path(ICompressionProvider provider) + { + string name = provider.GetType().Name; + foreach (byte[] payload in (byte[][])[[], [42], Payload()]) + { + byte[] compressed = provider.Compress(payload); + + byte[] destination = new byte[payload.Length]; + Assert.IsTrue(provider.TryDecompress(compressed, destination, out int written), $"{name} span rejected {payload.Length} bytes"); + Assert.AreEqual(payload.Length, written); + CollectionAssert.AreEqual(payload, destination); + + using (MemoryStream source = new(compressed)) + using (MemoryStream sink = new()) + { + Assert.IsTrue(provider.TryDecompress(source, sink), $"{name} stream rejected {payload.Length} bytes"); + CollectionAssert.AreEqual(payload, sink.ToArray()); + } + + using (AsyncOnlyStream source = new(compressed)) + using (AsyncOnlyStream sink = new()) + { + Assert.IsTrue(await provider.TryDecompressAsync(source, sink, TestContext.CancellationToken).ConfigureAwait(false), $"{name} async rejected {payload.Length} bytes"); + CollectionAssert.AreEqual(payload, sink.ToArray()); + } + } + } + + [TestMethod] + public void Gzip_Accepts_Concatenated_Members() + { + // GZipStream reads every member of a concatenated file, as gzip itself does, and so must the check on + // its trailer, which is the last member's alone. + ICompressionProvider gzip = new GzipCompressionProvider(); + byte[] first = Payload(); + byte[] second = [.. Payload().AsSpan(0, 5000)]; + byte[] concatenated = [.. gzip.Compress(first), .. gzip.Compress(second)]; + + CollectionAssert.AreEqual((byte[])[.. first, .. second], gzip.Decompress(concatenated)); + } + + private static IEnumerable Cuts(int length) + { + foreach (int cut in new[] { 1, 2, 3, 4, 8, 12, length / 3, length / 2, length - 9, length - 8, length - 5, length - 4, length - 2, length - 1 }) + { + if (cut > 0 && cut < length) + { + yield return cut; + } + } + } + + private static byte[] Payload() + { + Random random = new(80); + byte[] payload = new byte[100_000]; + for (int i = 0; i < payload.Length; i++) + { + payload[i] = (byte)(i % 3 == 0 ? random.Next(256) : i % 17); + } + + return payload; + } +} diff --git a/Shared/BoundedDecompression.cs b/Shared/BoundedDecompression.cs index 5876de2..ffa2e9e 100644 --- a/Shared/BoundedDecompression.cs +++ b/Shared/BoundedDecompression.cs @@ -23,6 +23,12 @@ namespace ktsu.Essentials; /// internal static class BoundedDecompression { + /// Whether is everything describes. + /// Everything the decompressor produced. + /// The whole compressed input. + /// if the input was not cut off. + internal delegate bool CompletenessCheck(ReadOnlySpan output, ReadOnlySpan input); + /// /// Decompresses into , failing once the /// destination is full and more output remains. @@ -30,14 +36,18 @@ internal static class BoundedDecompression /// The compressed data to decompress. /// The destination to write the decompressed data to. /// Wraps the compressed input in the provider's decompressing stream. + /// + /// Tells a stream that reached its own end from one whose input ran out, which the decompressing stream reports + /// the same way (issue #80), or for a format that cannot tell. + /// /// The number of bytes written to , or zero on failure. /// if the whole output fit in ; otherwise, . /// /// Errors in the compressed data surface as the decompressor throws them, for the caller's own - /// catch blocks to turn into . Input that ends early is treated exactly - /// as the stream overloads treat it, since both read the same decompressing stream to its end. + /// catch blocks to turn into . Input that ends early is reported as + /// decides, as the stream overloads report it. /// - internal static bool TryDecompress(ReadOnlySpan compressedData, Span destination, Func openDecompressor, out int bytesWritten) + internal static bool TryDecompress(ReadOnlySpan compressedData, Span destination, Func openDecompressor, CompletenessCheck? isComplete, out int bytesWritten) { bytesWritten = 0; @@ -50,8 +60,7 @@ internal static bool TryDecompress(ReadOnlySpan compressedData, Span int read = decompressor.Read(destination[written..]); if (read == 0) { - bytesWritten = written; - return true; + return Finish(compressedData, destination[..written], isComplete, out bytesWritten); } written += read; @@ -65,7 +74,18 @@ internal static bool TryDecompress(ReadOnlySpan compressedData, Span return false; } - bytesWritten = written; + return Finish(compressedData, destination[..written], isComplete, out bytesWritten); + } + + private static bool Finish(ReadOnlySpan compressedData, ReadOnlySpan output, CompletenessCheck? isComplete, out int bytesWritten) + { + if (isComplete is not null && !isComplete(output, compressedData)) + { + bytesWritten = 0; + return false; + } + + bytesWritten = output.Length; return true; } } diff --git a/Shared/CompressedStreamEnd.cs b/Shared/CompressedStreamEnd.cs new file mode 100644 index 0000000..13f1b3a --- /dev/null +++ b/Shared/CompressedStreamEnd.cs @@ -0,0 +1,289 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Essentials; + +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +/// +/// Checks that a Gzip or ZLib stream reached its own end rather than stopping where its input ran out. +/// +/// +/// +/// Linked into the Gzip and ZLib providers rather than placed in the interfaces package, following +/// BoundedDecompression, so the interfaces take no dependency for it. GZipStream and ZLibStream +/// report the end of their input as a clean end of stream, so a payload cut off part way decompressed to a +/// shorter one and was reported as a success (issue #80). Both formats end in a checksum of everything before it, +/// which the decompressor verifies only if it gets that far: comparing the last bytes of the input against a +/// checksum of the output is what tells a complete stream from a truncated one. +/// +/// +/// The input's last bytes are its trailer only when the compressed data is all the input there is, so data +/// followed by anything else is reported as a failure. A Gzip file is allowed several members, read back to back +/// as gzip itself reads them; its trailer then describes only the last member, so a trailer whose size is less +/// than the whole output is taken for one, the members before it having been checked by the decompressor as it +/// crossed their trailers. +/// +/// +internal abstract class CompressedStreamEnd +{ + /// How many bytes of trailer the longest format here ends with. + internal const int MaxTrailerLength = 8; + + private const int CopyBufferSize = 81920; + + /// Adds decompressed output to the running checksum. + /// The next run of output. + internal abstract void Append(ReadOnlySpan output); + + /// Whether the input ended in the trailer the output so far calls for. + /// The last bytes of the input, up to of them. + /// if the stream reached its own end. + internal abstract bool EndsWith(ReadOnlySpan inputTail); + + /// + /// Whether is everything describes, rather than what it + /// held before it was cut off. + /// + /// Everything the decompressor produced. + /// The whole compressed input. + /// if the input ended in the trailer for that output. + internal bool IsComplete(ReadOnlySpan output, ReadOnlySpan input) + { + // The providers compress empty data to no bytes at all, so no bytes read back as empty data. + if (input.IsEmpty) + { + return output.IsEmpty; + } + + Append(output); + return EndsWith(input[Math.Max(0, input.Length - MaxTrailerLength)..]); + } + + /// + /// Decompresses into , then reports whether + /// the stream reached its own end. + /// + /// The compressed input. + /// The stream to write the output to. + /// Wraps the input in the provider's decompressing stream. + /// if the whole stream was decompressed. + internal bool CopyTo(Stream compressedData, Stream destination, Func openDecompressor) + { + using TailRecordingStream input = new(compressedData); + using Stream decompressor = openDecompressor(input); + + byte[] buffer = new byte[CopyBufferSize]; + int read; + while ((read = decompressor.Read(buffer, 0, buffer.Length)) > 0) + { + Append(buffer.AsSpan(0, read)); + destination.Write(buffer, 0, read); + } + + // The providers compress empty data to no bytes at all, so no bytes read back as empty data. + return input.Tail.IsEmpty || EndsWith(input.Tail); + } + + /// The asynchronous counterpart of , touching only the streams' asynchronous members. + /// The compressed input. + /// The stream to write the output to. + /// Wraps the input in the provider's decompressing stream. + /// The cancellation token. + /// if the whole stream was decompressed. + internal async Task CopyToAsync(Stream compressedData, Stream destination, Func openDecompressor, CancellationToken cancellationToken) + { + using TailRecordingStream input = new(compressedData); + Stream decompressor = openDecompressor(input); + await using (decompressor.ConfigureAwait(false)) + { + byte[] buffer = new byte[CopyBufferSize]; + int read; + while ((read = await decompressor.ReadAsync(buffer.AsMemory(), cancellationToken).ConfigureAwait(false)) > 0) + { + Append(buffer.AsSpan(0, read)); + await destination.WriteAsync(buffer.AsMemory(0, read), cancellationToken).ConfigureAwait(false); + } + } + + // The providers compress empty data to no bytes at all, so no bytes read back as empty data. + return input.Tail.IsEmpty || EndsWith(input.Tail); + } + + /// Gzip's trailer: the CRC-32 of the member's output, then its length modulo 2^32, both little-endian. + internal sealed class Gzip : CompressedStreamEnd + { + private uint crc = 0xFFFFFFFFu; + private uint length; + + internal override void Append(ReadOnlySpan output) + { + crc = Crc32.Update(crc, output); + length = unchecked(length + (uint)output.Length); + } + + internal override bool EndsWith(ReadOnlySpan inputTail) + { + if (inputTail.Length < 8) + { + return false; + } + + uint trailerCrc = ReadUInt32LittleEndian(inputTail[^8..]); + uint trailerLength = ReadUInt32LittleEndian(inputTail[^4..]); + return (trailerCrc == ~crc && trailerLength == length) || trailerLength < length; + } + } + + /// ZLib's trailer: the Adler-32 of the output, big-endian. + internal sealed class ZLib : CompressedStreamEnd + { + private const uint Modulus = 65521; + + // The largest run whose sums cannot overflow 32 bits before they are reduced. + private const int MaxRun = 5552; + + private uint a = 1; + private uint b; + + internal override void Append(ReadOnlySpan output) + { + while (!output.IsEmpty) + { + int run = Math.Min(output.Length, MaxRun); + foreach (byte value in output[..run]) + { + a += value; + b += a; + } + + a %= Modulus; + b %= Modulus; + output = output[run..]; + } + } + + internal override bool EndsWith(ReadOnlySpan inputTail) + { + if (inputTail.Length < 4) + { + return false; + } + + ReadOnlySpan trailer = inputTail[^4..]; + uint expected = ((uint)trailer[0] << 24) | ((uint)trailer[1] << 16) | ((uint)trailer[2] << 8) | trailer[3]; + return expected == ((b << 16) | a); + } + } + + private static uint ReadUInt32LittleEndian(ReadOnlySpan bytes) + => bytes[0] | ((uint)bytes[1] << 8) | ((uint)bytes[2] << 16) | ((uint)bytes[3] << 24); + + /// The CRC-32 Gzip uses (the reflected IEEE 802.3 polynomial), table driven. + private static class Crc32 + { + private static readonly uint[] Table = CreateTable(); + + internal static uint Update(uint crc, ReadOnlySpan data) + { + foreach (byte value in data) + { + crc = Table[(crc ^ value) & 0xFF] ^ (crc >> 8); + } + + return crc; + } + + private static uint[] CreateTable() + { + uint[] table = new uint[256]; + for (uint n = 0; n < table.Length; n++) + { + uint c = n; + for (int k = 0; k < 8; k++) + { + c = (c & 1) != 0 ? 0xEDB88320u ^ (c >> 1) : c >> 1; + } + + table[n] = c; + } + + return table; + } + } + + /// + /// Passes reads through to the compressed input, keeping the last bytes read. + /// Disposing it leaves the input open, as the providers' decompressors are told to. + /// + private sealed class TailRecordingStream(Stream inner) : Stream + { + private readonly byte[] tail = new byte[MaxTrailerLength]; + private int tailLength; + + /// Gets the last bytes read, oldest first. + internal ReadOnlySpan Tail => tail.AsSpan(MaxTrailerLength - tailLength); + + 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 int Read(byte[] buffer, int offset, int count) + { + int read = inner.Read(buffer, offset, count); + Record(buffer.AsSpan(offset, read)); + return read; + } + + public override async Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + int read = await inner.ReadAsync(buffer.AsMemory(offset, count), cancellationToken).ConfigureAwait(false); + Record(buffer.AsSpan(offset, read)); + return read; + } + + public override async ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) + { + int read = await inner.ReadAsync(buffer, cancellationToken).ConfigureAwait(false); + Record(buffer.Span[..read]); + return read; + } + + public override void Flush() + { + } + + 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(); + + private void Record(ReadOnlySpan read) + { + if (read.Length >= MaxTrailerLength) + { + read[^MaxTrailerLength..].CopyTo(tail); + tailLength = MaxTrailerLength; + return; + } + + // Shift the kept bytes down to make room, keeping them in the order they were read. + tail.AsSpan(read.Length).CopyTo(tail); + read.CopyTo(tail.AsSpan(MaxTrailerLength - read.Length)); + tailLength = Math.Min(MaxTrailerLength, tailLength + read.Length); + } + } +} From 1ac587f29a057a0276dadb2ba62ea2a4a837c264 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 11:44:13 +0000 Subject: [PATCH 21/23] Make cache and key enumeration filters explicit Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- .../InMemoryCacheProvider.cs | 12 +++++------- Shared/FileRecordStore.cs | 3 +-- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/Essentials.CacheProviders.InMemory/InMemoryCacheProvider.cs b/Essentials.CacheProviders.InMemory/InMemoryCacheProvider.cs index 27accc1..5547d5b 100644 --- a/Essentials.CacheProviders.InMemory/InMemoryCacheProvider.cs +++ b/Essentials.CacheProviders.InMemory/InMemoryCacheProvider.cs @@ -7,6 +7,7 @@ namespace ktsu.Essentials.CacheProviders.InMemory; using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; +using System.Linq; using System.Threading; /// @@ -119,9 +120,9 @@ public TValue GetOrAdd(TKey key, Func factory, TimeSpan? expiratio while (true) { - if (cache.TryGetValue(key, out CacheEntry? observed) && !observed.IsExpired(clock())) + if (cache.TryGetValue(key, out CacheEntry? observed) && observed is { } existing && !existing.IsExpired(clock())) { - return observed.Value; + return existing.Value; } TValue value = factory(key); @@ -190,12 +191,9 @@ private void SweepIfDue(long now) { Interlocked.Exchange(ref setsSinceSweep, 0); Interlocked.Exchange(ref lastSweep, now); - foreach (KeyValuePair pair in cache) + foreach (KeyValuePair pair in cache.Where(pair => pair.Value.IsExpired(now))) { - if (pair.Value.IsExpired(now)) - { - RemoveEntry(pair.Key, pair.Value); - } + RemoveEntry(pair.Key, pair.Value); } } finally diff --git a/Shared/FileRecordStore.cs b/Shared/FileRecordStore.cs index 5438c3c..f09dc07 100644 --- a/Shared/FileRecordStore.cs +++ b/Shared/FileRecordStore.cs @@ -181,9 +181,8 @@ internal IEnumerable GetAllKeys() } List keys = []; - foreach (string file in fileSystem.Directory.GetFiles(Directory, "*" + serializer.FileExtension, SearchOption.TopDirectoryOnly)) + foreach (string? name in fileSystem.Directory.GetFiles(Directory, "*" + serializer.FileExtension, SearchOption.TopDirectoryOnly).Select(fileSystem.Path.GetFileNameWithoutExtension)) { - string? name = fileSystem.Path.GetFileNameWithoutExtension(file); if (string.IsNullOrEmpty(name)) { continue; From 4778d5b953dbd3697977299bc934913349ac1390 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 12:01:50 +0000 Subject: [PATCH 22/23] Retry a Windows read refused while a replace is pending A read landing as an overwriting move swaps the target can be refused with access denied rather than a sharing violation, because the replaced file is still pending deletion. The concurrent-store test caught it on the Windows runner. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- Shared/AtomicFile.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Shared/AtomicFile.cs b/Shared/AtomicFile.cs index 42519f2..40c17ce 100644 --- a/Shared/AtomicFile.cs +++ b/Shared/AtomicFile.cs @@ -99,11 +99,13 @@ internal static async Task ReadAllTextAsync(IFileSystem fileSystem, stri { return await fileSystem.File.ReadAllTextAsync(filePath, cancellationToken).ConfigureAwait(false); } - catch (IOException ex) when (IsSharingViolation(ex)) + catch (Exception ex) when ((ex is IOException io && IsSharingViolation(io)) || (ex is UnauthorizedAccessException && IsWindows)) { // Windows holds the target exclusively for the instant an overwriting move swaps it, so a - // read that lands then is refused rather than served either version. Only that refusal is - // retried; a missing file and every other error surface at once. + // read that lands then is refused rather than served either version: as a sharing violation, + // or as access denied while the replaced file is still pending deletion. Only those refusals + // are retried, and a real permission problem still surfaces once the attempts run out; a + // missing file and every other error surface at once. await Task.Delay(Backoff(attempt), cancellationToken).ConfigureAwait(false); } } From 2e03e5b9464c20fe6fdeb486f1c4d9c7f79206ce Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 12:37:06 +0000 Subject: [PATCH 23/23] Share the chunked stream loop and make the roll's moves visible to analysis The FNV hash and Xor, Caesar and BitRotate obfuscation stream paths each carried the same pooled-buffer loop, which SonarCloud counted as duplication; it now lives once in Shared/StreamChunks.cs. The file logger's roll recorded its moves through a local function, so Sonar read the undo loop as unreachable (S2583); the moves list is now passed in. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01TmbAehyjJjPCQdRASdECes --- CLAUDE.md | 1 + .../Essentials.HashProviders.FNV1_32.csproj | 4 ++ .../FNV1_32HashProvider.cs | 23 +++----- .../Essentials.HashProviders.FNV1_64.csproj | 4 ++ .../FNV1_64HashProvider.cs | 23 +++----- .../Essentials.HashProviders.FNV1a_32.csproj | 4 ++ .../FNV1a_32HashProvider.cs | 23 +++----- .../Essentials.HashProviders.FNV1a_64.csproj | 4 ++ .../FNV1a_64HashProvider.cs | 23 +++----- .../FileLoggingProvider.cs | 22 ++++---- .../BitRotateObfuscationProvider.cs | 52 +++++-------------- ...ials.ObfuscationProviders.BitRotate.csproj | 1 + .../CaesarObfuscationProvider.cs | 52 +++++-------------- ...entials.ObfuscationProviders.Caesar.csproj | 1 + ...Essentials.ObfuscationProviders.Xor.csproj | 1 + .../XorObfuscationProvider.cs | 30 ++++------- Shared/StreamChunks.cs | 52 +++++++++++++++++++ 17 files changed, 146 insertions(+), 174 deletions(-) create mode 100644 Shared/StreamChunks.cs diff --git a/CLAUDE.md b/CLAUDE.md index 9bdbf4f..fbc3d4c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -47,6 +47,7 @@ This is a .NET library (`ktsu.Essentials`) providing high-performance interfaces - `Shared/DeflateBound.cs` - zlib's `deflateBound` plus container overhead, linked into the Deflate, Gzip and ZLib providers as their `GetMaxCompressedLength` - `Shared/BoundedDecompression.cs` - The span overload of `TryDecompress` for Deflate, Gzip and ZLib: reads the decompressing stream straight into the destination and fails once it is full and more output remains, so the destination bounds the work. See issue #122. Brotli uses `BrotliDecoder` directly instead, which reports a truncated stream itself - `Shared/CompressedStreamEnd.cs` - Gzip's CRC-32 and length trailer and ZLib's Adler-32, checked against the output on every decompression path, because `GZipStream` and `ZLibStream` treat input that runs out as a clean end of stream. Raw Deflate has no trailer, so its truncation cannot be detected. See issue #80 +- `Shared/StreamChunks.cs` - Reads a stream a chunk at a time through a pooled buffer, linked into the FNV hash and the Xor, Caesar and BitRotate obfuscation providers, whose stream paths otherwise made a call per byte. See issue #91 - `Shared/SpecialFunctions.cs` - Incomplete gamma, incomplete beta, error function, normal quantile and cancellation-free `ExpM1`/`Log1P`, linked into the Normal, LogNormal, Exponential, Binomial, Poisson and Geometric providers rather than placed in the interfaces-only package - `Shared/DistributionArguments.cs` - Parameter validation shared by every distribution provider, linked in the same way - `Shared/ProviderRegistration.cs` - DI registration for providers whose `Add…Provider` overloads take arguments (Xor, Caesar, BitRotate and every distribution): a call replaces an earlier registration of the same concrete type in place, and `AddEssentials()` keeps a registration made before it, whether as the concrete type, the interface or both, moved to its default's position, rather than overwriting it; a concrete-only one gains an interface registration forwarding to it. Linked into those providers and `Essentials.All`. See issues #132 and #156 diff --git a/Essentials.HashProviders.FNV1_32/Essentials.HashProviders.FNV1_32.csproj b/Essentials.HashProviders.FNV1_32/Essentials.HashProviders.FNV1_32.csproj index 3bd8cea..e9c7748 100644 --- a/Essentials.HashProviders.FNV1_32/Essentials.HashProviders.FNV1_32.csproj +++ b/Essentials.HashProviders.FNV1_32/Essentials.HashProviders.FNV1_32.csproj @@ -16,4 +16,8 @@ + + + + diff --git a/Essentials.HashProviders.FNV1_32/FNV1_32HashProvider.cs b/Essentials.HashProviders.FNV1_32/FNV1_32HashProvider.cs index 5175b70..5c49747 100644 --- a/Essentials.HashProviders.FNV1_32/FNV1_32HashProvider.cs +++ b/Essentials.HashProviders.FNV1_32/FNV1_32HashProvider.cs @@ -4,7 +4,6 @@ namespace ktsu.Essentials.HashProviders.FNV1_32; using ktsu.Essentials; using System; -using System.Buffers; using System.IO; /// @@ -16,7 +15,6 @@ public class FNV1_32HashProvider : IHashProvider private const uint FnvPrime32 = 0x01000193; /// How many bytes the stream path reads at a time. - private const int StreamBufferSize = 81920; /// /// The length of the FNV-1 32-bit hash in bytes (4 bytes / 32 bits). @@ -88,25 +86,16 @@ public bool TryHash(Stream data, Span destination, out int bytesWritten) try { uint hash = FnvOffsetBasis32; - // A chunk at a time: the base Stream.ReadByte allocates and makes a Read call per byte, which on a - // stream that does not override it cost eleven times as long and 32 bytes of garbage per byte (issue #91). - byte[] buffer = ArrayPool.Shared.Rent(StreamBufferSize); - try - { - int read; - while ((read = data.Read(buffer, 0, buffer.Length)) > 0) + StreamChunks.ForEach( + data, + chunk => { - for (int i = 0; i < read; i++) + for (int i = 0; i < chunk.Length; i++) { hash *= FnvPrime32; - hash ^= buffer[i]; + hash ^= chunk[i]; } - } - } - finally - { - ArrayPool.Shared.Return(buffer, clearArray: true); - } + }); // Write the hash as little-endian bytes destination[0] = (byte)(hash & 0xFF); diff --git a/Essentials.HashProviders.FNV1_64/Essentials.HashProviders.FNV1_64.csproj b/Essentials.HashProviders.FNV1_64/Essentials.HashProviders.FNV1_64.csproj index 3bd8cea..e9c7748 100644 --- a/Essentials.HashProviders.FNV1_64/Essentials.HashProviders.FNV1_64.csproj +++ b/Essentials.HashProviders.FNV1_64/Essentials.HashProviders.FNV1_64.csproj @@ -16,4 +16,8 @@ + + + + diff --git a/Essentials.HashProviders.FNV1_64/FNV1_64HashProvider.cs b/Essentials.HashProviders.FNV1_64/FNV1_64HashProvider.cs index 5d61d96..e71e344 100644 --- a/Essentials.HashProviders.FNV1_64/FNV1_64HashProvider.cs +++ b/Essentials.HashProviders.FNV1_64/FNV1_64HashProvider.cs @@ -4,7 +4,6 @@ namespace ktsu.Essentials.HashProviders.FNV1_64; using ktsu.Essentials; using System; -using System.Buffers; using System.IO; /// @@ -16,7 +15,6 @@ public class FNV1_64HashProvider : IHashProvider private const ulong FnvPrime64 = 0x00000100000001b3; /// How many bytes the stream path reads at a time. - private const int StreamBufferSize = 81920; /// /// The length of the FNV-1 64-bit hash in bytes (8 bytes / 64 bits). @@ -92,25 +90,16 @@ public bool TryHash(Stream data, Span destination, out int bytesWritten) try { ulong hash = FnvOffsetBasis64; - // A chunk at a time: the base Stream.ReadByte allocates and makes a Read call per byte, which on a - // stream that does not override it cost eleven times as long and 32 bytes of garbage per byte (issue #91). - byte[] buffer = ArrayPool.Shared.Rent(StreamBufferSize); - try - { - int read; - while ((read = data.Read(buffer, 0, buffer.Length)) > 0) + StreamChunks.ForEach( + data, + chunk => { - for (int i = 0; i < read; i++) + for (int i = 0; i < chunk.Length; i++) { hash *= FnvPrime64; - hash ^= buffer[i]; + hash ^= chunk[i]; } - } - } - finally - { - ArrayPool.Shared.Return(buffer, clearArray: true); - } + }); // Write the hash as little-endian bytes destination[0] = (byte)(hash & 0xFF); diff --git a/Essentials.HashProviders.FNV1a_32/Essentials.HashProviders.FNV1a_32.csproj b/Essentials.HashProviders.FNV1a_32/Essentials.HashProviders.FNV1a_32.csproj index 3bd8cea..e9c7748 100644 --- a/Essentials.HashProviders.FNV1a_32/Essentials.HashProviders.FNV1a_32.csproj +++ b/Essentials.HashProviders.FNV1a_32/Essentials.HashProviders.FNV1a_32.csproj @@ -16,4 +16,8 @@ + + + + diff --git a/Essentials.HashProviders.FNV1a_32/FNV1a_32HashProvider.cs b/Essentials.HashProviders.FNV1a_32/FNV1a_32HashProvider.cs index b5c061d..f9d5741 100644 --- a/Essentials.HashProviders.FNV1a_32/FNV1a_32HashProvider.cs +++ b/Essentials.HashProviders.FNV1a_32/FNV1a_32HashProvider.cs @@ -4,7 +4,6 @@ namespace ktsu.Essentials.HashProviders.FNV1a_32; using ktsu.Essentials; using System; -using System.Buffers; using System.IO; /// @@ -16,7 +15,6 @@ public class FNV1a_32HashProvider : IHashProvider private const uint FnvPrime32 = 0x01000193; /// How many bytes the stream path reads at a time. - private const int StreamBufferSize = 81920; /// /// The length of the FNV-1a 32-bit hash in bytes (4 bytes / 32 bits). @@ -88,25 +86,16 @@ public bool TryHash(Stream data, Span destination, out int bytesWritten) try { uint hash = FnvOffsetBasis32; - // A chunk at a time: the base Stream.ReadByte allocates and makes a Read call per byte, which on a - // stream that does not override it cost eleven times as long and 32 bytes of garbage per byte (issue #91). - byte[] buffer = ArrayPool.Shared.Rent(StreamBufferSize); - try - { - int read; - while ((read = data.Read(buffer, 0, buffer.Length)) > 0) + StreamChunks.ForEach( + data, + chunk => { - for (int i = 0; i < read; i++) + for (int i = 0; i < chunk.Length; i++) { - hash ^= buffer[i]; + hash ^= chunk[i]; hash *= FnvPrime32; } - } - } - finally - { - ArrayPool.Shared.Return(buffer, clearArray: true); - } + }); // Write the hash as little-endian bytes destination[0] = (byte)(hash & 0xFF); diff --git a/Essentials.HashProviders.FNV1a_64/Essentials.HashProviders.FNV1a_64.csproj b/Essentials.HashProviders.FNV1a_64/Essentials.HashProviders.FNV1a_64.csproj index 3bd8cea..e9c7748 100644 --- a/Essentials.HashProviders.FNV1a_64/Essentials.HashProviders.FNV1a_64.csproj +++ b/Essentials.HashProviders.FNV1a_64/Essentials.HashProviders.FNV1a_64.csproj @@ -16,4 +16,8 @@ + + + + diff --git a/Essentials.HashProviders.FNV1a_64/FNV1a_64HashProvider.cs b/Essentials.HashProviders.FNV1a_64/FNV1a_64HashProvider.cs index 499cd73..111d8ad 100644 --- a/Essentials.HashProviders.FNV1a_64/FNV1a_64HashProvider.cs +++ b/Essentials.HashProviders.FNV1a_64/FNV1a_64HashProvider.cs @@ -4,7 +4,6 @@ namespace ktsu.Essentials.HashProviders.FNV1a_64; using ktsu.Essentials; using System; -using System.Buffers; using System.IO; /// @@ -16,7 +15,6 @@ public class FNV1a_64HashProvider : IHashProvider private const ulong FnvPrime64 = 0x00000100000001b3; /// How many bytes the stream path reads at a time. - private const int StreamBufferSize = 81920; /// /// The length of the FNV-1a 64-bit hash in bytes (8 bytes / 64 bits). @@ -92,25 +90,16 @@ public bool TryHash(Stream data, Span destination, out int bytesWritten) try { ulong hash = FnvOffsetBasis64; - // A chunk at a time: the base Stream.ReadByte allocates and makes a Read call per byte, which on a - // stream that does not override it cost eleven times as long and 32 bytes of garbage per byte (issue #91). - byte[] buffer = ArrayPool.Shared.Rent(StreamBufferSize); - try - { - int read; - while ((read = data.Read(buffer, 0, buffer.Length)) > 0) + StreamChunks.ForEach( + data, + chunk => { - for (int i = 0; i < read; i++) + for (int i = 0; i < chunk.Length; i++) { - hash ^= buffer[i]; + hash ^= chunk[i]; hash *= FnvPrime64; } - } - } - finally - { - ArrayPool.Shared.Return(buffer, clearArray: true); - } + }); // Write the hash as little-endian bytes destination[0] = (byte)(hash & 0xFF); diff --git a/Essentials.LoggingProviders.File/FileLoggingProvider.cs b/Essentials.LoggingProviders.File/FileLoggingProvider.cs index 69d98c3..07fac20 100644 --- a/Essentials.LoggingProviders.File/FileLoggingProvider.cs +++ b/Essentials.LoggingProviders.File/FileLoggingProvider.cs @@ -285,13 +285,13 @@ private bool TryRoll() try { - Move(FilePath, activeStaging); + Move(moves, FilePath, activeStaging); string oldest = GetRolledFilePath(maxFileCount - 1); if (fileSystem.File.Exists(oldest)) { oldestStaging = StagingPath(); - Move(oldest, oldestStaging); + Move(moves, oldest, oldestStaging); } for (int index = maxFileCount - 2; index >= 1; index--) @@ -299,11 +299,11 @@ private bool TryRoll() string source = GetRolledFilePath(index); if (fileSystem.File.Exists(source)) { - Move(source, GetRolledFilePath(index + 1)); + Move(moves, source, GetRolledFilePath(index + 1)); } } - Move(activeStaging, GetRolledFilePath(1)); + Move(moves, activeStaging, GetRolledFilePath(1)); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { @@ -335,12 +335,16 @@ private bool TryRoll() } return true; + } - void Move(string from, string to) - { - fileSystem.File.Move(from, to); - moves.Add((from, to)); - } + /// Moves a file and records the move, so a roll refused part way can undo it. + /// The moves made so far. + /// The file to move. + /// Where it goes. + private void Move(List<(string From, string To)> moves, string from, string to) + { + fileSystem.File.Move(from, to); + moves.Add((from, to)); } private string StagingPath() diff --git a/Essentials.ObfuscationProviders.BitRotate/BitRotateObfuscationProvider.cs b/Essentials.ObfuscationProviders.BitRotate/BitRotateObfuscationProvider.cs index 16dc4d8..54a0f29 100644 --- a/Essentials.ObfuscationProviders.BitRotate/BitRotateObfuscationProvider.cs +++ b/Essentials.ObfuscationProviders.BitRotate/BitRotateObfuscationProvider.cs @@ -4,7 +4,6 @@ namespace ktsu.Essentials.ObfuscationProviders.BitRotate; using ktsu.Essentials; using System; -using System.Buffers; using System.IO; /// @@ -13,9 +12,6 @@ namespace ktsu.Essentials.ObfuscationProviders.BitRotate; /// public class BitRotateObfuscationProvider : IObfuscationProvider { - /// How many bytes the stream paths transform at a time. - private const int StreamBufferSize = 81920; - private readonly int _bits; /// Initializes a new instance rotating by 3 bits. @@ -74,26 +70,16 @@ public bool TryObfuscate(Stream data, Stream destination) try { - // A chunk at a time: the base Stream.ReadByte and WriteByte allocate and make a call per byte on any - // stream that does not override them (issue #91). - byte[] buffer = ArrayPool.Shared.Rent(StreamBufferSize); - try - { - int read; - while ((read = data.Read(buffer, 0, buffer.Length)) > 0) + StreamChunks.ForEach( + data, + chunk => { - for (int i = 0; i < read; i++) + for (int i = 0; i < chunk.Length; i++) { - buffer[i] = RotateLeft(buffer[i], _bits); + chunk[i] = RotateLeft(chunk[i], _bits); } - - destination.Write(buffer, 0, read); - } - } - finally - { - ArrayPool.Shared.Return(buffer, clearArray: true); - } + }, + destination); return true; } @@ -136,26 +122,16 @@ public bool TryDeobfuscate(Stream obfuscatedData, Stream destination) try { - // A chunk at a time: the base Stream.ReadByte and WriteByte allocate and make a call per byte on any - // stream that does not override them (issue #91). - byte[] buffer = ArrayPool.Shared.Rent(StreamBufferSize); - try - { - int read; - while ((read = obfuscatedData.Read(buffer, 0, buffer.Length)) > 0) + StreamChunks.ForEach( + obfuscatedData, + chunk => { - for (int i = 0; i < read; i++) + for (int i = 0; i < chunk.Length; i++) { - buffer[i] = RotateRight(buffer[i], _bits); + chunk[i] = RotateRight(chunk[i], _bits); } - - destination.Write(buffer, 0, read); - } - } - finally - { - ArrayPool.Shared.Return(buffer, clearArray: true); - } + }, + destination); return true; } diff --git a/Essentials.ObfuscationProviders.BitRotate/Essentials.ObfuscationProviders.BitRotate.csproj b/Essentials.ObfuscationProviders.BitRotate/Essentials.ObfuscationProviders.BitRotate.csproj index b208b9e..c14b5d5 100644 --- a/Essentials.ObfuscationProviders.BitRotate/Essentials.ObfuscationProviders.BitRotate.csproj +++ b/Essentials.ObfuscationProviders.BitRotate/Essentials.ObfuscationProviders.BitRotate.csproj @@ -13,6 +13,7 @@ + diff --git a/Essentials.ObfuscationProviders.Caesar/CaesarObfuscationProvider.cs b/Essentials.ObfuscationProviders.Caesar/CaesarObfuscationProvider.cs index dc81602..7c52d60 100644 --- a/Essentials.ObfuscationProviders.Caesar/CaesarObfuscationProvider.cs +++ b/Essentials.ObfuscationProviders.Caesar/CaesarObfuscationProvider.cs @@ -3,7 +3,6 @@ namespace ktsu.Essentials.ObfuscationProviders.Caesar; using System; -using System.Buffers; using System.IO; using ktsu.Essentials; @@ -13,9 +12,6 @@ namespace ktsu.Essentials.ObfuscationProviders.Caesar; /// public class CaesarObfuscationProvider(byte shift = 13) : IObfuscationProvider { - /// How many bytes the stream paths transform at a time. - private const int StreamBufferSize = 81920; - private readonly byte _shift = shift; /// @@ -55,26 +51,16 @@ public bool TryObfuscate(Stream data, Stream destination) try { - // A chunk at a time: the base Stream.ReadByte and WriteByte allocate and make a call per byte on any - // stream that does not override them (issue #91). - byte[] buffer = ArrayPool.Shared.Rent(StreamBufferSize); - try - { - int read; - while ((read = data.Read(buffer, 0, buffer.Length)) > 0) + StreamChunks.ForEach( + data, + chunk => { - for (int i = 0; i < read; i++) + for (int i = 0; i < chunk.Length; i++) { - buffer[i] = (byte)(buffer[i] + _shift); + chunk[i] = (byte)(chunk[i] + _shift); } - - destination.Write(buffer, 0, read); - } - } - finally - { - ArrayPool.Shared.Return(buffer, clearArray: true); - } + }, + destination); return true; } @@ -117,26 +103,16 @@ public bool TryDeobfuscate(Stream obfuscatedData, Stream destination) try { - // A chunk at a time: the base Stream.ReadByte and WriteByte allocate and make a call per byte on any - // stream that does not override them (issue #91). - byte[] buffer = ArrayPool.Shared.Rent(StreamBufferSize); - try - { - int read; - while ((read = obfuscatedData.Read(buffer, 0, buffer.Length)) > 0) + StreamChunks.ForEach( + obfuscatedData, + chunk => { - for (int i = 0; i < read; i++) + for (int i = 0; i < chunk.Length; i++) { - buffer[i] = (byte)(buffer[i] - _shift); + chunk[i] = (byte)(chunk[i] - _shift); } - - destination.Write(buffer, 0, read); - } - } - finally - { - ArrayPool.Shared.Return(buffer, clearArray: true); - } + }, + destination); return true; } diff --git a/Essentials.ObfuscationProviders.Caesar/Essentials.ObfuscationProviders.Caesar.csproj b/Essentials.ObfuscationProviders.Caesar/Essentials.ObfuscationProviders.Caesar.csproj index b208b9e..c14b5d5 100644 --- a/Essentials.ObfuscationProviders.Caesar/Essentials.ObfuscationProviders.Caesar.csproj +++ b/Essentials.ObfuscationProviders.Caesar/Essentials.ObfuscationProviders.Caesar.csproj @@ -13,6 +13,7 @@ + diff --git a/Essentials.ObfuscationProviders.Xor/Essentials.ObfuscationProviders.Xor.csproj b/Essentials.ObfuscationProviders.Xor/Essentials.ObfuscationProviders.Xor.csproj index b208b9e..c14b5d5 100644 --- a/Essentials.ObfuscationProviders.Xor/Essentials.ObfuscationProviders.Xor.csproj +++ b/Essentials.ObfuscationProviders.Xor/Essentials.ObfuscationProviders.Xor.csproj @@ -13,6 +13,7 @@ + diff --git a/Essentials.ObfuscationProviders.Xor/XorObfuscationProvider.cs b/Essentials.ObfuscationProviders.Xor/XorObfuscationProvider.cs index 206c1da..a64bbf6 100644 --- a/Essentials.ObfuscationProviders.Xor/XorObfuscationProvider.cs +++ b/Essentials.ObfuscationProviders.Xor/XorObfuscationProvider.cs @@ -3,7 +3,6 @@ namespace ktsu.Essentials.ObfuscationProviders.Xor; using System; -using System.Buffers; using System.IO; using ktsu.Essentials; @@ -13,8 +12,6 @@ namespace ktsu.Essentials.ObfuscationProviders.Xor; /// public class XorObfuscationProvider : IObfuscationProvider { - /// How many bytes the stream paths transform at a time. - private const int StreamBufferSize = 81920; private readonly byte[] _key; @@ -72,28 +69,19 @@ public bool TryObfuscate(Stream data, Stream destination) try { - // A chunk at a time: the base Stream.ReadByte and WriteByte allocate and make a call per byte on any - // stream that does not override them (issue #91). - byte[] buffer = ArrayPool.Shared.Rent(StreamBufferSize); - try - { - long offset = 0; - int read; - while ((read = data.Read(buffer, 0, buffer.Length)) > 0) + long offset = 0; + StreamChunks.ForEach( + data, + chunk => { - for (int i = 0; i < read; i++) + for (int i = 0; i < chunk.Length; i++) { - buffer[i] ^= _key[(int)((offset + i) % _key.Length)]; + chunk[i] ^= _key[(int)((offset + i) % _key.Length)]; } - destination.Write(buffer, 0, read); - offset += read; - } - } - finally - { - ArrayPool.Shared.Return(buffer, clearArray: true); - } + offset += chunk.Length; + }, + destination); return true; } diff --git a/Shared/StreamChunks.cs b/Shared/StreamChunks.cs new file mode 100644 index 0000000..4a8bfca --- /dev/null +++ b/Shared/StreamChunks.cs @@ -0,0 +1,52 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Essentials; + +using System; +using System.Buffers; +using System.IO; + +/// +/// Reads a stream a chunk at a time through a pooled buffer, for the providers whose stream paths transform or +/// hash each byte in place. +/// +/// +/// Linked into the FNV hash and the Xor, Caesar and BitRotate obfuscation providers rather than placed in the +/// interfaces package. The base and allocate and +/// make a call per byte on any stream that does not override them, which cost eleven times as long and 32 bytes +/// of garbage per byte (issue #91). +/// +internal static class StreamChunks +{ + /// How many bytes are read at a time. + internal const int BufferSize = 81920; + + /// Receives one chunk, which it may change in place. + /// The bytes read. + internal delegate void ChunkAction(Span chunk); + + /// + /// Passes every chunk of to , then writes it to + /// when there is one. + /// + /// The stream to read to its end. + /// Receives each chunk in order. + /// Where each chunk goes once has seen it, if anywhere. + internal static void ForEach(Stream source, ChunkAction action, Stream? destination = null) + { + byte[] buffer = ArrayPool.Shared.Rent(BufferSize); + try + { + int read; + while ((read = source.Read(buffer, 0, buffer.Length)) > 0) + { + action(buffer.AsSpan(0, read)); + destination?.Write(buffer, 0, read); + } + } + finally + { + ArrayPool.Shared.Return(buffer, clearArray: true); + } + } +}