huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Don't call complete outside of main request loop (#13728)

This commit is contained in:
Justin Kotalik 2019-09-06 23:01:19 -07:00 committed by GitHub
parent c3fa16970d
commit 1db76280ae
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 78 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs
View File

@ -1009,6 +1009,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
return Task.CompletedTask;
}
_isLeasedMemoryInvalid = true;
if (_requestRejectedException != null || _applicationException != null)
{
if (HasResponseStarted)
@ -1339,8 +1341,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
if (_isLeasedMemoryInvalid)
{
throw new InvalidOperationException("Invalid ordering of calling StartAsync and Advance. " +
"Call StartAsync before calling GetMemory/GetSpan and Advance.");
throw new InvalidOperationException("Invalid ordering of calling StartAsync or CompleteAsync and Advance.");
}
if (_canWriteResponseBody)

46
src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs
View File

@ -80,6 +80,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
Stop();
// Make sure the writing side is completed.
_pipeWriter.Complete();
if (_fakeMemoryOwner != null)
{
_fakeMemoryOwner.Dispose();
@ -104,7 +107,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
lock (_dataWriterLock)
{
ThrowIfSuffixSent();
ThrowIfSuffixSentOrDisposed();
if (_completed)
{
@ -130,7 +133,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
{
lock (_dataWriterLock)
{
ThrowIfSuffixSent();
ThrowIfSuffixSentOrDisposed();
if (_completed)
{
@ -185,7 +188,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
lock (_dataWriterLock)
{
ThrowIfSuffixSent();
ThrowIfSuffixSentOrDisposed();
// This length check is important because we don't want to set _startedWritingDataFrames unless a data
// frame will actually be written causing the headers to be flushed.
@ -233,7 +236,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
{
lock (_dataWriterLock)
{
ThrowIfSuffixSent();
ThrowIfSuffixSentOrDisposed();
if (_completed)
{
@ -250,7 +253,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
{
lock (_dataWriterLock)
{
ThrowIfSuffixSent();
ThrowIfSuffixSentOrDisposed();
if (_completed)
{
@ -265,7 +268,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
{
lock (_dataWriterLock)
{
ThrowIfSuffixSent();
ThrowIfSuffixSentOrDisposed();
if (_completed)
{
@ -298,7 +301,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
lock (_dataWriterLock)
{
ThrowIfSuffixSent();
ThrowIfSuffixSentOrDisposed();
// This length check is important because we don't want to set _startedWritingDataFrames unless a data
// frame will actually be written causing the headers to be flushed.
@ -345,10 +348,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
_completed = true;
// Complete with an exception to prevent an end of stream data frame from being sent without an
// explicit call to WriteStreamSuffixAsync. ConnectionAbortedExceptions are swallowed, so the
// message doesn't matter
_pipeWriter.Complete(new OperationCanceledException());
_pipeReader.CancelPendingRead();
_frameWriter.AbortPendingStreamDataWrites(_flowControl);
}
@ -369,7 +369,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
{
readResult = await _pipeReader.ReadAsync();
if (readResult.IsCompleted && _stream.ResponseTrailers?.Count > 0)
if (readResult.IsCanceled)
{
// Response body is aborted, break and complete reader.
break;
}
else if (readResult.IsCompleted && _stream.ResponseTrailers?.Count > 0)
{
// Output is ending and there are trailers to write
// Write any remaining content then write trailers
@ -405,10 +410,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
_pipeReader.AdvanceTo(readResult.Buffer.End);
} while (!readResult.IsCompleted);
}
catch (OperationCanceledException)
{
// Writes should not throw for aborted streams/connections.
}
catch (Exception ex)
{
_log.LogCritical(ex, nameof(Http2OutputProducer) + "." + nameof(ProcessDataWrites) + " observed an unexpected exception.");
@ -435,12 +436,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
}
[StackTraceHidden]
private void ThrowIfSuffixSent()
private void ThrowIfSuffixSentOrDisposed()
{
if (_suffixSent)
{
ThrowSuffixSent();
}
if (_disposed)
{
ThrowDisposed();
}
}
[StackTraceHidden]
@ -449,6 +455,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
throw new InvalidOperationException("Writing is not allowed after writer was completed.");
}
[StackTraceHidden]
private static void ThrowDisposed()
{
throw new InvalidOperationException("Cannot write to response after the request has completed.");
}
private static Pipe CreateDataPipe(MemoryPool<byte> pool)
=> new Pipe(new PipeOptions
(

46
src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs
View File

@ -3737,6 +3737,52 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
Assert.Equal("Hello World", Encoding.UTF8.GetString(bodyFrame.Payload.Span));
}
[Fact]
public async Task CompleteAsync_AdvanceAfterComplete_AdvanceThrows()
{
var tcs = new TaskCompletionSource<object>();
var headers = new[]
{
new KeyValuePair<string, string>(HeaderNames.Method, "GET"),
new KeyValuePair<string, string>(HeaderNames.Path, "/"),
new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
};
await InitializeConnectionAsync(async context =>
{
var memory = context.Response.BodyWriter.GetMemory(12);
await context.Response.CompleteAsync();
try
{
context.Response.BodyWriter.Advance(memory.Length);
}
catch (InvalidOperationException)
{
tcs.SetResult(null);
return;
}
Assert.True(false);
});
await StartStreamAsync(1, headers, endStream: true);
var headersFrame = await ExpectAsync(Http2FrameType.HEADERS,
withLength: 55,
withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM),
withStreamId: 1);
await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false);
_hpackDecoder.Decode(headersFrame.PayloadSequence, endHeaders: false, handler: this);
Assert.Equal(3, _decodedHeaders.Count);
Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase);
Assert.Equal("200", _decodedHeaders[HeaderNames.Status]);
Assert.Equal("0", _decodedHeaders[HeaderNames.ContentLength]);
await tcs.Task.DefaultTimeout();
}
[Fact]
public async Task CompleteAsync_AfterPipeWrite_WithTrailers_SendsBodyAndTrailersWithEndStream()
{