From d3f2ca9c9aafc8ce1c76c688eaa82be2f9a69a51 Mon Sep 17 00:00:00 2001 From: "Chris Ross (ASP.NET)" Date: Tue, 18 Sep 2018 16:27:03 -0700 Subject: [PATCH 1/6] Do not inherit socket handles #2789 --- .../Internal/NativeMethods.cs | 31 ++++++++++ .../SocketTransport.cs | 1 + .../HandleInheritanceTests.cs | 60 +++++++++++++++++++ 3 files changed, 92 insertions(+) create mode 100644 src/Kestrel.Transport.Sockets/Internal/NativeMethods.cs create mode 100644 test/Kestrel.Transport.FunctionalTests/HandleInheritanceTests.cs diff --git a/src/Kestrel.Transport.Sockets/Internal/NativeMethods.cs b/src/Kestrel.Transport.Sockets/Internal/NativeMethods.cs new file mode 100644 index 0000000000..a77efc2c35 --- /dev/null +++ b/src/Kestrel.Transport.Sockets/Internal/NativeMethods.cs @@ -0,0 +1,31 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Net.Sockets; +using System.Runtime.InteropServices; + +namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal +{ + internal static class NativeMethods + { + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool SetHandleInformation(IntPtr hObject, HANDLE_FLAGS dwMask, HANDLE_FLAGS dwFlags); + + [Flags] + private enum HANDLE_FLAGS : uint + { + None = 0, + INHERIT = 1, + PROTECT_FROM_CLOSE = 2 + } + + internal static void DisableHandleInheritance(Socket socket) + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + SetHandleInformation(socket.Handle, HANDLE_FLAGS.INHERIT, 0); + } + } + } +} diff --git a/src/Kestrel.Transport.Sockets/SocketTransport.cs b/src/Kestrel.Transport.Sockets/SocketTransport.cs index 730c36c2d1..bc61f1a97f 100644 --- a/src/Kestrel.Transport.Sockets/SocketTransport.cs +++ b/src/Kestrel.Transport.Sockets/SocketTransport.cs @@ -80,6 +80,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets IPEndPoint endPoint = _endPointInformation.IPEndPoint; var listenSocket = new Socket(endPoint.AddressFamily, SocketType.Stream, ProtocolType.Tcp); + NativeMethods.DisableHandleInheritance(listenSocket); // Kestrel expects IPv6Any to bind to both IPv6 and IPv4 if (endPoint.Address == IPAddress.IPv6Any) diff --git a/test/Kestrel.Transport.FunctionalTests/HandleInheritanceTests.cs b/test/Kestrel.Transport.FunctionalTests/HandleInheritanceTests.cs new file mode 100644 index 0000000000..cbbcb8b454 --- /dev/null +++ b/test/Kestrel.Transport.FunctionalTests/HandleInheritanceTests.cs @@ -0,0 +1,60 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Diagnostics; +using System.Net.Sockets; +using System.Runtime.InteropServices; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Testing; +using Microsoft.AspNetCore.Testing.xunit; +using Xunit; + +namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests +{ + public class HandleInheritanceTests : TestApplicationErrorLoggerLoggedTest + { + [ConditionalFact] + [OSSkipCondition(OperatingSystems.MacOSX, SkipReason = "No fix available for Mac https://github.com/aspnet/KestrelHttpServer/pull/2944#issuecomment-426397600")] + public async Task SpawnChildProcess_DoesNotInheritListenHandle() + { + var hostBuilder = TransportSelector.GetWebHostBuilder() + .UseKestrel() + .ConfigureServices(AddTestLogging) + .UseUrls("http://127.0.0.1:0") + .Configure(app => + { + app.Run(context => + { + return context.Response.WriteAsync("Hello World"); + }); + }); + + using (var host = hostBuilder.Build()) + { + await host.StartAsync(); + + var processInfo = new ProcessStartInfo + { + FileName = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "cmd.exe" : "vi", + CreateNoWindow = true, + }; + using (var process = Process.Start(processInfo)) + { + var port = host.GetPort(); + await host.StopAsync(); + + // We should not be able to connect if the handle was correctly closed and not inherited by the child process. + using (var client = new TcpClient()) + { + await Assert.ThrowsAnyAsync(() => client.ConnectAsync("127.0.0.1", port)); + } + + process.Kill(); + } + } + } + } +} From 23a4e112613bc570ff859f9f70e3477a71565a1a Mon Sep 17 00:00:00 2001 From: "Chris Ross (ASP.NET)" Date: Thu, 18 Oct 2018 14:41:31 -0700 Subject: [PATCH 2/6] Send RST for canceled HTTP/2 writes #3007 --- .../Internal/Http2/Http2OutputProducer.cs | 5 +- .../Internal/Http2/Http2Stream.cs | 2 +- .../Http2/Http2StreamTests.cs | 85 +++++++++++++++++++ 3 files changed, 89 insertions(+), 3 deletions(-) diff --git a/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs b/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs index 0bdf5186a8..fa31210932 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs @@ -74,10 +74,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } } - // Review: This is called when a CancellationToken fires mid-write. In HTTP/1.x, this aborts the entire connection. - // Should we do that here? + // This is called when a CancellationToken fires mid-write. In HTTP/1.x, this aborts the entire connection. + // For HTTP/2 we abort the stream. void IHttpOutputAborter.Abort(ConnectionAbortedException abortReason) { + _stream.ResetAndAbort(abortReason, Http2ErrorCode.INTERNAL_ERROR); Dispose(); } diff --git a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs index 25623576ed..06f51fd096 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs @@ -427,7 +427,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 ResetAndAbort(abortReason, Http2ErrorCode.INTERNAL_ERROR); } - private void ResetAndAbort(ConnectionAbortedException abortReason, Http2ErrorCode error) + internal void ResetAndAbort(ConnectionAbortedException abortReason, Http2ErrorCode error) { // Future incoming frames will drain for a default grace period to avoid destabilizing the connection. var states = ApplyCompletionFlag(StreamCompletionFlags.Aborted); diff --git a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs index 1b94337efc..8375561f9b 100644 --- a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -1998,5 +1998,90 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await WaitForConnectionErrorAsync(ignoreNonGoAwayFrames: false, expectedLastStreamId: 1, Http2ErrorCode.INTERNAL_ERROR, CoreStrings.HPackErrorNotEnoughBuffer); } + + [Fact] + public async Task WriteAsync_PreCancelledCancellationToken_DoesNotAbort() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(async context => + { + // The cancellation is checked at the start of WriteAsync and no application state is changed. + await Assert.ThrowsAsync(() => context.Response.WriteAsync("hello,", new CancellationToken(true))); + Assert.False(context.Response.HasStarted); + }); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 55, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 0, + withFlags: (byte)Http2DataFrameFlags.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]); + } + + [Fact] + public async Task WriteAsync_CancellationTokenTriggeredDueToFlowControl_SendRST() + { + var cts = new CancellationTokenSource(); + var writeStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(async context => + { + await context.Response.Body.FlushAsync(); // https://github.com/aspnet/KestrelHttpServer/issues/3031 + var writeTask = context.Response.WriteAsync("hello,", cts.Token); + writeStarted.SetResult(0); + await Assert.ThrowsAsync(() => writeTask); + }); + + _clientSettings.InitialWindowSize = 0; + await SendSettingsAsync(); + await ExpectAsync(Http2FrameType.SETTINGS, + withLength: 0, + withFlags: (byte)Http2SettingsFrameFlags.ACK, + withStreamId: 0); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await writeStarted.Task; + + cts.Cancel(); + + await WaitForStreamErrorAsync(1, Http2ErrorCode.INTERNAL_ERROR, null); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); + + _hpackDecoder.Decode(headersFrame.PayloadSequence, endHeaders: false, handler: this); + + Assert.Equal(2, _decodedHeaders.Count); + Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); + Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); + } } } \ No newline at end of file From 277a5502fd44d565d8086480b829d1c9995a78ab Mon Sep 17 00:00:00 2001 From: "Chris Ross (ASP.NET)" Date: Fri, 19 Oct 2018 10:14:19 -0700 Subject: [PATCH 3/6] Flush response headers #3031 --- .../Internal/Http2/Http2FrameWriter.cs | 17 ++----- .../Internal/Http2/Http2OutputProducer.cs | 18 +------ .../Http2/Http2StreamTests.cs | 49 +++++++++++++++++++ 3 files changed, 54 insertions(+), 30 deletions(-) diff --git a/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs b/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs index abaea59245..1242c93131 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs @@ -104,19 +104,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } } - public Task FlushAsync(IHttpOutputAborter outputAborter, CancellationToken cancellationToken) - { - lock (_writeLock) - { - if (_completed) - { - return Task.CompletedTask; - } - - return _flusher.FlushAsync(outputAborter, cancellationToken); - } - } - public Task Write100ContinueAsync(int streamId) { lock (_writeLock) @@ -171,9 +158,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 throw new InvalidOperationException(hex.Message, hex); // Report the error to the user if this was the first write. } } + + _ = _flusher.FlushAsync(); } - public Task WriteResponseTrailers(int streamId, HttpResponseTrailers headers) + public Task WriteResponseTrailersAsync(int streamId, HttpResponseTrailers headers) { lock (_writeLock) { diff --git a/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs b/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs index fa31210932..83e517d05f 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs @@ -28,7 +28,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 private readonly object _dataWriterLock = new object(); private readonly Pipe _dataPipe; private readonly Task _dataWriteProcessingTask; - private bool _startedWritingDataFrames; private bool _completed; private bool _disposed; @@ -101,18 +100,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 return Task.CompletedTask; } - if (_startedWritingDataFrames) - { - // If there's already been response data written to the stream, just wait for that. Any header - // should be in front of the data frames in the connection pipe. Trailers could change things. - return _flusher.FlushAsync(this, cancellationToken); - } - else - { - // Flushing the connection pipe ensures headers already in the pipe are flushed even if no data - // frames have been written. - return _frameWriter.FlushAsync(this, cancellationToken); - } + return _flusher.FlushAsync(this, cancellationToken); } } @@ -160,8 +148,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 return Task.CompletedTask; } - _startedWritingDataFrames = true; - _dataPipe.Writer.Write(data); return _flusher.FlushAsync(this, cancellationToken); } @@ -212,7 +198,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 await _frameWriter.WriteDataAsync(_streamId, _flowControl, _stream.MinResponseDataRate, readResult.Buffer, endStream: false); } - await _frameWriter.WriteResponseTrailers(_streamId, _stream.Trailers); + await _frameWriter.WriteResponseTrailersAsync(_streamId, _stream.Trailers); } else { diff --git a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs index 8375561f9b..1cf437ecab 100644 --- a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -2083,5 +2083,54 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); } + + [Fact] + public async Task ResponseHeaders_NotBlockedByFlowControl() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(context => + { + return context.Response.WriteAsync("hello world"); + }); + + _clientSettings.InitialWindowSize = 0; + await SendSettingsAsync(); + await ExpectAsync(Http2FrameType.SETTINGS, + withLength: 0, + withFlags: (byte)Http2SettingsFrameFlags.ACK, + withStreamId: 0); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await SendWindowUpdateAsync(1, 11); + + await ExpectAsync(Http2FrameType.DATA, + withLength: 11, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.DATA, + withLength: 0, + withFlags: (byte)Http2DataFrameFlags.END_STREAM, + withStreamId: 1); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); + + _hpackDecoder.Decode(headersFrame.PayloadSequence, endHeaders: false, handler: this); + + Assert.Equal(2, _decodedHeaders.Count); + Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); + Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); + } } } \ No newline at end of file From ad3cba550955446cfa30db484c4160f569d16961 Mon Sep 17 00:00:00 2001 From: "Chris Ross (ASP.NET)" Date: Fri, 19 Oct 2018 18:05:56 -0700 Subject: [PATCH 4/6] Disable inheritance test on linux #3034 --- .../HandleInheritanceTests.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/Kestrel.Transport.FunctionalTests/HandleInheritanceTests.cs b/test/Kestrel.Transport.FunctionalTests/HandleInheritanceTests.cs index cbbcb8b454..cd75141197 100644 --- a/test/Kestrel.Transport.FunctionalTests/HandleInheritanceTests.cs +++ b/test/Kestrel.Transport.FunctionalTests/HandleInheritanceTests.cs @@ -17,7 +17,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests public class HandleInheritanceTests : TestApplicationErrorLoggerLoggedTest { [ConditionalFact] - [OSSkipCondition(OperatingSystems.MacOSX, SkipReason = "No fix available for Mac https://github.com/aspnet/KestrelHttpServer/pull/2944#issuecomment-426397600")] + [OSSkipCondition(OperatingSystems.Linux, SkipReason = "Fixed in 3.0 https://github.com/aspnet/KestrelHttpServer/issues/3040")] + [OSSkipCondition(OperatingSystems.MacOSX, SkipReason = "Fixed in 3.0 https://github.com/aspnet/KestrelHttpServer/issues/3040")] public async Task SpawnChildProcess_DoesNotInheritListenHandle() { var hostBuilder = TransportSelector.GetWebHostBuilder() @@ -38,7 +39,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests var processInfo = new ProcessStartInfo { - FileName = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "cmd.exe" : "vi", + FileName = "cmd.exe", CreateNoWindow = true, }; using (var process = Process.Start(processInfo)) From 2a610ee1b8d00eed8afe77252c966ecc1e695272 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Mon, 22 Oct 2018 11:38:53 -0700 Subject: [PATCH 5/6] Reduce flakiness of LargeUpload tests (#3036) * Move h2spec tests to own project --- KestrelHttpServer.sln | 15 ++++++ .../H2SpecCommands.cs | 2 +- .../H2SpecTests.cs | 30 ++++++----- .../Kestrel.H2Spec.FunctionalTests.csproj | 28 ++++++++++ .../MaxRequestBufferSizeTests.cs | 53 ++++++++++++++++--- .../RequestTests.cs | 49 +++++++++++++---- ...rel.Transport.Libuv.FunctionalTests.csproj | 1 - .../DiagnosticMemoryPoolFactory.cs | 0 8 files changed, 145 insertions(+), 33 deletions(-) rename test/{shared/TransportTestHelpers => Kestrel.H2Spec.FunctionalTests}/H2SpecCommands.cs (99%) rename test/{Kestrel.Transport.FunctionalTests/Http2 => Kestrel.H2Spec.FunctionalTests}/H2SpecTests.cs (84%) create mode 100644 test/Kestrel.H2Spec.FunctionalTests/Kestrel.H2Spec.FunctionalTests.csproj rename test/shared/{ => TransportTestHelpers}/DiagnosticMemoryPoolFactory.cs (100%) diff --git a/KestrelHttpServer.sln b/KestrelHttpServer.sln index 9bc9681d39..c9679e5676 100644 --- a/KestrelHttpServer.sln +++ b/KestrelHttpServer.sln @@ -134,6 +134,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Kestrel.Transport.Sockets.B EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Kestrel.Transport.Libuv.BindTests", "test\Kestrel.Transport.Libuv.BindTests\Kestrel.Transport.Libuv.BindTests.csproj", "{FB9C6B61-0A7B-4FFA-B772-A754316B262E}" EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Kestrel.H2Spec.FunctionalTests", "test\Kestrel.H2Spec.FunctionalTests\Kestrel.H2Spec.FunctionalTests.csproj", "{C4123E55-5760-4557-B89B-39E1258FD7F9}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -420,6 +422,18 @@ Global {FB9C6B61-0A7B-4FFA-B772-A754316B262E}.Release|x64.Build.0 = Release|Any CPU {FB9C6B61-0A7B-4FFA-B772-A754316B262E}.Release|x86.ActiveCfg = Release|Any CPU {FB9C6B61-0A7B-4FFA-B772-A754316B262E}.Release|x86.Build.0 = Release|Any CPU + {C4123E55-5760-4557-B89B-39E1258FD7F9}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {C4123E55-5760-4557-B89B-39E1258FD7F9}.Debug|Any CPU.Build.0 = Debug|Any CPU + {C4123E55-5760-4557-B89B-39E1258FD7F9}.Debug|x64.ActiveCfg = Debug|Any CPU + {C4123E55-5760-4557-B89B-39E1258FD7F9}.Debug|x64.Build.0 = Debug|Any CPU + {C4123E55-5760-4557-B89B-39E1258FD7F9}.Debug|x86.ActiveCfg = Debug|Any CPU + {C4123E55-5760-4557-B89B-39E1258FD7F9}.Debug|x86.Build.0 = Debug|Any CPU + {C4123E55-5760-4557-B89B-39E1258FD7F9}.Release|Any CPU.ActiveCfg = Release|Any CPU + {C4123E55-5760-4557-B89B-39E1258FD7F9}.Release|Any CPU.Build.0 = Release|Any CPU + {C4123E55-5760-4557-B89B-39E1258FD7F9}.Release|x64.ActiveCfg = Release|Any CPU + {C4123E55-5760-4557-B89B-39E1258FD7F9}.Release|x64.Build.0 = Release|Any CPU + {C4123E55-5760-4557-B89B-39E1258FD7F9}.Release|x86.ActiveCfg = Release|Any CPU + {C4123E55-5760-4557-B89B-39E1258FD7F9}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -450,6 +464,7 @@ Global {B5422347-E919-431D-9EF2-C352FFE4D6C1} = {D3273454-EA07-41D2-BF0B-FCC3675C2483} {9254C3EB-196B-402F-A059-34FEA6140500} = {D3273454-EA07-41D2-BF0B-FCC3675C2483} {FB9C6B61-0A7B-4FFA-B772-A754316B262E} = {D3273454-EA07-41D2-BF0B-FCC3675C2483} + {C4123E55-5760-4557-B89B-39E1258FD7F9} = {D3273454-EA07-41D2-BF0B-FCC3675C2483} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {2D10D020-6770-47CA-BB8D-2C23FE3AE071} diff --git a/test/shared/TransportTestHelpers/H2SpecCommands.cs b/test/Kestrel.H2Spec.FunctionalTests/H2SpecCommands.cs similarity index 99% rename from test/shared/TransportTestHelpers/H2SpecCommands.cs rename to test/Kestrel.H2Spec.FunctionalTests/H2SpecCommands.cs index bf33c519a8..09b57d2990 100644 --- a/test/shared/TransportTestHelpers/H2SpecCommands.cs +++ b/test/Kestrel.H2Spec.FunctionalTests/H2SpecCommands.cs @@ -10,7 +10,7 @@ using System.Runtime.InteropServices; using System.Xml; using Microsoft.Extensions.Logging; -namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests +namespace H2Spec.FunctionalTests { public static class H2SpecCommands { diff --git a/test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs b/test/Kestrel.H2Spec.FunctionalTests/H2SpecTests.cs similarity index 84% rename from test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs rename to test/Kestrel.H2Spec.FunctionalTests/H2SpecTests.cs index 4d9822cae5..45e66cbf47 100644 --- a/test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs +++ b/test/Kestrel.H2Spec.FunctionalTests/H2SpecTests.cs @@ -1,33 +1,35 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -#if NETCOREAPP2_2 - +using System; using System.IO; using System.Linq; using System.Net; using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Hosting.Server.Features; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel.Core; -using Microsoft.AspNetCore.Testing; using Microsoft.AspNetCore.Testing.xunit; +using Microsoft.Extensions.Logging.Testing; using Xunit; using Xunit.Abstractions; -namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2 +namespace H2Spec.FunctionalTests { [OSSkipCondition(OperatingSystems.MacOSX, SkipReason = "Missing SslStream ALPN support: https://github.com/dotnet/corefx/issues/30492")] [MinimumOSVersion(OperatingSystems.Windows, WindowsVersions.Win81, SkipReason = "Missing Windows ALPN support: https://en.wikipedia.org/wiki/Application-Layer_Protocol_Negotiation#Support")] - public class H2SpecTests : TestApplicationErrorLoggerLoggedTest + public class H2SpecTests : LoggedTest { + private static readonly string _testCertPath = Path.Combine(Directory.GetCurrentDirectory(), "shared", "TestCertificates", "testCert.pfx"); + [ConditionalTheory] [MemberData(nameof(H2SpecTestCases))] public async Task RunIndividualTestCase(H2SpecTestCase testCase) { - var hostBuilder = TransportSelector.GetWebHostBuilder() + var hostBuilder = new WebHostBuilder() .UseKestrel(options => { options.Listen(IPAddress.Loopback, 0, listenOptions => @@ -35,7 +37,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2 listenOptions.Protocols = HttpProtocols.Http2; if (testCase.Https) { - listenOptions.UseHttps(TestResources.TestCertificatePath, "testPassword"); + listenOptions.UseHttps(_testCertPath, "testPassword"); } }); }) @@ -46,7 +48,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2 { await host.StartAsync(); - H2SpecCommands.RunTest(testCase.Id, host.GetPort(), testCase.Https, Logger); + H2SpecCommands.RunTest(testCase.Id, GetPort(host), testCase.Https, Logger); } } @@ -129,9 +131,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2 await context.Response.WriteAsync("Hello World"); }); } + + private static int GetPort(IWebHost host) + { + return host.ServerFeatures.Get().Addresses + .Select(a => new Uri(a)) + .First() + .Port; + } } } -#elif NET461 // HTTP/2 is not supported -#else -#error TFMs need updating -#endif diff --git a/test/Kestrel.H2Spec.FunctionalTests/Kestrel.H2Spec.FunctionalTests.csproj b/test/Kestrel.H2Spec.FunctionalTests/Kestrel.H2Spec.FunctionalTests.csproj new file mode 100644 index 0000000000..5b45607b50 --- /dev/null +++ b/test/Kestrel.H2Spec.FunctionalTests/Kestrel.H2Spec.FunctionalTests.csproj @@ -0,0 +1,28 @@ + + + + H2Spec.FunctionalTests + H2Spec.FunctionalTests + + + $(DeveloperBuildTestTfms) + true + H2Spec.FunctionalTests + + + + + + + + + + + + + + + + + + diff --git a/test/Kestrel.Transport.FunctionalTests/MaxRequestBufferSizeTests.cs b/test/Kestrel.Transport.FunctionalTests/MaxRequestBufferSizeTests.cs index 5090e6d530..ad087574b2 100644 --- a/test/Kestrel.Transport.FunctionalTests/MaxRequestBufferSizeTests.cs +++ b/test/Kestrel.Transport.FunctionalTests/MaxRequestBufferSizeTests.cs @@ -51,7 +51,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests private static readonly string[] _requestLines = new[] { - "POST / HTTP/1.0\r\n", + "POST / HTTP/1.1\r\n", + "Host: \r\n", $"Content-Length: {_dataLength}\r\n", "\r\n" }; @@ -60,11 +61,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { get { + var totalHeaderSize = 0; + + for (var i = 1; i < _requestLines.Length - 1; i++) + { + totalHeaderSize += _requestLines[i].Length; + } + var maxRequestBufferSizeValues = new Tuple[] { - // Smallest buffer that can hold a test request line without causing + // Smallest buffer that can hold the test request headers without causing // the server to hang waiting for the end of the request line or // a header line. - Tuple.Create((long?)(_requestLines.Max(line => line.Length)), true), + Tuple.Create((long?)totalHeaderSize, true), // Small buffer, but large enough to hold all request headers. Tuple.Create((long?)16 * 1024, true), @@ -194,11 +202,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests startReadingRequestBody.TrySetResult(null); } - using (var reader = new StreamReader(stream, Encoding.ASCII)) - { - var response = reader.ReadToEnd(); - Assert.Contains($"bytesRead: {data.Length}", response); - } + await AssertStreamContains(stream, $"bytesRead: {data.Length}"); } } @@ -374,5 +378,38 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests } } } + + // THIS IS NOT GENERAL PURPOSE. If the initial characters could repeat, this is broken. However, since we're + // looking for /bytesWritten: \d+/ and the initial "b" cannot occur elsewhere in the pattern, this works. + private static async Task AssertStreamContains(Stream stream, string expectedSubstring) + { + var expectedBytes = Encoding.ASCII.GetBytes(expectedSubstring); + var exptectedLength = expectedBytes.Length; + var responseBuffer = new byte[exptectedLength]; + + var matchedChars = 0; + + while (matchedChars < exptectedLength) + { + var count = await stream.ReadAsync(responseBuffer, 0, exptectedLength - matchedChars).DefaultTimeout(); + + if (count == 0) + { + Assert.True(false, "Stream completed without expected substring."); + } + + for (var i = 0; i < count && matchedChars < exptectedLength; i++) + { + if (responseBuffer[i] == expectedBytes[matchedChars]) + { + matchedChars++; + } + else + { + matchedChars = 0; + } + } + } + } } } diff --git a/test/Kestrel.Transport.FunctionalTests/RequestTests.cs b/test/Kestrel.Transport.FunctionalTests/RequestTests.cs index 347ea3661f..4c1fea6660 100644 --- a/test/Kestrel.Transport.FunctionalTests/RequestTests.cs +++ b/test/Kestrel.Transport.FunctionalTests/RequestTests.cs @@ -53,7 +53,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests // https://github.com/aspnet/KestrelHttpServer/issues/520#issuecomment-188591242 // will be lost. [InlineData((long)int.MaxValue + 1, false)] - public void LargeUpload(long contentLength, bool checkBytes) + public async Task LargeUpload(long contentLength, bool checkBytes) { const int bufferLength = 1024 * 1024; Assert.True(contentLength % bufferLength == 0, $"{nameof(contentLength)} sent must be evenly divisible by {bufferLength}."); @@ -89,7 +89,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests total += received; } - await context.Response.WriteAsync(total.ToString(CultureInfo.InvariantCulture)); + await context.Response.WriteAsync($"bytesRead: {total.ToString()}"); }); }); @@ -100,8 +100,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests using (var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp)) { socket.Connect(new IPEndPoint(IPAddress.Loopback, host.GetPort())); - socket.Send(Encoding.ASCII.GetBytes("POST / HTTP/1.0\r\n")); - Thread.Sleep(5000); + socket.Send(Encoding.ASCII.GetBytes("POST / HTTP/1.1\r\nHost: \r\n")); socket.Send(Encoding.ASCII.GetBytes($"Content-Length: {contentLength}\r\n\r\n")); var contentBytes = new byte[bufferLength]; @@ -119,15 +118,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests socket.Send(contentBytes); } - var response = new StringBuilder(); - var responseBytes = new byte[4096]; - var received = 0; - while ((received = socket.Receive(responseBytes)) > 0) + using (var stream = new NetworkStream(socket)) { - response.Append(Encoding.ASCII.GetString(responseBytes, 0, received)); + await AssertStreamContains(stream, $"bytesRead: {contentLength}"); } - - Assert.Contains(contentLength.ToString(CultureInfo.InvariantCulture), response.ToString()); } } } @@ -906,5 +900,38 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests Assert.NotEmpty(facts["RemotePort"].Value()); } } + + // THIS IS NOT GENERAL PURPOSE. If the initial characters could repeat, this is broken. However, since we're + // looking for /bytesWritten: \d+/ and the initial "b" cannot occur elsewhere in the pattern, this works. + private static async Task AssertStreamContains(Stream stream, string expectedSubstring) + { + var expectedBytes = Encoding.ASCII.GetBytes(expectedSubstring); + var exptectedLength = expectedBytes.Length; + var responseBuffer = new byte[exptectedLength]; + + var matchedChars = 0; + + while (matchedChars < exptectedLength) + { + var count = await stream.ReadAsync(responseBuffer, 0, exptectedLength - matchedChars).DefaultTimeout(); + + if (count == 0) + { + Assert.True(false, "Stream completed without expected substring."); + } + + for (var i = 0; i < count && matchedChars < exptectedLength; i++) + { + if (responseBuffer[i] == expectedBytes[matchedChars]) + { + matchedChars++; + } + else + { + matchedChars = 0; + } + } + } + } } } diff --git a/test/Kestrel.Transport.Libuv.FunctionalTests/Kestrel.Transport.Libuv.FunctionalTests.csproj b/test/Kestrel.Transport.Libuv.FunctionalTests/Kestrel.Transport.Libuv.FunctionalTests.csproj index 7c9eb1fa1d..5169c5ed25 100644 --- a/test/Kestrel.Transport.Libuv.FunctionalTests/Kestrel.Transport.Libuv.FunctionalTests.csproj +++ b/test/Kestrel.Transport.Libuv.FunctionalTests/Kestrel.Transport.Libuv.FunctionalTests.csproj @@ -11,7 +11,6 @@ - diff --git a/test/shared/DiagnosticMemoryPoolFactory.cs b/test/shared/TransportTestHelpers/DiagnosticMemoryPoolFactory.cs similarity index 100% rename from test/shared/DiagnosticMemoryPoolFactory.cs rename to test/shared/TransportTestHelpers/DiagnosticMemoryPoolFactory.cs From 32532078d6cdb1933fc55986a292f27be5645f43 Mon Sep 17 00:00:00 2001 From: "Chris Ross (ASP.NET)" Date: Mon, 22 Oct 2018 14:51:29 -0700 Subject: [PATCH 6/6] Log binary data as a parameter #2860 --- samples/SampleApp/Startup.cs | 1 + src/Kestrel.Core/Adapter/Internal/LoggingStream.cs | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/samples/SampleApp/Startup.cs b/samples/SampleApp/Startup.cs index ad38e3920d..32890238ef 100644 --- a/samples/SampleApp/Startup.cs +++ b/samples/SampleApp/Startup.cs @@ -66,6 +66,7 @@ namespace SampleApp var hostBuilder = new WebHostBuilder() .ConfigureLogging((_, factory) => { + factory.SetMinimumLevel(LogLevel.Debug); factory.AddConsole(); }) .ConfigureAppConfiguration((hostingContext, config) => diff --git a/src/Kestrel.Core/Adapter/Internal/LoggingStream.cs b/src/Kestrel.Core/Adapter/Internal/LoggingStream.cs index e591d4acf4..42722f2413 100644 --- a/src/Kestrel.Core/Adapter/Internal/LoggingStream.cs +++ b/src/Kestrel.Core/Adapter/Internal/LoggingStream.cs @@ -174,13 +174,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Adapter.Internal builder.Append(" "); } builder.AppendLine(); + builder.Append("{0}"); + + var rawDataBuilder = new StringBuilder(); // Write the bytes as if they were ASCII for (int i = 0; i < buffer.Length; i++) { - builder.Append((char)buffer[i]); + rawDataBuilder.Append((char)buffer[i]); } - _logger.LogDebug(builder.ToString()); + _logger.LogDebug(builder.ToString(), rawDataBuilder.ToString()); } // The below APM methods call the underlying Read/WriteAsync methods which will still be logged.