From de67201ad4e5ba3f2c1968d5e3cc035111615879 Mon Sep 17 00:00:00 2001 From: samgerene Date: Sun, 11 Oct 2026 15:26:46 +0200 Subject: [PATCH 1/2] [Fix] Validate ArgusPipeHostOptions values when they are set (#67) Every option validates in its setter, so an invalid value fails at startup with an exception that names the option instead of failing later inside the host. Limits must be greater than zero, MaxRequestBodySize zero or greater, PipeName not null or empty, the request timeouts at least 1 ms and ShutdownDrainTimeout zero or greater, each at most the timers' maximum or Timeout.InfiniteTimeSpan. --- .../ArgusTransportExtensionsTestFixture.cs | 48 +++ ...gusPipeHostBackgroundServiceTestFixture.cs | 20 ++ .../Server/ArgusPipeHostOptionsTestFixture.cs | 334 ++++++++++++++++++ ArgusTransfer/Server/ArgusPipeHostOptions.cs | 245 ++++++++++++- CLAUDE.md | 3 + 5 files changed, 638 insertions(+), 12 deletions(-) create mode 100644 ArgusTransfer.Tests/Server/ArgusPipeHostOptionsTestFixture.cs diff --git a/ArgusTransfer.Tests/Extensions/ArgusTransportExtensionsTestFixture.cs b/ArgusTransfer.Tests/Extensions/ArgusTransportExtensionsTestFixture.cs index 4cc03c0..eceef55 100644 --- a/ArgusTransfer.Tests/Extensions/ArgusTransportExtensionsTestFixture.cs +++ b/ArgusTransfer.Tests/Extensions/ArgusTransportExtensionsTestFixture.cs @@ -21,7 +21,9 @@ namespace ArgusTransfer.Tests.Extensions { using System; + using System.Collections.Generic; using System.Linq; + using System.Reflection; using ArgusTransfer.Client; using ArgusTransfer.Extensions; @@ -30,6 +32,7 @@ namespace ArgusTransfer.Tests.Extensions using ArgusTransfer.Serialization; using ArgusTransfer.Server; + using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; @@ -335,6 +338,51 @@ public void Verify_that_AddArgusPipeHost_resolves_without_ambiguity() Assert.That(hostedServices.OfType().Count(), Is.EqualTo(1)); } + [Test] + public void Verify_that_AddArgusPipeHost_with_an_invalid_option_fails_when_the_host_is_created() + { + var services = new ServiceCollection(); + + services.AddLogging(); + services.AddSingleton(); + services.AddArgusPipeHost(options => options.RequestReadTimeout = TimeSpan.FromSeconds(-30)); + + var provider = services.BuildServiceProvider(); + + // The generic host resolves its hosted services when it starts, so the invalid option stops the startup with + // an exception that names it, instead of failing every request later + Assert.That( + () => provider.GetServices().ToList(), + Throws.TypeOf().With.Message.Contains(nameof(ArgusPipeHostOptions.RequestReadTimeout))); + Assert.That( + () => provider.GetRequiredService>().Value, + Throws.TypeOf()); + } + + [Test] + public void Verify_that_an_invalid_option_bound_from_configuration_fails_with_an_exception_that_names_the_option() + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary { ["Argus:RequestReadTimeout"] = "-00:00:30" }) + .Build(); + + var services = new ServiceCollection(); + + services.AddLogging(); + services.AddSingleton(); + services.Configure(configuration.GetSection("Argus")); + services.AddArgusPipeHost(); + + var provider = services.BuildServiceProvider(); + + // The configuration binder sets the option through reflection, which wraps the exception of the setter + Assert.That( + () => provider.GetServices().ToList(), + Throws.TypeOf() + .With.InnerException.TypeOf() + .And.InnerException.Message.Contains(nameof(ArgusPipeHostOptions.RequestReadTimeout))); + } + [Test] public void Verify_that_AddArgusBodySerializer_does_not_duplicate() { diff --git a/ArgusTransfer.Tests/Server/ArgusPipeHostBackgroundServiceTestFixture.cs b/ArgusTransfer.Tests/Server/ArgusPipeHostBackgroundServiceTestFixture.cs index 6fd4042..d83c05a 100644 --- a/ArgusTransfer.Tests/Server/ArgusPipeHostBackgroundServiceTestFixture.cs +++ b/ArgusTransfer.Tests/Server/ArgusPipeHostBackgroundServiceTestFixture.cs @@ -1277,6 +1277,26 @@ public async Task Verify_that_large_body_over_the_limit_gets_BadRequest_promptly cts.Dispose(); } + [Test] + public async Task Verify_that_a_MaxRequestBodySize_of_zero_accepts_a_body_larger_than_the_default_limit() + { + var pipeName = $"argus-no-body-limit-test-{Guid.NewGuid():N}"; + var (hostService, cts) = await this.StartHostAsync(CreateEchoRouter(), new ArgusPipeHostOptions { PipeName = pipeName, MaxRequestBodySize = 0 }); + + using var client = new ArgusClient(pipeName); + + // A 2 MB body: twice the default limit of 1 MB + var body = new string('x', 2_000_000); + var response = await client.PostAsync("/echo", body, timeout: TimeSpan.FromSeconds(10)); + + Assert.That(response.StatusCode, Is.EqualTo(ArgusStatusCode.Ok)); + Assert.That(response.Body, Has.Length.EqualTo(body.Length)); + + await cts.CancelAsync(); + await hostService.StopAsync(CancellationToken.None); + cts.Dispose(); + } + [Test] public async Task Verify_that_oversized_headers_sent_by_ArgusClient_get_BadRequest_promptly() { diff --git a/ArgusTransfer.Tests/Server/ArgusPipeHostOptionsTestFixture.cs b/ArgusTransfer.Tests/Server/ArgusPipeHostOptionsTestFixture.cs new file mode 100644 index 0000000..89a6993 --- /dev/null +++ b/ArgusTransfer.Tests/Server/ArgusPipeHostOptionsTestFixture.cs @@ -0,0 +1,334 @@ +// ------------------------------------------------------------------------------------------------- +// +// +// Copyright (c) 2025-2026 Sam Gerené +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// +// ------------------------------------------------------------------------------------------------ + +namespace ArgusTransfer.Tests.Server +{ + using System; + using System.Collections.Generic; + using System.Threading; + using System.Threading.Tasks; + + using ArgusTransfer.Server; + + using NUnit.Framework; + + /// + /// Suite of tests for the class + /// + [TestFixture] + public class ArgusPipeHostOptionsTestFixture + { + /// + /// The timeout options for which is not a valid value, with their names + /// + private static readonly Dictionary> RequestTimeouts = new() + { + [nameof(ArgusPipeHostOptions.RequestReadTimeout)] = (options, value) => options.RequestReadTimeout = value, + [nameof(ArgusPipeHostOptions.RequestTimeout)] = (options, value) => options.RequestTimeout = value, + [nameof(ArgusPipeHostOptions.ResponseWriteTimeout)] = (options, value) => options.ResponseWriteTimeout = value + }; + + /// + /// The options that must be greater than zero, with their names + /// + private static readonly Dictionary> PositiveOptions = new() + { + [nameof(ArgusPipeHostOptions.MaxConcurrentRequests)] = (options, value) => options.MaxConcurrentRequests = value, + [nameof(ArgusPipeHostOptions.MaxConcurrentConnections)] = (options, value) => options.MaxConcurrentConnections = value, + [nameof(ArgusPipeHostOptions.MaxRequestHeaderSize)] = (options, value) => options.MaxRequestHeaderSize = value + }; + + [Test] + public void Verify_that_the_defaults_are_valid_values() + { + var options = new ArgusPipeHostOptions(); + + Assert.That(options.PipeName, Is.EqualTo("argus")); + Assert.That(options.MaxRequestBodySize, Is.EqualTo(1_048_576)); + Assert.That(options.MaxRequestHeaderSize, Is.EqualTo(32 * 1024)); + Assert.That(options.ShutdownDrainTimeout, Is.EqualTo(TimeSpan.FromSeconds(30))); + Assert.That(options.RequestTimeout, Is.EqualTo(TimeSpan.FromSeconds(60))); + Assert.That(options.MaxConcurrentRequests, Is.EqualTo(10)); + Assert.That(options.RequestReadTimeout, Is.EqualTo(TimeSpan.FromSeconds(30))); + Assert.That(options.ResponseWriteTimeout, Is.EqualTo(TimeSpan.FromSeconds(30))); + Assert.That(options.MaxConcurrentConnections, Is.EqualTo(100)); + + // Assigning each default back must be accepted + Assert.That( + () => new ArgusPipeHostOptions + { + PipeName = options.PipeName, + MaxRequestBodySize = options.MaxRequestBodySize, + MaxRequestHeaderSize = options.MaxRequestHeaderSize, + ShutdownDrainTimeout = options.ShutdownDrainTimeout, + RequestTimeout = options.RequestTimeout, + MaxConcurrentRequests = options.MaxConcurrentRequests, + RequestReadTimeout = options.RequestReadTimeout, + ResponseWriteTimeout = options.ResponseWriteTimeout, + MaxConcurrentConnections = options.MaxConcurrentConnections + }, + Throws.Nothing); + } + + [TestCase(nameof(ArgusPipeHostOptions.MaxConcurrentRequests), 0)] + [TestCase(nameof(ArgusPipeHostOptions.MaxConcurrentRequests), -1)] + [TestCase(nameof(ArgusPipeHostOptions.MaxConcurrentConnections), 0)] + [TestCase(nameof(ArgusPipeHostOptions.MaxConcurrentConnections), -1)] + [TestCase(nameof(ArgusPipeHostOptions.MaxRequestHeaderSize), 0)] + [TestCase(nameof(ArgusPipeHostOptions.MaxRequestHeaderSize), -1)] + public void Verify_that_a_zero_or_negative_limit_is_rejected(string option, int value) + { + var options = new ArgusPipeHostOptions(); + + Assert.That( + () => PositiveOptions[option](options, value), + Throws.TypeOf().With.Message.Contains(option)); + } + + [TestCase(nameof(ArgusPipeHostOptions.MaxConcurrentRequests))] + [TestCase(nameof(ArgusPipeHostOptions.MaxConcurrentConnections))] + [TestCase(nameof(ArgusPipeHostOptions.MaxRequestHeaderSize))] + public void Verify_that_a_limit_of_one_is_accepted(string option) + { + var options = new ArgusPipeHostOptions(); + + Assert.That(() => PositiveOptions[option](options, 1), Throws.Nothing); + } + + [Test] + public void Verify_that_a_negative_MaxRequestBodySize_is_rejected_and_zero_means_no_limit() + { + var options = new ArgusPipeHostOptions(); + + Assert.That( + () => options.MaxRequestBodySize = -1, + Throws.TypeOf().With.Message.Contains(nameof(ArgusPipeHostOptions.MaxRequestBodySize))); + + options.MaxRequestBodySize = 0; + + Assert.That(options.MaxRequestBodySize, Is.Zero); + } + + [Test] + public void Verify_that_a_null_PipeName_is_rejected() + { + var options = new ArgusPipeHostOptions(); + + Assert.That( + () => options.PipeName = null, + Throws.TypeOf().With.Message.Contains(nameof(ArgusPipeHostOptions.PipeName))); + Assert.That(options.PipeName, Is.EqualTo("argus")); + } + + [TestCase("")] + [TestCase(" ")] + public void Verify_that_an_empty_PipeName_is_rejected(string pipeName) + { + var options = new ArgusPipeHostOptions(); + + Assert.That( + () => options.PipeName = pipeName, + Throws.TypeOf().With.Message.Contains(nameof(ArgusPipeHostOptions.PipeName))); + Assert.That(options.PipeName, Is.EqualTo("argus")); + } + + [TestCase(nameof(ArgusPipeHostOptions.RequestReadTimeout))] + [TestCase(nameof(ArgusPipeHostOptions.RequestTimeout))] + [TestCase(nameof(ArgusPipeHostOptions.ResponseWriteTimeout))] + public void Verify_that_an_invalid_timeout_is_rejected(string option) + { + var options = new ArgusPipeHostOptions(); + var set = RequestTimeouts[option]; + + var invalidValues = new[] + { + TimeSpan.Zero, + TimeSpan.FromTicks(1), + ArgusPipeHostOptions.MinTimeout - TimeSpan.FromTicks(1), + TimeSpan.FromSeconds(-30), + TimeSpan.FromMilliseconds(-2), + TimeSpan.MinValue, + ArgusPipeHostOptions.MaxTimeout + TimeSpan.FromMilliseconds(1), + TimeSpan.MaxValue + }; + + foreach (var value in invalidValues) + { + Assert.That( + () => set(options, value), + Throws.TypeOf().With.Message.Contains(option), + $"{option} = {value}"); + } + } + + [TestCase(nameof(ArgusPipeHostOptions.RequestReadTimeout))] + [TestCase(nameof(ArgusPipeHostOptions.RequestTimeout))] + [TestCase(nameof(ArgusPipeHostOptions.ResponseWriteTimeout))] + public void Verify_that_a_timeout_of_at_least_one_millisecond_or_infinite_is_accepted(string option) + { + var options = new ArgusPipeHostOptions(); + var set = RequestTimeouts[option]; + + var validValues = new[] + { + ArgusPipeHostOptions.MinTimeout, + TimeSpan.FromHours(1), + ArgusPipeHostOptions.MaxTimeout, + Timeout.InfiniteTimeSpan + }; + + foreach (var value in validValues) + { + Assert.That(() => set(options, value), Throws.Nothing, $"{option} = {value}"); + } + } + + [Test] + public void Verify_that_ShutdownDrainTimeout_accepts_zero_and_infinite_and_rejects_other_negative_values() + { + var options = new ArgusPipeHostOptions(); + + // Zero cancels the in-flight requests immediately, which is a meaningful choice at shutdown + options.ShutdownDrainTimeout = TimeSpan.Zero; + Assert.That(options.ShutdownDrainTimeout, Is.EqualTo(TimeSpan.Zero)); + + options.ShutdownDrainTimeout = Timeout.InfiniteTimeSpan; + Assert.That(options.ShutdownDrainTimeout, Is.EqualTo(Timeout.InfiniteTimeSpan)); + + options.ShutdownDrainTimeout = ArgusPipeHostOptions.MaxTimeout; + Assert.That(options.ShutdownDrainTimeout, Is.EqualTo(ArgusPipeHostOptions.MaxTimeout)); + + // Less than a millisecond drains like zero, which is valid here, unlike for the request timeouts + options.ShutdownDrainTimeout = TimeSpan.FromTicks(1); + Assert.That(options.ShutdownDrainTimeout, Is.EqualTo(TimeSpan.FromTicks(1))); + + var invalidValues = new[] + { + TimeSpan.FromTicks(-1), + TimeSpan.FromMilliseconds(-2), + TimeSpan.FromSeconds(-30), + TimeSpan.MinValue, + ArgusPipeHostOptions.MaxTimeout + TimeSpan.FromMilliseconds(1), + TimeSpan.MaxValue + }; + + foreach (var value in invalidValues) + { + Assert.That( + () => options.ShutdownDrainTimeout = value, + Throws.TypeOf().With.Message.Contains(nameof(ArgusPipeHostOptions.ShutdownDrainTimeout)), + $"{nameof(ArgusPipeHostOptions.ShutdownDrainTimeout)} = {value}"); + } + + Assert.That(options.ShutdownDrainTimeout, Is.EqualTo(TimeSpan.FromTicks(1))); + } + + [Test] + public void Verify_that_a_rejected_value_leaves_the_option_unchanged() + { + var options = new ArgusPipeHostOptions { RequestTimeout = TimeSpan.FromSeconds(5), MaxConcurrentRequests = 3 }; + + Assert.That(() => options.RequestTimeout = TimeSpan.Zero, Throws.TypeOf()); + Assert.That(() => options.MaxConcurrentRequests = 0, Throws.TypeOf()); + + Assert.That(options.RequestTimeout, Is.EqualTo(TimeSpan.FromSeconds(5))); + Assert.That(options.MaxConcurrentRequests, Is.EqualTo(3)); + } + + [Test] + public void Verify_that_each_option_returns_the_value_it_was_given() + { + // Every value is different, so an option that stores into the field of another one is noticed + var options = new ArgusPipeHostOptions + { + PipeName = "argus-options-test", + MaxRequestBodySize = 11, + MaxRequestHeaderSize = 12, + ShutdownDrainTimeout = TimeSpan.FromSeconds(13), + RequestTimeout = TimeSpan.FromSeconds(14), + MaxConcurrentRequests = 15, + RequestReadTimeout = TimeSpan.FromSeconds(16), + ResponseWriteTimeout = TimeSpan.FromSeconds(17), + MaxConcurrentConnections = 18 + }; + + Assert.That(options.PipeName, Is.EqualTo("argus-options-test")); + Assert.That(options.MaxRequestBodySize, Is.EqualTo(11)); + Assert.That(options.MaxRequestHeaderSize, Is.EqualTo(12)); + Assert.That(options.ShutdownDrainTimeout, Is.EqualTo(TimeSpan.FromSeconds(13))); + Assert.That(options.RequestTimeout, Is.EqualTo(TimeSpan.FromSeconds(14))); + Assert.That(options.MaxConcurrentRequests, Is.EqualTo(15)); + Assert.That(options.RequestReadTimeout, Is.EqualTo(TimeSpan.FromSeconds(16))); + Assert.That(options.ResponseWriteTimeout, Is.EqualTo(TimeSpan.FromSeconds(17))); + Assert.That(options.MaxConcurrentConnections, Is.EqualTo(18)); + } + + [Test] + public void Verify_that_an_out_of_range_exception_carries_the_rejected_value() + { + var options = new ArgusPipeHostOptions(); + var timeout = TimeSpan.FromSeconds(-30); + + // The rejected value ends up in the exception message, and so in the log of a host that fails to start + Assert.That( + () => options.MaxRequestBodySize = -1, + Throws.TypeOf() + .With.Property(nameof(ArgumentOutOfRangeException.ActualValue)).EqualTo(-1L) + .And.Property(nameof(ArgumentOutOfRangeException.ParamName)).EqualTo("value")); + Assert.That( + () => options.MaxConcurrentRequests = 0, + Throws.TypeOf() + .With.Property(nameof(ArgumentOutOfRangeException.ActualValue)).EqualTo(0) + .And.Property(nameof(ArgumentOutOfRangeException.ParamName)).EqualTo("value")); + Assert.That( + () => options.RequestTimeout = timeout, + Throws.TypeOf() + .With.Property(nameof(ArgumentOutOfRangeException.ActualValue)).EqualTo(timeout) + .And.Property(nameof(ArgumentOutOfRangeException.ParamName)).EqualTo("value")); + } + + [Test] + public void Verify_that_Compression_may_be_set_to_null() + { + var options = new ArgusPipeHostOptions(); + + Assert.That(() => options.Compression = null, Throws.Nothing); + } + + [Test] + public async Task Verify_that_the_longest_accepted_timeout_is_the_longest_the_timers_support() + { + using var cts = new CancellationTokenSource(); + + // The host applies the timeouts with CancelAfter and Task.Delay; what the options accept must not throw there + Assert.That(() => cts.CancelAfter(ArgusPipeHostOptions.MaxTimeout), Throws.Nothing); + + var delay = Task.Delay(ArgusPipeHostOptions.MaxTimeout, cts.Token); + + Assert.That( + () => cts.CancelAfter(ArgusPipeHostOptions.MaxTimeout + TimeSpan.FromMilliseconds(1)), + Throws.TypeOf()); + + await cts.CancelAsync(); + + Assert.That(async () => await delay, Throws.InstanceOf()); + } + } +} diff --git a/ArgusTransfer/Server/ArgusPipeHostOptions.cs b/ArgusTransfer/Server/ArgusPipeHostOptions.cs index 130e070..e7d7c6c 100644 --- a/ArgusTransfer/Server/ArgusPipeHostOptions.cs +++ b/ArgusTransfer/Server/ArgusPipeHostOptions.cs @@ -27,14 +27,95 @@ namespace ArgusTransfer.Server using ArgusTransfer.Serialization; /// - /// Configuration options for the + /// Configuration options for the . Every option validates the value it is + /// given, so an invalid value fails where it is configured and not later, while a request is being handled. /// public class ArgusPipeHostOptions { + /// + /// The longest finite timeout the options accept, about 49.7 days: the longest duration the timers behind the + /// timeouts support. Use for no timeout. + /// + internal static readonly TimeSpan MaxTimeout = TimeSpan.FromMilliseconds(uint.MaxValue - 1); + + /// + /// The shortest timeout the request timeouts accept, one millisecond: the timers behind the timeouts count whole + /// milliseconds, so a shorter duration would expire immediately. + /// + internal static readonly TimeSpan MinTimeout = TimeSpan.FromMilliseconds(1); + + /// + /// Backing field for + /// + private string pipeName = "argus"; + + /// + /// Backing field for + /// + private long maxRequestBodySize = 1_048_576; + + /// + /// Backing field for + /// + private int maxRequestHeaderSize = 32 * 1024; + + /// + /// Backing field for + /// + private TimeSpan shutdownDrainTimeout = TimeSpan.FromSeconds(30); + + /// + /// Backing field for + /// + private TimeSpan requestTimeout = TimeSpan.FromSeconds(60); + + /// + /// Backing field for + /// + private int maxConcurrentRequests = 10; + + /// + /// Backing field for + /// + private TimeSpan requestReadTimeout = TimeSpan.FromSeconds(30); + + /// + /// Backing field for + /// + private TimeSpan responseWriteTimeout = TimeSpan.FromSeconds(30); + + /// + /// Backing field for + /// + private int maxConcurrentConnections = 100; + /// /// Gets or sets the name of the named pipe to listen on. Defaults to "argus" /// - public string PipeName { get; set; } = "argus"; + /// + /// Thrown when the value is null + /// + /// + /// Thrown when the value is empty or consists only of white space + /// + public string PipeName + { + get => this.pipeName; + set + { + if (value is null) + { + throw new ArgumentNullException(nameof(value), $"{nameof(this.PipeName)} must not be null."); + } + + if (string.IsNullOrWhiteSpace(value)) + { + throw new ArgumentException($"{nameof(this.PipeName)} must not be empty or consist only of white space.", nameof(value)); + } + + this.pipeName = value; + } + } /// /// Gets or sets the applied to the named pipe on Windows. When @@ -49,33 +130,81 @@ public class ArgusPipeHostOptions /// /// Gets or sets the maximum allowed request body size in bytes. Defaults to 1 MB (1,048,576 bytes). /// Requests with a Content-Length exceeding this limit are rejected with a 400 Bad Request response. + /// A value of 0 disables the limit. /// - public long MaxRequestBodySize { get; set; } = 1_048_576; + /// + /// Thrown when the value is negative + /// + public long MaxRequestBodySize + { + get => this.maxRequestBodySize; + set + { + if (value < 0) + { + throw new ArgumentOutOfRangeException(nameof(value), value, $"{nameof(this.MaxRequestBodySize)} must be 0 (no limit) or greater."); + } + + this.maxRequestBodySize = value; + } + } /// /// Gets or sets the maximum size in bytes of a request's header block -- the request line plus all header lines. /// Defaults to 32 KB (32,768 bytes). The host stops reading as soon as the limit is exceeded and rejects the request /// with a 400 Bad Request response, so a client cannot exhaust memory with an endless header line or endless headers. /// - public int MaxRequestHeaderSize { get; set; } = 32 * 1024; + /// + /// Thrown when the value is zero or negative + /// + public int MaxRequestHeaderSize + { + get => this.maxRequestHeaderSize; + set => this.maxRequestHeaderSize = EnsurePositive(value, nameof(this.MaxRequestHeaderSize)); + } /// /// Gets or sets the maximum time to wait for in-flight requests to complete during shutdown. - /// Defaults to 30 seconds. After this timeout, remaining requests are cancelled. + /// Defaults to 30 seconds. After this timeout, remaining requests are cancelled. A value of + /// cancels them immediately; waits until they + /// have completed or the host's own shutdown timeout expires. /// - public TimeSpan ShutdownDrainTimeout { get; set; } = TimeSpan.FromSeconds(30); + /// + /// Thrown when the value is negative and not , or greater than + /// about 49.7 days + /// + public TimeSpan ShutdownDrainTimeout + { + get => this.shutdownDrainTimeout; + set => this.shutdownDrainTimeout = EnsureTimeout(value, allowZero: true, nameof(this.ShutdownDrainTimeout)); + } /// /// Gets or sets the maximum time allowed for a single request handler to execute. /// Defaults to 60 seconds. Set to to disable the timeout. /// - public TimeSpan RequestTimeout { get; set; } = TimeSpan.FromSeconds(60); + /// + /// Thrown when the value is less than one millisecond and not , or greater + /// than about 49.7 days + /// + public TimeSpan RequestTimeout + { + get => this.requestTimeout; + set => this.requestTimeout = EnsureTimeout(value, allowZero: false, nameof(this.RequestTimeout)); + } /// /// Gets or sets the maximum number of requests that can be processed concurrently. /// Defaults to 10. When the limit is reached, new requests receive a 503 Service Unavailable response. /// - public int MaxConcurrentRequests { get; set; } = 10; + /// + /// Thrown when the value is zero or negative + /// + public int MaxConcurrentRequests + { + get => this.maxConcurrentRequests; + set => this.maxConcurrentRequests = EnsurePositive(value, nameof(this.MaxConcurrentRequests)); + } /// /// Gets or sets the maximum time a client may take to send its complete request -- request line, headers and @@ -83,7 +212,15 @@ public class ArgusPipeHostOptions /// connection, so idle or deliberately slow clients cannot hold a connection indefinitely. Set to /// to disable the timeout. /// - public TimeSpan RequestReadTimeout { get; set; } = TimeSpan.FromSeconds(30); + /// + /// Thrown when the value is less than one millisecond and not , or greater + /// than about 49.7 days + /// + public TimeSpan RequestReadTimeout + { + get => this.requestReadTimeout; + set => this.requestReadTimeout = EnsureTimeout(value, allowZero: false, nameof(this.RequestReadTimeout)); + } /// /// Gets or sets the maximum time the host may take to send a complete response -- status line, headers and @@ -94,7 +231,15 @@ public class ArgusPipeHostOptions /// handler's stream takes to produce the data. Increase it, or disable it, on a host that streams large or /// long-lived responses. Set to to disable the timeout. /// - public TimeSpan ResponseWriteTimeout { get; set; } = TimeSpan.FromSeconds(30); + /// + /// Thrown when the value is less than one millisecond and not , or greater + /// than about 49.7 days + /// + public TimeSpan ResponseWriteTimeout + { + get => this.responseWriteTimeout; + set => this.responseWriteTimeout = EnsureTimeout(value, allowZero: false, nameof(this.ResponseWriteTimeout)); + } /// /// Gets or sets the maximum number of simultaneous client connections, including connections whose request is still @@ -105,15 +250,91 @@ public class ArgusPipeHostOptions /// read. In both cases the client's own timeout applies. Together with /// and , this bounds the memory used for requests. /// - public int MaxConcurrentConnections { get; set; } = 100; + /// + /// Thrown when the value is zero or negative + /// + public int MaxConcurrentConnections + { + get => this.maxConcurrentConnections; + set => this.maxConcurrentConnections = EnsurePositive(value, nameof(this.MaxConcurrentConnections)); + } /// /// Gets or sets the for request and response bodies. Requests compressed with /// one of the configured encodings are always decoded; limits both the compressed /// and the decompressed size. When is true, responses whose body /// reaches are compressed if the request's - /// Accept-Encoding header accepts one of the configured encodings. Disabled by default. + /// Accept-Encoding header accepts one of the configured encodings. Disabled by default. When set to + /// null, the host uses the default options. /// public ArgusCompressionOptions Compression { get; set; } = new ArgusCompressionOptions(); + + /// + /// Checks that the value of an option is greater than zero + /// + /// + /// The value to check + /// + /// + /// The name of the option, for the exception message + /// + /// + /// The + /// + /// + /// Thrown when the value is zero or negative + /// + private static int EnsurePositive(int value, string option) + { + if (value <= 0) + { + throw new ArgumentOutOfRangeException(nameof(value), value, $"{option} must be greater than zero."); + } + + return value; + } + + /// + /// Checks that the value of a timeout option is a duration the host's timers accept: at least one millisecond (or + /// zero when is set) up to , or + /// . The timers count whole milliseconds, so a shorter positive duration + /// would expire immediately, like zero. + /// + /// + /// The value to check + /// + /// + /// true when is a meaningful value for the option + /// + /// + /// The name of the option, for the exception message + /// + /// + /// The + /// + /// + /// Thrown when the value is not an accepted duration + /// + private static TimeSpan EnsureTimeout(TimeSpan value, bool allowZero, string option) + { + if (value == Timeout.InfiniteTimeSpan) + { + return value; + } + + var tooSmall = allowZero ? value < TimeSpan.Zero : value < MinTimeout; + + if (tooSmall || value > MaxTimeout) + { + var lowerBound = allowZero ? "zero or greater" : "at least 1 millisecond"; + + throw new ArgumentOutOfRangeException( + nameof(value), + value, + $"{option} must be {lowerBound} and at most {MaxTimeout}, or Timeout.InfiniteTimeSpan for no timeout."); + } + + return value; + } } } diff --git a/CLAUDE.md b/CLAUDE.md index ad4becf..c3a67d1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -103,6 +103,8 @@ Exception handling is layered: if routing throws (anything except `OperationCanc Connection limits: `ArgusPipeHostOptions.MaxConcurrentConnections` (default 100) is enforced by a connection semaphore acquired *after* the next pipe instance is created and *before* `WaitForConnectionAsync`, so at the limit new connections are not read (no 503 before the request is read, which would deadlock on the unbuffered pipe). The next instance must exist while waiting: on Linux/macOS (Unix domain sockets) the listening socket is shared by all instances of a pipe name and closes when the last one is disposed, which would reset clients queued in the backlog. Clients at the limit either wait in `ConnectAsync` or connect immediately (backlog on Unix; the ready instance on Windows) and wait to be read -- tests must not assume a connect timeout; `MaxConcurrentRequests` still limits processing. `RequestReadTimeout` (default 30 s) bounds receiving the complete request; on expiry the host logs a warning and closes the connection. `ResponseWriteTimeout` (default 30 s, `Timeout.InfiniteTimeSpan` disables) bounds sending the complete response (a total budget like `RequestReadTimeout`, not an idle timeout; for a streamed body it includes reading the handler's `BodyStream`): every response write in the connection task goes through `WriteResponseAsync`, which links a timeout to the drain token and on expiry logs a warning and returns so the connection is closed -- the pipe is unbuffered, so a client that never reads would otherwise hold the connection, a `MaxConcurrentRequests` slot and a `MaxConcurrentConnections` slot until shutdown. Never call `responseSerializer.WriteAsync` on the pipe directly. Tests for small responses that are not read are Windows-only (`[Platform("Win")]`): a Unix socket buffers them, so the write completes without a reader. The `Test (Windows)` job in `CodeQuality.yml` runs them in CI; Sonar analysis and coverage stay in the Linux job. `StopAsync` drains requests for `ShutdownDrainTimeout`, then cancels them and waits `CancellationGracePeriod` (internal, default 5 s); the drain token source and both semaphores are disposed only when every request task has finished -- otherwise disposal is skipped with a warning, so a handler that ignores cancellation can still release them. The accept loop (`AcceptConnectionAsync`) survives pipe errors: any exception other than cancellation on shutdown is logged and retried with exponential backoff (`AcceptRetryInitialDelay`, internal, default 1 s, doubling up to 30 s, reset after a successful accept), so a transient error never stops the host. +Option validation (#67): `ArgusPipeHostOptions` validates in its property setters, like `ArgusRetryPolicy` and `ArgusCompressionOptions` -- there is no `IValidateOptions` and the host does not re-check. Limits (`MaxConcurrentRequests`, `MaxConcurrentConnections`, `MaxRequestHeaderSize`) must be greater than zero; `MaxRequestBodySize` zero (no limit) or greater; `PipeName` not null/empty/white space; the three request timeouts at least 1 ms (`MinTimeout`; the timers count whole milliseconds, so less would expire at once like zero), `ShutdownDrainTimeout` zero or greater, each at most the internal `ArgusPipeHostOptions.MaxTimeout` (the timers' maximum, about 49.7 days) or `Timeout.InfiniteTimeSpan`. The exception (`ArgumentOutOfRangeException`/`ArgumentException`, parameter `value`, message naming the option) surfaces when the configure callback runs, i.e. when the host resolves its hosted services. Options bound from configuration (`services.Configure(section)`) are set by the binder through reflection, so there it arrives as the `InnerException` of a `TargetInvocationException`. A new option needs the same setter validation and a case in `ArgusPipeHostOptionsTestFixture`. + Header limits: the serializers' `MaxHeaderSize` (default 32 KB; host sets it from `ArgusPipeHostOptions.MaxRequestHeaderSize`) bounds the request/status line plus all header lines via the internal `ArgusHeaderBudget`; `ArgusWireReader.ReadLineAsync(maxLength, …)` stops buffering as soon as a line exceeds its limit (`ArgusLineTooLongException`, an internal `ArgusProtocolException`). Chunk-size lines on the byte path are capped at 1 KB. The string reader is not limited. Request rejection: readers throw `ArgusProtocolException` (derives from `InvalidOperationException`) for violations by the received message -- body or decompressed size over the limit, unsupported or corrupt `Content-Encoding`. The host answers only `ArgusProtocolException` and `FormatException` (malformed request) with 400 and the message; any other failure is a server fault (500, or a logged closed connection once the response has started). A response whose `Content-Encoding` the host cannot produce is replaced by the generic 500 before writing. Writing-side and configuration errors (unsupported encoding on an outgoing message, text overloads with a compressed body, unknown preferred encoding) stay plain `InvalidOperationException`. @@ -126,6 +128,7 @@ Authentication: `ArgusAuthenticationMiddleware` calls the singleton `IArgusAuthe - **README.md is a landing page**, and also the NuGet package readme (`PackageReadmeFile`): pitch, why/when to use, feature list, install, a quick start, documentation links, contributing, SBOM, license. Do **not** add detailed feature sections to it. - **Document features in the wiki** (`ArgusTransfer.wiki` repository, pushed to `samgerene/ArgusTransfer.wiki`): add or update the relevant page and the sidebar. A new feature gets at most a one-line entry in the README feature list, linking to its wiki page. +- **Record changes in `CHANGELOG.md`** ([Keep a Changelog](https://keepachangelog.com/en/1.1.0/) format): every pull request with a change a user of the library can notice adds one line under `## [Unreleased]`, in `Added`, `Changed`, `Fixed` or `Security`, with a link to its issue or pull request. Start the line with `**Behavior change:**` when existing code or configuration can behave differently. At release time `[Unreleased]` becomes `[x.y.z] - yyyy-mm-dd` and the compare links at the bottom are updated; `PackageReleaseNotes` is written from that section. - Links in the README must be absolute URLs (relative links break on nuget.org). - The README quick start must compile and run: verify it in a scratch console project (`dotnet new console`, project reference to `ArgusTransfer`, package `Microsoft.Extensions.Hosting`) whenever it or the APIs it uses change. From 763ff4e07cfb008ecb1acaaeb2d4e1020fda94fa Mon Sep 17 00:00:00 2001 From: samgerene Date: Sun, 11 Oct 2026 15:26:46 +0200 Subject: [PATCH 2/2] [Add] CHANGELOG.md with the releases 0.1.0 to 1.2.1 and the unreleased changes Releases are reconstructed from the PackageReleaseNotes at each tag, the unreleased section from the commits merged since 1.2.1. --- CHANGELOG.md | 140 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..7edd162 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,140 @@ +# Changelog + +All notable changes to ArgusTransfer are documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and the project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). Numbers such as [#67](https://github.com/samgerene/ArgusTransfer/issues/67) refer to the issue or pull request of the change. + +## [Unreleased] + +### Added + +- `ArgusProblemDetails`: a structured error response format in the style of RFC 7807 Problem Details ([#29](https://github.com/samgerene/ArgusTransfer/issues/29)). +- `ArgusRetryPolicy`: optional automatic retry of transient named pipe failures in `ArgusClient` ([#30](https://github.com/samgerene/ArgusTransfer/issues/30)). +- Global exception handling middleware, `ArgusExceptionHandlerMiddleware` ([#31](https://github.com/samgerene/ArgusTransfer/issues/31)). +- `Content-Encoding` support for compressed request and response bodies (gzip) ([#34](https://github.com/samgerene/ArgusTransfer/issues/34)). +- `Authorization` header convention for process identity ([#35](https://github.com/samgerene/ArgusTransfer/issues/35)). +- Built-in authentication middleware with pluggable handlers ([#36](https://github.com/samgerene/ArgusTransfer/issues/36)). +- `ArgusPipeHostOptions.MaxRequestHeaderSize`: a limit on the size of the header block ([#64](https://github.com/samgerene/ArgusTransfer/issues/64)). +- `ArgusPipeHostOptions.RequestReadTimeout` and `MaxConcurrentConnections`: a request read timeout and a connection limit for the pipe host ([#65](https://github.com/samgerene/ArgusTransfer/issues/65)). +- `ArgusClient.MaxResponseBodySize`: an optional maximum response body size on the client ([#76](https://github.com/samgerene/ArgusTransfer/issues/76)). +- `ArgusPipeHostOptions.ResponseWriteTimeout`, so a client that does not read its response cannot hold a connection ([#85](https://github.com/samgerene/ArgusTransfer/issues/85)). + +### Changed + +- **Behavior change:** `ArgusPipeHostOptions` validates its values when they are set. A host configured with a zero or negative limit, a request timeout shorter than one millisecond, a negative `ShutdownDrainTimeout`, a timeout longer than about 49.7 days or an empty `PipeName` now fails at startup with an exception that names the option, instead of failing later. A negative `MaxRequestBodySize` is rejected; use `0` for no limit. Use `Timeout.InfiniteTimeSpan` for no timeout ([#67](https://github.com/samgerene/ArgusTransfer/issues/67)). +- The host answers only request-format violations with 400, through `ArgusProtocolException`; other failures are server faults ([#63](https://github.com/samgerene/ArgusTransfer/issues/63)). +- Routes are percent-encoded on the wire, and header names and values are validated ([#69](https://github.com/samgerene/ArgusTransfer/issues/69)). +- Route templates are validated at registration, so a mistyped template fails at startup ([#73](https://github.com/samgerene/ArgusTransfer/issues/73)). +- Route templates are parsed and middleware pipelines composed once, instead of for every request ([#74](https://github.com/samgerene/ArgusTransfer/issues/74)). +- The response serializer is negotiated from the `Accept` header ([#75](https://github.com/samgerene/ArgusTransfer/issues/75)). +- The host's built-in 400, 406 and 503 responses are problem details ([#77](https://github.com/samgerene/ArgusTransfer/issues/77)). +- `ArgusClient.SendAsync` leaves the caller's request unchanged and validates it ([#79](https://github.com/samgerene/ArgusTransfer/issues/79)). +- The protocol version in request and status lines is validated ([#81](https://github.com/samgerene/ArgusTransfer/issues/81)). +- Documented why `IArgusClient` is `IDisposable` and what `ArgusClient.Dispose` does ([#82](https://github.com/samgerene/ArgusTransfer/issues/82)). +- The README is restructured into a landing page with a verified quick start ([#61](https://github.com/samgerene/ArgusTransfer/issues/61)). +- Updated the dependencies. + +### Fixed + +- Byte-safe message framing for non-ASCII and binary bodies ([#33](https://github.com/samgerene/ArgusTransfer/issues/33)). +- A pipeline that completes without a response is answered with a generic 500 instead of a closed connection ([#62](https://github.com/samgerene/ArgusTransfer/issues/62)). +- The client reads the response while writing the request, so an early rejection by the host does not deadlock ([#68](https://github.com/samgerene/ArgusTransfer/issues/68)). +- The pipe accept loop keeps running after an unexpected exception ([#70](https://github.com/samgerene/ArgusTransfer/issues/70)). +- Shutdown resources are disposed only after all requests have finished ([#71](https://github.com/samgerene/ArgusTransfer/issues/71)). +- `AddArgusModules` can no longer scan the wrong assembly ([#72](https://github.com/samgerene/ArgusTransfer/issues/72)). +- A null query parameter value is written as an empty value ([#78](https://github.com/samgerene/ArgusTransfer/issues/78)). +- `ArgusLoggingMiddleware` logs a completion entry when the pipeline throws ([#80](https://github.com/samgerene/ArgusTransfer/issues/80)). +- Corrected stale file headers and the "Argus Health" wording ([#83](https://github.com/samgerene/ArgusTransfer/issues/83)). + +## [1.2.1] - 2026-04-26 + +### Fixed + +- The default pipe ACL also grants the account that runs the service `FullControl`, so subsequent pipe instances can be created. Authenticated Users alone lack `CreateNewInstance`, which broke the host loop after the first connection. + +## [1.2.0] - 2026-04-26 + +### Added + +- `ArgusPipeHostOptions.PipeSecurity` for cross-session named-pipe access. The default ACL grants Authenticated Users `ReadWrite | Synchronize`, so clients in a user session can connect to a pipe owned by a service that runs as LocalSystem. + +## [1.1.0] - 2026-04-26 + +### Added + +- `EnsureSuccessStatusCode` helper and `EnsureSuccessAsync` client overloads ([#23](https://github.com/samgerene/ArgusTransfer/issues/23)). + +### Changed + +- Updated `Microsoft.Extensions.DependencyInjection`, `Microsoft.Extensions.Hosting.Abstractions` and `Microsoft.Extensions.Logging.Abstractions` to version 10.0.7. + +## [1.0.2] - 2026-03-28 + +### Changed + +- `ArgusTransportExtensions` registers `ArgusPipeHostBackgroundService` with a factory that resolves `IArgusBodySerializerRegistry` first and falls back to `IArgusBodySerializer`. + +## [1.0.1] - 2026-03-28 + +### Security + +- Hardened chunked transfer encoding against denial of service: chunk size parsing uses `TryParse` with a non-negative check, so a malicious peer cannot trigger excessive memory allocation. +- Parsed `Content-Length` headers are checked to be non-negative in both the request and the response deserializer. + +### Fixed + +- `ArgusClient.SendAsync` throws `ObjectDisposedException` after disposal, to fail fast. +- Resource leak in `ArgusClient.SendAsync`: the `StreamWriter` and `StreamReader` are now disposed, with `leaveOpen: true`. + +### Changed + +- Performance: header parsing uses `AsSpan().Trim().ToString()` instead of `Substring().Trim()`, to reduce intermediate string allocations. +- Performance: `ArgusBodySerializerRegistry` uses `TryGetValue` instead of a `ContainsKey` and indexer double lookup. +- Performance: chunked encoding uses the `ReadAsync(Memory)` overload instead of the inexact `Read(byte[], int, int)`. +- Performance: the sample's `SampleClientService` caches its `JsonSerializerOptions` instance instead of allocating one per call. +- Introduced `ArgusHeaderNames` constants (`Accept`, `ContentType`, `ContentLength`, `CorrelationToken`, `Timestamp`), replacing all string literals. +- Reordered the `IArgusClient` and `ArgusClient` method overloads to be adjacent by name. +- Reordered `ArgusResponseSerializer` so all `Write` overloads are grouped, followed by all `WriteAsync` overloads. +- Refactored `GuidExtensions.FromShortGuidArray` to use a private iterator method. +- Refactored `ArgusClient.SendAsync` to use `requestSerializer.WriteAsync`. + +## [1.0.0] - 2026-03-28 + +### Added + +- `IArgusClient` interface. +- Query parameters. +- `Accept` header as a first-class citizen on `ArgusRequest` ([#1](https://github.com/samgerene/ArgusTransfer/issues/1)). +- Content negotiation with `IArgusBodySerializerRegistry` ([#1](https://github.com/samgerene/ArgusTransfer/issues/1)). +- Timeout per request ([#4](https://github.com/samgerene/ArgusTransfer/issues/4)). +- Request timeout on the server ([#14](https://github.com/samgerene/ArgusTransfer/issues/14)). +- Status codes 401 and 403 ([#8](https://github.com/samgerene/ArgusTransfer/issues/8)). +- Status codes 409, 501 and 503 ([#9](https://github.com/samgerene/ArgusTransfer/issues/9), [#10](https://github.com/samgerene/ArgusTransfer/issues/10)). +- Status codes 204 and 422 ([#13](https://github.com/samgerene/ArgusTransfer/issues/13), [#12](https://github.com/samgerene/ArgusTransfer/issues/12)). +- The `HEAD` verb, which returns no body ([#24](https://github.com/samgerene/ArgusTransfer/issues/24)). +- Maximum body size checking ([#15](https://github.com/samgerene/ArgusTransfer/issues/15)). +- Graceful shutdown that drains in-flight requests ([#16](https://github.com/samgerene/ArgusTransfer/issues/16)). +- Concurrency gating ([#17](https://github.com/samgerene/ArgusTransfer/issues/17)). +- Streaming body support for large payloads ([#7](https://github.com/samgerene/ArgusTransfer/issues/7)). + +## [0.2.0] - 2026-03-15 + +### Added + +- `ArgusContext`. + +## [0.1.0] - 2026-03-02 + +### Added + +- Initial version. + +[Unreleased]: https://github.com/samgerene/ArgusTransfer/compare/1.2.1...development +[1.2.1]: https://github.com/samgerene/ArgusTransfer/compare/1.2.0...1.2.1 +[1.2.0]: https://github.com/samgerene/ArgusTransfer/compare/1.1.0...1.2.0 +[1.1.0]: https://github.com/samgerene/ArgusTransfer/compare/1.0.2...1.1.0 +[1.0.2]: https://github.com/samgerene/ArgusTransfer/compare/1.0.1...1.0.2 +[1.0.1]: https://github.com/samgerene/ArgusTransfer/compare/1.0.0...1.0.1 +[1.0.0]: https://github.com/samgerene/ArgusTransfer/compare/0.2.0...1.0.0 +[0.2.0]: https://github.com/samgerene/ArgusTransfer/compare/0.1.0...0.2.0 +[0.1.0]: https://github.com/samgerene/ArgusTransfer/releases/tag/0.1.0