Clean up DeadlineMiddleware/Interceptor edge cases

src/IceRpc.Deadline/DeadlineInterceptor.cs

@@ -25,22 +25,32 @@ namespace IceRpc.Deadline;
 /// <seealso cref="DeadlineInvokerBuilderExtensions"/>
 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;
     private readonly TimeProvider _timeProvider;
 
     /// <summary>Constructs a Deadline interceptor.</summary>
     /// <param name="next">The next invoker in the invocation pipeline.</param>
-    /// <param name="defaultTimeout">The default timeout. When not infinite, the interceptor adds a deadline to requests
-    /// without a deadline.</param>
-    /// <param name="timeProvider">The optional time provider used to obtain the current time. If <see langword="null"/>, it uses
-    /// <see cref="TimeProvider.System"/>.</param>
+    /// <param name="defaultTimeout">The default timeout applied to requests without a deadline. Must be positive and
+    /// must not exceed <see cref="int.MaxValue" /> milliseconds (~24.8 days), the maximum supported by
+    /// <see cref="CancellationTokenSource.CancelAfter(TimeSpan)" />, or <see cref="Timeout.InfiniteTimeSpan" /> to
+    /// disable this behavior.</param>
+    /// <param name="timeProvider">The optional time provider used to obtain the current time. If
+    /// <see langword="null"/>, it uses <see cref="TimeProvider.System"/>.</param>
     /// <param name="alwaysEnforceDeadline">When <see langword="true" /> and the request carries a deadline, the
     /// interceptor always creates a cancellation token source to enforce this deadline. When <see langword="false" />
     /// 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 <see langword="false" />.
     /// </param>
+    /// <exception cref="ArgumentException">Thrown if <paramref name="defaultTimeout" /> is not
+    /// <see cref="Timeout.InfiniteTimeSpan" />, not positive, or exceeds the maximum supported value.</exception>
+    /// <remarks>A request carrying an <see cref="IDeadlineFeature" /> whose computed remaining timeout exceeds
+    /// the <see cref="CancellationTokenSource.CancelAfter(TimeSpan)" /> maximum (~24.8 days) is silently clamped to
+    /// that maximum at invocation time.</remarks>
     public DeadlineInterceptor(IInvoker next, TimeSpan defaultTimeout, bool alwaysEnforceDeadline, TimeProvider? timeProvider = null)
     {
         if (defaultTimeout != Timeout.InfiniteTimeSpan && defaultTimeout <= TimeSpan.Zero)
@@ -50,6 +60,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 +113,13 @@ public Task<IncomingResponse> InvokeAsync(OutgoingRequest request, CancellationT
 
         async Task<IncomingResponse> 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)
+            {
+                timeout = _maxCancelAfterDelay;
+            }
+
             using var timeoutTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
             timeoutTokenSource.CancelAfter(timeout);
 

src/IceRpc.Deadline/DeadlineMiddleware.cs

@@ -2,17 +2,24 @@
 
 using IceRpc.Extensions.DependencyInjection;
 using IceRpc.Features;
+using System.Buffers;
 using ZeroC.Slice.Codec;
 
 namespace IceRpc.Deadline;
 
 /// <summary>Represents a middleware that decodes deadline fields into deadline features. When the decoded deadline
 /// expires, this middleware cancels the dispatch and returns an <see cref="OutgoingResponse" /> with status code
 /// <see cref="StatusCode.DeadlineExceeded" />.</summary>
+/// <remarks>A peer-encoded deadline whose computed remaining timeout exceeds the
+/// <see cref="CancellationTokenSource.CancelAfter(TimeSpan)" /> maximum (~24.8 days) is silently clamped to that
+/// maximum.</remarks>
 /// <seealso cref="DeadlineRouterExtensions"/>
 /// <seealso cref="DeadlineDispatcherBuilderExtensions"/>
 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;
 
@@ -31,16 +38,12 @@ public ValueTask<OutgoingResponse> 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.
+        if (request.Fields.TryGetValue(RequestFieldKey.Deadline, out ReadOnlySequence<byte> 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)
             {
@@ -50,10 +53,19 @@ public ValueTask<OutgoingResponse> 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.
+            if (timeout > _maxCancelAfterDelay)
+            {
+                timeout = _maxCancelAfterDelay;
+            }
+
             request.Features = request.Features.With<IDeadlineFeature>(new DeadlineFeature(deadline));
+            return PerformDispatchAsync(timeout);
         }
 
-        return timeout is null ? _next.DispatchAsync(request, cancellationToken) : PerformDispatchAsync(timeout.Value);
+        return _next.DispatchAsync(request, cancellationToken);
 
         async ValueTask<OutgoingResponse> PerformDispatchAsync(TimeSpan timeout)
         {

src/IceRpc/Features/DeadlineFeature.cs

@@ -5,10 +5,32 @@ namespace IceRpc.Features;
 /// <summary>Default implementation of <see cref="IDeadlineFeature" />.</summary>
 public sealed class DeadlineFeature : IDeadlineFeature
 {
+    // The maximum delay CancellationTokenSource.CancelAfter(TimeSpan) accepts.
+    private static readonly TimeSpan MaxCancelAfterDelay = TimeSpan.FromMilliseconds(int.MaxValue);
+
     /// <summary>Creates a deadline from a timeout.</summary>
-    /// <param name="timeout">The timeout.</param>
+    /// <param name="timeout">The timeout. Must be positive and must not exceed
+    /// <see cref="int.MaxValue" /> milliseconds (~24.8 days), the maximum supported by
+    /// <see cref="CancellationTokenSource.CancelAfter(TimeSpan)" />.</param>
     /// <returns>A new deadline equal to now plus the timeout.</returns>
-    public static DeadlineFeature FromTimeout(TimeSpan timeout) => new(DateTime.UtcNow + timeout);
+    /// <exception cref="ArgumentException">Thrown if <paramref name="timeout" /> is not positive, or exceeds the
+    /// maximum supported value.</exception>
+    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);
+    }
 
     /// <inheritdoc/>
     public DateTime Value { get; }

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));
     }
 
+    /// <summary>Verifies the interceptor clamps an extreme-future IDeadlineFeature value instead of letting
+    /// CancelAfter throw ArgumentOutOfRangeException.</summary>
+    [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<IDeadlineFeature>(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<ArgumentException>());
     }
 
+    /// <summary>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.</summary>
+    [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<ArgumentException>());
+    }
+
     [Test]
     public void Constructor_accepts_infinite_timeout()
     {

tests/IceRpc.Deadline.Tests/DeadlineMiddlewareTests.cs

@@ -56,6 +56,82 @@ public async Task Dispatch_fails_after_the_deadline_expires()
         pipeReader.Complete();
     }
 
+    /// <summary>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.</summary>
+    [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, ReadOnlySequence<byte>>
+            {
+                [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();
+    }
+
+    /// <summary>Verifies the middleware clamps an extreme-future peer-encoded deadline instead of letting
+    /// CancelAfter throw ArgumentOutOfRangeException.</summary>
+    [Test]
+    public async Task Dispatch_with_extreme_future_deadline_does_not_throw()
+    {
+        // Arrange
+        bool nextCalled = false;
+        var dispatcher = new InlineDispatcher((request, cancellationToken) =>
+        {
+            nextCalled = true;
+            return 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, ReadOnlySequence<byte>>
+            {
+                [RequestFieldKey.Deadline] = readResult.Buffer
+            }
+        };
+
+        // Act/Assert
+        Assert.That(async () => await sut.DispatchAsync(request, CancellationToken.None), Throws.Nothing);
+        Assert.That(nextCalled, Is.True);
+
+        // Cleanup
+        pipeReader.Complete();
+    }
+
     /// <summary>Verifies that the deadline decoded by the middleware has the expected value.</summary>
     [Test]
     public async Task Deadline_decoded_by_middleware_has_expected_value()

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<ArgumentException>());
+        Assert.That(() => DeadlineFeature.FromTimeout(TimeSpan.FromSeconds(-1)), Throws.TypeOf<ArgumentException>());
+    }
+
+    /// <summary>Verifies FromTimeout rejects a timeout beyond CancelAfter's supported maximum instead of
+    /// letting DateTime.UtcNow + timeout overflow with ArgumentOutOfRangeException.</summary>
+    [Test]
+    public void FromTimeout_rejects_timeout_beyond_cancel_after_max()
+    {
+        Assert.That(() => DeadlineFeature.FromTimeout(TimeSpan.MaxValue), Throws.TypeOf<ArgumentException>());
+    }
+}