Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check for sentinel value when setting HTTP/3 error code #57934

Merged
merged 5 commits into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/Servers/Kestrel/Core/src/Internal/Http3/Http3Connection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ private void UpdateHighestOpenedRequestStreamId(long streamId)
public string ConnectionId => _context.ConnectionId;
public ITimeoutControl TimeoutControl => _context.TimeoutControl;

// The default error value is -1. If it hasn't been changed before abort is called then default to HTTP/3's NoError value.
private Http3ErrorCode Http3ErrorCodeOrNoError => _errorCodeFeature.Error == -1 ? Http3ErrorCode.NoError : (Http3ErrorCode)_errorCodeFeature.Error;

public void StopProcessingNextRequest(ConnectionEndReason reason)
=> StopProcessingNextRequest(serverInitiated: true, reason);

Expand Down Expand Up @@ -505,12 +508,14 @@ public async Task ProcessRequestsAsync<TContext>(IHttpApplication<TContext> appl
}
}

var errorCode = Http3ErrorCodeOrNoError;

// Abort active request streams.
lock (_streams)
{
foreach (var stream in _streams.Values)
{
stream.Abort(CreateConnectionAbortError(error, clientAbort), (Http3ErrorCode)_errorCodeFeature.Error);
stream.Abort(CreateConnectionAbortError(error, clientAbort), errorCode);
}
}

Expand Down Expand Up @@ -546,7 +551,7 @@ public async Task ProcessRequestsAsync<TContext>(IHttpApplication<TContext> appl
}

// Complete
Abort(CreateConnectionAbortError(error, clientAbort), (Http3ErrorCode)_errorCodeFeature.Error, reason);
Abort(CreateConnectionAbortError(error, clientAbort), errorCode, reason);

// Wait for active requests to complete.
while (_activeRequestCount > 0)
Expand Down Expand Up @@ -905,7 +910,7 @@ public void OnInputOrOutputCompleted()
TryStopAcceptingStreams();

// Abort the connection using the error code the client used. For a graceful close, this should be H3_NO_ERROR.
Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient), (Http3ErrorCode)_errorCodeFeature.Error, ConnectionEndReason.TransportCompleted);
Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient), Http3ErrorCodeOrNoError, ConnectionEndReason.TransportCompleted);
}

internal WebTransportSession OpenNewWebTransportSession(Http3Stream http3Stream)
Expand Down
2 changes: 1 addition & 1 deletion src/Servers/Kestrel/shared/test/Http3/Http3InMemory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ public void TriggerTick(TimeSpan timeSpan = default)

public async Task InitializeConnectionAsync(RequestDelegate application)
{
MultiplexedConnectionContext = new TestMultiplexedConnectionContext(this)
MultiplexedConnectionContext ??= new TestMultiplexedConnectionContext(this)
{
ConnectionId = "TEST"
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,84 @@ await Http3Api.InitializeConnectionAsync(async c =>
await tcs.Task;
}

[Fact]
public async Task ErrorCodeIsValidOnConnectionTimeout()
{
// This test loosely repros the scenario in https://github.com/dotnet/aspnetcore/issues/57933.
// In particular, there's a request from the server and, once a response has been sent,
// the (simulated) transport throws a QuicException that surfaces through AcceptAsync.
// This test confirms that Http3Connection.ProcessRequestsAsync doesn't (indirectly) cause
// IProtocolErrorCodeFeature.Error to be set to (or left at) -1, which System.Net.Quic will
// not accept.

// Used to signal that a request has been sent and a response has been received
var requestTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
// Used to signal that the connection context has been aborted
var abortTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);

// InitializeConnectionAsync consumes the connection context, so set it first
Http3Api.MultiplexedConnectionContext = new ThrowingMultiplexedConnectionContext(Http3Api, skipCount: 2, requestTcs, abortTcs);
await Http3Api.InitializeConnectionAsync(_echoApplication);

await Http3Api.CreateControlStream();
await Http3Api.GetInboundControlStream();
var requestStream = await Http3Api.CreateRequestStream(Headers, endStream: true);
var responseHeaders = await requestStream.ExpectHeadersAsync();

await requestStream.ExpectReceiveEndOfStream();
await requestStream.OnDisposedTask.DefaultTimeout();

requestTcs.SetResult();

// By the time the connection context is aborted, the error code feature has been updated
await abortTcs.Task.DefaultTimeout();

Http3Api.CloseServerGracefully();

var errorCodeFeature = Http3Api.MultiplexedConnectionContext.Features.Get<IProtocolErrorCodeFeature>();
Assert.InRange(errorCodeFeature.Error, 0, (1L << 62) - 1); // Valid range for HTTP/3 error codes
}

private sealed class ThrowingMultiplexedConnectionContext : TestMultiplexedConnectionContext
{
private int _skipCount;
private readonly TaskCompletionSource _requestTcs;
private readonly TaskCompletionSource _abortTcs;

/// <summary>
/// After <paramref name="skipCount"/> calls to <see cref="AcceptAsync"/>, the next call will throw a <see cref="QuicException"/>
/// (after waiting for <see cref="_requestTcs"/> to be set).
///
/// <paramref name="abortTcs"/> lets this type signal that <see cref="Abort"/> has been called.
/// </summary>
public ThrowingMultiplexedConnectionContext(Http3InMemory testBase, int skipCount, TaskCompletionSource requestTcs, TaskCompletionSource abortTcs)
: base(testBase)
{
_skipCount = skipCount;
_requestTcs = requestTcs;
_abortTcs = abortTcs;
}

public override async ValueTask<ConnectionContext> AcceptAsync(CancellationToken cancellationToken = default)
{
if (_skipCount-- <= 0)
{
await _requestTcs.Task.DefaultTimeout();
throw new System.Net.Quic.QuicException(
System.Net.Quic.QuicError.ConnectionTimeout,
applicationErrorCode: null,
"Connection timed out waiting for a response from the peer.");
}
return await base.AcceptAsync(cancellationToken);
}

public override void Abort(ConnectionAbortedException abortReason)
{
_abortTcs.SetResult();
base.Abort(abortReason);
}
}

private async Task<ConnectionContext> MakeRequestAsync(int index, KeyValuePair<string, string>[] headers, bool sendData, bool waitForServerDispose)
{
var requestStream = await Http3Api.CreateRequestStream(headers, endStream: !sendData);
Expand Down
Loading