Fix Kestrel HTTP/2 debug assert failure (#23062)

This commit is contained in:
Stephen Halter 2020-06-18 15:13:18 -07:00 committed by GitHub
parent 7e6b5d5fea
commit 6abc3c759f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 35 additions and 49 deletions

View File

@ -44,7 +44,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
public override void AdvanceTo(SequencePosition consumed, SequencePosition examined) public override void AdvanceTo(SequencePosition consumed, SequencePosition examined)
{ {
OnAdvance(_readResult, consumed, examined); TrackConsumedAndExaminedBytes(_readResult, consumed, examined);
_requestBodyPipe.Reader.AdvanceTo(consumed, examined); _requestBodyPipe.Reader.AdvanceTo(consumed, examined);
} }
@ -351,7 +351,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
consumed = reader.Position; consumed = reader.Position;
examined = reader.Position; examined = reader.Position;
AddAndCheckConsumedBytes(reader.Consumed); AddAndCheckObservedBytes(reader.Consumed);
_inputLength = chunkSize; _inputLength = chunkSize;
_mode = Mode.Extension; _mode = Mode.Extension;
return; return;
@ -370,7 +370,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
{ {
consumed = reader.Position; consumed = reader.Position;
AddAndCheckConsumedBytes(reader.Consumed); AddAndCheckObservedBytes(reader.Consumed);
_inputLength = chunkSize; _inputLength = chunkSize;
_mode = chunkSize > 0 ? Mode.Data : Mode.Trailer; _mode = chunkSize > 0 ? Mode.Data : Mode.Trailer;
return; return;
@ -398,7 +398,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
// End marker not found yet // End marker not found yet
consumed = buffer.End; consumed = buffer.End;
examined = buffer.End; examined = buffer.End;
AddAndCheckConsumedBytes(buffer.Length); AddAndCheckObservedBytes(buffer.Length);
return; return;
}; };
@ -410,7 +410,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
{ {
consumed = extensionCursor; consumed = extensionCursor;
examined = buffer.End; examined = buffer.End;
AddAndCheckConsumedBytes(charsToByteCRExclusive); AddAndCheckObservedBytes(charsToByteCRExclusive);
return; return;
} }
@ -424,14 +424,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
consumed = suffixBuffer.End; consumed = suffixBuffer.End;
examined = suffixBuffer.End; examined = suffixBuffer.End;
AddAndCheckConsumedBytes(charsToByteCRExclusive + 2); AddAndCheckObservedBytes(charsToByteCRExclusive + 2);
} }
else else
{ {
// Don't consume suffixSpan[1] in case it is also a \r. // Don't consume suffixSpan[1] in case it is also a \r.
buffer = buffer.Slice(charsToByteCRExclusive + 1); buffer = buffer.Slice(charsToByteCRExclusive + 1);
consumed = extensionCursor; consumed = extensionCursor;
AddAndCheckConsumedBytes(charsToByteCRExclusive + 1); AddAndCheckObservedBytes(charsToByteCRExclusive + 1);
} }
} while (_mode == Mode.Extension); } while (_mode == Mode.Extension);
} }
@ -445,7 +445,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
Copy(buffer.Slice(0, actual), writableBuffer); Copy(buffer.Slice(0, actual), writableBuffer);
_inputLength -= actual; _inputLength -= actual;
AddAndCheckConsumedBytes(actual); AddAndCheckObservedBytes(actual);
if (_inputLength == 0) if (_inputLength == 0)
{ {
@ -472,7 +472,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
if (suffixSpan[0] == '\r' && suffixSpan[1] == '\n') if (suffixSpan[0] == '\r' && suffixSpan[1] == '\n')
{ {
consumed = suffixBuffer.End; consumed = suffixBuffer.End;
AddAndCheckConsumedBytes(2); AddAndCheckObservedBytes(2);
_mode = Mode.Prefix; _mode = Mode.Prefix;
} }
else else
@ -500,7 +500,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
if (trailerSpan[0] == '\r' && trailerSpan[1] == '\n') if (trailerSpan[0] == '\r' && trailerSpan[1] == '\n')
{ {
consumed = trailerBuffer.End; consumed = trailerBuffer.End;
AddAndCheckConsumedBytes(2); AddAndCheckObservedBytes(2);
_mode = Mode.Complete; _mode = Mode.Complete;
// No trailers // No trailers
_context.OnTrailersComplete(); _context.OnTrailersComplete();

View File

@ -13,7 +13,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
{ {
private ReadResult _readResult; private ReadResult _readResult;
private readonly long _contentLength; private readonly long _contentLength;
private long _inputLength; private long _unexaminedInputLength;
private bool _readCompleted; private bool _readCompleted;
private bool _isReading; private bool _isReading;
private int _userCanceled; private int _userCanceled;
@ -24,7 +24,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
{ {
RequestKeepAlive = keepAlive; RequestKeepAlive = keepAlive;
_contentLength = contentLength; _contentLength = contentLength;
_inputLength = _contentLength; _unexaminedInputLength = _contentLength;
} }
public override ValueTask<ReadResult> ReadAsync(CancellationToken cancellationToken = default) public override ValueTask<ReadResult> ReadAsync(CancellationToken cancellationToken = default)
@ -181,7 +181,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
private long CreateReadResultFromConnectionReadResult() private long CreateReadResultFromConnectionReadResult()
{ {
var initialLength = _readResult.Buffer.Length; var initialLength = _readResult.Buffer.Length;
var maxLength = _inputLength + _examinedUnconsumedBytes; var maxLength = _unexaminedInputLength + _examinedUnconsumedBytes;
if (initialLength < maxLength) if (initialLength < maxLength)
{ {
@ -226,7 +226,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
return; return;
} }
_inputLength -= OnAdvance(_readResult, consumed, examined); _unexaminedInputLength -= TrackConsumedAndExaminedBytes(_readResult, consumed, examined);
_context.Input.AdvanceTo(consumed, examined); _context.Input.AdvanceTo(consumed, examined);
} }

View File

@ -18,7 +18,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
private readonly HttpProtocol _context; private readonly HttpProtocol _context;
private bool _send100Continue = true; private bool _send100Continue = true;
private long _consumedBytes; private long _observedBytes;
private bool _stopped; private bool _stopped;
protected bool _timingEnabled; protected bool _timingEnabled;
@ -76,7 +76,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
public virtual void Reset() public virtual void Reset()
{ {
_send100Continue = true; _send100Continue = true;
_consumedBytes = 0; _observedBytes = 0;
_stopped = false; _stopped = false;
_timingEnabled = false; _timingEnabled = false;
_backpressure = false; _backpressure = false;
@ -160,15 +160,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
{ {
} }
protected virtual void OnDataRead(long bytesRead) protected void AddAndCheckObservedBytes(long observedBytes)
{ {
} _observedBytes += observedBytes;
protected void AddAndCheckConsumedBytes(long consumedBytes) if (_observedBytes > _context.MaxRequestBodySize)
{
_consumedBytes += consumedBytes;
if (_consumedBytes > _context.MaxRequestBodySize)
{ {
KestrelBadHttpRequestException.Throw(RequestRejectionReason.RequestBodyTooLarge); KestrelBadHttpRequestException.Throw(RequestRejectionReason.RequestBodyTooLarge);
} }
@ -209,7 +205,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
} }
} }
protected long OnAdvance(ReadResult readResult, SequencePosition consumed, SequencePosition examined) protected long TrackConsumedAndExaminedBytes(ReadResult readResult, SequencePosition consumed, SequencePosition examined)
{ {
// This code path is fairly hard to understand so let's break it down with an example // This code path is fairly hard to understand so let's break it down with an example
// ReadAsync returns a ReadResult of length 50. // ReadAsync returns a ReadResult of length 50.
@ -252,18 +248,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
totalLength = readResult.Buffer.Length; totalLength = readResult.Buffer.Length;
} }
var newlyExamined = examinedLength - _examinedUnconsumedBytes; var newlyExaminedBytes = examinedLength - _examinedUnconsumedBytes;
_examinedUnconsumedBytes += newlyExaminedBytes - consumedLength;
if (newlyExamined > 0)
{
OnDataRead(newlyExamined);
_examinedUnconsumedBytes += newlyExamined;
}
_examinedUnconsumedBytes -= consumedLength;
_alreadyTimedBytes = totalLength - consumedLength; _alreadyTimedBytes = totalLength - consumedLength;
return newlyExamined; return newlyExaminedBytes;
} }
} }
} }

View File

@ -39,13 +39,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
} }
} }
protected override void OnDataRead(long bytesRead)
{
// The HTTP/2 flow control window cannot be larger than 2^31-1 which limits bytesRead.
_context.OnDataRead((int)bytesRead);
AddAndCheckConsumedBytes(bytesRead);
}
public override void Reset() public override void Reset()
{ {
base.Reset(); base.Reset();
@ -59,8 +52,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
public override void AdvanceTo(SequencePosition consumed, SequencePosition examined) public override void AdvanceTo(SequencePosition consumed, SequencePosition examined)
{ {
OnAdvance(_readResult, consumed, examined); var newlyExaminedBytes = TrackConsumedAndExaminedBytes(_readResult, consumed, examined);
// Ensure we consume data from the RequestBodyPipe before sending WINDOW_UPDATES to the client.
_context.RequestBodyPipe.Reader.AdvanceTo(consumed, examined); _context.RequestBodyPipe.Reader.AdvanceTo(consumed, examined);
// The HTTP/2 flow control window cannot be larger than 2^31-1 which limits bytesRead.
_context.OnDataRead((int)newlyExaminedBytes);
AddAndCheckObservedBytes(newlyExaminedBytes);
} }
public override bool TryRead(out ReadResult readResult) public override bool TryRead(out ReadResult readResult)

View File

@ -446,7 +446,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
var flushTask = RequestBodyPipe.Writer.FlushAsync(); var flushTask = RequestBodyPipe.Writer.FlushAsync();
// It shouldn't be possible for the RequestBodyPipe to fill up an return an incomplete task if // It shouldn't be possible for the RequestBodyPipe to fill up an return an incomplete task if
// _inputFlowControl.Advance() didn't throw. // _inputFlowControl.Advance() didn't throw.
Debug.Assert(flushTask.IsCompleted); Debug.Assert(flushTask.IsCompletedSuccessfully);
} }
} }
} }

View File

@ -33,11 +33,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3
{ {
} }
protected override void OnDataRead(long bytesRead)
{
AddAndCheckConsumedBytes(bytesRead);
}
public static MessageBody For(Http3Stream context) public static MessageBody For(Http3Stream context)
{ {
return new Http3MessageBody(context); return new Http3MessageBody(context);
@ -50,8 +45,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3
public override void AdvanceTo(SequencePosition consumed, SequencePosition examined) public override void AdvanceTo(SequencePosition consumed, SequencePosition examined)
{ {
OnAdvance(_readResult, consumed, examined); var newlyExaminedBytes = TrackConsumedAndExaminedBytes(_readResult, consumed, examined);
_context.RequestBodyPipe.Reader.AdvanceTo(consumed, examined); _context.RequestBodyPipe.Reader.AdvanceTo(consumed, examined);
AddAndCheckObservedBytes(newlyExaminedBytes);
} }
public override bool TryRead(out ReadResult readResult) public override bool TryRead(out ReadResult readResult)