Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Commit 1d3090f

Browse files
TratcherTratcher
committed
Only reset the request body pipe for HTTP/1 #3006 
1 parent 97acb95 commit 1d3090f

File tree

3 files changed

+39
-28
lines changed

3 files changed

+39
-28
lines changed

‎src/Kestrel.Core/Internal/Http/Http1MessageBody.cs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,25 @@ public override Task StopAsync()
137137
return Task.CompletedTask;
138138
}
139139

140+
// PumpTask catches all Exceptions internally.
141+
if (_pumpTask.IsCompleted)
142+
{
143+
// At this point both the request body pipe reader and writer should be completed.
144+
_context.RequestBodyPipe.Reset();
145+
return Task.CompletedTask;
146+
}
147+
148+
return StopAsyncAwaited();
149+
}
150+
151+
private async Task StopAsyncAwaited()
152+
{
140153
_canceled = true;
141154
_context.Input.CancelPendingRead();
142-
return _pumpTask;
155+
await _pumpTask;
156+
157+
// At this point both the request body pipe reader and writer should be completed.
158+
_context.RequestBodyPipe.Reset();
143159
}
144160

145161
protected override Task OnConsumeAsync()

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -630,9 +630,6 @@ private async Task ProcessRequests<TContext>(IHttpApplication<TContext> applicat
630630

631631
// Wait for MessageBody.PumpAsync() to call RequestBodyPipe.Writer.Complete().
632632
await messageBody.StopAsync();
633-
634-
// At this point both the request body pipe reader and writer should be completed.
635-
RequestBodyPipe.Reset();
636633
}
637634
}
638635
}

‎test/Kestrel.Core.Tests/MessageBodyTests.cs

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ public async Task CanReadFromContentLength(HttpVersion httpVersion)
4747
count = stream.Read(buffer, 0, buffer.Length);
4848
Assert.Equal(0, count);
4949

50+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
5051
await body.StopAsync();
5152
}
5253
}
@@ -73,6 +74,7 @@ public async Task CanReadAsyncFromContentLength(HttpVersion httpVersion)
7374
count = await stream.ReadAsync(buffer, 0, buffer.Length);
7475
Assert.Equal(0, count);
7576

77+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
7678
await body.StopAsync();
7779
}
7880
}
@@ -101,6 +103,7 @@ public async Task CanReadFromChunkedEncoding()
101103
count = stream.Read(buffer, 0, buffer.Length);
102104
Assert.Equal(0, count);
103105

106+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
104107
await body.StopAsync();
105108
}
106109
}
@@ -127,6 +130,7 @@ public async Task CanReadAsyncFromChunkedEncoding()
127130
count = await stream.ReadAsync(buffer, 0, buffer.Length);
128131
Assert.Equal(0, count);
129132

133+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
130134
await body.StopAsync();
131135
}
132136
}
@@ -152,6 +156,7 @@ public async Task ReadExitsGivenIncompleteChunkedExtension()
152156
Assert.Equal(5, await readTask.DefaultTimeout());
153157
Assert.Equal(0, await stream.ReadAsync(buffer, 0, buffer.Length));
154158

159+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
155160
await body.StopAsync();
156161
}
157162
}
@@ -173,6 +178,7 @@ public async Task ReadThrowsGivenChunkPrefixGreaterThanMaxInt()
173178
Assert.IsType<OverflowException>(ex.InnerException);
174179
Assert.Equal(CoreStrings.BadRequest_BadChunkSizeData, ex.Message);
175180

181+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
176182
await body.StopAsync();
177183
}
178184
}
@@ -194,6 +200,7 @@ public async Task ReadThrowsGivenChunkPrefixGreaterThan8Bytes()
194200

195201
Assert.Equal(CoreStrings.BadRequest_BadChunkSizeData, ex.Message);
196202

203+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
197204
await body.StopAsync();
198205
}
199206
}
@@ -221,6 +228,7 @@ public async Task CanReadFromRemainingData(HttpVersion httpVersion)
221228

222229
input.Fin();
223230

231+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
224232
await body.StopAsync();
225233
}
226234
}
@@ -246,6 +254,7 @@ public async Task CanReadAsyncFromRemainingData(HttpVersion httpVersion)
246254

247255
input.Fin();
248256

257+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
249258
await body.StopAsync();
250259
}
251260
}
@@ -316,6 +325,7 @@ public async Task CanHandleLargeBlocks()
316325
Assert.Equal(8197, requestArray.Length);
317326
AssertASCII(largeInput + "Hello", new ArraySegment<byte>(requestArray, 0, requestArray.Length));
318327

328+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
319329
await body.StopAsync();
320330
}
321331
}
@@ -381,6 +391,7 @@ public async Task CopyToAsyncDoesNotCompletePipeReader()
381391

382392
Assert.Equal(0, await body.ReadAsync(new ArraySegment<byte>(new byte[1])));
383393

394+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
384395
await body.StopAsync();
385396
}
386397
}
@@ -398,6 +409,7 @@ public async Task ConsumeAsyncConsumesAllRemainingInput()
398409

399410
Assert.Equal(0, await body.ReadAsync(new ArraySegment<byte>(new byte[1])));
400411

412+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
401413
await body.StopAsync();
402414
}
403415
}
@@ -494,6 +506,7 @@ public async Task ConnectionUpgradeKeepAlive(string headerConnection)
494506

495507
input.Fin();
496508

509+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
497510
await body.StopAsync();
498511
}
499512
}
@@ -520,6 +533,7 @@ public async Task UpgradeConnectionAcceptsContentLengthZero()
520533

521534
input.Fin();
522535

536+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
523537
await body.StopAsync();
524538
}
525539
}
@@ -543,34 +557,11 @@ public async Task PumpAsyncDoesNotReturnAfterCancelingInput()
543557
input.Add("b");
544558
Assert.Equal(1, await stream.ReadAsync(new byte[1], 0, 1));
545559

560+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
546561
await body.StopAsync();
547562
}
548563
}
549564

550-
[Fact]
551-
public async Task StopAsyncPreventsFurtherDataConsumption()
552-
{
553-
using (var input = new TestInput())
554-
{
555-
var body = Http1MessageBody.For(HttpVersion.Http11, new HttpRequestHeaders { HeaderContentLength = "2" }, input.Http1Connection);
556-
var stream = new HttpRequestStream(Mock.Of<IHttpBodyControlFeature>());
557-
stream.StartAcceptingReads(body);
558-
559-
// Add some input and consume it to ensure PumpAsync is running
560-
input.Add("a");
561-
Assert.Equal(1, await stream.ReadAsync(new byte[1], 0, 1));
562-
563-
await body.StopAsync();
564-
565-
// Add some more data. Checking for cancellation and exiting the loop
566-
// should take priority over reading this data.
567-
input.Add("b");
568-
569-
// There shouldn't be any additional data available
570-
Assert.Equal(0, await stream.ReadAsync(new byte[1], 0, 1));
571-
}
572-
}
573-
574565
[Fact]
575566
public async Task ReadAsyncThrowsOnTimeout()
576567
{
@@ -592,6 +583,7 @@ public async Task ReadAsyncThrowsOnTimeout()
592583
var exception = await Assert.ThrowsAsync<BadHttpRequestException>(async () => await body.ReadAsync(new Memory<byte>(new byte[1])));
593584
Assert.Equal(StatusCodes.Status408RequestTimeout, exception.StatusCode);
594585

586+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
595587
await body.StopAsync();
596588
}
597589
}
@@ -622,6 +614,7 @@ public async Task ConsumeAsyncCompletesAndDoesNotThrowOnTimeout()
622614
It.IsAny<string>(),
623615
It.Is<BadHttpRequestException>(ex => ex.Reason == RequestRejectionReason.RequestBodyTimeout)));
624616

617+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
625618
await body.StopAsync();
626619
}
627620
}
@@ -650,6 +643,7 @@ public async Task CopyToAsyncThrowsOnTimeout()
650643
Assert.Equal(StatusCodes.Status408RequestTimeout, exception.StatusCode);
651644
}
652645

646+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
653647
await body.StopAsync();
654648
}
655649
}
@@ -676,6 +670,7 @@ public async Task LogsWhenStartsReadingRequestBody()
676670

677671
input.Fin();
678672

673+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
679674
await body.StopAsync();
680675
}
681676
}
@@ -706,6 +701,7 @@ public async Task LogsWhenStopsReadingRequestBody()
706701

707702
await logEvent.Task.DefaultTimeout();
708703

704+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
709705
await body.StopAsync();
710706
}
711707
}
@@ -765,6 +761,7 @@ public async Task OnlyEnforcesRequestBodyTimeoutAfterSending100Continue()
765761
input.Add("a");
766762
await readTask;
767763

764+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
768765
await body.StopAsync();
769766
}
770767
}
@@ -797,6 +794,7 @@ public async Task DoesNotEnforceRequestBodyTimeoutOnUpgradeRequests()
797794
mockTimeoutControl.Verify(timeoutControl => timeoutControl.PauseTimingReads(), Times.Never);
798795
mockTimeoutControl.Verify(timeoutControl => timeoutControl.ResumeTimingReads(), Times.Never);
799796

797+
input.Http1Connection.RequestBodyPipe.Reader.Complete();
800798
await body.StopAsync();
801799
}
802800
}

0 commit comments

Comments
 (0)