Skip to content

Commit 6abc3c7

Browse files
halter73halter73
authored
Fix Kestrel HTTP/2 debug assert failure (#23062)
1 parent 7e6b5d5 commit 6abc3c7

File tree

6 files changed

+35
-49
lines changed

6 files changed

+35
-49
lines changed

‎src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public override void AdvanceTo(SequencePosition consumed)
4444

4545
public override void AdvanceTo(SequencePosition consumed, SequencePosition examined)
4646
{
47-
OnAdvance(_readResult, consumed, examined);
47+
TrackConsumedAndExaminedBytes(_readResult, consumed, examined);
4848
_requestBodyPipe.Reader.AdvanceTo(consumed, examined);
4949
}
5050

@@ -351,7 +351,7 @@ private void ParseChunkedPrefix(in ReadOnlySequence<byte> buffer, out SequencePo
351351
consumed = reader.Position;
352352
examined = reader.Position;
353353

354-
AddAndCheckConsumedBytes(reader.Consumed);
354+
AddAndCheckObservedBytes(reader.Consumed);
355355
_inputLength = chunkSize;
356356
_mode = Mode.Extension;
357357
return;
@@ -370,7 +370,7 @@ private void ParseChunkedPrefix(in ReadOnlySequence<byte> buffer, out SequencePo
370370
{
371371
consumed = reader.Position;
372372

373-
AddAndCheckConsumedBytes(reader.Consumed);
373+
AddAndCheckObservedBytes(reader.Consumed);
374374
_inputLength = chunkSize;
375375
_mode = chunkSize > 0 ? Mode.Data : Mode.Trailer;
376376
return;
@@ -398,7 +398,7 @@ private void ParseExtension(ReadOnlySequence<byte> buffer, out SequencePosition
398398
// End marker not found yet
399399
consumed = buffer.End;
400400
examined = buffer.End;
401-
AddAndCheckConsumedBytes(buffer.Length);
401+
AddAndCheckObservedBytes(buffer.Length);
402402
return;
403403
};
404404

@@ -410,7 +410,7 @@ private void ParseExtension(ReadOnlySequence<byte> buffer, out SequencePosition
410410
{
411411
consumed = extensionCursor;
412412
examined = buffer.End;
413-
AddAndCheckConsumedBytes(charsToByteCRExclusive);
413+
AddAndCheckObservedBytes(charsToByteCRExclusive);
414414
return;
415415
}
416416

@@ -424,14 +424,14 @@ private void ParseExtension(ReadOnlySequence<byte> buffer, out SequencePosition
424424

425425
consumed = suffixBuffer.End;
426426
examined = suffixBuffer.End;
427-
AddAndCheckConsumedBytes(charsToByteCRExclusive + 2);
427+
AddAndCheckObservedBytes(charsToByteCRExclusive + 2);
428428
}
429429
else
430430
{
431431
// Don't consume suffixSpan[1] in case it is also a \r.
432432
buffer = buffer.Slice(charsToByteCRExclusive + 1);
433433
consumed = extensionCursor;
434-
AddAndCheckConsumedBytes(charsToByteCRExclusive + 1);
434+
AddAndCheckObservedBytes(charsToByteCRExclusive + 1);
435435
}
436436
} while (_mode == Mode.Extension);
437437
}
@@ -445,7 +445,7 @@ private void ReadChunkedData(in ReadOnlySequence<byte> buffer, PipeWriter writab
445445
Copy(buffer.Slice(0, actual), writableBuffer);
446446

447447
_inputLength -= actual;
448-
AddAndCheckConsumedBytes(actual);
448+
AddAndCheckObservedBytes(actual);
449449

450450
if (_inputLength == 0)
451451
{
@@ -472,7 +472,7 @@ private void ParseChunkedSuffix(in ReadOnlySequence<byte> buffer, out SequencePo
472472
if (suffixSpan[0] == '\r' && suffixSpan[1] == '\n')
473473
{
474474
consumed = suffixBuffer.End;
475-
AddAndCheckConsumedBytes(2);
475+
AddAndCheckObservedBytes(2);
476476
_mode = Mode.Prefix;
477477
}
478478
else
@@ -500,7 +500,7 @@ private void ParseChunkedTrailer(in ReadOnlySequence<byte> buffer, out SequenceP
500500
if (trailerSpan[0] == '\r' && trailerSpan[1] == '\n')
501501
{
502502
consumed = trailerBuffer.End;
503-
AddAndCheckConsumedBytes(2);
503+
AddAndCheckObservedBytes(2);
504504
_mode = Mode.Complete;
505505
// No trailers
506506
_context.OnTrailersComplete();

‎src/Servers/Kestrel/Core/src/Internal/Http/Http1ContentLengthMessageBody.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ internal sealed class Http1ContentLengthMessageBody : Http1MessageBody
1313
{
1414
private ReadResult _readResult;
1515
private readonly long _contentLength;
16-
private long _inputLength;
16+
private long _unexaminedInputLength;
1717
private bool _readCompleted;
1818
private bool _isReading;
1919
private int _userCanceled;
@@ -24,7 +24,7 @@ public Http1ContentLengthMessageBody(bool keepAlive, long contentLength, Http1Co
2424
{
2525
RequestKeepAlive = keepAlive;
2626
_contentLength = contentLength;
27-
_inputLength = _contentLength;
27+
_unexaminedInputLength = _contentLength;
2828
}
2929

3030
public override ValueTask<ReadResult> ReadAsync(CancellationToken cancellationToken = default)
@@ -181,7 +181,7 @@ public override bool TryReadInternal(out ReadResult readResult)
181181
private long CreateReadResultFromConnectionReadResult()
182182
{
183183
var initialLength = _readResult.Buffer.Length;
184-
var maxLength = _inputLength + _examinedUnconsumedBytes;
184+
var maxLength = _unexaminedInputLength + _examinedUnconsumedBytes;
185185

186186
if (initialLength < maxLength)
187187
{
@@ -226,7 +226,7 @@ public override void AdvanceTo(SequencePosition consumed, SequencePosition exami
226226
return;
227227
}
228228

229-
_inputLength -= OnAdvance(_readResult, consumed, examined);
229+
_unexaminedInputLength -= TrackConsumedAndExaminedBytes(_readResult, consumed, examined);
230230
_context.Input.AdvanceTo(consumed, examined);
231231
}
232232

‎src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ internal abstract class MessageBody
1818
private readonly HttpProtocol _context;
1919

2020
private bool _send100Continue = true;
21-
private long _consumedBytes;
21+
private long _observedBytes;
2222
private bool _stopped;
2323

2424
protected bool _timingEnabled;
@@ -76,7 +76,7 @@ public virtual Task StopAsync()
7676
public virtual void Reset()
7777
{
7878
_send100Continue = true;
79-
_consumedBytes = 0;
79+
_observedBytes = 0;
8080
_stopped = false;
8181
_timingEnabled = false;
8282
_backpressure = false;
@@ -160,15 +160,11 @@ protected virtual void OnReadStarted()
160160
{
161161
}
162162

163-
protected virtual void OnDataRead(long bytesRead)
163+
protected void AddAndCheckObservedBytes(long observedBytes)
164164
{
165-
}
166-
167-
protected void AddAndCheckConsumedBytes(long consumedBytes)
168-
{
169-
_consumedBytes += consumedBytes;
165+
_observedBytes += observedBytes;
170166

171-
if (_consumedBytes > _context.MaxRequestBodySize)
167+
if (_observedBytes > _context.MaxRequestBodySize)
172168
{
173169
KestrelBadHttpRequestException.Throw(RequestRejectionReason.RequestBodyTooLarge);
174170
}
@@ -209,7 +205,7 @@ protected void StopTimingRead(long bytesInReadResult)
209205
}
210206
}
211207

212-
protected long OnAdvance(ReadResult readResult, SequencePosition consumed, SequencePosition examined)
208+
protected long TrackConsumedAndExaminedBytes(ReadResult readResult, SequencePosition consumed, SequencePosition examined)
213209
{
214210
// This code path is fairly hard to understand so let's break it down with an example
215211
// ReadAsync returns a ReadResult of length 50.
@@ -252,18 +248,11 @@ protected long OnAdvance(ReadResult readResult, SequencePosition consumed, Seque
252248
totalLength = readResult.Buffer.Length;
253249
}
254250

255-
var newlyExamined = examinedLength - _examinedUnconsumedBytes;
256-
257-
if (newlyExamined > 0)
258-
{
259-
OnDataRead(newlyExamined);
260-
_examinedUnconsumedBytes += newlyExamined;
261-
}
262-
263-
_examinedUnconsumedBytes -= consumedLength;
251+
var newlyExaminedBytes = examinedLength - _examinedUnconsumedBytes;
252+
_examinedUnconsumedBytes += newlyExaminedBytes - consumedLength;
264253
_alreadyTimedBytes = totalLength - consumedLength;
265254

266-
return newlyExamined;
255+
return newlyExaminedBytes;
267256
}
268257
}
269258
}

‎src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,6 @@ protected override void OnReadStarted()
3939
}
4040
}
4141

42-
protected override void OnDataRead(long bytesRead)
43-
{
44-
// The HTTP/2 flow control window cannot be larger than 2^31-1 which limits bytesRead.
45-
_context.OnDataRead((int)bytesRead);
46-
AddAndCheckConsumedBytes(bytesRead);
47-
}
48-
4942
public override void Reset()
5043
{
5144
base.Reset();
@@ -59,8 +52,14 @@ public override void AdvanceTo(SequencePosition consumed)
5952

6053
public override void AdvanceTo(SequencePosition consumed, SequencePosition examined)
6154
{
62-
OnAdvance(_readResult, consumed, examined);
55+
var newlyExaminedBytes = TrackConsumedAndExaminedBytes(_readResult, consumed, examined);
56+
57+
// Ensure we consume data from the RequestBodyPipe before sending WINDOW_UPDATES to the client.
6358
_context.RequestBodyPipe.Reader.AdvanceTo(consumed, examined);
59+
60+
// The HTTP/2 flow control window cannot be larger than 2^31-1 which limits bytesRead.
61+
_context.OnDataRead((int)newlyExaminedBytes);
62+
AddAndCheckObservedBytes(newlyExaminedBytes);
6463
}
6564

6665
public override bool TryRead(out ReadResult readResult)

‎src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,7 @@ public Task OnDataAsync(Http2Frame dataFrame, in ReadOnlySequence<byte> payload)
446446
var flushTask = RequestBodyPipe.Writer.FlushAsync();
447447
// It shouldn't be possible for the RequestBodyPipe to fill up an return an incomplete task if
448448
// _inputFlowControl.Advance() didn't throw.
449-
Debug.Assert(flushTask.IsCompleted);
449+
Debug.Assert(flushTask.IsCompletedSuccessfully);
450450
}
451451
}
452452
}

‎src/Servers/Kestrel/Core/src/Internal/Http3/Http3MessageBody.cs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,6 @@ protected override void OnReadStarted()
3333
{
3434
}
3535

36-
protected override void OnDataRead(long bytesRead)
37-
{
38-
AddAndCheckConsumedBytes(bytesRead);
39-
}
40-
4136
public static MessageBody For(Http3Stream context)
4237
{
4338
return new Http3MessageBody(context);
@@ -50,8 +45,11 @@ public override void AdvanceTo(SequencePosition consumed)
5045

5146
public override void AdvanceTo(SequencePosition consumed, SequencePosition examined)
5247
{
53-
OnAdvance(_readResult, consumed, examined);
48+
var newlyExaminedBytes = TrackConsumedAndExaminedBytes(_readResult, consumed, examined);
49+
5450
_context.RequestBodyPipe.Reader.AdvanceTo(consumed, examined);
51+
52+
AddAndCheckObservedBytes(newlyExaminedBytes);
5553
}
5654

5755
public override bool TryRead(out ReadResult readResult)

0 commit comments

Comments
 (0)