Skip to content

Commit 62f891e

Browse files
committed
Feedback
1 parent dca5a37 commit 62f891e

File tree

4 files changed

+63
-18
lines changed

4 files changed

+63
-18
lines changed

src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -355,11 +355,7 @@ private void WriteDataWrittenBeforeHeaders(ref BufferWriter<PipeWriter> writer)
355355

356356
_position = 0;
357357

358-
if (_currentSegmentOwner != null)
359-
{
360-
_currentSegmentOwner.Dispose();
361-
_currentSegmentOwner = null;
362-
}
358+
DisposeCurrentSegment();
363359
}
364360
}
365361

@@ -382,15 +378,19 @@ public void Dispose()
382378
}
383379
}
384380

385-
if (_currentSegmentOwner != null)
386-
{
387-
_currentSegmentOwner.Dispose();
388-
}
381+
DisposeCurrentSegment();
389382

390383
CompletePipe();
391384
}
392385
}
393386

387+
private void DisposeCurrentSegment()
388+
{
389+
_currentSegmentOwner?.Dispose();
390+
_currentSegmentOwner = null;
391+
_currentSegment = default;
392+
}
393+
394394
private void CompletePipe()
395395
{
396396
if (!_pipeWriterCompleted)
@@ -658,7 +658,7 @@ private readonly struct CompletedBuffer
658658
public Memory<byte> Buffer { get; }
659659
public int Length { get; }
660660

661-
public ReadOnlySpan<byte> Span => Buffer.Span;
661+
public ReadOnlySpan<byte> Span => Buffer.Span.Slice(0, Length);
662662

663663
public CompletedBuffer(IMemoryOwner<byte> owner, Memory<byte> buffer, int length)
664664
{

src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public abstract partial class HttpProtocol : IDefaultHttpContextContainer, IHttp
4949
// If we are writing with GetMemory/Advance before calling StartAsync, assume we can write and throw away contents if we can't.
5050
private bool _canWriteResponseBody = true;
5151
private bool _hasAdvanced;
52-
private bool _requireGetMemoryNextCall;
52+
private bool _isLeasedMemoryInvalid = true;
5353
private bool _autoChunk;
5454
protected Exception _applicationException;
5555
private BadHttpRequestException _requestRejectedException;
@@ -925,7 +925,7 @@ private void ProduceStart(bool appCompleted)
925925
return;
926926
}
927927

928-
_requireGetMemoryNextCall = true;
928+
_isLeasedMemoryInvalid = true;
929929

930930
_requestProcessingStatus = RequestProcessingStatus.HeadersCommitted;
931931

@@ -1283,7 +1283,7 @@ public void Advance(int bytes)
12831283
_hasAdvanced = true;
12841284
}
12851285

1286-
if (_requireGetMemoryNextCall)
1286+
if (_isLeasedMemoryInvalid)
12871287
{
12881288
throw new InvalidOperationException("Invalid ordering of calling StartAsync and Advance. " +
12891289
"Call StartAsync before calling GetMemory/GetSpan and Advance.");
@@ -1305,13 +1305,13 @@ public void Advance(int bytes)
13051305

13061306
public Memory<byte> GetMemory(int sizeHint = 0)
13071307
{
1308-
_requireGetMemoryNextCall = false;
1308+
_isLeasedMemoryInvalid = false;
13091309
return Output.GetMemory(sizeHint);
13101310
}
13111311

13121312
public Span<byte> GetSpan(int sizeHint = 0)
13131313
{
1314-
_requireGetMemoryNextCall = false;
1314+
_isLeasedMemoryInvalid = false;
13151315
return Output.GetSpan(sizeHint);
13161316
}
13171317

src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2625,6 +2625,52 @@ await ExpectAsync(Http2FrameType.SETTINGS,
26252625

26262626
[Fact]
26272627
public async Task GetMemoryAdvance_Works()
2628+
{
2629+
var headers = new[]
2630+
{
2631+
new KeyValuePair<string, string>(HeaderNames.Method, "GET"),
2632+
new KeyValuePair<string, string>(HeaderNames.Path, "/"),
2633+
new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
2634+
};
2635+
await InitializeConnectionAsync(httpContext =>
2636+
{
2637+
var response = httpContext.Response;
2638+
var memory = response.BodyPipe.GetMemory();
2639+
var fisrtPartOfResponse = Encoding.ASCII.GetBytes("hello,");
2640+
fisrtPartOfResponse.CopyTo(memory);
2641+
response.BodyPipe.Advance(6);
2642+
2643+
memory = response.BodyPipe.GetMemory();
2644+
var secondPartOfResponse = Encoding.ASCII.GetBytes(" world");
2645+
secondPartOfResponse.CopyTo(memory);
2646+
response.BodyPipe.Advance(6);
2647+
return Task.CompletedTask;
2648+
});
2649+
2650+
await StartStreamAsync(1, headers, endStream: true);
2651+
2652+
var headersFrame = await ExpectAsync(Http2FrameType.HEADERS,
2653+
withLength: 37,
2654+
withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS,
2655+
withStreamId: 1);
2656+
var dataFrame = await ExpectAsync(Http2FrameType.DATA,
2657+
withLength: 12,
2658+
withFlags: (byte)Http2DataFrameFlags.END_STREAM,
2659+
withStreamId: 1);
2660+
2661+
await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false);
2662+
2663+
_hpackDecoder.Decode(headersFrame.PayloadSequence, endHeaders: false, handler: this);
2664+
2665+
Assert.Equal(2, _decodedHeaders.Count);
2666+
Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase);
2667+
Assert.Equal("200", _decodedHeaders[HeaderNames.Status]);
2668+
2669+
Assert.True(_helloWorldBytes.AsSpan().SequenceEqual(dataFrame.PayloadSequence.ToArray()));
2670+
}
2671+
2672+
[Fact]
2673+
public async Task GetMemoryAdvance_WithStartAsync_Works()
26282674
{
26292675
var headers = new[]
26302676
{
@@ -2634,7 +2680,7 @@ public async Task GetMemoryAdvance_Works()
26342680
};
26352681
await InitializeConnectionAsync(async httpContext =>
26362682
{
2637-
await Task.CompletedTask;
2683+
await httpContext.Response.StartAsync();
26382684
var response = httpContext.Response;
26392685
var memory = response.BodyPipe.GetMemory();
26402686
var fisrtPartOfResponse = Encoding.ASCII.GetBytes("hello,");
@@ -3214,7 +3260,6 @@ await InitializeConnectionAsync(async httpContext =>
32143260
{
32153261
var response = httpContext.Response;
32163262
response.ContentLength = 54;
3217-
await Task.CompletedTask;
32183263
var memory = response.BodyPipe.GetMemory(4096);
32193264
var fisrtPartOfResponse = Encoding.ASCII.GetBytes("hello,");
32203265
fisrtPartOfResponse.CopyTo(memory);

src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3286,7 +3286,7 @@ public async Task AdvanceWithTooLargeOfAValueThrowInvalidOperationException(bool
32863286
{
32873287
await response.StartAsync();
32883288
}
3289-
Assert.Throws<ArgumentOutOfRangeException>(() => response.BodyPipe.Advance(1));
3289+
Assert.Throws<InvalidOperationException>(() => response.BodyPipe.Advance(1));
32903290
}, testContext))
32913291
{
32923292
using (var connection = server.CreateConnection())

0 commit comments

Comments
 (0)