diff --git a/src/Http/Http.Extensions/src/HttpResponseJsonExtensions.cs b/src/Http/Http.Extensions/src/HttpResponseJsonExtensions.cs index b35c4f2b3214..aa0d67b37160 100644 --- a/src/Http/Http.Extensions/src/HttpResponseJsonExtensions.cs +++ b/src/Http/Http.Extensions/src/HttpResponseJsonExtensions.cs @@ -18,7 +18,7 @@ public static partial class HttpResponseJsonExtensions { /// /// Write the specified value as JSON to the response body. The response content-type will be set to - /// application/json; charset=utf-8 and the status code set to 200. + /// application/json; charset=utf-8. /// /// The type of object to write. /// The response to write JSON to. @@ -36,7 +36,7 @@ public static Task WriteAsJsonAsync( /// /// Write the specified value as JSON to the response body. The response content-type will be set to - /// application/json; charset=utf-8 and the status code set to 200. + /// application/json; charset=utf-8. /// /// The type of object to write. /// The response to write JSON to. @@ -56,7 +56,7 @@ public static Task WriteAsJsonAsync( /// /// Write the specified value as JSON to the response body. The response content-type will be set to - /// the specified content-type and the status code set to 200. + /// the specified content-type. /// /// The type of object to write. /// The response to write JSON to. @@ -86,7 +86,7 @@ public static Task WriteAsJsonAsync( /// /// Write the specified value as JSON to the response body. The response content-type will be set to - /// application/json; charset=utf-8 and the status code set to 200. + /// application/json; charset=utf-8. /// /// The response to write JSON to. /// The value to write as JSON. @@ -105,7 +105,7 @@ public static Task WriteAsJsonAsync( /// /// Write the specified value as JSON to the response body. The response content-type will be set to - /// application/json; charset=utf-8 and the status code set to 200. + /// application/json; charset=utf-8. /// /// The response to write JSON to. /// The value to write as JSON. @@ -126,7 +126,7 @@ public static Task WriteAsJsonAsync( /// /// Write the specified value as JSON to the response body. The response content-type will be set to - /// the specified content-type and the status code set to 200. + /// the specified content-type. /// /// The response to write JSON to. /// The value to write as JSON. diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs index 156a90dbb909..a84a3dfab550 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs @@ -192,7 +192,7 @@ public static MessageBody For( } [StackTraceHidden] - private void ThrowIfReaderCompleted() + protected void ThrowIfReaderCompleted() { if (_readerCompleted) { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1UpgradeMessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1UpgradeMessageBody.cs index 03a44428d443..066f83b2ff8e 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1UpgradeMessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1UpgradeMessageBody.cs @@ -14,6 +14,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http /// internal sealed class Http1UpgradeMessageBody : Http1MessageBody { + private int _userCanceled; + public Http1UpgradeMessageBody(Http1Connection context, bool keepAlive) : base(context, keepAlive) { @@ -23,6 +25,18 @@ public Http1UpgradeMessageBody(Http1Connection context, bool keepAlive) // This returns IsEmpty so we can avoid draining the body (since it's basically an endless stream) public override bool IsEmpty => true; + public override ValueTask ReadAsync(CancellationToken cancellationToken = default) + { + ThrowIfReaderCompleted(); + return ReadAsyncInternal(cancellationToken); + } + + public override bool TryRead(out ReadResult result) + { + ThrowIfReaderCompleted(); + return TryReadInternal(out result); + } + public override void AdvanceTo(SequencePosition consumed, SequencePosition examined) { _context.Input.AdvanceTo(consumed, examined); @@ -30,6 +44,7 @@ public override void AdvanceTo(SequencePosition consumed, SequencePosition exami public override void CancelPendingRead() { + Interlocked.Exchange(ref _userCanceled, 1); _context.Input.CancelPendingRead(); } @@ -45,12 +60,49 @@ public override ValueTask StopAsync() public override bool TryReadInternal(out ReadResult readResult) { - return _context.Input.TryRead(out readResult); + // Ignore the canceled readResult unless it was canceled by the user. + do + { + if (!_context.Input.TryRead(out readResult)) + { + return false; + } + } while (readResult.IsCanceled && Interlocked.Exchange(ref _userCanceled, 0) == 0); + + return true; } public override ValueTask ReadAsyncInternal(CancellationToken cancellationToken = default) { - return _context.Input.ReadAsync(cancellationToken); + ReadResult readResult; + + // Ignore the canceled readResult unless it was canceled by the user. + do + { + var readTask = _context.Input.ReadAsync(cancellationToken); + + if (!readTask.IsCompletedSuccessfully) + { + return ReadAsyncInternalAwaited(readTask, cancellationToken); + } + + readResult = readTask.GetAwaiter().GetResult(); + } while (readResult.IsCanceled && Interlocked.Exchange(ref _userCanceled, 0) == 0); + + return new ValueTask(readResult); + } + + private async ValueTask ReadAsyncInternalAwaited(ValueTask readTask, CancellationToken cancellationToken = default) + { + var readResult = await readTask; + + // Ignore the canceled readResult unless it was canceled by the user. + while (readResult.IsCanceled && Interlocked.Exchange(ref _userCanceled, 0) == 0) + { + readResult = await _context.Input.ReadAsync(cancellationToken); + } + + return readResult; } } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/UpgradeTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/UpgradeTests.cs index 3261086d3aee..9c3fbc67a245 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/UpgradeTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/UpgradeTests.cs @@ -3,7 +3,10 @@ using System; using System.IO; +using System.IO.Pipelines; using System.Threading.Tasks; +using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; @@ -343,6 +346,51 @@ await connection.Receive("HTTP/1.1 101 Switching Protocols", } } + [Fact] + public async Task DoesNotThrowGivenCanceledReadResult() + { + var appCompletedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + await using var server = new TestServer(async context => + { + try + { + var upgradeFeature = context.Features.Get(); + var duplexStream = await upgradeFeature.UpgradeAsync(); + + // Kestrel will call Transport.Input.CancelPendingRead() during shutdown so idle connections + // can wake up and shutdown gracefully. We manually call CancelPendingRead() to simulate this and + // ensure the Stream returned by UpgradeAsync doesn't throw in this case. + // https://github.com/dotnet/aspnetcore/issues/26482 + var connectionTransportFeature = context.Features.Get(); + connectionTransportFeature.Transport.Input.CancelPendingRead(); + + // Use ReadAsync() instead of CopyToAsync() for this test since IsCanceled is only checked in + // HttpRequestStream.ReadAsync() and not HttpRequestStream.CopyToAsync() + Assert.Equal(0, await duplexStream.ReadAsync(new byte[1])); + appCompletedTcs.SetResult(null); + } + catch (Exception ex) + { + appCompletedTcs.SetException(ex); + throw; + } + }, + new TestServiceContext(LoggerFactory)); + + using (var connection = server.CreateConnection()) + { + await connection.SendEmptyGetWithUpgrade(); + await connection.Receive("HTTP/1.1 101 Switching Protocols", + "Connection: Upgrade", + $"Date: {server.Context.DateHeaderValue}", + "", + ""); + } + + await appCompletedTcs.Task.DefaultTimeout(); + } + [Fact] public async Task DoesNotCloseConnectionWithout101Response() {