Skip to content

HTTP/3 RequestHeadersTimeout #30638

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

Merged
merged 1 commit into from
Apr 23, 2021
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
3 changes: 3 additions & 0 deletions src/Servers/Kestrel/Core/src/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -683,4 +683,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l
<data name="Http3StreamErrorRequestEndedNoHeaders" xml:space="preserve">
<value>Request stream ended without headers.</value>
</data>
<data name="Http3ControlStreamHeaderTimeout" xml:space="preserve">
<value>Reading the control stream header timed out.</value>
</data>
</root>
154 changes: 92 additions & 62 deletions src/Servers/Kestrel/Core/src/Internal/Http3/Http3Connection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
Expand All @@ -14,14 +15,15 @@
using Microsoft.AspNetCore.Connections.Features;
using Microsoft.AspNetCore.Hosting.Server;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
using Microsoft.Extensions.Logging;

namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3
{
internal class Http3Connection : ITimeoutHandler
internal class Http3Connection : ITimeoutHandler, IHttp3StreamLifetimeHandler
{
internal readonly Dictionary<long, Http3Stream> _streams = new Dictionary<long, Http3Stream>();
internal readonly Dictionary<long, IHttp3Stream> _streams = new Dictionary<long, IHttp3Stream>();

private long _highestOpenedStreamId;
private readonly object _sync = new object();
Expand Down Expand Up @@ -84,9 +86,6 @@ public async Task ProcessStreamsAsync<TContext>(IHttpApplication<TContext> httpA
{
try
{
// Ensure TimeoutControl._lastTimestamp is initialized before anything that could set timeouts runs.
_timeoutControl.Initialize(_systemClock.UtcNowTicks);

var connectionHeartbeatFeature = _context.ConnectionFeatures.Get<IConnectionHeartbeatFeature>();
var connectionLifetimeNotificationFeature = _context.ConnectionFeatures.Get<IConnectionLifetimeNotificationFeature>();

Expand Down Expand Up @@ -198,11 +197,42 @@ public void Tick()
return;
}

// It's safe to use UtcNowUnsynchronized since Tick is called by the Heartbeat.
var now = _systemClock.UtcNowUnsynchronized;
_timeoutControl.Tick(now);
UpdateStartingStreams();
}

private void UpdateStartingStreams()
{
var now = _systemClock.UtcNow.Ticks;

lock (_streams)
{
foreach (var stream in _streams.Values)
{
if (stream.ReceivedHeader)
{
continue;
}

if (stream.HeaderTimeoutTicks == default)
{
// On expiration overflow, use max value.
var expirationTicks = now + _context.ServiceContext.ServerOptions.Limits.RequestHeadersTimeout.Ticks;
stream.HeaderTimeoutTicks = expirationTicks >= 0 ? expirationTicks : long.MaxValue;
}

// TODO cancel process stream loop to update logic.
if (stream.HeaderTimeoutTicks < now)
{
if (stream.IsRequestStream)
{
stream.Abort(new ConnectionAbortedException(CoreStrings.BadRequest_RequestHeadersTimeout), Http3ErrorCode.RequestRejected);
}
else
{
stream.Abort(new ConnectionAbortedException(CoreStrings.Http3ControlStreamHeaderTimeout), Http3ErrorCode.StreamCreationError);
}
}
}
}
}

public void OnTimeout(TimeoutReason reason)
Expand All @@ -213,13 +243,11 @@ public void OnTimeout(TimeoutReason reason)
// TODO what timeouts should we handle here? Is keep alive something we should care about?
switch (reason)
{
case TimeoutReason.KeepAlive:
SendGoAway(GetHighestStreamId()).Preserve();
break;
case TimeoutReason.TimeoutFeature:
SendGoAway(GetHighestStreamId()).Preserve();
break;
case TimeoutReason.RequestHeaders:
case TimeoutReason.RequestHeaders: // Request header timeout is handled in starting stream queue
case TimeoutReason.KeepAlive: // Keep-alive is handled by msquic
case TimeoutReason.ReadDataRate:
case TimeoutReason.WriteDataRate:
case TimeoutReason.RequestBodyDrain:
Expand All @@ -245,8 +273,6 @@ internal async Task InnerProcessStreamsAsync<TContext>(IHttpApplication<TContext
// TODO should we await the control stream task?
var controlTask = CreateControlStream(application);

_timeoutControl.SetTimeout(Limits.KeepAliveTimeout.Ticks, TimeoutReason.KeepAlive);

try
{
while (_isClosed == 0)
Expand Down Expand Up @@ -277,29 +303,36 @@ internal async Task InnerProcessStreamsAsync<TContext>(IHttpApplication<TContext
streamContext.LocalEndPoint as IPEndPoint,
streamContext.RemoteEndPoint as IPEndPoint,
streamContext.Transport,
this,
streamContext,
_serverSettings);
httpConnectionContext.TimeoutControl = _context.TimeoutControl;

var streamId = streamIdFeature.StreamId;

if (!quicStreamFeature.CanWrite)
{
// Unidirectional stream
var stream = new Http3ControlStream<TContext>(application, this, httpConnectionContext);
var stream = new Http3ControlStream<TContext>(application, httpConnectionContext);
lock (_streams)
{
_streams[streamId] = stream;
}

ThreadPool.UnsafeQueueUserWorkItem(stream, preferLocal: false);
}
else
{
var streamId = streamIdFeature.StreamId;

// Request stream
UpdateHighestStreamId(streamId);

var http3Stream = new Http3Stream<TContext>(application, this, httpConnectionContext);
var stream = http3Stream;
var stream = new Http3Stream<TContext>(application, httpConnectionContext);
lock (_streams)
{
_activeRequestCount++;
_streams[streamId] = http3Stream;
_streams[streamId] = stream;
}

KestrelEventSource.Log.RequestQueuedStart(stream, AspNetCore.Http.HttpProtocol.Http3);
ThreadPool.UnsafeQueueUserWorkItem(stream, preferLocal: false);
}
Expand Down Expand Up @@ -363,8 +396,6 @@ internal async Task InnerProcessStreamsAsync<TContext>(IHttpApplication<TContext
{
await _streamCompletionAwaitable;
}

_timeoutControl.CancelTimeout();
}
catch
{
Expand Down Expand Up @@ -410,17 +441,6 @@ private void UpdateConnectionState()
SendGoAway(GetHighestStreamId()).Preserve();
}
}
else
{
// TODO should keep-alive timeout be a thing for HTTP/3? MsQuic currently tracks this for us?
if (_timeoutControl.TimerReason == TimeoutReason.None)
{
_timeoutControl.SetTimeout(Limits.KeepAliveTimeout.Ticks, TimeoutReason.KeepAlive);
}

// Only reason should be keep-alive.
Debug.Assert(_timeoutControl.TimerReason == TimeoutReason.KeepAlive);
}
}
}

Expand Down Expand Up @@ -450,11 +470,12 @@ private async ValueTask<Http3ControlStream> CreateNewUnidirectionalStreamAsync<T
streamContext.LocalEndPoint as IPEndPoint,
streamContext.RemoteEndPoint as IPEndPoint,
streamContext.Transport,
this,
streamContext,
_serverSettings);
httpConnectionContext.TimeoutControl = _context.TimeoutControl;

return new Http3ControlStream<TContext>(application, this, httpConnectionContext);
return new Http3ControlStream<TContext>(application, httpConnectionContext);
}

private ValueTask<FlushResult> SendGoAway(long id)
Expand All @@ -466,33 +487,10 @@ private ValueTask<FlushResult> SendGoAway(long id)
return OutboundControlStream.SendGoAway(id);
}
}
return new ValueTask<FlushResult>();
return default;
}

public void ApplyMaxHeaderListSize(long value)
{
}

internal void ApplyBlockedStream(long value)
{
}

internal void ApplyMaxTableCapacity(long value)
{
}

internal void RemoveStream(long streamId)
{
lock (_streams)
{
_activeRequestCount--;
_streams.Remove(streamId);
}

_streamCompletionAwaitable.Complete();
}

public bool SetInboundControlStream(Http3ControlStream stream)
public bool OnInboundControlStream(Http3ControlStream stream)
{
lock (_sync)
{
Expand All @@ -505,7 +503,7 @@ public bool SetInboundControlStream(Http3ControlStream stream)
}
}

public bool SetInboundEncoderStream(Http3ControlStream stream)
public bool OnInboundEncoderStream(Http3ControlStream stream)
{
lock (_sync)
{
Expand All @@ -518,7 +516,7 @@ public bool SetInboundEncoderStream(Http3ControlStream stream)
}
}

public bool SetInboundDecoderStream(Http3ControlStream stream)
public bool OnInboundDecoderStream(Http3ControlStream stream)
{
lock (_sync)
{
Expand All @@ -531,6 +529,38 @@ public bool SetInboundDecoderStream(Http3ControlStream stream)
}
}

public void OnStreamCompleted(IHttp3Stream stream)
{
lock (_streams)
{
_activeRequestCount--;
_streams.Remove(stream.StreamId);
}

_streamCompletionAwaitable.Complete();
}

public void OnStreamConnectionError(Http3ConnectionErrorException ex)
{
Log.Http3ConnectionError(ConnectionId, ex);
Abort(new ConnectionAbortedException(ex.Message, ex), ex.ErrorCode);
}

public void OnInboundControlStreamSetting(Http3SettingType type, long value)
{
switch (type)
{
case Http3SettingType.QPackMaxTableCapacity:
break;
case Http3SettingType.MaxFieldSectionSize:
break;
case Http3SettingType.QPackBlockedStreams:
break;
default:
throw new InvalidOperationException("Unexpected setting: " + type);
}
}

private static class GracefulCloseInitiator
{
public const int None = 0;
Expand Down
Loading