Skip to content

Commit 75f4159

Browse files
halter73halter73
authored
Accurately count only newly examined bytes (#12639)
- Ensure Kestrel count all bytes read using TryRead - Ensure Kestrel doesn't double count examined but not consumed bytes
1 parent 4debc9c commit 75f4159

File tree

4 files changed

+80
-62
lines changed

4 files changed

+80
-62
lines changed

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,8 @@ public override void AdvanceTo(SequencePosition consumed)
4444

4545
public override void AdvanceTo(SequencePosition consumed, SequencePosition examined)
4646
{
47-
var dataLength = _readResult.Buffer.Slice(_readResult.Buffer.Start, consumed).Length;
47+
OnAdvance(_readResult, consumed, examined);
4848
_requestBodyPipe.Reader.AdvanceTo(consumed, examined);
49-
OnDataRead(dataLength);
5049
}
5150

5251
public override bool TryRead(out ReadResult readResult)
@@ -63,6 +62,7 @@ public override bool TryReadInternal(out ReadResult readResult)
6362
var boolResult = _requestBodyPipe.Reader.TryRead(out _readResult);
6463

6564
readResult = _readResult;
65+
CountBytesRead(readResult.Buffer.Length);
6666

6767
if (_readResult.IsCompleted)
6868
{

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

+6-14
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ internal sealed class Http1ContentLengthMessageBody : Http1MessageBody
1717
private bool _readCompleted;
1818
private bool _isReading;
1919
private int _userCanceled;
20-
private long _totalExaminedInPreviousReadResult;
2120
private bool _finalAdvanceCalled;
2221

2322
public Http1ContentLengthMessageBody(bool keepAlive, long contentLength, Http1Connection context)
@@ -85,6 +84,8 @@ public override async ValueTask<ReadResult> ReadAsyncInternal(CancellationToken
8584
if (Interlocked.Exchange(ref _userCanceled, 0) == 1)
8685
{
8786
// Ignore the readResult if it wasn't by the user.
87+
CreateReadResultFromConnectionReadResult();
88+
8889
break;
8990
}
9091
else
@@ -156,6 +157,7 @@ public override bool TryReadInternal(out ReadResult readResult)
156157
CreateReadResultFromConnectionReadResult();
157158

158159
readResult = _readResult;
160+
CountBytesRead(readResult.Buffer.Length);
159161

160162
return true;
161163
}
@@ -174,11 +176,11 @@ public override Task ConsumeAsync()
174176

175177
private void CreateReadResultFromConnectionReadResult()
176178
{
177-
if (_readResult.Buffer.Length >= _inputLength + _totalExaminedInPreviousReadResult)
179+
if (_readResult.Buffer.Length >= _inputLength + _examinedUnconsumedBytes)
178180
{
179181
_readCompleted = true;
180182
_readResult = new ReadResult(
181-
_readResult.Buffer.Slice(0, _inputLength + _totalExaminedInPreviousReadResult),
183+
_readResult.Buffer.Slice(0, _inputLength + _examinedUnconsumedBytes),
182184
_readResult.IsCanceled && Interlocked.Exchange(ref _userCanceled, 0) == 1,
183185
_readCompleted);
184186
}
@@ -217,18 +219,8 @@ public override void AdvanceTo(SequencePosition consumed, SequencePosition exami
217219
return;
218220
}
219221

220-
var consumedLength = _readResult.Buffer.Slice(_readResult.Buffer.Start, consumed).Length;
221-
var examinedLength = consumedLength + _readResult.Buffer.Slice(consumed, examined).Length;
222-
222+
_inputLength -= OnAdvance(_readResult, consumed, examined);
223223
_context.Input.AdvanceTo(consumed, examined);
224-
225-
var newlyExamined = examinedLength - _totalExaminedInPreviousReadResult;
226-
227-
OnDataRead(newlyExamined);
228-
_totalExaminedInPreviousReadResult += newlyExamined;
229-
_inputLength -= newlyExamined;
230-
231-
_totalExaminedInPreviousReadResult -= consumedLength;
232224
}
233225

234226
protected override void OnReadStarting()

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

+70-3
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ internal abstract class MessageBody
2323
protected bool _timingEnabled;
2424
protected bool _backpressure;
2525
protected long _alreadyTimedBytes;
26+
protected long _examinedUnconsumedBytes;
2627

2728
protected MessageBody(HttpProtocol context)
2829
{
@@ -165,16 +166,82 @@ protected ValueTask<ReadResult> StartTimingReadAsync(ValueTask<ReadResult> readA
165166
return readAwaitable;
166167
}
167168

168-
protected void StopTimingRead(long bytesRead)
169+
protected void CountBytesRead(long bytesInReadResult)
169170
{
170-
_context.TimeoutControl.BytesRead(bytesRead - _alreadyTimedBytes);
171-
_alreadyTimedBytes = 0;
171+
var numFirstSeenBytes = bytesInReadResult - _alreadyTimedBytes;
172+
173+
if (numFirstSeenBytes > 0)
174+
{
175+
_context.TimeoutControl.BytesRead(numFirstSeenBytes);
176+
}
177+
}
178+
179+
protected void StopTimingRead(long bytesInReadResult)
180+
{
181+
CountBytesRead(bytesInReadResult);
172182

173183
if (_backpressure)
174184
{
175185
_backpressure = false;
176186
_context.TimeoutControl.StopTimingRead();
177187
}
178188
}
189+
190+
protected long OnAdvance(ReadResult readResult, SequencePosition consumed, SequencePosition examined)
191+
{
192+
// This code path is fairly hard to understand so let's break it down with an example
193+
// ReadAsync returns a ReadResult of length 50.
194+
// Advance(25, 40). The examined length would be 40 and consumed length would be 25.
195+
// _totalExaminedInPreviousReadResult starts at 0. newlyExamined is 40.
196+
// OnDataRead is called with length 40.
197+
// _totalExaminedInPreviousReadResult is now 40 - 25 = 15.
198+
199+
// The next call to ReadAsync returns 50 again
200+
// Advance(5, 5) is called
201+
// newlyExamined is 5 - 15, or -10.
202+
// Update _totalExaminedInPreviousReadResult to 10 as we consumed 5.
203+
204+
// The next call to ReadAsync returns 50 again
205+
// _totalExaminedInPreviousReadResult is 10
206+
// Advance(50, 50) is called
207+
// newlyExamined = 50 - 10 = 40
208+
// _totalExaminedInPreviousReadResult is now 50
209+
// _totalExaminedInPreviousReadResult is finally 0 after subtracting consumedLength.
210+
211+
long examinedLength, consumedLength, totalLength;
212+
213+
if (consumed.Equals(examined))
214+
{
215+
examinedLength = readResult.Buffer.Slice(readResult.Buffer.Start, examined).Length;
216+
consumedLength = examinedLength;
217+
}
218+
else
219+
{
220+
consumedLength = readResult.Buffer.Slice(readResult.Buffer.Start, consumed).Length;
221+
examinedLength = consumedLength + readResult.Buffer.Slice(consumed, examined).Length;
222+
}
223+
224+
if (examined.Equals(readResult.Buffer.End))
225+
{
226+
totalLength = examinedLength;
227+
}
228+
else
229+
{
230+
totalLength = readResult.Buffer.Length;
231+
}
232+
233+
var newlyExamined = examinedLength - _examinedUnconsumedBytes;
234+
235+
if (newlyExamined > 0)
236+
{
237+
OnDataRead(newlyExamined);
238+
_examinedUnconsumedBytes += newlyExamined;
239+
}
240+
241+
_examinedUnconsumedBytes -= consumedLength;
242+
_alreadyTimedBytes = totalLength - consumedLength;
243+
244+
return newlyExamined;
245+
}
179246
}
180247
}

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

+2-43
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ internal sealed class Http2MessageBody : MessageBody
1414
{
1515
private readonly Http2Stream _context;
1616
private ReadResult _readResult;
17-
private long _alreadyExaminedInNextReadResult;
1817

1918
private Http2MessageBody(Http2Stream context)
2019
: base(context)
@@ -64,55 +63,15 @@ public override void AdvanceTo(SequencePosition consumed)
6463

6564
public override void AdvanceTo(SequencePosition consumed, SequencePosition examined)
6665
{
67-
// This code path is fairly hard to understand so let's break it down with an example
68-
// ReadAsync returns a ReadResult of length 50.
69-
// Advance(25, 40). The examined length would be 40 and consumed length would be 25.
70-
// _totalExaminedInPreviousReadResult starts at 0. newlyExamined is 40.
71-
// OnDataRead is called with length 40.
72-
// _totalExaminedInPreviousReadResult is now 40 - 25 = 15.
73-
74-
// The next call to ReadAsync returns 50 again
75-
// Advance(5, 5) is called
76-
// newlyExamined is 5 - 15, or -10.
77-
// Update _totalExaminedInPreviousReadResult to 10 as we consumed 5.
78-
79-
// The next call to ReadAsync returns 50 again
80-
// _totalExaminedInPreviousReadResult is 10
81-
// Advance(50, 50) is called
82-
// newlyExamined = 50 - 10 = 40
83-
// _totalExaminedInPreviousReadResult is now 50
84-
// _totalExaminedInPreviousReadResult is finally 0 after subtracting consumedLength.
85-
86-
long examinedLength;
87-
long consumedLength;
88-
if (consumed.Equals(examined))
89-
{
90-
examinedLength = _readResult.Buffer.Slice(_readResult.Buffer.Start, examined).Length;
91-
consumedLength = examinedLength;
92-
}
93-
else
94-
{
95-
consumedLength = _readResult.Buffer.Slice(_readResult.Buffer.Start, consumed).Length;
96-
examinedLength = consumedLength + _readResult.Buffer.Slice(consumed, examined).Length;
97-
}
98-
66+
OnAdvance(_readResult, consumed, examined);
9967
_context.RequestBodyPipe.Reader.AdvanceTo(consumed, examined);
100-
101-
var newlyExamined = examinedLength - _alreadyExaminedInNextReadResult;
102-
103-
if (newlyExamined > 0)
104-
{
105-
OnDataRead(newlyExamined);
106-
_alreadyExaminedInNextReadResult += newlyExamined;
107-
}
108-
109-
_alreadyExaminedInNextReadResult -= consumedLength;
11068
}
11169

11270
public override bool TryRead(out ReadResult readResult)
11371
{
11472
var result = _context.RequestBodyPipe.Reader.TryRead(out readResult);
11573
_readResult = readResult;
74+
CountBytesRead(readResult.Buffer.Length);
11675

11776
return result;
11877
}

0 commit comments

Comments
 (0)