From 02b1ccb3fdda60702b1c1979b2958bc9ca62c156 Mon Sep 17 00:00:00 2001 From: Jose Date: Thu, 23 Apr 2026 17:23:38 +0200 Subject: [PATCH 1/7] Stop treating DateTime.MinValue as the "absent" deadline sentinel DeadlineMiddleware used the decoded DateTime.MinValue as its "field absent" sentinel. A peer encoding ticks=0 decodes to exactly that value, so an explicitly encoded MinValue deadline was indistinguishable from an absent field and bypassed deadline enforcement entirely. Check field presence explicitly via TryGetValue, then decode and enforce. Add a regression test encoding ticks=0 (DateTime(0, Utc) so ToUniversalTime doesn't shift it) and assert the middleware returns DeadlineExceeded and does not call the next dispatcher. Closes #4419 --- src/IceRpc.Deadline/DeadlineMiddleware.cs | 19 ++++----- .../DeadlineMiddlewareTests.cs | 41 +++++++++++++++++++ 2 files changed, 50 insertions(+), 10 deletions(-) diff --git a/src/IceRpc.Deadline/DeadlineMiddleware.cs b/src/IceRpc.Deadline/DeadlineMiddleware.cs index 067e6d3801..124808fe1d 100644 --- a/src/IceRpc.Deadline/DeadlineMiddleware.cs +++ b/src/IceRpc.Deadline/DeadlineMiddleware.cs @@ -2,6 +2,7 @@ using IceRpc.Extensions.DependencyInjection; using IceRpc.Features; +using System.Buffers; using ZeroC.Slice.Codec; namespace IceRpc.Deadline; @@ -31,16 +32,13 @@ public ValueTask DispatchAsync( IncomingRequest request, CancellationToken cancellationToken = default) { - TimeSpan? timeout = null; - - // not found returns default == DateTime.MinValue. - DateTime deadline = request.Fields.DecodeValue( - RequestFieldKey.Deadline, - (ref SliceDecoder decoder) => decoder.DecodeTimeStamp()); - - if (deadline != DateTime.MinValue) + // Check explicit field presence rather than relying on a decoded-value sentinel: a peer encoding ticks=0 + // decodes to DateTime.MinValue, which would otherwise be indistinguishable from an absent field. + if (request.Fields.TryGetValue(RequestFieldKey.Deadline, out ReadOnlySequence value)) { - timeout = deadline - _timeProvider.GetUtcNow().UtcDateTime; + DateTime deadline = value.DecodeSliceBuffer( + (ref SliceDecoder decoder) => decoder.DecodeTimeStamp()); + TimeSpan timeout = deadline - _timeProvider.GetUtcNow().UtcDateTime; if (timeout <= TimeSpan.Zero) { @@ -51,9 +49,10 @@ public ValueTask DispatchAsync( } request.Features = request.Features.With(new DeadlineFeature(deadline)); + return PerformDispatchAsync(timeout); } - return timeout is null ? _next.DispatchAsync(request, cancellationToken) : PerformDispatchAsync(timeout.Value); + return _next.DispatchAsync(request, cancellationToken); async ValueTask PerformDispatchAsync(TimeSpan timeout) { diff --git a/tests/IceRpc.Deadline.Tests/DeadlineMiddlewareTests.cs b/tests/IceRpc.Deadline.Tests/DeadlineMiddlewareTests.cs index 26f1e7ddce..6333e36f5d 100644 --- a/tests/IceRpc.Deadline.Tests/DeadlineMiddlewareTests.cs +++ b/tests/IceRpc.Deadline.Tests/DeadlineMiddlewareTests.cs @@ -56,6 +56,47 @@ public async Task Dispatch_fails_after_the_deadline_expires() pipeReader.Complete(); } + /// Verifies that a request with an explicitly encoded DateTime.MinValue deadline returns + /// DeadlineExceeded and does not fall through to the next dispatcher. This is the decoded form of a peer-encoded + /// ticks=0 deadline, which must be treated as an expired deadline rather than as an absent field. + [Test] + public async Task Dispatch_fails_when_deadline_is_min_value() + { + // Arrange + bool nextCalled = false; + var dispatcher = new InlineDispatcher((request, cancellationToken) => + { + nextCalled = true; + return new(new OutgoingResponse(request)); + }); + + var sut = new DeadlineMiddleware(dispatcher); + + // Encode an explicit ticks=0 deadline: the Utc kind avoids ToUniversalTime shifting it away from 0 in + // timezones other than UTC. Decoded server-side as DateTime.MinValue, which collides with the "field + // absent" sentinel the middleware used to rely on. + PipeReader pipeReader = WriteDeadline(new DateTime(0, DateTimeKind.Utc)); + pipeReader.TryRead(out var readResult); + + using var request = new IncomingRequest(Protocol.IceRpc, FakeConnectionContext.Instance) + { + Fields = new Dictionary> + { + [RequestFieldKey.Deadline] = readResult.Buffer + } + }; + + // Act + OutgoingResponse response = await sut.DispatchAsync(request, CancellationToken.None); + + // Assert + Assert.That(response.StatusCode, Is.EqualTo(StatusCode.DeadlineExceeded)); + Assert.That(nextCalled, Is.False); + + // Cleanup + pipeReader.Complete(); + } + /// Verifies that the deadline decoded by the middleware has the expected value. [Test] public async Task Deadline_decoded_by_middleware_has_expected_value() From 38c8f11ffef1b935ea8ee47e3730ea6c701086e5 Mon Sep 17 00:00:00 2001 From: Jose Date: Thu, 23 Apr 2026 18:02:05 +0200 Subject: [PATCH 2/7] Clamp extreme-future deadlines to CancelAfter's supported maximum CancellationTokenSource.CancelAfter(TimeSpan) rejects values greater than int.MaxValue milliseconds (~24.8 days). A peer-encoded deadline thousands of years in the future (or a caller-supplied IDeadlineFeature near DateTime.MaxValue) produced a TimeSpan that CancelAfter rejected with ArgumentOutOfRangeException, leaking as a generic InternalError response on the server side. - DeadlineMiddleware: clamp the computed timeout before PerformDispatchAsync. - DeadlineInterceptor: clamp the timeout in PerformInvokeAsync for the peer-deadline-via-feature path, and reject a configured defaultTimeout greater than the CancelAfter maximum at construction time. At the clamp bound (~24.8 days) the deadline is effectively infinite for RPC purposes; matches the precedent set by HttpClient.Timeout. Closes #4420 --- src/IceRpc.Deadline/DeadlineInterceptor.cs | 18 ++++++++++ src/IceRpc.Deadline/DeadlineMiddleware.cs | 11 ++++++ .../DeadlineInterceptorTests.cs | 36 +++++++++++++++++++ .../DeadlineMiddlewareTests.cs | 30 ++++++++++++++++ 4 files changed, 95 insertions(+) diff --git a/src/IceRpc.Deadline/DeadlineInterceptor.cs b/src/IceRpc.Deadline/DeadlineInterceptor.cs index 1782029fc7..71450e03f4 100644 --- a/src/IceRpc.Deadline/DeadlineInterceptor.cs +++ b/src/IceRpc.Deadline/DeadlineInterceptor.cs @@ -25,6 +25,9 @@ namespace IceRpc.Deadline; /// public class DeadlineInterceptor : IInvoker { + // The maximum delay CancellationTokenSource.CancelAfter(TimeSpan) accepts. + private static readonly TimeSpan MaxCancelAfterDelay = TimeSpan.FromMilliseconds(int.MaxValue); + private readonly bool _alwaysEnforceDeadline; private readonly IInvoker _next; private readonly TimeSpan _defaultTimeout; @@ -50,6 +53,13 @@ public DeadlineInterceptor(IInvoker next, TimeSpan defaultTimeout, bool alwaysEn nameof(defaultTimeout)); } + if (defaultTimeout != Timeout.InfiniteTimeSpan && defaultTimeout > MaxCancelAfterDelay) + { + throw new ArgumentException( + $"The {nameof(defaultTimeout)} value must not exceed {MaxCancelAfterDelay} or be Timeout.InfiniteTimeSpan.", + nameof(defaultTimeout)); + } + _next = next; _alwaysEnforceDeadline = alwaysEnforceDeadline; _defaultTimeout = defaultTimeout; @@ -96,6 +106,14 @@ public Task InvokeAsync(OutgoingRequest request, CancellationT async Task PerformInvokeAsync(TimeSpan timeout) { + // Clamp to CancelAfter's supported maximum. A caller-provided IDeadlineFeature value near + // DateTime.MaxValue would otherwise cause CancelAfter to throw ArgumentOutOfRangeException. At this + // bound the deadline is effectively infinite for RPC purposes. + if (timeout > MaxCancelAfterDelay) + { + timeout = MaxCancelAfterDelay; + } + using var timeoutTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); timeoutTokenSource.CancelAfter(timeout); diff --git a/src/IceRpc.Deadline/DeadlineMiddleware.cs b/src/IceRpc.Deadline/DeadlineMiddleware.cs index 124808fe1d..fb5b3d0346 100644 --- a/src/IceRpc.Deadline/DeadlineMiddleware.cs +++ b/src/IceRpc.Deadline/DeadlineMiddleware.cs @@ -14,6 +14,9 @@ namespace IceRpc.Deadline; /// public class DeadlineMiddleware : IDispatcher { + // The maximum delay CancellationTokenSource.CancelAfter(TimeSpan) accepts. + private static readonly TimeSpan MaxCancelAfterDelay = TimeSpan.FromMilliseconds(int.MaxValue); + private readonly IDispatcher _next; private readonly TimeProvider _timeProvider; @@ -48,6 +51,14 @@ public ValueTask DispatchAsync( "The request deadline has expired.")); } + // Clamp to CancelAfter's supported maximum. A peer-encoded deadline thousands of years in the future + // would otherwise cause CancelAfter to throw ArgumentOutOfRangeException, surfacing as a generic + // InternalError response. At this bound the deadline is effectively infinite for RPC purposes. + if (timeout > MaxCancelAfterDelay) + { + timeout = MaxCancelAfterDelay; + } + request.Features = request.Features.With(new DeadlineFeature(deadline)); return PerformDispatchAsync(timeout); } diff --git a/tests/IceRpc.Deadline.Tests/DeadlineInterceptorTests.cs b/tests/IceRpc.Deadline.Tests/DeadlineInterceptorTests.cs index 1d354c14dc..cc2941d34a 100644 --- a/tests/IceRpc.Deadline.Tests/DeadlineInterceptorTests.cs +++ b/tests/IceRpc.Deadline.Tests/DeadlineInterceptorTests.cs @@ -135,6 +135,28 @@ public async Task Deadline_interceptor_does_not_enforce_deadline_by_default() Assert.That(token!.Value, Is.EqualTo(cts.Token)); } + /// Verifies the interceptor clamps an extreme-future IDeadlineFeature value instead of letting + /// CancelAfter throw ArgumentOutOfRangeException. + [Test] + public async Task Invoke_with_extreme_future_deadline_does_not_throw() + { + // Arrange + var invoker = new InlineInvoker((request, cancellationToken) => + Task.FromResult(new IncomingResponse(request, FakeConnectionContext.Instance))); + + var sut = new DeadlineInterceptor(invoker, Timeout.InfiniteTimeSpan, alwaysEnforceDeadline: true); + // Close to DateTime.MaxValue but not equal, so the interceptor's "== DateTime.MaxValue" short-circuit + // does not kick in and the CancelAfter path is actually exercised. + DateTime extreme = DateTime.SpecifyKind(DateTime.MaxValue.AddDays(-1), DateTimeKind.Utc); + using var request = new OutgoingRequest(new ServiceAddress(Protocol.IceRpc)) + { + Features = new FeatureCollection().With(new DeadlineFeature(extreme)) + }; + + // Act/Assert + Assert.That(async () => await sut.InvokeAsync(request, default), Throws.Nothing); + } + [Test] public void Deadline_interceptor_can_enforce_application_deadline() { @@ -229,6 +251,20 @@ public void Constructor_rejects_invalid_default_timeout(int milliseconds) Throws.TypeOf()); } + /// Verifies the constructor rejects a default timeout beyond CancelAfter's supported maximum. + /// TimeSpan.MaxValue would otherwise later cause DateTime overflow (now + timeout) or CancelAfter to throw + /// ArgumentOutOfRangeException at first invoke. + [Test] + public void Constructor_rejects_default_timeout_beyond_cancel_after_max() + { + var invoker = new InlineInvoker((request, cancellationToken) => + Task.FromResult(new IncomingResponse(request, FakeConnectionContext.Instance))); + + Assert.That( + () => new DeadlineInterceptor(invoker, TimeSpan.MaxValue, alwaysEnforceDeadline: false), + Throws.TypeOf()); + } + [Test] public void Constructor_accepts_infinite_timeout() { diff --git a/tests/IceRpc.Deadline.Tests/DeadlineMiddlewareTests.cs b/tests/IceRpc.Deadline.Tests/DeadlineMiddlewareTests.cs index 6333e36f5d..b31e2ce597 100644 --- a/tests/IceRpc.Deadline.Tests/DeadlineMiddlewareTests.cs +++ b/tests/IceRpc.Deadline.Tests/DeadlineMiddlewareTests.cs @@ -97,6 +97,36 @@ public async Task Dispatch_fails_when_deadline_is_min_value() pipeReader.Complete(); } + /// Verifies the middleware clamps an extreme-future peer-encoded deadline instead of letting + /// CancelAfter throw ArgumentOutOfRangeException. + [Test] + public async Task Dispatch_with_extreme_future_deadline_does_not_throw() + { + // Arrange + var dispatcher = new InlineDispatcher((request, cancellationToken) => + new(new OutgoingResponse(request))); + + var sut = new DeadlineMiddleware(dispatcher); + + PipeReader pipeReader = WriteDeadline( + DateTime.SpecifyKind(DateTime.MaxValue, DateTimeKind.Utc)); + pipeReader.TryRead(out var readResult); + + using var request = new IncomingRequest(Protocol.IceRpc, FakeConnectionContext.Instance) + { + Fields = new Dictionary> + { + [RequestFieldKey.Deadline] = readResult.Buffer + } + }; + + // Act/Assert + Assert.That(async () => await sut.DispatchAsync(request, CancellationToken.None), Throws.Nothing); + + // Cleanup + pipeReader.Complete(); + } + /// Verifies that the deadline decoded by the middleware has the expected value. [Test] public async Task Deadline_decoded_by_middleware_has_expected_value() From ff6801d602953598082e7337dbfb64fc0e9d6218 Mon Sep 17 00:00:00 2001 From: Jose Date: Thu, 23 Apr 2026 18:06:44 +0200 Subject: [PATCH 3/7] Validate and document the deadline timeout cap Add the same CancelAfter-maximum cap to DeadlineFeature.FromTimeout so that the failure surfaces at the call site instead of inside the interceptor or as a later DateTime overflow. Also reject non-positive timeouts here, matching the DeadlineInterceptor constructor's validation of its defaultTimeout. Document the cap on: - DeadlineFeature.FromTimeout (param docs + ArgumentException). - DeadlineInterceptor constructor's defaultTimeout param (+ class-level remark about silent clamping for extreme IDeadlineFeature values). - DeadlineMiddleware class remarks (silent clamping of peer deadlines). --- src/IceRpc.Deadline/DeadlineInterceptor.cs | 12 +++++++-- src/IceRpc.Deadline/DeadlineMiddleware.cs | 3 +++ src/IceRpc/Features/DeadlineFeature.cs | 26 +++++++++++++++++-- .../Features/DeadlineFeatureTests.cs | 24 +++++++++++++++++ 4 files changed, 61 insertions(+), 4 deletions(-) create mode 100644 tests/IceRpc.Tests/Features/DeadlineFeatureTests.cs diff --git a/src/IceRpc.Deadline/DeadlineInterceptor.cs b/src/IceRpc.Deadline/DeadlineInterceptor.cs index 71450e03f4..13a70067ff 100644 --- a/src/IceRpc.Deadline/DeadlineInterceptor.cs +++ b/src/IceRpc.Deadline/DeadlineInterceptor.cs @@ -35,8 +35,11 @@ public class DeadlineInterceptor : IInvoker /// Constructs a Deadline interceptor. /// The next invoker in the invocation pipeline. - /// The default timeout. When not infinite, the interceptor adds a deadline to requests - /// without a deadline. + /// The default timeout. When not , the + /// interceptor adds a deadline to requests without a deadline. Must be positive and must not exceed + /// milliseconds (~24.8 days), the maximum supported by + /// , or equal to + /// . /// The optional time provider used to obtain the current time. If , it uses /// . /// When and the request carries a deadline, the @@ -44,6 +47,11 @@ public class DeadlineInterceptor : IInvoker /// and the request carries a deadline, the interceptor creates a cancellation token source to enforce this deadline /// only when the invocation's cancellation token cannot be canceled. The default value is . /// + /// Thrown if is not positive, not + /// , and does not exceed the maximum supported value. + /// A request carrying an whose computed remaining timeout exceeds + /// the maximum is silently clamped to that + /// maximum at invocation time. public DeadlineInterceptor(IInvoker next, TimeSpan defaultTimeout, bool alwaysEnforceDeadline, TimeProvider? timeProvider = null) { if (defaultTimeout != Timeout.InfiniteTimeSpan && defaultTimeout <= TimeSpan.Zero) diff --git a/src/IceRpc.Deadline/DeadlineMiddleware.cs b/src/IceRpc.Deadline/DeadlineMiddleware.cs index fb5b3d0346..fc10b3a7f5 100644 --- a/src/IceRpc.Deadline/DeadlineMiddleware.cs +++ b/src/IceRpc.Deadline/DeadlineMiddleware.cs @@ -10,6 +10,9 @@ namespace IceRpc.Deadline; /// Represents a middleware that decodes deadline fields into deadline features. When the decoded deadline /// expires, this middleware cancels the dispatch and returns an with status code /// . +/// A peer-encoded deadline whose computed remaining timeout exceeds the +/// maximum (~24.8 days) is silently clamped to that +/// maximum. At this bound the deadline is effectively infinite for RPC purposes. /// /// public class DeadlineMiddleware : IDispatcher diff --git a/src/IceRpc/Features/DeadlineFeature.cs b/src/IceRpc/Features/DeadlineFeature.cs index 54b44654c2..e40e645489 100644 --- a/src/IceRpc/Features/DeadlineFeature.cs +++ b/src/IceRpc/Features/DeadlineFeature.cs @@ -5,10 +5,32 @@ namespace IceRpc.Features; /// Default implementation of . public sealed class DeadlineFeature : IDeadlineFeature { + // The maximum delay CancellationTokenSource.CancelAfter(TimeSpan) accepts. + private static readonly TimeSpan MaxCancelAfterDelay = TimeSpan.FromMilliseconds(int.MaxValue); + /// Creates a deadline from a timeout. - /// The timeout. + /// The timeout. Must be positive and must not exceed + /// milliseconds (~24.8 days), the maximum supported by + /// . /// A new deadline equal to now plus the timeout. - public static DeadlineFeature FromTimeout(TimeSpan timeout) => new(DateTime.UtcNow + timeout); + /// Thrown if is not positive, or exceeds the + /// maximum supported value. + public static DeadlineFeature FromTimeout(TimeSpan timeout) + { + if (timeout <= TimeSpan.Zero) + { + throw new ArgumentException( + $"The {nameof(timeout)} value must be positive.", + nameof(timeout)); + } + if (timeout > MaxCancelAfterDelay) + { + throw new ArgumentException( + $"The {nameof(timeout)} value must not exceed {MaxCancelAfterDelay}.", + nameof(timeout)); + } + return new(DateTime.UtcNow + timeout); + } /// public DateTime Value { get; } diff --git a/tests/IceRpc.Tests/Features/DeadlineFeatureTests.cs b/tests/IceRpc.Tests/Features/DeadlineFeatureTests.cs new file mode 100644 index 0000000000..64239f5cac --- /dev/null +++ b/tests/IceRpc.Tests/Features/DeadlineFeatureTests.cs @@ -0,0 +1,24 @@ +// Copyright (c) ZeroC, Inc. + +using NUnit.Framework; + +namespace IceRpc.Features.Tests; + +[Parallelizable(scope: ParallelScope.All)] +public class DeadlineFeatureTests +{ + [Test] + public void FromTimeout_rejects_non_positive_timeout() + { + Assert.That(() => DeadlineFeature.FromTimeout(TimeSpan.Zero), Throws.TypeOf()); + Assert.That(() => DeadlineFeature.FromTimeout(TimeSpan.FromSeconds(-1)), Throws.TypeOf()); + } + + /// Verifies FromTimeout rejects a timeout beyond CancelAfter's supported maximum instead of + /// letting DateTime.UtcNow + timeout overflow with ArgumentOutOfRangeException. + [Test] + public void FromTimeout_rejects_timeout_beyond_cancel_after_max() + { + Assert.That(() => DeadlineFeature.FromTimeout(TimeSpan.MaxValue), Throws.TypeOf()); + } +} From 10e2a936ca9c4e8faf1aadbe4814c7e0b8a6992d Mon Sep 17 00:00:00 2001 From: Jose Date: Thu, 23 Apr 2026 18:09:26 +0200 Subject: [PATCH 4/7] Fix inverted wording in defaultTimeout exception doc --- src/IceRpc.Deadline/DeadlineInterceptor.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/IceRpc.Deadline/DeadlineInterceptor.cs b/src/IceRpc.Deadline/DeadlineInterceptor.cs index 13a70067ff..f515639adf 100644 --- a/src/IceRpc.Deadline/DeadlineInterceptor.cs +++ b/src/IceRpc.Deadline/DeadlineInterceptor.cs @@ -47,8 +47,9 @@ public class DeadlineInterceptor : IInvoker /// and the request carries a deadline, the interceptor creates a cancellation token source to enforce this deadline /// only when the invocation's cancellation token cannot be canceled. The default value is . /// - /// Thrown if is not positive, not - /// , and does not exceed the maximum supported value. + /// Thrown if is not + /// and is either non-positive or exceeds the maximum supported + /// value. /// A request carrying an whose computed remaining timeout exceeds /// the maximum is silently clamped to that /// maximum at invocation time. From 33f0f101042388ed093d93607850c84c967b5813 Mon Sep 17 00:00:00 2001 From: Jose Date: Tue, 28 Apr 2026 11:36:20 +0200 Subject: [PATCH 5/7] Polish defaultTimeout doc and tighten extreme-future test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Split the InfiniteTimeSpan exception out of the constraint clause in DeadlineInterceptor's defaultTimeout doc so it reads behavior → constraint → explicit exception. - Assert the next dispatcher is invoked in the middleware extreme-future clamp test so a regression that swallowed the request would be caught. --- src/IceRpc.Deadline/DeadlineInterceptor.cs | 8 ++++---- tests/IceRpc.Deadline.Tests/DeadlineMiddlewareTests.cs | 7 ++++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/IceRpc.Deadline/DeadlineInterceptor.cs b/src/IceRpc.Deadline/DeadlineInterceptor.cs index f515639adf..0214766749 100644 --- a/src/IceRpc.Deadline/DeadlineInterceptor.cs +++ b/src/IceRpc.Deadline/DeadlineInterceptor.cs @@ -38,10 +38,10 @@ public class DeadlineInterceptor : IInvoker /// The default timeout. When not , the /// interceptor adds a deadline to requests without a deadline. Must be positive and must not exceed /// milliseconds (~24.8 days), the maximum supported by - /// , or equal to - /// . - /// The optional time provider used to obtain the current time. If , it uses - /// . + /// . is + /// also accepted and disables the default deadline behavior. + /// The optional time provider used to obtain the current time. If + /// , it uses . /// When and the request carries a deadline, the /// interceptor always creates a cancellation token source to enforce this deadline. When /// and the request carries a deadline, the interceptor creates a cancellation token source to enforce this deadline diff --git a/tests/IceRpc.Deadline.Tests/DeadlineMiddlewareTests.cs b/tests/IceRpc.Deadline.Tests/DeadlineMiddlewareTests.cs index b31e2ce597..c7ad4527b2 100644 --- a/tests/IceRpc.Deadline.Tests/DeadlineMiddlewareTests.cs +++ b/tests/IceRpc.Deadline.Tests/DeadlineMiddlewareTests.cs @@ -103,8 +103,12 @@ public async Task Dispatch_fails_when_deadline_is_min_value() public async Task Dispatch_with_extreme_future_deadline_does_not_throw() { // Arrange + bool nextCalled = false; var dispatcher = new InlineDispatcher((request, cancellationToken) => - new(new OutgoingResponse(request))); + { + nextCalled = true; + return new(new OutgoingResponse(request)); + }); var sut = new DeadlineMiddleware(dispatcher); @@ -122,6 +126,7 @@ public async Task Dispatch_with_extreme_future_deadline_does_not_throw() // Act/Assert Assert.That(async () => await sut.DispatchAsync(request, CancellationToken.None), Throws.Nothing); + Assert.That(nextCalled, Is.True); // Cleanup pipeReader.Complete(); From 678e1761736818f4695d9d48967224899a994d71 Mon Sep 17 00:00:00 2001 From: Jose Date: Thu, 30 Apr 2026 10:58:29 +0200 Subject: [PATCH 6/7] Apply suggestions from code review Co-authored-by: Austin Henriksen --- src/IceRpc.Deadline/DeadlineInterceptor.cs | 10 ++++------ src/IceRpc.Deadline/DeadlineMiddleware.cs | 5 ++--- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/IceRpc.Deadline/DeadlineInterceptor.cs b/src/IceRpc.Deadline/DeadlineInterceptor.cs index 0214766749..9967c17d24 100644 --- a/src/IceRpc.Deadline/DeadlineInterceptor.cs +++ b/src/IceRpc.Deadline/DeadlineInterceptor.cs @@ -48,11 +48,10 @@ public class DeadlineInterceptor : IInvoker /// only when the invocation's cancellation token cannot be canceled. The default value is . /// /// Thrown if is not - /// and is either non-positive or exceeds the maximum supported - /// value. + /// , not positive, or exceeds the maximum supported value. /// A request carrying an whose computed remaining timeout exceeds - /// the maximum is silently clamped to that - /// maximum at invocation time. + /// the maximum (~24.8 days) is silently clamped to + /// that maximum at invocation time. public DeadlineInterceptor(IInvoker next, TimeSpan defaultTimeout, bool alwaysEnforceDeadline, TimeProvider? timeProvider = null) { if (defaultTimeout != Timeout.InfiniteTimeSpan && defaultTimeout <= TimeSpan.Zero) @@ -116,8 +115,7 @@ public Task InvokeAsync(OutgoingRequest request, CancellationT async Task PerformInvokeAsync(TimeSpan timeout) { // Clamp to CancelAfter's supported maximum. A caller-provided IDeadlineFeature value near - // DateTime.MaxValue would otherwise cause CancelAfter to throw ArgumentOutOfRangeException. At this - // bound the deadline is effectively infinite for RPC purposes. + // DateTime.MaxValue would otherwise cause CancelAfter to throw ArgumentOutOfRangeException. if (timeout > MaxCancelAfterDelay) { timeout = MaxCancelAfterDelay; diff --git a/src/IceRpc.Deadline/DeadlineMiddleware.cs b/src/IceRpc.Deadline/DeadlineMiddleware.cs index fc10b3a7f5..7fbaaff8a1 100644 --- a/src/IceRpc.Deadline/DeadlineMiddleware.cs +++ b/src/IceRpc.Deadline/DeadlineMiddleware.cs @@ -38,8 +38,7 @@ public ValueTask DispatchAsync( IncomingRequest request, CancellationToken cancellationToken = default) { - // Check explicit field presence rather than relying on a decoded-value sentinel: a peer encoding ticks=0 - // decodes to DateTime.MinValue, which would otherwise be indistinguishable from an absent field. + // Check explicit field presence rather than relying on a decoded-value sentinel. if (request.Fields.TryGetValue(RequestFieldKey.Deadline, out ReadOnlySequence value)) { DateTime deadline = value.DecodeSliceBuffer( @@ -56,7 +55,7 @@ public ValueTask DispatchAsync( // Clamp to CancelAfter's supported maximum. A peer-encoded deadline thousands of years in the future // would otherwise cause CancelAfter to throw ArgumentOutOfRangeException, surfacing as a generic - // InternalError response. At this bound the deadline is effectively infinite for RPC purposes. + // InternalError response. if (timeout > MaxCancelAfterDelay) { timeout = MaxCancelAfterDelay; From 6cc8051eb9e5746437911375b4fef5ff9f8231a7 Mon Sep 17 00:00:00 2001 From: Jose Date: Thu, 30 Apr 2026 11:15:46 +0200 Subject: [PATCH 7/7] Review fixes --- src/IceRpc.Deadline/DeadlineInterceptor.cs | 19 +++++++++---------- src/IceRpc.Deadline/DeadlineMiddleware.cs | 8 ++++---- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/IceRpc.Deadline/DeadlineInterceptor.cs b/src/IceRpc.Deadline/DeadlineInterceptor.cs index 9967c17d24..2990e9ac47 100644 --- a/src/IceRpc.Deadline/DeadlineInterceptor.cs +++ b/src/IceRpc.Deadline/DeadlineInterceptor.cs @@ -26,7 +26,7 @@ namespace IceRpc.Deadline; public class DeadlineInterceptor : IInvoker { // The maximum delay CancellationTokenSource.CancelAfter(TimeSpan) accepts. - private static readonly TimeSpan MaxCancelAfterDelay = TimeSpan.FromMilliseconds(int.MaxValue); + private static readonly TimeSpan _maxCancelAfterDelay = TimeSpan.FromMilliseconds(int.MaxValue); private readonly bool _alwaysEnforceDeadline; private readonly IInvoker _next; @@ -35,11 +35,10 @@ public class DeadlineInterceptor : IInvoker /// Constructs a Deadline interceptor. /// The next invoker in the invocation pipeline. - /// The default timeout. When not , the - /// interceptor adds a deadline to requests without a deadline. Must be positive and must not exceed - /// milliseconds (~24.8 days), the maximum supported by - /// . is - /// also accepted and disables the default deadline behavior. + /// The default timeout applied to requests without a deadline. Must be positive and + /// must not exceed milliseconds (~24.8 days), the maximum supported by + /// , or to + /// disable this behavior. /// The optional time provider used to obtain the current time. If /// , it uses . /// When and the request carries a deadline, the @@ -61,10 +60,10 @@ public DeadlineInterceptor(IInvoker next, TimeSpan defaultTimeout, bool alwaysEn nameof(defaultTimeout)); } - if (defaultTimeout != Timeout.InfiniteTimeSpan && defaultTimeout > MaxCancelAfterDelay) + if (defaultTimeout != Timeout.InfiniteTimeSpan && defaultTimeout > _maxCancelAfterDelay) { throw new ArgumentException( - $"The {nameof(defaultTimeout)} value must not exceed {MaxCancelAfterDelay} or be Timeout.InfiniteTimeSpan.", + $"The {nameof(defaultTimeout)} value must not exceed {_maxCancelAfterDelay} or be Timeout.InfiniteTimeSpan.", nameof(defaultTimeout)); } @@ -116,9 +115,9 @@ async Task PerformInvokeAsync(TimeSpan timeout) { // Clamp to CancelAfter's supported maximum. A caller-provided IDeadlineFeature value near // DateTime.MaxValue would otherwise cause CancelAfter to throw ArgumentOutOfRangeException. - if (timeout > MaxCancelAfterDelay) + if (timeout > _maxCancelAfterDelay) { - timeout = MaxCancelAfterDelay; + timeout = _maxCancelAfterDelay; } using var timeoutTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); diff --git a/src/IceRpc.Deadline/DeadlineMiddleware.cs b/src/IceRpc.Deadline/DeadlineMiddleware.cs index 7fbaaff8a1..4627c29c31 100644 --- a/src/IceRpc.Deadline/DeadlineMiddleware.cs +++ b/src/IceRpc.Deadline/DeadlineMiddleware.cs @@ -12,13 +12,13 @@ namespace IceRpc.Deadline; /// . /// A peer-encoded deadline whose computed remaining timeout exceeds the /// maximum (~24.8 days) is silently clamped to that -/// maximum. At this bound the deadline is effectively infinite for RPC purposes. +/// maximum. /// /// public class DeadlineMiddleware : IDispatcher { // The maximum delay CancellationTokenSource.CancelAfter(TimeSpan) accepts. - private static readonly TimeSpan MaxCancelAfterDelay = TimeSpan.FromMilliseconds(int.MaxValue); + private static readonly TimeSpan _maxCancelAfterDelay = TimeSpan.FromMilliseconds(int.MaxValue); private readonly IDispatcher _next; private readonly TimeProvider _timeProvider; @@ -56,9 +56,9 @@ public ValueTask DispatchAsync( // Clamp to CancelAfter's supported maximum. A peer-encoded deadline thousands of years in the future // would otherwise cause CancelAfter to throw ArgumentOutOfRangeException, surfacing as a generic // InternalError response. - if (timeout > MaxCancelAfterDelay) + if (timeout > _maxCancelAfterDelay) { - timeout = MaxCancelAfterDelay; + timeout = _maxCancelAfterDelay; } request.Features = request.Features.With(new DeadlineFeature(deadline));