From debf43053189e7d47969fdb7c2d2c7323d47a696 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Wed, 1 Jun 2016 12:46:33 -0700 Subject: [PATCH 01/37] Limit size of memory buffer when reading request (#304) --- .../Internal/Http/Connection.cs | 10 ++- .../Internal/Http/SocketInput.cs | 88 +++++++++++++++++++ .../KestrelServerOptions.cs | 26 ++++-- 3 files changed, 118 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs index 647254316..d327f5587 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs @@ -49,7 +49,15 @@ public Connection(ListenerContext context, UvStreamHandle socket) : base(context ConnectionId = GenerateConnectionId(Interlocked.Increment(ref _lastConnectionId)); - _rawSocketInput = new SocketInput(Memory, ThreadPool); + if (ServerOptions.MaxInputBufferLength == -1) + { + _rawSocketInput = new SocketInput(Memory, ThreadPool); + } + else + { + _rawSocketInput = new SocketInput(Memory, ThreadPool, ServerOptions.MaxInputBufferLength, this, Thread); + } + _rawSocketOutput = new SocketOutput(Thread, _socket, Memory, this, ConnectionId, Log, ThreadPool, WriteReqPool); } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs index 3082edf70..fef4dac63 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs @@ -32,6 +32,8 @@ public class SocketInput : ICriticalNotifyCompletion, IDisposable private bool _consuming; private bool _disposed; + private readonly BufferLengthConnectionController _bufferLengthConnectionController; + public SocketInput(MemoryPool memory, IThreadPool threadPool) { _memory = memory; @@ -39,6 +41,12 @@ public SocketInput(MemoryPool memory, IThreadPool threadPool) _awaitableState = _awaitableIsNotCompleted; } + public SocketInput(MemoryPool memory, IThreadPool threadPool, int maxBufferLength, IConnectionControl connectionControl, + KestrelThread connectionThread) : this(memory, threadPool) + { + _bufferLengthConnectionController = new BufferLengthConnectionController(maxBufferLength, connectionControl, connectionThread); + } + public bool RemoteIntakeFin { get; set; } public bool IsCompleted => ReferenceEquals(_awaitableState, _awaitableIsCompleted); @@ -63,6 +71,9 @@ public void IncomingData(byte[] buffer, int offset, int count) { lock (_sync) { + // Must call Add() before bytes are available to consumer, to ensure that Length is >= 0 + _bufferLengthConnectionController?.Add(count); + if (count > 0) { if (_tail == null) @@ -93,6 +104,9 @@ public void IncomingComplete(int count, Exception error) { lock (_sync) { + // Must call Add() before bytes are available to consumer, to ensure that Length is >= 0 + _bufferLengthConnectionController?.Add(count); + if (_pinned != null) { _pinned.End += count; @@ -189,10 +203,16 @@ public void ConsumingComplete( { if (!consumed.IsDefault) { + var lengthConsumed = new MemoryPoolIterator(_head).GetLength(consumed); + returnStart = _head; returnEnd = consumed.Block; _head = consumed.Block; _head.Start = consumed.Index; + + // Must call Subtract() after bytes have been freed, to avoid producer starting too early and growing + // buffer beyond max length. + _bufferLengthConnectionController?.Subtract(lengthConsumed); } if (!examined.IsDefault && @@ -321,5 +341,73 @@ private static void ReturnBlocks(MemoryPoolBlock block, MemoryPoolBlock end) returnBlock.Pool.Return(returnBlock); } } + + private class BufferLengthConnectionController + { + private readonly int _maxLength; + private readonly IConnectionControl _connectionControl; + private readonly KestrelThread _connectionThread; + + private readonly object _lock = new object(); + + private int _length; + private bool _connectionPaused; + + public BufferLengthConnectionController(int maxLength, IConnectionControl connectionControl, KestrelThread connectionThread) + { + _maxLength = maxLength; + _connectionControl = connectionControl; + _connectionThread = connectionThread; + } + + public int Length + { + get + { + return _length; + } + set + { + // Caller should ensure that bytes are never consumed before the producer has called Add() + Debug.Assert(value >= 0); + + _length = value; + } + } + + public void Add(int count) + { + lock (_lock) + { + // Add() should never be called while connection is paused, since ConnectionControl.Pause() runs on a libuv thread + // and should take effect immediately. + Debug.Assert(!_connectionPaused); + + Length += count; + if (Length >= _maxLength) + { + _connectionPaused = true; + _connectionControl.Pause(); + } + } + } + + public void Subtract(int count) + { + lock (_lock) + { + Length -= count; + + if (_connectionPaused && Length < _maxLength) + { + _connectionPaused = false; + _connectionThread.Post( + (connectionControl) => ((IConnectionControl)connectionControl).Resume(), + _connectionControl); + } + } + } + } + } } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs index 7a6c7953e..cc8353c93 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs @@ -8,16 +8,32 @@ namespace Microsoft.AspNetCore.Server.Kestrel { public class KestrelServerOptions { + /// + /// Gets or sets whether the Server header should be included in each response. + /// + public bool AddServerHeader { get; set; } = true; + public IServiceProvider ApplicationServices { get; set; } public IConnectionFilter ConnectionFilter { get; set; } - public bool NoDelay { get; set; } = true; + private int _maxInputBufferLength = 1024 * 1024; + public int MaxInputBufferLength + { + get + { + return _maxInputBufferLength; + } + set + { + if (value < -1 || value == 0) + { + throw new ArgumentOutOfRangeException("value", "Value must be positive or -1."); + } + } + } - /// - /// Gets or sets whether the Server header should be included in each response. - /// - public bool AddServerHeader { get; set; } = true; + public bool NoDelay { get; set; } = true; /// /// The amount of time after the server begins shutting down before connections will be forcefully closed. From 6a3c653a510477e34e4ce6e65ec5df57875a5bd9 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Wed, 1 Jun 2016 18:13:39 -0700 Subject: [PATCH 02/37] Fix bug in set_MaxInputBufferLength. --- .../KestrelServerOptions.cs | 1 + .../KestrelServerOptionsTests.cs | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs index cc8353c93..83850c8f8 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs @@ -30,6 +30,7 @@ public int MaxInputBufferLength { throw new ArgumentOutOfRangeException("value", "Value must be positive or -1."); } + _maxInputBufferLength = value; } } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/KestrelServerOptionsTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/KestrelServerOptionsTests.cs index ff03086b7..07abf7673 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/KestrelServerOptionsTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/KestrelServerOptionsTests.cs @@ -11,6 +11,33 @@ namespace Microsoft.AspNetCore.Server.KestrelTests { public class KestrelServerInformationTests { + [Fact] + public void MaxInputBufferLengthDefault() + { + Assert.Equal(1024 * 1024, (new KestrelServerOptions()).MaxInputBufferLength); + } + + [Theory] + [InlineData(-2)] + [InlineData(0)] + public void MaxInputBufferInvalid(int value) + { + Assert.Throws(() => + { + (new KestrelServerOptions()).MaxInputBufferLength = value; + }); + } + + [Theory] + [InlineData(-1)] + [InlineData(1)] + public void MaxInputBufferValid(int value) + { + var o = new KestrelServerOptions(); + o.MaxInputBufferLength = value; + Assert.Equal(value, o.MaxInputBufferLength); + } + [Fact] public void SetThreadCountUsingProcessorCount() { From 7fb1028f7325b7f8c7e63c92d3656e24b470f257 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Wed, 1 Jun 2016 18:47:38 -0700 Subject: [PATCH 03/37] Verify connection is paused when input buffer is full --- .../SocketInputTests.cs | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/SocketInputTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/SocketInputTests.cs index 15c7bc0c2..cddda569f 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/SocketInputTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/SocketInputTests.cs @@ -7,12 +7,68 @@ using Microsoft.AspNetCore.Server.Kestrel.Internal; using Microsoft.AspNetCore.Server.Kestrel.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; +using Microsoft.AspNetCore.Server.KestrelTests.TestHelpers; using Xunit; namespace Microsoft.AspNetCore.Server.KestrelTests { public class SocketInputTests { + [Fact] + public void ConnectionPausedWhenInputBufferFullUsingIncomingData() + { + const int maxBufferLength = 10; + var data = new byte[5]; + + var mockLibuv = new MockLibuv(); + var connectionControl = new TestConnectionControl(); + + using (var kestrelEngine = new KestrelEngine(mockLibuv, new TestServiceContext())) + { + kestrelEngine.Start(1); + + using (var memory = new MemoryPool()) + using (var socketInput = new SocketInput(memory, null, + maxBufferLength, connectionControl, kestrelEngine.Threads[0])) + { + Assert.Equal(false, connectionControl.Paused); + + socketInput.IncomingData(data, 0, data.Length); + Assert.Equal(false, connectionControl.Paused); + + socketInput.IncomingData(data, 0, data.Length); + Assert.Equal(true, connectionControl.Paused); + } + } + } + + [Fact] + public void ConnectionPausedWhenInputBufferFullUsingIncomingComplete() + { + const int maxBufferLength = 10; + + var mockLibuv = new MockLibuv(); + var connectionControl = new TestConnectionControl(); + + using (var kestrelEngine = new KestrelEngine(mockLibuv, new TestServiceContext())) + { + kestrelEngine.Start(1); + + using (var memory = new MemoryPool()) + using (var socketInput = new SocketInput(memory, null, + maxBufferLength, connectionControl, kestrelEngine.Threads[0])) + { + Assert.Equal(false, connectionControl.Paused); + + socketInput.IncomingComplete(5, null); + Assert.Equal(false, connectionControl.Paused); + + socketInput.IncomingComplete(5, null); + Assert.Equal(true, connectionControl.Paused); + } + } + } + [Fact] public async Task ConcurrentReadsFailGracefully() { @@ -115,5 +171,39 @@ private async Task AwaitAsTaskAsync(SocketInput socketInput) { await socketInput; } + + private class TestConnectionControl : IConnectionControl + { + public bool Paused { get; set; } + + public void End(ProduceEndType endType) + { + throw new NotImplementedException(); + } + + public void Pause() + { + if (Paused) + { + throw new InvalidOperationException("already paused"); + } + else + { + Paused = true; + } + } + + public void Resume() + { + if (Paused) + { + Paused = false; + } + else + { + throw new InvalidOperationException("not paused"); + } + } + } } } From 8832119016234c06dfaaa5075dabfb5ebc0bd37b Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Thu, 2 Jun 2016 14:54:08 -0700 Subject: [PATCH 04/37] Add test to verify connection resumed when buffer not full --- .../SocketInputTests.cs | 76 ++++++++++++------- 1 file changed, 48 insertions(+), 28 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/SocketInputTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/SocketInputTests.cs index cddda569f..5be979681 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/SocketInputTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/SocketInputTests.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Server.Kestrel; using Microsoft.AspNetCore.Server.Kestrel.Internal; @@ -17,54 +18,73 @@ public class SocketInputTests [Fact] public void ConnectionPausedWhenInputBufferFullUsingIncomingData() { - const int maxBufferLength = 10; - var data = new byte[5]; - - var mockLibuv = new MockLibuv(); var connectionControl = new TestConnectionControl(); - - using (var kestrelEngine = new KestrelEngine(mockLibuv, new TestServiceContext())) + using (var memory = new MemoryPool()) + using (var socketInput = new SocketInput(memory, null, 10, connectionControl, null)) { - kestrelEngine.Start(1); - - using (var memory = new MemoryPool()) - using (var socketInput = new SocketInput(memory, null, - maxBufferLength, connectionControl, kestrelEngine.Threads[0])) - { - Assert.Equal(false, connectionControl.Paused); + Assert.Equal(false, connectionControl.Paused); - socketInput.IncomingData(data, 0, data.Length); - Assert.Equal(false, connectionControl.Paused); + var data = new byte[5]; + socketInput.IncomingData(data, 0, data.Length); + Assert.Equal(false, connectionControl.Paused); - socketInput.IncomingData(data, 0, data.Length); - Assert.Equal(true, connectionControl.Paused); - } + socketInput.IncomingData(data, 0, data.Length); + Assert.Equal(true, connectionControl.Paused); } } [Fact] public void ConnectionPausedWhenInputBufferFullUsingIncomingComplete() { - const int maxBufferLength = 10; - - var mockLibuv = new MockLibuv(); var connectionControl = new TestConnectionControl(); + using (var memory = new MemoryPool()) + using (var socketInput = new SocketInput(memory, null, 10, connectionControl, null)) + { + Assert.Equal(false, connectionControl.Paused); - using (var kestrelEngine = new KestrelEngine(mockLibuv, new TestServiceContext())) + socketInput.IncomingComplete(5, null); + Assert.Equal(false, connectionControl.Paused); + + socketInput.IncomingComplete(5, null); + Assert.Equal(true, connectionControl.Paused); + } + } + + [Fact] + public void ConnectionResumedWhenInputBufferNotFull() + { + using (var kestrelEngine = new KestrelEngine(new MockLibuv(), new TestServiceContext())) { kestrelEngine.Start(1); + var connectionControl = new TestConnectionControl(); using (var memory = new MemoryPool()) - using (var socketInput = new SocketInput(memory, null, - maxBufferLength, connectionControl, kestrelEngine.Threads[0])) + using (var socketInput = new SocketInput(memory, null, 10, connectionControl, kestrelEngine.Threads[0])) { - Assert.Equal(false, connectionControl.Paused); + var data = new byte[20]; + socketInput.IncomingData(data, 0, data.Length); + Assert.Equal(true, connectionControl.Paused); - socketInput.IncomingComplete(5, null); - Assert.Equal(false, connectionControl.Paused); + var iterator = socketInput.ConsumingStart(); + iterator.Skip(5); + socketInput.ConsumingComplete(iterator, iterator); + Assert.Equal(true, connectionControl.Paused); - socketInput.IncomingComplete(5, null); + iterator = socketInput.ConsumingStart(); + iterator.Skip(5); + socketInput.ConsumingComplete(iterator, iterator); + + // TODO: Replace sleep with mock + Thread.Sleep(100); Assert.Equal(true, connectionControl.Paused); + + iterator = socketInput.ConsumingStart(); + iterator.Skip(1); + socketInput.ConsumingComplete(iterator, iterator); + + // TODO: Replace sleep with mock + Thread.Sleep(100); + Assert.Equal(false, connectionControl.Paused); } } } From ccd6e5c2d364a764c3220a978994d2b977cbb671 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Thu, 2 Jun 2016 19:13:24 -0700 Subject: [PATCH 05/37] Functional tests for MaxInputBufferLength - Tests currently fail sometimes, caused by server reading more bytes than sent by client. --- .../MaxInputBufferLengthTests.cs | 148 ++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs new file mode 100644 index 000000000..e469d8ee8 --- /dev/null +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs @@ -0,0 +1,148 @@ +using System; +using System.IO; +using System.Net; +using System.Net.Http; +using System.Net.Sockets; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Xunit; + +namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests +{ + public class MaxInputBufferLengthTests + { + [Theory] + [InlineData(-1)] + [InlineData(10 * 1024 * 1024)] + [InlineData(Int32.MaxValue)] + public void LargeUploadNotPaused(int maxInputBufferLength) + { + const int totalBytes = 10 * 1024 * 1024; + var startReadingRequestBody = new ManualResetEvent(false); + + using (var host = StartWebHost(totalBytes, maxInputBufferLength, startReadingRequestBody)) + { + var bytesWritten = 0; + using (var socket = CreateSocketForHttpPost(host)) + { + var buffer = new byte[4096]; + + while (bytesWritten < totalBytes) + { + var size = Math.Min(totalBytes - bytesWritten, buffer.Length); + bytesWritten += socket.Send(buffer, 0, size, SocketFlags.None); + } + + Assert.Equal(totalBytes, bytesWritten); + + // Tell server to start reading request body + startReadingRequestBody.Set(); + + buffer = new byte[4096]; + socket.Receive(buffer); + Assert.Contains($"bytesRead: {totalBytes}", Encoding.ASCII.GetString(buffer)); + } + } + } + + [Theory] + [InlineData(16 * 1024)] + [InlineData(1024 * 1024)] + [InlineData(5 * 1024 * 1024)] + public void LargeUploadPausedWhenInputBufferFull(int maxInputBufferLength) + { + const int totalBytes = 10 * 1024 * 1024; + var startReadingRequestBody = new ManualResetEvent(false); + + using (var host = StartWebHost(totalBytes, maxInputBufferLength, startReadingRequestBody)) + { + var bytesWritten = 0; + using (var socket = CreateSocketForHttpPost(host)) + { + var buffer = new byte[4096]; + try + { + while (bytesWritten < totalBytes) + { + var size = Math.Min(totalBytes - bytesWritten, buffer.Length); + bytesWritten += socket.Send(buffer, 0, size, SocketFlags.None); + } + + Assert.Equal("SocketException", "No Exception"); + } + catch (SocketException) + { + // When the input buffer is full (plus some amount of OS buffers), socket.Send() should + // throw a SocketException, since the server called IConnectionControl.Pause(). + + // Verify the number of bytes written is greater than or equal to the max input buffer size, + // but less than the total bytes. + Assert.InRange(bytesWritten, maxInputBufferLength, totalBytes - 1); + + // Tell server to start reading request body + startReadingRequestBody.Set(); + + while (bytesWritten < totalBytes) + { + var size = Math.Min(totalBytes - bytesWritten, buffer.Length); + bytesWritten += socket.Send(buffer, 0, size, SocketFlags.None); + } + } + + Assert.Equal(totalBytes, bytesWritten); + + buffer = new byte[4096]; + socket.Receive(buffer); + Assert.Contains($"bytesRead: {totalBytes}", Encoding.ASCII.GetString(buffer)); + } + } + } + + private static IWebHost StartWebHost(int totalBytes, int maxInputBufferLength, ManualResetEvent startReadingRequestBody) + { + var host = new WebHostBuilder() + .UseKestrel(options => + { + options.MaxInputBufferLength = maxInputBufferLength; + }) + .UseUrls("http://127.0.0.1:0/") + .Configure(app => app.Run(async context => + { + var bytesRead = 0; + + startReadingRequestBody.WaitOne(); + + var buffer = new byte[4096]; + while (bytesRead < totalBytes) + { + bytesRead += await context.Request.Body.ReadAsync(buffer, 0, buffer.Length); + } + + await context.Response.WriteAsync($"bytesRead: {bytesRead.ToString()}"); + } + )) + .Build(); + + host.Start(); + + return host; + } + + private static Socket CreateSocketForHttpPost(IWebHost host) + { + var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); + + socket.SendTimeout = 100; + socket.ReceiveTimeout = 100; + + socket.Connect(IPAddress.Loopback, host.GetPort()); + socket.Send(Encoding.ASCII.GetBytes($"POST / HTTP/1.0\r\n\r\n")); + + return socket; + } + } +} From a18b3eceb34b1722f09fd27ab755765b7ca40d96 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Fri, 3 Jun 2016 15:38:40 -0700 Subject: [PATCH 06/37] Functional tests pass reliably on Windows, but rely on Socket.Send() sending the full number of requested bytes when a SocketException is thrown, which is not guaranteed to be true on all platforms. --- .../MaxInputBufferLengthTests.cs | 146 ++++++++++++------ 1 file changed, 95 insertions(+), 51 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs index e469d8ee8..e6d1fa72a 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs @@ -1,11 +1,8 @@ using System; -using System.IO; using System.Net; -using System.Net.Http; using System.Net.Sockets; using System.Text; using System.Threading; -using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; @@ -15,61 +12,76 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { public class MaxInputBufferLengthTests { + private static readonly int _packetSize = 4096; + private static readonly byte[] _data = new byte[10 * 1024 * 1024]; + + static MaxInputBufferLengthTests() + { + // Fixed seed for reproducability + (new Random(0)).NextBytes(_data); + } + [Theory] - [InlineData(-1)] - [InlineData(10 * 1024 * 1024)] - [InlineData(Int32.MaxValue)] - public void LargeUploadNotPaused(int maxInputBufferLength) + [InlineData(-1, true)] + [InlineData(-1, false)] + [InlineData(10 * 1024 * 1024, true)] + [InlineData(10 * 1024 * 1024, false)] + [InlineData(Int32.MaxValue, true)] + [InlineData(Int32.MaxValue, false)] + public void LargeUploadNotPaused(int maxInputBufferLength, bool sendContentLengthHeader) { - const int totalBytes = 10 * 1024 * 1024; var startReadingRequestBody = new ManualResetEvent(false); + var clientFinishedSendingRequestBody = new ManualResetEvent(false); - using (var host = StartWebHost(totalBytes, maxInputBufferLength, startReadingRequestBody)) + using (var host = StartWebHost(maxInputBufferLength, startReadingRequestBody, clientFinishedSendingRequestBody)) { - var bytesWritten = 0; - using (var socket = CreateSocketForHttpPost(host)) + using (var socket = CreateSocketForHttpPost(host, sendContentLengthHeader ? _data.Length : -1)) { - var buffer = new byte[4096]; - - while (bytesWritten < totalBytes) + var bytesWritten = 0; + while (bytesWritten < _data.Length) { - var size = Math.Min(totalBytes - bytesWritten, buffer.Length); - bytesWritten += socket.Send(buffer, 0, size, SocketFlags.None); + var size = Math.Min(_data.Length - bytesWritten, _packetSize); + bytesWritten += socket.Send(_data, bytesWritten, size, SocketFlags.None); } + socket.Shutdown(SocketShutdown.Send); + clientFinishedSendingRequestBody.Set(); - Assert.Equal(totalBytes, bytesWritten); + Assert.Equal(_data.Length, bytesWritten); // Tell server to start reading request body startReadingRequestBody.Set(); - buffer = new byte[4096]; - socket.Receive(buffer); - Assert.Contains($"bytesRead: {totalBytes}", Encoding.ASCII.GetString(buffer)); + var buffer = new byte[_packetSize]; + var bytesRead = socket.Receive(buffer); + Assert.Contains($"bytesRead: {_data.Length}", Encoding.ASCII.GetString(buffer, 0, bytesRead)); } } } [Theory] - [InlineData(16 * 1024)] - [InlineData(1024 * 1024)] - [InlineData(5 * 1024 * 1024)] - public void LargeUploadPausedWhenInputBufferFull(int maxInputBufferLength) + [InlineData(16 * 1024, true)] + [InlineData(16 * 1024, false)] + [InlineData(1024 * 1024, true)] + [InlineData(1024 * 1024, false)] + [InlineData(5 * 1024 * 1024, true)] + [InlineData(5 * 1024 * 1024, false)] + public void LargeUploadPausedWhenInputBufferFull(int maxInputBufferLength, bool sendContentLengthHeader) { - const int totalBytes = 10 * 1024 * 1024; var startReadingRequestBody = new ManualResetEvent(false); - - using (var host = StartWebHost(totalBytes, maxInputBufferLength, startReadingRequestBody)) + var clientFinishedSendingRequestBody = new ManualResetEvent(false); + + using (var host = StartWebHost(maxInputBufferLength, startReadingRequestBody, clientFinishedSendingRequestBody)) { - var bytesWritten = 0; - using (var socket = CreateSocketForHttpPost(host)) + using (var socket = CreateSocketForHttpPost(host, sendContentLengthHeader ? _data.Length : -1)) { - var buffer = new byte[4096]; + var bytesWritten = 0; try { - while (bytesWritten < totalBytes) + socket.SendTimeout = 100; + while (bytesWritten < _data.Length) { - var size = Math.Min(totalBytes - bytesWritten, buffer.Length); - bytesWritten += socket.Send(buffer, 0, size, SocketFlags.None); + var size = Math.Min(_data.Length - bytesWritten, _packetSize); + bytesWritten += socket.Send(_data, bytesWritten, size, SocketFlags.None); } Assert.Equal("SocketException", "No Exception"); @@ -79,30 +91,36 @@ public void LargeUploadPausedWhenInputBufferFull(int maxInputBufferLength) // When the input buffer is full (plus some amount of OS buffers), socket.Send() should // throw a SocketException, since the server called IConnectionControl.Pause(). + bytesWritten += _packetSize; + // Verify the number of bytes written is greater than or equal to the max input buffer size, // but less than the total bytes. - Assert.InRange(bytesWritten, maxInputBufferLength, totalBytes - 1); + Assert.InRange(bytesWritten, maxInputBufferLength, _data.Length - 1); // Tell server to start reading request body startReadingRequestBody.Set(); - while (bytesWritten < totalBytes) + socket.SendTimeout = 10 * 1000; + while (bytesWritten < _data.Length) { - var size = Math.Min(totalBytes - bytesWritten, buffer.Length); - bytesWritten += socket.Send(buffer, 0, size, SocketFlags.None); + var size = Math.Min(_data.Length - bytesWritten, _packetSize); + bytesWritten += socket.Send(_data, bytesWritten, size, SocketFlags.None); } + socket.Shutdown(SocketShutdown.Send); + clientFinishedSendingRequestBody.Set(); } - Assert.Equal(totalBytes, bytesWritten); + Assert.Equal(_data.Length, bytesWritten); - buffer = new byte[4096]; - socket.Receive(buffer); - Assert.Contains($"bytesRead: {totalBytes}", Encoding.ASCII.GetString(buffer)); + var buffer = new byte[_packetSize]; + var bytesRead = socket.Receive(buffer); + Assert.Contains($"bytesRead: {_data.Length}", Encoding.ASCII.GetString(buffer, 0, bytesRead)); } } } - private static IWebHost StartWebHost(int totalBytes, int maxInputBufferLength, ManualResetEvent startReadingRequestBody) + private static IWebHost StartWebHost(int maxInputBufferLength, ManualResetEvent startReadingRequestBody, + ManualResetEvent clientFinishedSendingRequestBody) { var host = new WebHostBuilder() .UseKestrel(options => @@ -112,14 +130,34 @@ private static IWebHost StartWebHost(int totalBytes, int maxInputBufferLength, M .UseUrls("http://127.0.0.1:0/") .Configure(app => app.Run(async context => { + startReadingRequestBody.WaitOne(); + + var buffer = new byte[_data.Length]; var bytesRead = 0; + while (bytesRead < buffer.Length) + { + bytesRead += await context.Request.Body.ReadAsync(buffer, bytesRead, buffer.Length - bytesRead); + } - startReadingRequestBody.WaitOne(); + clientFinishedSendingRequestBody.WaitOne(); + + // Verify client didn't send extra bytes + if (context.Request.Body.ReadByte() != -1) + { + context.Response.StatusCode = 500; + await context.Response.WriteAsync("Client sent more bytes than _data.Length"); + return; + } - var buffer = new byte[4096]; - while (bytesRead < totalBytes) + // Verify bytes received match _data + for (int i=0; i < _data.Length; i++) { - bytesRead += await context.Request.Body.ReadAsync(buffer, 0, buffer.Length); + if (buffer[i] != _data[i]) + { + context.Response.StatusCode = 500; + await context.Response.WriteAsync($"Bytes received do not match _data at position {i}"); + return; + } } await context.Response.WriteAsync($"bytesRead: {bytesRead.ToString()}"); @@ -132,16 +170,22 @@ private static IWebHost StartWebHost(int totalBytes, int maxInputBufferLength, M return host; } - private static Socket CreateSocketForHttpPost(IWebHost host) + private static Socket CreateSocketForHttpPost(IWebHost host, int contentLength) { var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); - socket.SendTimeout = 100; - socket.ReceiveTimeout = 100; + // Timeouts large enough to prevent false positives, but small enough to fail quickly. + socket.SendTimeout = 10 * 1000; + socket.ReceiveTimeout = 10 * 1000; socket.Connect(IPAddress.Loopback, host.GetPort()); - socket.Send(Encoding.ASCII.GetBytes($"POST / HTTP/1.0\r\n\r\n")); - + socket.Send(Encoding.ASCII.GetBytes($"POST / HTTP/1.0\r\n")); + if (contentLength > -1) + { + socket.Send(Encoding.ASCII.GetBytes($"Content-Length: {contentLength}\r\n")); + } + socket.Send(Encoding.ASCII.GetBytes("\r\n")); + return socket; } } From 4ae8d49f7347d918d52337563dad698500fd9dc9 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Fri, 3 Jun 2016 17:15:44 -0700 Subject: [PATCH 07/37] Refactor test and make more reliable - Send bytes in a background task - Block test thread until send task has gone a while without writing bytes, rather than using timeout and SocketException. - Refactor to use mostly the same code for both pause and no-pause scenarios. --- .../MaxInputBufferLengthTests.cs | 147 +++++++----------- 1 file changed, 60 insertions(+), 87 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs index e6d1fa72a..56607c5cb 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs @@ -3,6 +3,7 @@ using System.Net.Sockets; using System.Text; using System.Threading; +using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; @@ -12,114 +13,86 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { public class MaxInputBufferLengthTests { - private static readonly int _packetSize = 4096; - private static readonly byte[] _data = new byte[10 * 1024 * 1024]; - - static MaxInputBufferLengthTests() - { - // Fixed seed for reproducability - (new Random(0)).NextBytes(_data); - } - [Theory] - [InlineData(-1, true)] - [InlineData(-1, false)] - [InlineData(10 * 1024 * 1024, true)] - [InlineData(10 * 1024 * 1024, false)] - [InlineData(Int32.MaxValue, true)] - [InlineData(Int32.MaxValue, false)] - public void LargeUploadNotPaused(int maxInputBufferLength, bool sendContentLengthHeader) + [InlineData(16 * 1024, true, true)] + [InlineData(16 * 1024, false, true)] + [InlineData(1024 * 1024, true, true)] + [InlineData(1024 * 1024, false, true)] + [InlineData(5 * 1024 * 1024, true, true)] + [InlineData(5 * 1024 * 1024, false, true)] + [InlineData(10 * 1024 * 1024, true, false)] + [InlineData(10 * 1024 * 1024, false, false)] + [InlineData(Int32.MaxValue, true, false)] + [InlineData(Int32.MaxValue, false, false)] + [InlineData(-1, true, false)] + [InlineData(-1, false, false)] + public async Task LargeUpload(int maxInputBufferLength, bool sendContentLengthHeader, bool expectPause) { - var startReadingRequestBody = new ManualResetEvent(false); - var clientFinishedSendingRequestBody = new ManualResetEvent(false); - - using (var host = StartWebHost(maxInputBufferLength, startReadingRequestBody, clientFinishedSendingRequestBody)) - { - using (var socket = CreateSocketForHttpPost(host, sendContentLengthHeader ? _data.Length : -1)) - { - var bytesWritten = 0; - while (bytesWritten < _data.Length) - { - var size = Math.Min(_data.Length - bytesWritten, _packetSize); - bytesWritten += socket.Send(_data, bytesWritten, size, SocketFlags.None); - } - socket.Shutdown(SocketShutdown.Send); - clientFinishedSendingRequestBody.Set(); - - Assert.Equal(_data.Length, bytesWritten); - - // Tell server to start reading request body - startReadingRequestBody.Set(); + // Parameters + var data = new byte[10 * 1024 * 1024]; + var bytesWrittenTimeout = TimeSpan.FromMilliseconds(100); + var maxSendSize = 4096; - var buffer = new byte[_packetSize]; - var bytesRead = socket.Receive(buffer); - Assert.Contains($"bytesRead: {_data.Length}", Encoding.ASCII.GetString(buffer, 0, bytesRead)); - } - } - } + // Initialize data with random bytes + (new Random()).NextBytes(data); - [Theory] - [InlineData(16 * 1024, true)] - [InlineData(16 * 1024, false)] - [InlineData(1024 * 1024, true)] - [InlineData(1024 * 1024, false)] - [InlineData(5 * 1024 * 1024, true)] - [InlineData(5 * 1024 * 1024, false)] - public void LargeUploadPausedWhenInputBufferFull(int maxInputBufferLength, bool sendContentLengthHeader) - { var startReadingRequestBody = new ManualResetEvent(false); var clientFinishedSendingRequestBody = new ManualResetEvent(false); - - using (var host = StartWebHost(maxInputBufferLength, startReadingRequestBody, clientFinishedSendingRequestBody)) + var bytesWrittenEvent = new AutoResetEvent(false); + + using (var host = StartWebHost(maxInputBufferLength, data, startReadingRequestBody, clientFinishedSendingRequestBody)) { - using (var socket = CreateSocketForHttpPost(host, sendContentLengthHeader ? _data.Length : -1)) + using (var socket = CreateSocketForHttpPost(host, sendContentLengthHeader ? data.Length : -1)) { var bytesWritten = 0; - try + + var sendTask = Task.Run(() => { - socket.SendTimeout = 100; - while (bytesWritten < _data.Length) + while (bytesWritten < data.Length) { - var size = Math.Min(_data.Length - bytesWritten, _packetSize); - bytesWritten += socket.Send(_data, bytesWritten, size, SocketFlags.None); + var size = Math.Min(data.Length - bytesWritten, maxSendSize); + bytesWritten += socket.Send(data, bytesWritten, size, SocketFlags.None); + bytesWrittenEvent.Set(); } - Assert.Equal("SocketException", "No Exception"); - } - catch (SocketException) - { - // When the input buffer is full (plus some amount of OS buffers), socket.Send() should - // throw a SocketException, since the server called IConnectionControl.Pause(). + Assert.Equal(data.Length, bytesWritten); + socket.Shutdown(SocketShutdown.Send); + clientFinishedSendingRequestBody.Set(); + }); - bytesWritten += _packetSize; + if (expectPause) + { + // Block until the send task has gone a while without writing bytes, which likely means + // the server input buffer is full. + while (bytesWrittenEvent.WaitOne(bytesWrittenTimeout)) { } // Verify the number of bytes written is greater than or equal to the max input buffer size, // but less than the total bytes. - Assert.InRange(bytesWritten, maxInputBufferLength, _data.Length - 1); + Assert.InRange(bytesWritten, maxInputBufferLength, data.Length - 1); // Tell server to start reading request body startReadingRequestBody.Set(); - socket.SendTimeout = 10 * 1000; - while (bytesWritten < _data.Length) - { - var size = Math.Min(_data.Length - bytesWritten, _packetSize); - bytesWritten += socket.Send(_data, bytesWritten, size, SocketFlags.None); - } - socket.Shutdown(SocketShutdown.Send); - clientFinishedSendingRequestBody.Set(); + // Wait for sendTask to finish sending the remaining bytes + await sendTask; } + else + { + // Ensure all bytes can be sent before the server starts reading + await sendTask; - Assert.Equal(_data.Length, bytesWritten); + // Tell server to start reading request body + startReadingRequestBody.Set(); + } - var buffer = new byte[_packetSize]; + var buffer = new byte[maxSendSize]; var bytesRead = socket.Receive(buffer); - Assert.Contains($"bytesRead: {_data.Length}", Encoding.ASCII.GetString(buffer, 0, bytesRead)); + Assert.Contains($"bytesRead: {data.Length}", Encoding.ASCII.GetString(buffer, 0, bytesRead)); } } } - private static IWebHost StartWebHost(int maxInputBufferLength, ManualResetEvent startReadingRequestBody, + private static IWebHost StartWebHost(int maxInputBufferLength, byte[] expectedBody, ManualResetEvent startReadingRequestBody, ManualResetEvent clientFinishedSendingRequestBody) { var host = new WebHostBuilder() @@ -132,7 +105,7 @@ private static IWebHost StartWebHost(int maxInputBufferLength, ManualResetEvent { startReadingRequestBody.WaitOne(); - var buffer = new byte[_data.Length]; + var buffer = new byte[expectedBody.Length]; var bytesRead = 0; while (bytesRead < buffer.Length) { @@ -145,17 +118,17 @@ private static IWebHost StartWebHost(int maxInputBufferLength, ManualResetEvent if (context.Request.Body.ReadByte() != -1) { context.Response.StatusCode = 500; - await context.Response.WriteAsync("Client sent more bytes than _data.Length"); + await context.Response.WriteAsync("Client sent more bytes than expectedBody.Length"); return; } - // Verify bytes received match _data - for (int i=0; i < _data.Length; i++) + // Verify bytes received match expectedBody + for (int i = 0; i < expectedBody.Length; i++) { - if (buffer[i] != _data[i]) + if (buffer[i] != expectedBody[i]) { context.Response.StatusCode = 500; - await context.Response.WriteAsync($"Bytes received do not match _data at position {i}"); + await context.Response.WriteAsync($"Bytes received do not match expectedBody at position {i}"); return; } } @@ -179,13 +152,13 @@ private static Socket CreateSocketForHttpPost(IWebHost host, int contentLength) socket.ReceiveTimeout = 10 * 1000; socket.Connect(IPAddress.Loopback, host.GetPort()); - socket.Send(Encoding.ASCII.GetBytes($"POST / HTTP/1.0\r\n")); + socket.Send(Encoding.ASCII.GetBytes("POST / HTTP/1.0\r\n")); if (contentLength > -1) { socket.Send(Encoding.ASCII.GetBytes($"Content-Length: {contentLength}\r\n")); } socket.Send(Encoding.ASCII.GetBytes("\r\n")); - + return socket; } } From 7bb1f0b20890bf85967553b5cd4124b299b6330f Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Fri, 3 Jun 2016 17:30:46 -0700 Subject: [PATCH 08/37] Improve range check for bytesWritten when client is paused. --- .../MaxInputBufferLengthTests.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs index 56607c5cb..308bf77d0 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs @@ -66,9 +66,17 @@ public async Task LargeUpload(int maxInputBufferLength, bool sendContentLengthHe // the server input buffer is full. while (bytesWrittenEvent.WaitOne(bytesWrittenTimeout)) { } - // Verify the number of bytes written is greater than or equal to the max input buffer size, - // but less than the total bytes. - Assert.InRange(bytesWritten, maxInputBufferLength, data.Length - 1); + // Verify the number of bytes written before the client was paused. + // + // The minimum is (maxInputBufferLength - maxSendSize + 1), since if bytesWritten is + // (maxInputBufferLength - maxSendSize) or smaller, the client should be able to + // complete another send. + // + // The maximum is harder to determine, since there can be OS-level buffers in both the client + // and server, which allow the client to send more than maxInputBufferLength before getting + // paused. We assume the combined buffers are smaller than the difference between + // data.Length and maxInputBufferLength. + Assert.InRange(bytesWritten, maxInputBufferLength - maxSendSize + 1, data.Length - 1); // Tell server to start reading request body startReadingRequestBody.Set(); From 507f555fc602043969f9dea42af9720cde3847b7 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Mon, 6 Jun 2016 12:18:22 -0700 Subject: [PATCH 09/37] Use `null` instead of `-1` to make buffer size unlimited --- .../Internal/Http/Connection.cs | 6 +++--- .../KestrelServerOptions.cs | 8 ++++---- .../MaxInputBufferLengthTests.cs | 10 +++++----- .../KestrelServerOptionsTests.cs | 6 +++--- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs index d327f5587..807da1aa7 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs @@ -49,13 +49,13 @@ public Connection(ListenerContext context, UvStreamHandle socket) : base(context ConnectionId = GenerateConnectionId(Interlocked.Increment(ref _lastConnectionId)); - if (ServerOptions.MaxInputBufferLength == -1) + if (ServerOptions.MaxInputBufferLength.HasValue) { - _rawSocketInput = new SocketInput(Memory, ThreadPool); + _rawSocketInput = new SocketInput(Memory, ThreadPool, ServerOptions.MaxInputBufferLength.Value, this, Thread); } else { - _rawSocketInput = new SocketInput(Memory, ThreadPool, ServerOptions.MaxInputBufferLength, this, Thread); + _rawSocketInput = new SocketInput(Memory, ThreadPool); } _rawSocketOutput = new SocketOutput(Thread, _socket, Memory, this, ConnectionId, Log, ThreadPool, WriteReqPool); diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs index 83850c8f8..bd2ba09d0 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs @@ -17,8 +17,8 @@ public class KestrelServerOptions public IConnectionFilter ConnectionFilter { get; set; } - private int _maxInputBufferLength = 1024 * 1024; - public int MaxInputBufferLength + private int? _maxInputBufferLength = 1024 * 1024; + public int? MaxInputBufferLength { get { @@ -26,9 +26,9 @@ public int MaxInputBufferLength } set { - if (value < -1 || value == 0) + if (value.HasValue && value.Value <= 0) { - throw new ArgumentOutOfRangeException("value", "Value must be positive or -1."); + throw new ArgumentOutOfRangeException("value", "Value must be null or a positive integer."); } _maxInputBufferLength = value; } diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs index 308bf77d0..e21b71484 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs @@ -24,9 +24,9 @@ public class MaxInputBufferLengthTests [InlineData(10 * 1024 * 1024, false, false)] [InlineData(Int32.MaxValue, true, false)] [InlineData(Int32.MaxValue, false, false)] - [InlineData(-1, true, false)] - [InlineData(-1, false, false)] - public async Task LargeUpload(int maxInputBufferLength, bool sendContentLengthHeader, bool expectPause) + [InlineData(null, true, false)] + [InlineData(null, false, false)] + public async Task LargeUpload(int? maxInputBufferLength, bool sendContentLengthHeader, bool expectPause) { // Parameters var data = new byte[10 * 1024 * 1024]; @@ -76,7 +76,7 @@ public async Task LargeUpload(int maxInputBufferLength, bool sendContentLengthHe // and server, which allow the client to send more than maxInputBufferLength before getting // paused. We assume the combined buffers are smaller than the difference between // data.Length and maxInputBufferLength. - Assert.InRange(bytesWritten, maxInputBufferLength - maxSendSize + 1, data.Length - 1); + Assert.InRange(bytesWritten, maxInputBufferLength.Value - maxSendSize + 1, data.Length - 1); // Tell server to start reading request body startReadingRequestBody.Set(); @@ -100,7 +100,7 @@ public async Task LargeUpload(int maxInputBufferLength, bool sendContentLengthHe } } - private static IWebHost StartWebHost(int maxInputBufferLength, byte[] expectedBody, ManualResetEvent startReadingRequestBody, + private static IWebHost StartWebHost(int? maxInputBufferLength, byte[] expectedBody, ManualResetEvent startReadingRequestBody, ManualResetEvent clientFinishedSendingRequestBody) { var host = new WebHostBuilder() diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/KestrelServerOptionsTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/KestrelServerOptionsTests.cs index 07abf7673..89825d4a5 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/KestrelServerOptionsTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/KestrelServerOptionsTests.cs @@ -18,7 +18,7 @@ public void MaxInputBufferLengthDefault() } [Theory] - [InlineData(-2)] + [InlineData(-1)] [InlineData(0)] public void MaxInputBufferInvalid(int value) { @@ -29,9 +29,9 @@ public void MaxInputBufferInvalid(int value) } [Theory] - [InlineData(-1)] + [InlineData(null)] [InlineData(1)] - public void MaxInputBufferValid(int value) + public void MaxInputBufferValid(int? value) { var o = new KestrelServerOptions(); o.MaxInputBufferLength = value; From 818bcb966ad8bd5096153a88f98e0399323e0c45 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Mon, 6 Jun 2016 18:52:06 -0700 Subject: [PATCH 10/37] Move buffer length tracking from SocketInput to Connection - Required for connection filters, which use use two instances of SocketInput. We need to ensure the combined buffers do not exceed the limit, and to coordinate the calls to `IConnectionControl` between the two instances of SocketInput. --- .../Filter/Internal/FilteredStreamAdapter.cs | 5 +- .../Internal/Http/Connection.cs | 76 ++++++++++++++-- .../Internal/Http/IBufferLengthControl.cs | 13 +++ .../Internal/Http/SocketInput.cs | 88 ++----------------- 4 files changed, 93 insertions(+), 89 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IBufferLengthControl.cs diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Filter/Internal/FilteredStreamAdapter.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Filter/Internal/FilteredStreamAdapter.cs index bcd6b709e..8a3712a79 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Filter/Internal/FilteredStreamAdapter.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Filter/Internal/FilteredStreamAdapter.cs @@ -24,9 +24,10 @@ public FilteredStreamAdapter( Stream filteredStream, MemoryPool memory, IKestrelTrace logger, - IThreadPool threadPool) + IThreadPool threadPool, + IBufferLengthControl bufferLengthControl) { - SocketInput = new SocketInput(memory, threadPool); + SocketInput = new SocketInput(memory, threadPool, bufferLengthControl); SocketOutput = new StreamSocketOutput(connectionId, filteredStream, memory, logger); _connectionId = connectionId; diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs index 807da1aa7..c49718b83 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Diagnostics; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Server.Kestrel.Filter; @@ -41,6 +42,8 @@ public class Connection : ConnectionContext, IConnectionControl private ConnectionState _connectionState; private TaskCompletionSource _socketClosedTcs; + private BufferLengthControl _bufferLengthControl; + public Connection(ListenerContext context, UvStreamHandle socket) : base(context) { _socket = socket; @@ -51,13 +54,10 @@ public Connection(ListenerContext context, UvStreamHandle socket) : base(context if (ServerOptions.MaxInputBufferLength.HasValue) { - _rawSocketInput = new SocketInput(Memory, ThreadPool, ServerOptions.MaxInputBufferLength.Value, this, Thread); - } - else - { - _rawSocketInput = new SocketInput(Memory, ThreadPool); + _bufferLengthControl = new BufferLengthControl(ServerOptions.MaxInputBufferLength.Value, this, Thread); } + _rawSocketInput = new SocketInput(Memory, ThreadPool, _bufferLengthControl); _rawSocketOutput = new SocketOutput(Thread, _socket, Memory, this, ConnectionId, Log, ThreadPool, WriteReqPool); } @@ -225,7 +225,7 @@ private void ApplyConnectionFilter() if (_filterContext.Connection != _libuvStream) { - _filteredStreamAdapter = new FilteredStreamAdapter(ConnectionId, _filterContext.Connection, Memory, Log, ThreadPool); + _filteredStreamAdapter = new FilteredStreamAdapter(ConnectionId, _filterContext.Connection, Memory, Log, ThreadPool, _bufferLengthControl); SocketInput = _filteredStreamAdapter.SocketInput; SocketOutput = _filteredStreamAdapter.SocketOutput; @@ -392,5 +392,69 @@ private enum ConnectionState Disconnecting, SocketClosed } + + private class BufferLengthControl : IBufferLengthControl + { + private readonly int _maxLength; + private readonly IConnectionControl _connectionControl; + private readonly KestrelThread _connectionThread; + + private readonly object _lock = new object(); + + private int _length; + private bool _connectionPaused; + + public BufferLengthControl(int maxLength, IConnectionControl connectionControl, KestrelThread connectionThread) + { + _maxLength = maxLength; + _connectionControl = connectionControl; + _connectionThread = connectionThread; + } + + private int Length + { + get + { + return _length; + } + set + { + // Caller should ensure that bytes are never consumed before the producer has called Add() + Debug.Assert(value >= 0); + _length = value; + } + } + + public void Add(int count) + { + lock (_lock) + { + Length += count; + if (!_connectionPaused && Length >= _maxLength) + { + _connectionPaused = true; + _connectionThread.Post( + (connectionControl) => ((IConnectionControl)connectionControl).Pause(), + _connectionControl); + } + } + } + + public void Subtract(int count) + { + lock (_lock) + { + Length -= count; + if (_connectionPaused && Length < _maxLength) + { + _connectionPaused = false; + _connectionThread.Post( + (connectionControl) => ((IConnectionControl)connectionControl).Resume(), + _connectionControl); + } + } + } + } + } } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IBufferLengthControl.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IBufferLengthControl.cs new file mode 100644 index 000000000..28fa24c4b --- /dev/null +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IBufferLengthControl.cs @@ -0,0 +1,13 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; + +namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http +{ + public interface IBufferLengthControl + { + void Add(int count); + void Subtract(int count); + } +} diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs index fef4dac63..c895b7f99 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs @@ -18,6 +18,7 @@ public class SocketInput : ICriticalNotifyCompletion, IDisposable private readonly MemoryPool _memory; private readonly IThreadPool _threadPool; + private readonly IBufferLengthControl _bufferLengthControl; private readonly ManualResetEventSlim _manualResetEvent = new ManualResetEventSlim(false, 0); private Action _awaitableState; @@ -32,21 +33,14 @@ public class SocketInput : ICriticalNotifyCompletion, IDisposable private bool _consuming; private bool _disposed; - private readonly BufferLengthConnectionController _bufferLengthConnectionController; - - public SocketInput(MemoryPool memory, IThreadPool threadPool) + public SocketInput(MemoryPool memory, IThreadPool threadPool, IBufferLengthControl bufferLengthControl) { _memory = memory; _threadPool = threadPool; + _bufferLengthControl = bufferLengthControl; _awaitableState = _awaitableIsNotCompleted; } - public SocketInput(MemoryPool memory, IThreadPool threadPool, int maxBufferLength, IConnectionControl connectionControl, - KestrelThread connectionThread) : this(memory, threadPool) - { - _bufferLengthConnectionController = new BufferLengthConnectionController(maxBufferLength, connectionControl, connectionThread); - } - public bool RemoteIntakeFin { get; set; } public bool IsCompleted => ReferenceEquals(_awaitableState, _awaitableIsCompleted); @@ -72,7 +66,7 @@ public void IncomingData(byte[] buffer, int offset, int count) lock (_sync) { // Must call Add() before bytes are available to consumer, to ensure that Length is >= 0 - _bufferLengthConnectionController?.Add(count); + _bufferLengthControl?.Add(count); if (count > 0) { @@ -105,7 +99,7 @@ public void IncomingComplete(int count, Exception error) lock (_sync) { // Must call Add() before bytes are available to consumer, to ensure that Length is >= 0 - _bufferLengthConnectionController?.Add(count); + _bufferLengthControl?.Add(count); if (_pinned != null) { @@ -210,9 +204,9 @@ public void ConsumingComplete( _head = consumed.Block; _head.Start = consumed.Index; - // Must call Subtract() after bytes have been freed, to avoid producer starting too early and growing + // Must call Subtract() after _head has been advanced, to avoid producer starting too early and growing // buffer beyond max length. - _bufferLengthConnectionController?.Subtract(lengthConsumed); + _bufferLengthControl?.Subtract(lengthConsumed); } if (!examined.IsDefault && @@ -341,73 +335,5 @@ private static void ReturnBlocks(MemoryPoolBlock block, MemoryPoolBlock end) returnBlock.Pool.Return(returnBlock); } } - - private class BufferLengthConnectionController - { - private readonly int _maxLength; - private readonly IConnectionControl _connectionControl; - private readonly KestrelThread _connectionThread; - - private readonly object _lock = new object(); - - private int _length; - private bool _connectionPaused; - - public BufferLengthConnectionController(int maxLength, IConnectionControl connectionControl, KestrelThread connectionThread) - { - _maxLength = maxLength; - _connectionControl = connectionControl; - _connectionThread = connectionThread; - } - - public int Length - { - get - { - return _length; - } - set - { - // Caller should ensure that bytes are never consumed before the producer has called Add() - Debug.Assert(value >= 0); - - _length = value; - } - } - - public void Add(int count) - { - lock (_lock) - { - // Add() should never be called while connection is paused, since ConnectionControl.Pause() runs on a libuv thread - // and should take effect immediately. - Debug.Assert(!_connectionPaused); - - Length += count; - if (Length >= _maxLength) - { - _connectionPaused = true; - _connectionControl.Pause(); - } - } - } - - public void Subtract(int count) - { - lock (_lock) - { - Length -= count; - - if (_connectionPaused && Length < _maxLength) - { - _connectionPaused = false; - _connectionThread.Post( - (connectionControl) => ((IConnectionControl)connectionControl).Resume(), - _connectionControl); - } - } - } - } - } } From 3716dba7b2db6026d5e621637cb803fe54bf08f2 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 7 Jun 2016 11:40:27 -0700 Subject: [PATCH 11/37] Make bufferLengthControl an optional parameter - Prevents breaking tests which call SocketInput.ctor() --- .../Internal/Http/SocketInput.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs index c895b7f99..8f493e4ba 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs @@ -33,7 +33,7 @@ public class SocketInput : ICriticalNotifyCompletion, IDisposable private bool _consuming; private bool _disposed; - public SocketInput(MemoryPool memory, IThreadPool threadPool, IBufferLengthControl bufferLengthControl) + public SocketInput(MemoryPool memory, IThreadPool threadPool, IBufferLengthControl bufferLengthControl = null) { _memory = memory; _threadPool = threadPool; From ae07e9295cb4f3ab9f29ddd878678f722458a214 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 7 Jun 2016 15:40:44 -0700 Subject: [PATCH 12/37] Update SocketInputTests - Add Moq to KestrelTests --- .../SocketInputTests.cs | 103 ++++-------------- .../project.json | 6 +- 2 files changed, 25 insertions(+), 84 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/SocketInputTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/SocketInputTests.cs index 5be979681..66dad80d7 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/SocketInputTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/SocketInputTests.cs @@ -2,89 +2,62 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Threading; using System.Threading.Tasks; -using Microsoft.AspNetCore.Server.Kestrel; using Microsoft.AspNetCore.Server.Kestrel.Internal; using Microsoft.AspNetCore.Server.Kestrel.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; using Microsoft.AspNetCore.Server.KestrelTests.TestHelpers; +using Moq; using Xunit; namespace Microsoft.AspNetCore.Server.KestrelTests { public class SocketInputTests { - [Fact] - public void ConnectionPausedWhenInputBufferFullUsingIncomingData() + public static readonly TheoryData> MockBufferLengthControlData = + new TheoryData>() { new Mock(), null }; + + [Theory] + [MemberData("MockBufferLengthControlData")] + public void IncomingDataCallsBufferLengthControlAdd(Mock mockBufferLengthControl) { - var connectionControl = new TestConnectionControl(); using (var memory = new MemoryPool()) - using (var socketInput = new SocketInput(memory, null, 10, connectionControl, null)) + using (var socketInput = new SocketInput(memory, null, mockBufferLengthControl?.Object)) { - Assert.Equal(false, connectionControl.Paused); - - var data = new byte[5]; - socketInput.IncomingData(data, 0, data.Length); - Assert.Equal(false, connectionControl.Paused); - - socketInput.IncomingData(data, 0, data.Length); - Assert.Equal(true, connectionControl.Paused); + socketInput.IncomingData(new byte[5], 0, 5); + mockBufferLengthControl?.Verify(b => b.Add(5)); } } - [Fact] - public void ConnectionPausedWhenInputBufferFullUsingIncomingComplete() + [Theory] + [MemberData("MockBufferLengthControlData")] + public void IncomingCompleteCallsBufferLengthControlAdd(Mock mockBufferLengthControl) { - var connectionControl = new TestConnectionControl(); using (var memory = new MemoryPool()) - using (var socketInput = new SocketInput(memory, null, 10, connectionControl, null)) + using (var socketInput = new SocketInput(memory, null, mockBufferLengthControl?.Object)) { - Assert.Equal(false, connectionControl.Paused); - - socketInput.IncomingComplete(5, null); - Assert.Equal(false, connectionControl.Paused); - socketInput.IncomingComplete(5, null); - Assert.Equal(true, connectionControl.Paused); + mockBufferLengthControl?.Verify(b => b.Add(5)); } } - [Fact] - public void ConnectionResumedWhenInputBufferNotFull() + [Theory] + [MemberData("MockBufferLengthControlData")] + public void ConsumingCompleteCallsBufferLengthControlSubtract(Mock mockBufferLengthControl) { using (var kestrelEngine = new KestrelEngine(new MockLibuv(), new TestServiceContext())) { kestrelEngine.Start(1); - var connectionControl = new TestConnectionControl(); using (var memory = new MemoryPool()) - using (var socketInput = new SocketInput(memory, null, 10, connectionControl, kestrelEngine.Threads[0])) + using (var socketInput = new SocketInput(memory, null, mockBufferLengthControl?.Object)) { - var data = new byte[20]; - socketInput.IncomingData(data, 0, data.Length); - Assert.Equal(true, connectionControl.Paused); + socketInput.IncomingData(new byte[20], 0, 20); var iterator = socketInput.ConsumingStart(); iterator.Skip(5); socketInput.ConsumingComplete(iterator, iterator); - Assert.Equal(true, connectionControl.Paused); - - iterator = socketInput.ConsumingStart(); - iterator.Skip(5); - socketInput.ConsumingComplete(iterator, iterator); - - // TODO: Replace sleep with mock - Thread.Sleep(100); - Assert.Equal(true, connectionControl.Paused); - - iterator = socketInput.ConsumingStart(); - iterator.Skip(1); - socketInput.ConsumingComplete(iterator, iterator); - - // TODO: Replace sleep with mock - Thread.Sleep(100); - Assert.Equal(false, connectionControl.Paused); + mockBufferLengthControl?.Verify(b => b.Subtract(5)); } } } @@ -191,39 +164,5 @@ private async Task AwaitAsTaskAsync(SocketInput socketInput) { await socketInput; } - - private class TestConnectionControl : IConnectionControl - { - public bool Paused { get; set; } - - public void End(ProduceEndType endType) - { - throw new NotImplementedException(); - } - - public void Pause() - { - if (Paused) - { - throw new InvalidOperationException("already paused"); - } - else - { - Paused = true; - } - } - - public void Resume() - { - if (Paused) - { - Paused = false; - } - else - { - throw new InvalidOperationException("not paused"); - } - } - } } } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/project.json b/test/Microsoft.AspNetCore.Server.KestrelTests/project.json index 2078e0839..89561cace 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/project.json +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/project.json @@ -20,7 +20,8 @@ "System.Net.Http": "4.1.0-*", "System.Net.Http.WinHttpHandler": "4.0.0-*", "System.Net.Sockets": "4.1.0-*", - "System.Runtime.Handles": "4.0.1-*" + "System.Runtime.Handles": "4.0.1-*", + "moq.netcore": "4.4.0-beta8" }, "imports": [ "dnxcore50", @@ -29,7 +30,8 @@ }, "net451": { "dependencies": { - "xunit.runner.console": "2.1.0" + "xunit.runner.console": "2.1.0", + "Moq": "4.2.1312.1622" }, "frameworkAssemblies": { "System.Net.Http": "4.0.0.0" From 15fc50e8dad321e8d2b24d6549a270775ec4b2b1 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 7 Jun 2016 16:17:02 -0700 Subject: [PATCH 13/37] UvStreamHandle.ReadStop() should be idempotent --- .../Internal/Networking/UvStreamHandle.cs | 6 ++--- .../UvStreamHandleTests.cs | 26 +++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) create mode 100644 test/Microsoft.AspNetCore.Server.KestrelTests/UvStreamHandleTests.cs diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Networking/UvStreamHandle.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Networking/UvStreamHandle.cs index bc0bbc64a..1a930f604 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Networking/UvStreamHandle.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Networking/UvStreamHandle.cs @@ -105,16 +105,16 @@ public void ReadStart( } } + // UvStreamHandle.ReadStop() should be idempotent to match uv_read_stop() public void ReadStop() { - if (!_readVitality.IsAllocated) + if (_readVitality.IsAllocated) { - throw new InvalidOperationException("TODO: ReadStart must be called before ReadStop may be called"); + _readVitality.Free(); } _allocCallback = null; _readCallback = null; _readState = null; - _readVitality.Free(); _uv.read_stop(this); } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/UvStreamHandleTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/UvStreamHandleTests.cs new file mode 100644 index 000000000..51dbce5b5 --- /dev/null +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/UvStreamHandleTests.cs @@ -0,0 +1,26 @@ +using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; +using Microsoft.AspNetCore.Server.Kestrel.Internal.Networking; +using Microsoft.AspNetCore.Server.KestrelTests.TestHelpers; +using Moq; +using Xunit; + +namespace Microsoft.AspNetCore.Server.KestrelTests +{ + public class UvStreamHandleTests + { + [Fact] + public void ReadStopIsIdempotent() + { + var mockKestrelTrace = Mock.Of(); + var mockUvLoopHandle = new Mock(mockKestrelTrace).Object; + mockUvLoopHandle.Init(new MockLibuv()); + + // Need to mock UvTcpHandle instead of UvStreamHandle, since the latter lacks an Init() method + var mockUvStreamHandle = new Mock(mockKestrelTrace).Object; + mockUvStreamHandle.Init(mockUvLoopHandle, null); + + mockUvStreamHandle.ReadStop(); + mockUvStreamHandle.ReadStop(); + } + } +} From fd8edf668da1bc8152594d8ba82cb2a541bae88f Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 7 Jun 2016 17:00:59 -0700 Subject: [PATCH 14/37] Replace InlineData with MemberData to reduce duplicate code --- .../MaxInputBufferLengthTests.cs | 41 +++++++++++++------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs index e21b71484..a78df39f3 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs @@ -1,4 +1,6 @@ using System; +using System.Collections.Generic; +using System.Linq; using System.Net; using System.Net.Sockets; using System.Text; @@ -13,19 +15,34 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { public class MaxInputBufferLengthTests { + private const int _dataLength = 10 * 1024 * 1024; + + public static IEnumerable LargeUploadData + { + get + { + var maxInputBufferLengthValues = new int?[] { + 16 * 1024, + 1024 * 1024, + 5 * 1024 * 1024, + 10 * 1024 * 1024, + Int32.MaxValue, + null + }; + + var sendContentLengthHeaderValues = new[] { + true, + false + }; + + return from b in maxInputBufferLengthValues + from s in sendContentLengthHeaderValues + select new object[] { b, s, b.HasValue && b.Value < _dataLength }; + } + } + [Theory] - [InlineData(16 * 1024, true, true)] - [InlineData(16 * 1024, false, true)] - [InlineData(1024 * 1024, true, true)] - [InlineData(1024 * 1024, false, true)] - [InlineData(5 * 1024 * 1024, true, true)] - [InlineData(5 * 1024 * 1024, false, true)] - [InlineData(10 * 1024 * 1024, true, false)] - [InlineData(10 * 1024 * 1024, false, false)] - [InlineData(Int32.MaxValue, true, false)] - [InlineData(Int32.MaxValue, false, false)] - [InlineData(null, true, false)] - [InlineData(null, false, false)] + [MemberData("LargeUploadData")] public async Task LargeUpload(int? maxInputBufferLength, bool sendContentLengthHeader, bool expectPause) { // Parameters From 7cd1f9bc7f2b0325f0dc7747d443427c2daa5d41 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Wed, 8 Jun 2016 15:10:40 -0700 Subject: [PATCH 15/37] Resume() should catch UvExceptions thrown by ReadStart() --- .../Internal/Http/Connection.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs index c49718b83..3b2e0a11a 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs @@ -324,7 +324,16 @@ void IConnectionControl.Pause() void IConnectionControl.Resume() { Log.ConnectionResume(ConnectionId); - _socket.ReadStart(_allocCallback, _readCallback, this); + try + { + _socket.ReadStart(_allocCallback, _readCallback, this); + } + catch (UvException ex) + { + // ReadStart() can throw a UvException in some cases (e.g. socket is no longer connected). + // This should be treated the same as OnRead() seeing a "normalDone" condition. + _rawSocketInput.IncomingComplete(0, ex); + } } void IConnectionControl.End(ProduceEndType endType) From 10d64465fbd5acea4459565db0d035be296f2c3c Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Wed, 8 Jun 2016 17:04:50 -0700 Subject: [PATCH 16/37] Add SSL to functional tests. --- .../IWebHostPortExtensions.cs | 21 +++- .../MaxInputBufferLengthTests.cs | 90 +++++++++++++----- .../TestResources/testCert.pfx | Bin 0 -> 2483 bytes .../project.json | 15 ++- 4 files changed, 96 insertions(+), 30 deletions(-) create mode 100644 test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/TestResources/testCert.pfx diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/IWebHostPortExtensions.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/IWebHostPortExtensions.cs index 9b3509503..625a10a7a 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/IWebHostPortExtensions.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/IWebHostPortExtensions.cs @@ -10,16 +10,35 @@ namespace Microsoft.AspNetCore.Hosting { public static class IWebHostPortExtensions { + public static string GetHost(this IWebHost host) + { + return host.GetUris().First().Host; + } + public static int GetPort(this IWebHost host) { return host.GetPorts().First(); } + public static int GetPort(this IWebHost host, string scheme) + { + return host.GetUris() + .Where(u => u.Scheme.Equals(scheme, StringComparison.OrdinalIgnoreCase)) + .Select(u => u.Port) + .First(); + } + public static IEnumerable GetPorts(this IWebHost host) + { + return host.GetUris() + .Select(u => u.Port); + } + + public static IEnumerable GetUris(this IWebHost host) { return host.ServerFeatures.Get().Addresses .Select(a => a.Replace("://+", "://localhost")) - .Select(a => (new Uri(a)).Port); + .Select(a => new Uri(a)); } } } diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs index a78df39f3..047a8ae98 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs @@ -1,8 +1,11 @@ using System; using System.Collections.Generic; +using System.IO; using System.Linq; using System.Net; +using System.Net.Security; using System.Net.Sockets; +using System.Security.Authentication; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -29,24 +32,27 @@ public static IEnumerable LargeUploadData Int32.MaxValue, null }; - - var sendContentLengthHeaderValues = new[] { - true, - false - }; - - return from b in maxInputBufferLengthValues - from s in sendContentLengthHeaderValues - select new object[] { b, s, b.HasValue && b.Value < _dataLength }; + var sendContentLengthHeaderValues = new[] { true, false }; + var sslValues = new[] { true, false }; + + return from maxInputBufferLength in maxInputBufferLengthValues + from sendContentLengthHeader in sendContentLengthHeaderValues + from ssl in sslValues + select new object[] { + maxInputBufferLength, + sendContentLengthHeader, + ssl, + maxInputBufferLength.HasValue && maxInputBufferLength.Value < _dataLength + }; } } [Theory] [MemberData("LargeUploadData")] - public async Task LargeUpload(int? maxInputBufferLength, bool sendContentLengthHeader, bool expectPause) + public async Task LargeUpload(int? maxInputBufferLength, bool sendContentLengthHeader, bool ssl, bool expectPause) { // Parameters - var data = new byte[10 * 1024 * 1024]; + var data = new byte[_dataLength]; var bytesWrittenTimeout = TimeSpan.FromMilliseconds(100); var maxSendSize = 4096; @@ -59,8 +65,12 @@ public async Task LargeUpload(int? maxInputBufferLength, bool sendContentLengthH using (var host = StartWebHost(maxInputBufferLength, data, startReadingRequestBody, clientFinishedSendingRequestBody)) { - using (var socket = CreateSocketForHttpPost(host, sendContentLengthHeader ? data.Length : -1)) + var port = host.GetPort(ssl ? "https" : "http"); + using (var socket = CreateSocket(port)) + using (var stream = await CreateStreamAsync(socket, ssl, host.GetHost())) { + WritePostRequestHeaders(stream, sendContentLengthHeader ? (int?)data.Length : null); + var bytesWritten = 0; var sendTask = Task.Run(() => @@ -68,7 +78,8 @@ public async Task LargeUpload(int? maxInputBufferLength, bool sendContentLengthH while (bytesWritten < data.Length) { var size = Math.Min(data.Length - bytesWritten, maxSendSize); - bytesWritten += socket.Send(data, bytesWritten, size, SocketFlags.None); + stream.Write(data, bytesWritten, size); + bytesWritten += size; bytesWrittenEvent.Set(); } @@ -110,9 +121,11 @@ public async Task LargeUpload(int? maxInputBufferLength, bool sendContentLengthH startReadingRequestBody.Set(); } - var buffer = new byte[maxSendSize]; - var bytesRead = socket.Receive(buffer); - Assert.Contains($"bytesRead: {data.Length}", Encoding.ASCII.GetString(buffer, 0, bytesRead)); + using (var reader = new StreamReader(stream, Encoding.ASCII)) + { + var response = reader.ReadToEnd(); + Assert.Contains($"bytesRead: {data.Length}", response); + } } } } @@ -124,8 +137,10 @@ private static IWebHost StartWebHost(int? maxInputBufferLength, byte[] expectedB .UseKestrel(options => { options.MaxInputBufferLength = maxInputBufferLength; + options.UseHttps(@"TestResources/testCert.pfx", "testPassword"); }) - .UseUrls("http://127.0.0.1:0/") + .UseUrls("http://127.0.0.1:0/", "https://127.0.0.1:0/") + .UseContentRoot(Directory.GetCurrentDirectory()) .Configure(app => app.Run(async context => { startReadingRequestBody.WaitOne(); @@ -159,8 +174,7 @@ private static IWebHost StartWebHost(int? maxInputBufferLength, byte[] expectedB } await context.Response.WriteAsync($"bytesRead: {bytesRead.ToString()}"); - } - )) + })) .Build(); host.Start(); @@ -168,7 +182,7 @@ private static IWebHost StartWebHost(int? maxInputBufferLength, byte[] expectedB return host; } - private static Socket CreateSocketForHttpPost(IWebHost host, int contentLength) + private static Socket CreateSocket(int port) { var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); @@ -176,15 +190,39 @@ private static Socket CreateSocketForHttpPost(IWebHost host, int contentLength) socket.SendTimeout = 10 * 1000; socket.ReceiveTimeout = 10 * 1000; - socket.Connect(IPAddress.Loopback, host.GetPort()); - socket.Send(Encoding.ASCII.GetBytes("POST / HTTP/1.0\r\n")); - if (contentLength > -1) + socket.Connect(IPAddress.Loopback, port); + + return socket; + } + + private static void WritePostRequestHeaders(Stream stream, int? contentLength) + { + using (var writer = new StreamWriter(stream, Encoding.ASCII, bufferSize: 1024, leaveOpen: true)) { - socket.Send(Encoding.ASCII.GetBytes($"Content-Length: {contentLength}\r\n")); + writer.WriteLine("POST / HTTP/1.0"); + if (contentLength.HasValue) + { + writer.WriteLine($"Content-Length: {contentLength.Value}"); + } + writer.WriteLine(); } - socket.Send(Encoding.ASCII.GetBytes("\r\n")); + } - return socket; + private static async Task CreateStreamAsync(Socket socket, bool ssl, string targetHost) + { + var networkStream = new NetworkStream(socket); + if (ssl) + { + var sslStream = new SslStream(networkStream, leaveInnerStreamOpen: false, + userCertificateValidationCallback: (a, b, c, d) => true); + await sslStream.AuthenticateAsClientAsync(targetHost, clientCertificates: null, + enabledSslProtocols: SslProtocols.Tls11 | SslProtocols.Tls12, checkCertificateRevocation: false); + return sslStream; + } + else + { + return networkStream; + } } } } diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/TestResources/testCert.pfx b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/TestResources/testCert.pfx new file mode 100644 index 0000000000000000000000000000000000000000..7118908c2d730670c16e9f8b2c532a262c951989 GIT binary patch literal 2483 zcmaKuc|27A8pqF>IWr86E&Q@(n=B)p$ug!;QVB6xij*z;uPLG!yCz#DQB)+9G$9m9 zQU)=DWXU?*EZIwG!+0d++P@yZ4Xhoagg?p6B~|Ue7tN=Ny=UD?x#1n1MTq z#c9MHh+D#gd|(a(cN}8i91v^=GcdgW3SmA$49p~gM-dys3jVWdg8+!iVL)pz1LDE5 zSb=|GAn(@R=(Ux!MfS9@}sFu-xDd zIt2+mqSq$glwy_6UNs<2?(qERU!gJ;5j}Pp&6trxG=wi)=@k(w2+fJVnc+qvXVzy(>Om4;L|^)R`t*3nTpAmEmTl(#i!RV#a0t#u6>Q9mY`-Nmcs7$XjXT7 zUmCD`O~_j7!%R#I?cG-7C^hcH)@l?WC1vyw$FFu_(r)jhOq6p}W8sG7NO{YTy8tG4 zrb$tTkag*G?(7lfoGx$4YWui>{{@}-FB2ub=}RX{1zx?j)s-##J9|G7E1@-;7Nuln z9MQoX7FJ76+D#XXT@ZZmLZCufIdf3@OigG6m8I7!GT=7VD|>?6e!z9=eT}*E_tSn6 zl+clHCZ-kcIR#gen#LjMJW8>0QtViaQB#FhqsCb0YPYr3;jRITl@V9Aph24D?r2d` zetCyyCg<*O-u+M& zW^ptmT|}p$VAOZpmbQ1{5fK-6ytEvre#Po}6c2URn`viQAF2+e?Z~PK2&pd>7=7)I zTCYm)@3PFRu_6a6Kb)IpCzQ%e3l%O#SDA+$Pq{Dk{HCqi7z>qd{nVpebffL7h{c4( zmhXn~G+C27S3(IfC)q2KON=YwqHXEo%zc40DgWLzF{%RIdr@RcLu90qMSHf!Y}JaqP<={8_Rfe;ddR5= zKEo;^Yip&^m((#{czE{kUga3-@`*;&EwO}Jt>QdURP2P>ob^j-A!qld-0S_pm)kjs zkNo48oZnMt){W~o8g^f;4#?lRLr-T@f}wH1o~-Iq=NEVtTVEZ`vrW~!>2yh%;Bc~H zHl&OK>n@d`*e19*9#v>zZpU?I);f7}IPIfSSk#N|ujE492Itg)l!)TJ19@FE^x|p= zH16NC7OfK&|6_!AnWfTIf^YPOa&`|nbk3VR0vql6&s@y1V3QOU%(`Re+kJgrz?r9!{^wOQ4W-eng23gc}f(LxIs zH_Ls~5izbjcRQH#WH6s6hR;zn>j_R8aJ$A)6xNneu8UI-vWV8Z@HZu&WwvG5q{1ZS zdZeVf{Pv5-u281~y;aJe*x%Uv0@biMZ$vPbKj}O`(SOWQc~kJX` zXR&d4DtAe@2RH$^ z0os5*;0eIUeJi3Uh`A%44x(XzjClG8BO~-r_A}odiRuHo2-86#`mhrgN5p~<$RLY? zq(kynfFA5{v#p+EA1 z5aoe1763EQHorRm`C&ktKn(OQ1n)$Q{GZz&jRb`eDEMpl<0O#+)DMV(T7nsIzCG{QuM->B9g7Lrl2SE&gW`M!~(un|y0fIn=b^6_$ z9{zEzgYI~39xn0ZP*9qBL%fg7rg$ttt&TOmvfNNO<6FT0ZavM$Y4CYLQGIcIYv9Y& zBGPUh&QTfW;V2!)oIra@s&d968y-y}Y|ww(R$GzWS*V&)k@W0>Slem{|HdTCjm;_5 zwY*A8W3nUbemE^_f0ng$tbd<`sr?TO-_&VCw+F#7P@LkIl$1PzTBoPY1b88EIO>UO zP-NK7+g2yD3U6g3i|iA6+su>54sf_Sk0F=)1|9odnCM4u2Rs z=&Y?-V&VquSN%3FJ2~ZGweP~iLs|w=l@9yu$tj@}Dp?e-2JUsqOoswdXb=E%&0te_ zA2M+{5Hf-dqD7=yw*r@A*xkn(1IS~nfP}k}e?4Bt|9g(eph4hFX_|S6nj1&Sz9z^= zRw~<&-9d@FzTn6S*RVE{Wj5lgLJr9HLB8S9CgOm*>XA8*y4`JE;^s$=bqD#U4;e5C&x&ggKIAVL zrQ)Yd8|{>7Z(6*B&7&4&9(*vDOfHMuR-Dk1IZia*XM^EZUD^{?cWG>J>KrtElc*{K zaVl(7SN2cH4I6Q$bZOpJ8e5LKaG7p;?tJ~#+9QrTYU@f#5`Vo7cEX!szCT}iX-K^2 w#3o+=C+lQz2J+SOEzVX(eJ)e7=eicC{rr9U2VGDcdH?_b literal 0 HcmV?d00001 diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/project.json b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/project.json index 449431010..d5abedc09 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/project.json +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/project.json @@ -7,7 +7,8 @@ "Microsoft.AspNetCore.Server.Kestrel.Https": "1.0.0-*", "Microsoft.AspNetCore.Testing": "1.0.0-*", "Newtonsoft.Json": "9.0.1-beta1", - "xunit": "2.1.0" + "xunit": "2.1.0", + "Microsoft.Extensions.Logging.Console": "1.0.0-rc3-21270" }, "frameworks": { "netcoreapp1.0": { @@ -36,7 +37,15 @@ } }, "buildOptions": { - "allowUnsafe": true + "allowUnsafe": true, + "copyToOutput": { + "include": "TestResources/testCert.pfx" + } }, - "testRunner": "xunit" + "testRunner": "xunit", + "publishOptions": { + "include": [ + "TestResources/testCert.pfx" + ] + } } \ No newline at end of file From 7fa15876299b462264ff7ab845639d9a5a65bc57 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Wed, 8 Jun 2016 18:25:51 -0700 Subject: [PATCH 17/37] Add test where maxInputBufferLength is (_dataLength - 1) --- .../MaxInputBufferLengthTests.cs | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs index 047a8ae98..142c9aa70 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs @@ -24,13 +24,18 @@ public static IEnumerable LargeUploadData { get { - var maxInputBufferLengthValues = new int?[] { - 16 * 1024, - 1024 * 1024, - 5 * 1024 * 1024, - 10 * 1024 * 1024, - Int32.MaxValue, - null + var maxInputBufferLengthValues = new Tuple[] { + Tuple.Create((int?)16 * 1024, true), + Tuple.Create((int?)1024 * 1024, true), + Tuple.Create((int?)5 * 1024 * 1024, true), + + // Even though maxInputBufferLength < _dataLength, client should not be paused since the + // OS-level buffers in client and/or server will handle the overflow + Tuple.Create((int?)10 * 1024 * 1024 - 1, false), + + Tuple.Create((int?)10 * 1024 * 1024, false), + Tuple.Create((int?)Int32.MaxValue, false), + Tuple.Create((int?)null, false) }; var sendContentLengthHeaderValues = new[] { true, false }; var sslValues = new[] { true, false }; @@ -39,10 +44,10 @@ public static IEnumerable LargeUploadData from sendContentLengthHeader in sendContentLengthHeaderValues from ssl in sslValues select new object[] { - maxInputBufferLength, + maxInputBufferLength.Item1, sendContentLengthHeader, ssl, - maxInputBufferLength.HasValue && maxInputBufferLength.Value < _dataLength + maxInputBufferLength.Item2 }; } } From 9d411823f4048cc5ec89cfa910a7faf8efc04d49 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Thu, 9 Jun 2016 13:08:09 -0700 Subject: [PATCH 18/37] Extract BufferLengthControl from a private nested class to a public class --- .../Internal/Http/BufferLengthControl.cs | 67 +++++++++++++++++++ .../Internal/Http/Connection.cs | 64 ------------------ 2 files changed, 67 insertions(+), 64 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferLengthControl.cs diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferLengthControl.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferLengthControl.cs new file mode 100644 index 000000000..85307056b --- /dev/null +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferLengthControl.cs @@ -0,0 +1,67 @@ +using System.Diagnostics; + +namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http +{ + public class BufferLengthControl : IBufferLengthControl + { + private readonly int _maxLength; + private readonly IConnectionControl _connectionControl; + private readonly KestrelThread _connectionThread; + + private readonly object _lock = new object(); + + private int _length; + private bool _connectionPaused; + + public BufferLengthControl(int maxLength, IConnectionControl connectionControl, KestrelThread connectionThread) + { + _maxLength = maxLength; + _connectionControl = connectionControl; + _connectionThread = connectionThread; + } + + private int Length + { + get + { + return _length; + } + set + { + // Caller should ensure that bytes are never consumed before the producer has called Add() + Debug.Assert(value >= 0); + _length = value; + } + } + + public void Add(int count) + { + lock (_lock) + { + Length += count; + if (!_connectionPaused && Length >= _maxLength) + { + _connectionPaused = true; + _connectionThread.Post( + (connectionControl) => ((IConnectionControl)connectionControl).Pause(), + _connectionControl); + } + } + } + + public void Subtract(int count) + { + lock (_lock) + { + Length -= count; + if (_connectionPaused && Length < _maxLength) + { + _connectionPaused = false; + _connectionThread.Post( + (connectionControl) => ((IConnectionControl)connectionControl).Resume(), + _connectionControl); + } + } + } + } +} diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs index 3b2e0a11a..501f84c01 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs @@ -401,69 +401,5 @@ private enum ConnectionState Disconnecting, SocketClosed } - - private class BufferLengthControl : IBufferLengthControl - { - private readonly int _maxLength; - private readonly IConnectionControl _connectionControl; - private readonly KestrelThread _connectionThread; - - private readonly object _lock = new object(); - - private int _length; - private bool _connectionPaused; - - public BufferLengthControl(int maxLength, IConnectionControl connectionControl, KestrelThread connectionThread) - { - _maxLength = maxLength; - _connectionControl = connectionControl; - _connectionThread = connectionThread; - } - - private int Length - { - get - { - return _length; - } - set - { - // Caller should ensure that bytes are never consumed before the producer has called Add() - Debug.Assert(value >= 0); - _length = value; - } - } - - public void Add(int count) - { - lock (_lock) - { - Length += count; - if (!_connectionPaused && Length >= _maxLength) - { - _connectionPaused = true; - _connectionThread.Post( - (connectionControl) => ((IConnectionControl)connectionControl).Pause(), - _connectionControl); - } - } - } - - public void Subtract(int count) - { - lock (_lock) - { - Length -= count; - if (_connectionPaused && Length < _maxLength) - { - _connectionPaused = false; - _connectionThread.Post( - (connectionControl) => ((IConnectionControl)connectionControl).Resume(), - _connectionControl); - } - } - } - } - } } From 93cb201e24193217cc28919abdb6fb4820f6a349 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Thu, 9 Jun 2016 14:19:21 -0700 Subject: [PATCH 19/37] Fix IConnectionControl.Resume() to correctly match behavior of OnRead()+normalDone --- .../Internal/Http/Connection.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs index 501f84c01..b9cd6e436 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs @@ -328,11 +328,12 @@ void IConnectionControl.Resume() { _socket.ReadStart(_allocCallback, _readCallback, this); } - catch (UvException ex) + catch (UvException) { // ReadStart() can throw a UvException in some cases (e.g. socket is no longer connected). // This should be treated the same as OnRead() seeing a "normalDone" condition. - _rawSocketInput.IncomingComplete(0, ex); + Log.ConnectionReadFin(ConnectionId); + _rawSocketInput.IncomingComplete(0, null); } } From fa4f94c37fbaccd5b79700b0122760267af55840 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Thu, 9 Jun 2016 14:34:32 -0700 Subject: [PATCH 20/37] Doc comment for KestrelServerOptions.MaxInputBufferLength --- .../KestrelServerOptions.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs index bd2ba09d0..adc8377fb 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs @@ -8,6 +8,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel { public class KestrelServerOptions { + private int? _maxInputBufferLength = 1024 * 1024; + /// /// Gets or sets whether the Server header should be included in each response. /// @@ -17,7 +19,9 @@ public class KestrelServerOptions public IConnectionFilter ConnectionFilter { get; set; } - private int? _maxInputBufferLength = 1024 * 1024; + /// + /// Maximum number of bytes used to buffer input for each connection. + /// public int? MaxInputBufferLength { get From fea3dfe1fd04fa98665aeeab7b6925b750576c42 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Thu, 9 Jun 2016 15:00:08 -0700 Subject: [PATCH 21/37] Add test for MaxInputBufferSize=1. Add comments explaining why each size is tested. --- .../MaxInputBufferLengthTests.cs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs index 142c9aa70..3416add8b 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs @@ -25,16 +25,31 @@ public static IEnumerable LargeUploadData get { var maxInputBufferLengthValues = new Tuple[] { + // Smallest allowed buffer. Server should call pause/resume between each read. + Tuple.Create((int?)1, true), + + // Small buffer, but large enough to hold all request headers. Tuple.Create((int?)16 * 1024, true), + + // Default buffer. Tuple.Create((int?)1024 * 1024, true), + + // Larger than default, but still 5MB lower than data, so client should be paused. + // On Windows, the client is usually paused at (MaxInputBufferSize + 700,000). Tuple.Create((int?)5 * 1024 * 1024, true), // Even though maxInputBufferLength < _dataLength, client should not be paused since the - // OS-level buffers in client and/or server will handle the overflow + // OS-level buffers in client and/or server will handle the overflow. Tuple.Create((int?)10 * 1024 * 1024 - 1, false), + // Buffer is exactly the same size as data. Exposed race condition where + // IConnectionControl.Resume() was called after socket was disconnected. Tuple.Create((int?)10 * 1024 * 1024, false), + + // Largest possible buffer, should never trigger backpressure. Tuple.Create((int?)Int32.MaxValue, false), + + // Disables all code related to computing and limiting the size of the input buffer. Tuple.Create((int?)null, false) }; var sendContentLengthHeaderValues = new[] { true, false }; From 5e695700c57724cfec179defa32c1303c400b877 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Thu, 9 Jun 2016 16:33:11 -0700 Subject: [PATCH 22/37] Add default value to doc comment for KestrelServerOptions.MaxInputBufferLength. --- src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs index adc8377fb..717fb5854 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs @@ -21,6 +21,7 @@ public class KestrelServerOptions /// /// Maximum number of bytes used to buffer input for each connection. + /// Default is 1,048,576 bytes (1 MB). /// public int? MaxInputBufferLength { From d40c8b99605c30d0ee96b71d58eae089ac2a4539 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Thu, 9 Jun 2016 16:35:54 -0700 Subject: [PATCH 23/37] Add comment explaining why MaxInputBufferLength defaults to 1MB. --- src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs index 717fb5854..150e02e2b 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs @@ -8,6 +8,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel { public class KestrelServerOptions { + // Matches the default client_max_body_size in nginx. Also large enough that most requests + // should be under the limit. private int? _maxInputBufferLength = 1024 * 1024; /// From 549aab9392c2dfd35a0c0b75228a2b791882b619 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Thu, 9 Jun 2016 17:00:45 -0700 Subject: [PATCH 24/37] Reduce lock contention if count is 0 --- .../Internal/Http/BufferLengthControl.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferLengthControl.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferLengthControl.cs index 85307056b..4c3f21c65 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferLengthControl.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferLengthControl.cs @@ -36,6 +36,14 @@ private int Length public void Add(int count) { + Debug.Assert(count >= 0); + + if (count == 0) + { + // No-op and avoid taking lock to reduce contention + return; + } + lock (_lock) { Length += count; @@ -51,6 +59,14 @@ public void Add(int count) public void Subtract(int count) { + Debug.Assert(count >= 0); + + if (count == 0) + { + // No-op and avoid taking lock to reduce contention + return; + } + lock (_lock) { Length -= count; From f977e9b69713073eb4ff39e7ff40c1fe4b38a609 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Thu, 9 Jun 2016 17:41:06 -0700 Subject: [PATCH 25/37] Use Write("\r\n") instead of WriteLine() - HTTP requires "\r\n", and WriteLine() uses "\n" on Mac/Linux --- .../MaxInputBufferLengthTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs index 3416add8b..ae712071e 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs @@ -219,12 +219,12 @@ private static void WritePostRequestHeaders(Stream stream, int? contentLength) { using (var writer = new StreamWriter(stream, Encoding.ASCII, bufferSize: 1024, leaveOpen: true)) { - writer.WriteLine("POST / HTTP/1.0"); + writer.Write("POST / HTTP/1.0\r\n"); if (contentLength.HasValue) { - writer.WriteLine($"Content-Length: {contentLength.Value}"); + writer.Write($"Content-Length: {contentLength.Value}\r\n"); } - writer.WriteLine(); + writer.Write("\r\n"); } } From aaab270f30ddfd93fec73748a8787fb06e69e11e Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Thu, 9 Jun 2016 17:41:44 -0700 Subject: [PATCH 26/37] Only compute consumed length if _bufferLengthControl is not null. --- .../Internal/Http/SocketInput.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs index 8f493e4ba..342e43b7e 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs @@ -197,7 +197,6 @@ public void ConsumingComplete( { if (!consumed.IsDefault) { - var lengthConsumed = new MemoryPoolIterator(_head).GetLength(consumed); returnStart = _head; returnEnd = consumed.Block; @@ -206,7 +205,7 @@ public void ConsumingComplete( // Must call Subtract() after _head has been advanced, to avoid producer starting too early and growing // buffer beyond max length. - _bufferLengthControl?.Subtract(lengthConsumed); + _bufferLengthControl?.Subtract(new MemoryPoolIterator(returnStart).GetLength(consumed)); } if (!examined.IsDefault && From 7cdb1e8109175f4f01bbf9f1dd110d43f5b079cc Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Thu, 9 Jun 2016 18:37:59 -0700 Subject: [PATCH 27/37] Compute lengthConsumed before modifying _head or consumed - Reverts regression introduced in 584f314 --- .../Internal/Http/SocketInput.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs index 342e43b7e..3ef7fbe1a 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs @@ -197,6 +197,12 @@ public void ConsumingComplete( { if (!consumed.IsDefault) { + // Compute lengthConsumed before modifying _head or consumed + var lengthConsumed = 0; + if (_bufferLengthControl != null) + { + lengthConsumed = new MemoryPoolIterator(_head).GetLength(consumed); + } returnStart = _head; returnEnd = consumed.Block; @@ -205,7 +211,7 @@ public void ConsumingComplete( // Must call Subtract() after _head has been advanced, to avoid producer starting too early and growing // buffer beyond max length. - _bufferLengthControl?.Subtract(new MemoryPoolIterator(returnStart).GetLength(consumed)); + _bufferLengthControl?.Subtract(lengthConsumed); } if (!examined.IsDefault && From 0084e4a4d5024dab155bdf50384ecfe9325502fb Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Thu, 9 Jun 2016 18:44:41 -0700 Subject: [PATCH 28/37] Make functional test use more async --- .../MaxInputBufferLengthTests.cs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs index ae712071e..bdcee7950 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs @@ -89,16 +89,16 @@ public async Task LargeUpload(int? maxInputBufferLength, bool sendContentLengthH using (var socket = CreateSocket(port)) using (var stream = await CreateStreamAsync(socket, ssl, host.GetHost())) { - WritePostRequestHeaders(stream, sendContentLengthHeader ? (int?)data.Length : null); + await WritePostRequestHeaders(stream, sendContentLengthHeader ? (int?)data.Length : null); var bytesWritten = 0; - var sendTask = Task.Run(() => + Func sendFunc = async () => { while (bytesWritten < data.Length) { var size = Math.Min(data.Length - bytesWritten, maxSendSize); - stream.Write(data, bytesWritten, size); + await stream.WriteAsync(data, bytesWritten, size); bytesWritten += size; bytesWrittenEvent.Set(); } @@ -106,7 +106,9 @@ public async Task LargeUpload(int? maxInputBufferLength, bool sendContentLengthH Assert.Equal(data.Length, bytesWritten); socket.Shutdown(SocketShutdown.Send); clientFinishedSendingRequestBody.Set(); - }); + }; + + var sendTask = sendFunc(); if (expectPause) { @@ -215,16 +217,16 @@ private static Socket CreateSocket(int port) return socket; } - private static void WritePostRequestHeaders(Stream stream, int? contentLength) + private static async Task WritePostRequestHeaders(Stream stream, int? contentLength) { using (var writer = new StreamWriter(stream, Encoding.ASCII, bufferSize: 1024, leaveOpen: true)) { - writer.Write("POST / HTTP/1.0\r\n"); + await writer.WriteAsync("POST / HTTP/1.0\r\n"); if (contentLength.HasValue) { - writer.Write($"Content-Length: {contentLength.Value}\r\n"); + await writer.WriteAsync($"Content-Length: {contentLength.Value}\r\n"); } - writer.Write("\r\n"); + await writer.WriteAsync("\r\n"); } } From 272c6262bc53ac0fd3eac2d90111be39f3fd7a9c Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Fri, 10 Jun 2016 13:39:19 -0700 Subject: [PATCH 29/37] References to other projects should use "*" versions --- .../project.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/project.json b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/project.json index d5abedc09..51a86e1a7 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/project.json +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/project.json @@ -6,9 +6,9 @@ "Microsoft.AspNetCore.Server.Kestrel": "1.0.0-*", "Microsoft.AspNetCore.Server.Kestrel.Https": "1.0.0-*", "Microsoft.AspNetCore.Testing": "1.0.0-*", + "Microsoft.Extensions.Logging.Console": "1.0.0-*", "Newtonsoft.Json": "9.0.1-beta1", - "xunit": "2.1.0", - "Microsoft.Extensions.Logging.Console": "1.0.0-rc3-21270" + "xunit": "2.1.0" }, "frameworks": { "netcoreapp1.0": { From 27283eed66887c8ebf5ac374fec6aba5b201ec09 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Fri, 10 Jun 2016 13:50:39 -0700 Subject: [PATCH 30/37] Increase size of data sent by test client. On Linux, the client isn't paused until around 10MB after the server sends backpressure. --- .../MaxInputBufferLengthTests.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs index bdcee7950..8b038bbc4 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs @@ -18,7 +18,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { public class MaxInputBufferLengthTests { - private const int _dataLength = 10 * 1024 * 1024; + private const int _dataLength = 20 * 1024 * 1024; public static IEnumerable LargeUploadData { @@ -34,17 +34,18 @@ public static IEnumerable LargeUploadData // Default buffer. Tuple.Create((int?)1024 * 1024, true), - // Larger than default, but still 5MB lower than data, so client should be paused. - // On Windows, the client is usually paused at (MaxInputBufferSize + 700,000). + // Larger than default, but still significantly lower than data, so client should be paused. + // On Windows, the client is usually paused around (MaxInputBufferLength + 700,000). + // On Linux, the client is usually paused around (MaxInputBufferLength + 10,000,000). Tuple.Create((int?)5 * 1024 * 1024, true), // Even though maxInputBufferLength < _dataLength, client should not be paused since the // OS-level buffers in client and/or server will handle the overflow. - Tuple.Create((int?)10 * 1024 * 1024 - 1, false), + Tuple.Create((int?)_dataLength - 1, false), // Buffer is exactly the same size as data. Exposed race condition where // IConnectionControl.Resume() was called after socket was disconnected. - Tuple.Create((int?)10 * 1024 * 1024, false), + Tuple.Create((int?)_dataLength, false), // Largest possible buffer, should never trigger backpressure. Tuple.Create((int?)Int32.MaxValue, false), From 64aabce57c4ed8f632a3ab50c2c53be4b9673073 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Sun, 12 Jun 2016 11:18:01 -0700 Subject: [PATCH 31/37] Poll instead of blocking test method - Blocking causes deadlock on single-core machines --- .../MaxInputBufferLengthTests.cs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs index 8b038bbc4..bd6de5e42 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs @@ -26,8 +26,8 @@ public static IEnumerable LargeUploadData { var maxInputBufferLengthValues = new Tuple[] { // Smallest allowed buffer. Server should call pause/resume between each read. - Tuple.Create((int?)1, true), - + Tuple.Create((int?)1, true), + // Small buffer, but large enough to hold all request headers. Tuple.Create((int?)16 * 1024, true), @@ -38,7 +38,7 @@ public static IEnumerable LargeUploadData // On Windows, the client is usually paused around (MaxInputBufferLength + 700,000). // On Linux, the client is usually paused around (MaxInputBufferLength + 10,000,000). Tuple.Create((int?)5 * 1024 * 1024, true), - + // Even though maxInputBufferLength < _dataLength, client should not be paused since the // OS-level buffers in client and/or server will handle the overflow. Tuple.Create((int?)_dataLength - 1, false), @@ -75,6 +75,7 @@ public async Task LargeUpload(int? maxInputBufferLength, bool sendContentLengthH // Parameters var data = new byte[_dataLength]; var bytesWrittenTimeout = TimeSpan.FromMilliseconds(100); + var bytesWrittenPollingInterval = TimeSpan.FromMilliseconds(bytesWrittenTimeout.TotalMilliseconds / 10); var maxSendSize = 4096; // Initialize data with random bytes @@ -82,7 +83,7 @@ public async Task LargeUpload(int? maxInputBufferLength, bool sendContentLengthH var startReadingRequestBody = new ManualResetEvent(false); var clientFinishedSendingRequestBody = new ManualResetEvent(false); - var bytesWrittenEvent = new AutoResetEvent(false); + var lastBytesWritten = DateTime.MaxValue; using (var host = StartWebHost(maxInputBufferLength, data, startReadingRequestBody, clientFinishedSendingRequestBody)) { @@ -101,7 +102,7 @@ public async Task LargeUpload(int? maxInputBufferLength, bool sendContentLengthH var size = Math.Min(data.Length - bytesWritten, maxSendSize); await stream.WriteAsync(data, bytesWritten, size); bytesWritten += size; - bytesWrittenEvent.Set(); + lastBytesWritten = DateTime.Now; } Assert.Equal(data.Length, bytesWritten); @@ -115,7 +116,10 @@ public async Task LargeUpload(int? maxInputBufferLength, bool sendContentLengthH { // Block until the send task has gone a while without writing bytes, which likely means // the server input buffer is full. - while (bytesWrittenEvent.WaitOne(bytesWrittenTimeout)) { } + while ((DateTime.Now - lastBytesWritten) < bytesWrittenTimeout) + { + await Task.Delay(bytesWrittenPollingInterval); + } // Verify the number of bytes written before the client was paused. // From b17b94e5acc7c66117b73f0dc3b3de0b60ef50eb Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Mon, 13 Jun 2016 14:12:34 -0700 Subject: [PATCH 32/37] Improve doc comment for MaxInputBufferLength. --- src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs index 150e02e2b..027b79155 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs @@ -23,7 +23,7 @@ public class KestrelServerOptions /// /// Maximum number of bytes used to buffer input for each connection. - /// Default is 1,048,576 bytes (1 MB). + /// Default is 1,048,576 bytes (1 MB). If value is null, the number of bytes is unlimited. /// public int? MaxInputBufferLength { From 1469b42e717f379ec363d80d15a4568d6a5a9b5e Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Mon, 13 Jun 2016 14:14:20 -0700 Subject: [PATCH 33/37] Further improve doc comment for MaxInputBufferLength. --- src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs index 027b79155..fe51fed12 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs @@ -23,7 +23,7 @@ public class KestrelServerOptions /// /// Maximum number of bytes used to buffer input for each connection. - /// Default is 1,048,576 bytes (1 MB). If value is null, the number of bytes is unlimited. + /// Default is 1,048,576 bytes (1 MB). If value is null, the length of the input buffer is unlimited. /// public int? MaxInputBufferLength { From 3fa77f9ca1a6a6a6a0d979bfe4a7df78e2a27775 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Mon, 13 Jun 2016 15:08:59 -0700 Subject: [PATCH 34/37] Change type of MaxInputBufferLength from int? to long? --- .../Internal/Http/BufferLengthControl.cs | 8 +++---- .../KestrelServerOptions.cs | 4 ++-- .../MaxInputBufferLengthTests.cs | 22 +++++++++---------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferLengthControl.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferLengthControl.cs index 4c3f21c65..baa611cc8 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferLengthControl.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferLengthControl.cs @@ -4,23 +4,23 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http { public class BufferLengthControl : IBufferLengthControl { - private readonly int _maxLength; + private readonly long _maxLength; private readonly IConnectionControl _connectionControl; private readonly KestrelThread _connectionThread; private readonly object _lock = new object(); - private int _length; + private long _length; private bool _connectionPaused; - public BufferLengthControl(int maxLength, IConnectionControl connectionControl, KestrelThread connectionThread) + public BufferLengthControl(long maxLength, IConnectionControl connectionControl, KestrelThread connectionThread) { _maxLength = maxLength; _connectionControl = connectionControl; _connectionThread = connectionThread; } - private int Length + private long Length { get { diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs index fe51fed12..ae5b32631 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs @@ -10,7 +10,7 @@ public class KestrelServerOptions { // Matches the default client_max_body_size in nginx. Also large enough that most requests // should be under the limit. - private int? _maxInputBufferLength = 1024 * 1024; + private long? _maxInputBufferLength = 1024 * 1024; /// /// Gets or sets whether the Server header should be included in each response. @@ -25,7 +25,7 @@ public class KestrelServerOptions /// Maximum number of bytes used to buffer input for each connection. /// Default is 1,048,576 bytes (1 MB). If value is null, the length of the input buffer is unlimited. /// - public int? MaxInputBufferLength + public long? MaxInputBufferLength { get { diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs index bd6de5e42..cee92a9f0 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs @@ -24,34 +24,34 @@ public static IEnumerable LargeUploadData { get { - var maxInputBufferLengthValues = new Tuple[] { + var maxInputBufferLengthValues = new Tuple[] { // Smallest allowed buffer. Server should call pause/resume between each read. - Tuple.Create((int?)1, true), + Tuple.Create((long?)1, true), // Small buffer, but large enough to hold all request headers. - Tuple.Create((int?)16 * 1024, true), + Tuple.Create((long?)16 * 1024, true), // Default buffer. - Tuple.Create((int?)1024 * 1024, true), + Tuple.Create((long?)1024 * 1024, true), // Larger than default, but still significantly lower than data, so client should be paused. // On Windows, the client is usually paused around (MaxInputBufferLength + 700,000). // On Linux, the client is usually paused around (MaxInputBufferLength + 10,000,000). - Tuple.Create((int?)5 * 1024 * 1024, true), + Tuple.Create((long?)5 * 1024 * 1024, true), // Even though maxInputBufferLength < _dataLength, client should not be paused since the // OS-level buffers in client and/or server will handle the overflow. - Tuple.Create((int?)_dataLength - 1, false), + Tuple.Create((long?)_dataLength - 1, false), // Buffer is exactly the same size as data. Exposed race condition where // IConnectionControl.Resume() was called after socket was disconnected. - Tuple.Create((int?)_dataLength, false), + Tuple.Create((long?)_dataLength, false), // Largest possible buffer, should never trigger backpressure. - Tuple.Create((int?)Int32.MaxValue, false), + Tuple.Create((long?)long.MaxValue, false), // Disables all code related to computing and limiting the size of the input buffer. - Tuple.Create((int?)null, false) + Tuple.Create((long?)null, false) }; var sendContentLengthHeaderValues = new[] { true, false }; var sslValues = new[] { true, false }; @@ -70,7 +70,7 @@ from ssl in sslValues [Theory] [MemberData("LargeUploadData")] - public async Task LargeUpload(int? maxInputBufferLength, bool sendContentLengthHeader, bool ssl, bool expectPause) + public async Task LargeUpload(long? maxInputBufferLength, bool sendContentLengthHeader, bool ssl, bool expectPause) { // Parameters var data = new byte[_dataLength]; @@ -157,7 +157,7 @@ public async Task LargeUpload(int? maxInputBufferLength, bool sendContentLengthH } } - private static IWebHost StartWebHost(int? maxInputBufferLength, byte[] expectedBody, ManualResetEvent startReadingRequestBody, + private static IWebHost StartWebHost(long? maxInputBufferLength, byte[] expectedBody, ManualResetEvent startReadingRequestBody, ManualResetEvent clientFinishedSendingRequestBody) { var host = new WebHostBuilder() From 342f26b8560689ca98e27b6d00121c3cd7714787 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Mon, 13 Jun 2016 16:32:15 -0700 Subject: [PATCH 35/37] Rename "MaxInputBufferLength" to "MaxRequestBufferSize" - Also rename "Length" to "Size" in the implementation classes --- .../Filter/Internal/FilteredStreamAdapter.cs | 4 +-- ...rLengthControl.cs => BufferSizeControl.cs} | 24 ++++++------- .../Internal/Http/Connection.cs | 10 +++--- ...LengthControl.cs => IBufferSizeControl.cs} | 2 +- .../Internal/Http/SocketInput.cs | 14 ++++---- .../KestrelServerOptions.cs | 12 +++---- .../MaxInputBufferLengthTests.cs | 34 +++++++++---------- .../KestrelServerOptionsTests.cs | 14 ++++---- .../SocketInputTests.cs | 28 +++++++-------- 9 files changed, 71 insertions(+), 71 deletions(-) rename src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/{BufferLengthControl.cs => BufferSizeControl.cs} (76%) rename src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/{IBufferLengthControl.cs => IBufferSizeControl.cs} (85%) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Filter/Internal/FilteredStreamAdapter.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Filter/Internal/FilteredStreamAdapter.cs index 8a3712a79..e17a988b4 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Filter/Internal/FilteredStreamAdapter.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Filter/Internal/FilteredStreamAdapter.cs @@ -25,9 +25,9 @@ public FilteredStreamAdapter( MemoryPool memory, IKestrelTrace logger, IThreadPool threadPool, - IBufferLengthControl bufferLengthControl) + IBufferSizeControl bufferSizeControl) { - SocketInput = new SocketInput(memory, threadPool, bufferLengthControl); + SocketInput = new SocketInput(memory, threadPool, bufferSizeControl); SocketOutput = new StreamSocketOutput(connectionId, filteredStream, memory, logger); _connectionId = connectionId; diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferLengthControl.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferSizeControl.cs similarity index 76% rename from src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferLengthControl.cs rename to src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferSizeControl.cs index baa611cc8..7fbec15ec 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferLengthControl.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/BufferSizeControl.cs @@ -2,35 +2,35 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http { - public class BufferLengthControl : IBufferLengthControl + public class BufferSizeControl : IBufferSizeControl { - private readonly long _maxLength; + private readonly long _maxSize; private readonly IConnectionControl _connectionControl; private readonly KestrelThread _connectionThread; private readonly object _lock = new object(); - private long _length; + private long _size; private bool _connectionPaused; - public BufferLengthControl(long maxLength, IConnectionControl connectionControl, KestrelThread connectionThread) + public BufferSizeControl(long maxSize, IConnectionControl connectionControl, KestrelThread connectionThread) { - _maxLength = maxLength; + _maxSize = maxSize; _connectionControl = connectionControl; _connectionThread = connectionThread; } - private long Length + private long Size { get { - return _length; + return _size; } set { // Caller should ensure that bytes are never consumed before the producer has called Add() Debug.Assert(value >= 0); - _length = value; + _size = value; } } @@ -46,8 +46,8 @@ public void Add(int count) lock (_lock) { - Length += count; - if (!_connectionPaused && Length >= _maxLength) + Size += count; + if (!_connectionPaused && Size >= _maxSize) { _connectionPaused = true; _connectionThread.Post( @@ -69,8 +69,8 @@ public void Subtract(int count) lock (_lock) { - Length -= count; - if (_connectionPaused && Length < _maxLength) + Size -= count; + if (_connectionPaused && Size < _maxSize) { _connectionPaused = false; _connectionThread.Post( diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs index b9cd6e436..f3eeac1bd 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs @@ -42,7 +42,7 @@ public class Connection : ConnectionContext, IConnectionControl private ConnectionState _connectionState; private TaskCompletionSource _socketClosedTcs; - private BufferLengthControl _bufferLengthControl; + private BufferSizeControl _bufferSizeControl; public Connection(ListenerContext context, UvStreamHandle socket) : base(context) { @@ -52,12 +52,12 @@ public Connection(ListenerContext context, UvStreamHandle socket) : base(context ConnectionId = GenerateConnectionId(Interlocked.Increment(ref _lastConnectionId)); - if (ServerOptions.MaxInputBufferLength.HasValue) + if (ServerOptions.MaxRequestBufferSize.HasValue) { - _bufferLengthControl = new BufferLengthControl(ServerOptions.MaxInputBufferLength.Value, this, Thread); + _bufferSizeControl = new BufferSizeControl(ServerOptions.MaxRequestBufferSize.Value, this, Thread); } - _rawSocketInput = new SocketInput(Memory, ThreadPool, _bufferLengthControl); + _rawSocketInput = new SocketInput(Memory, ThreadPool, _bufferSizeControl); _rawSocketOutput = new SocketOutput(Thread, _socket, Memory, this, ConnectionId, Log, ThreadPool, WriteReqPool); } @@ -225,7 +225,7 @@ private void ApplyConnectionFilter() if (_filterContext.Connection != _libuvStream) { - _filteredStreamAdapter = new FilteredStreamAdapter(ConnectionId, _filterContext.Connection, Memory, Log, ThreadPool, _bufferLengthControl); + _filteredStreamAdapter = new FilteredStreamAdapter(ConnectionId, _filterContext.Connection, Memory, Log, ThreadPool, _bufferSizeControl); SocketInput = _filteredStreamAdapter.SocketInput; SocketOutput = _filteredStreamAdapter.SocketOutput; diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IBufferLengthControl.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IBufferSizeControl.cs similarity index 85% rename from src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IBufferLengthControl.cs rename to src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IBufferSizeControl.cs index 28fa24c4b..3d05cfe4f 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IBufferLengthControl.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IBufferSizeControl.cs @@ -5,7 +5,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http { - public interface IBufferLengthControl + public interface IBufferSizeControl { void Add(int count); void Subtract(int count); diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs index 3ef7fbe1a..e093df9c8 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketInput.cs @@ -18,7 +18,7 @@ public class SocketInput : ICriticalNotifyCompletion, IDisposable private readonly MemoryPool _memory; private readonly IThreadPool _threadPool; - private readonly IBufferLengthControl _bufferLengthControl; + private readonly IBufferSizeControl _bufferSizeControl; private readonly ManualResetEventSlim _manualResetEvent = new ManualResetEventSlim(false, 0); private Action _awaitableState; @@ -33,11 +33,11 @@ public class SocketInput : ICriticalNotifyCompletion, IDisposable private bool _consuming; private bool _disposed; - public SocketInput(MemoryPool memory, IThreadPool threadPool, IBufferLengthControl bufferLengthControl = null) + public SocketInput(MemoryPool memory, IThreadPool threadPool, IBufferSizeControl bufferSizeControl = null) { _memory = memory; _threadPool = threadPool; - _bufferLengthControl = bufferLengthControl; + _bufferSizeControl = bufferSizeControl; _awaitableState = _awaitableIsNotCompleted; } @@ -66,7 +66,7 @@ public void IncomingData(byte[] buffer, int offset, int count) lock (_sync) { // Must call Add() before bytes are available to consumer, to ensure that Length is >= 0 - _bufferLengthControl?.Add(count); + _bufferSizeControl?.Add(count); if (count > 0) { @@ -99,7 +99,7 @@ public void IncomingComplete(int count, Exception error) lock (_sync) { // Must call Add() before bytes are available to consumer, to ensure that Length is >= 0 - _bufferLengthControl?.Add(count); + _bufferSizeControl?.Add(count); if (_pinned != null) { @@ -199,7 +199,7 @@ public void ConsumingComplete( { // Compute lengthConsumed before modifying _head or consumed var lengthConsumed = 0; - if (_bufferLengthControl != null) + if (_bufferSizeControl != null) { lengthConsumed = new MemoryPoolIterator(_head).GetLength(consumed); } @@ -211,7 +211,7 @@ public void ConsumingComplete( // Must call Subtract() after _head has been advanced, to avoid producer starting too early and growing // buffer beyond max length. - _bufferLengthControl?.Subtract(lengthConsumed); + _bufferSizeControl?.Subtract(lengthConsumed); } if (!examined.IsDefault && diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs index ae5b32631..3591408bb 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServerOptions.cs @@ -10,7 +10,7 @@ public class KestrelServerOptions { // Matches the default client_max_body_size in nginx. Also large enough that most requests // should be under the limit. - private long? _maxInputBufferLength = 1024 * 1024; + private long? _maxRequestBufferSize = 1024 * 1024; /// /// Gets or sets whether the Server header should be included in each response. @@ -22,14 +22,14 @@ public class KestrelServerOptions public IConnectionFilter ConnectionFilter { get; set; } /// - /// Maximum number of bytes used to buffer input for each connection. - /// Default is 1,048,576 bytes (1 MB). If value is null, the length of the input buffer is unlimited. + /// Maximum size of the request buffer. Default is 1,048,576 bytes (1 MB). + /// If value is null, the size of the request buffer is unlimited. /// - public long? MaxInputBufferLength + public long? MaxRequestBufferSize { get { - return _maxInputBufferLength; + return _maxRequestBufferSize; } set { @@ -37,7 +37,7 @@ public long? MaxInputBufferLength { throw new ArgumentOutOfRangeException("value", "Value must be null or a positive integer."); } - _maxInputBufferLength = value; + _maxRequestBufferSize = value; } } diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs index cee92a9f0..0eb78ccfe 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs @@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { - public class MaxInputBufferLengthTests + public class MaxRequestBufferSizeTests { private const int _dataLength = 20 * 1024 * 1024; @@ -24,7 +24,7 @@ public static IEnumerable LargeUploadData { get { - var maxInputBufferLengthValues = new Tuple[] { + var maxRequestBufferSizeValues = new Tuple[] { // Smallest allowed buffer. Server should call pause/resume between each read. Tuple.Create((long?)1, true), @@ -35,11 +35,11 @@ public static IEnumerable LargeUploadData Tuple.Create((long?)1024 * 1024, true), // Larger than default, but still significantly lower than data, so client should be paused. - // On Windows, the client is usually paused around (MaxInputBufferLength + 700,000). - // On Linux, the client is usually paused around (MaxInputBufferLength + 10,000,000). + // On Windows, the client is usually paused around (MaxRequestBufferSize + 700,000). + // On Linux, the client is usually paused around (MaxRequestBufferSize + 10,000,000). Tuple.Create((long?)5 * 1024 * 1024, true), - // Even though maxInputBufferLength < _dataLength, client should not be paused since the + // Even though maxRequestBufferSize < _dataLength, client should not be paused since the // OS-level buffers in client and/or server will handle the overflow. Tuple.Create((long?)_dataLength - 1, false), @@ -56,21 +56,21 @@ public static IEnumerable LargeUploadData var sendContentLengthHeaderValues = new[] { true, false }; var sslValues = new[] { true, false }; - return from maxInputBufferLength in maxInputBufferLengthValues + return from maxRequestBufferSize in maxRequestBufferSizeValues from sendContentLengthHeader in sendContentLengthHeaderValues from ssl in sslValues select new object[] { - maxInputBufferLength.Item1, + maxRequestBufferSize.Item1, sendContentLengthHeader, ssl, - maxInputBufferLength.Item2 + maxRequestBufferSize.Item2 }; } } [Theory] [MemberData("LargeUploadData")] - public async Task LargeUpload(long? maxInputBufferLength, bool sendContentLengthHeader, bool ssl, bool expectPause) + public async Task LargeUpload(long? maxRequestBufferSize, bool sendContentLengthHeader, bool ssl, bool expectPause) { // Parameters var data = new byte[_dataLength]; @@ -85,7 +85,7 @@ public async Task LargeUpload(long? maxInputBufferLength, bool sendContentLength var clientFinishedSendingRequestBody = new ManualResetEvent(false); var lastBytesWritten = DateTime.MaxValue; - using (var host = StartWebHost(maxInputBufferLength, data, startReadingRequestBody, clientFinishedSendingRequestBody)) + using (var host = StartWebHost(maxRequestBufferSize, data, startReadingRequestBody, clientFinishedSendingRequestBody)) { var port = host.GetPort(ssl ? "https" : "http"); using (var socket = CreateSocket(port)) @@ -123,15 +123,15 @@ public async Task LargeUpload(long? maxInputBufferLength, bool sendContentLength // Verify the number of bytes written before the client was paused. // - // The minimum is (maxInputBufferLength - maxSendSize + 1), since if bytesWritten is - // (maxInputBufferLength - maxSendSize) or smaller, the client should be able to + // The minimum is (maxRequestBufferSize - maxSendSize + 1), since if bytesWritten is + // (maxRequestBufferSize - maxSendSize) or smaller, the client should be able to // complete another send. // // The maximum is harder to determine, since there can be OS-level buffers in both the client - // and server, which allow the client to send more than maxInputBufferLength before getting + // and server, which allow the client to send more than maxRequestBufferSize before getting // paused. We assume the combined buffers are smaller than the difference between - // data.Length and maxInputBufferLength. - Assert.InRange(bytesWritten, maxInputBufferLength.Value - maxSendSize + 1, data.Length - 1); + // data.Length and maxRequestBufferSize. + Assert.InRange(bytesWritten, maxRequestBufferSize.Value - maxSendSize + 1, data.Length - 1); // Tell server to start reading request body startReadingRequestBody.Set(); @@ -157,13 +157,13 @@ public async Task LargeUpload(long? maxInputBufferLength, bool sendContentLength } } - private static IWebHost StartWebHost(long? maxInputBufferLength, byte[] expectedBody, ManualResetEvent startReadingRequestBody, + private static IWebHost StartWebHost(long? maxRequestBufferSize, byte[] expectedBody, ManualResetEvent startReadingRequestBody, ManualResetEvent clientFinishedSendingRequestBody) { var host = new WebHostBuilder() .UseKestrel(options => { - options.MaxInputBufferLength = maxInputBufferLength; + options.MaxRequestBufferSize = maxRequestBufferSize; options.UseHttps(@"TestResources/testCert.pfx", "testPassword"); }) .UseUrls("http://127.0.0.1:0/", "https://127.0.0.1:0/") diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/KestrelServerOptionsTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/KestrelServerOptionsTests.cs index 89825d4a5..9b05072c2 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/KestrelServerOptionsTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/KestrelServerOptionsTests.cs @@ -12,30 +12,30 @@ namespace Microsoft.AspNetCore.Server.KestrelTests public class KestrelServerInformationTests { [Fact] - public void MaxInputBufferLengthDefault() + public void MaxRequestBufferSizeDefault() { - Assert.Equal(1024 * 1024, (new KestrelServerOptions()).MaxInputBufferLength); + Assert.Equal(1024 * 1024, (new KestrelServerOptions()).MaxRequestBufferSize); } [Theory] [InlineData(-1)] [InlineData(0)] - public void MaxInputBufferInvalid(int value) + public void MaxRequestBufferSizeInvalid(int value) { Assert.Throws(() => { - (new KestrelServerOptions()).MaxInputBufferLength = value; + (new KestrelServerOptions()).MaxRequestBufferSize = value; }); } [Theory] [InlineData(null)] [InlineData(1)] - public void MaxInputBufferValid(int? value) + public void MaxRequestBufferSizeValid(int? value) { var o = new KestrelServerOptions(); - o.MaxInputBufferLength = value; - Assert.Equal(value, o.MaxInputBufferLength); + o.MaxRequestBufferSize = value; + Assert.Equal(value, o.MaxRequestBufferSize); } [Fact] diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/SocketInputTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/SocketInputTests.cs index 66dad80d7..ecef0c153 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/SocketInputTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/SocketInputTests.cs @@ -14,50 +14,50 @@ namespace Microsoft.AspNetCore.Server.KestrelTests { public class SocketInputTests { - public static readonly TheoryData> MockBufferLengthControlData = - new TheoryData>() { new Mock(), null }; + public static readonly TheoryData> MockBufferSizeControlData = + new TheoryData>() { new Mock(), null }; [Theory] - [MemberData("MockBufferLengthControlData")] - public void IncomingDataCallsBufferLengthControlAdd(Mock mockBufferLengthControl) + [MemberData("MockBufferSizeControlData")] + public void IncomingDataCallsBufferSizeControlAdd(Mock mockBufferSizeControl) { using (var memory = new MemoryPool()) - using (var socketInput = new SocketInput(memory, null, mockBufferLengthControl?.Object)) + using (var socketInput = new SocketInput(memory, null, mockBufferSizeControl?.Object)) { socketInput.IncomingData(new byte[5], 0, 5); - mockBufferLengthControl?.Verify(b => b.Add(5)); + mockBufferSizeControl?.Verify(b => b.Add(5)); } } [Theory] - [MemberData("MockBufferLengthControlData")] - public void IncomingCompleteCallsBufferLengthControlAdd(Mock mockBufferLengthControl) + [MemberData("MockBufferSizeControlData")] + public void IncomingCompleteCallsBufferSizeControlAdd(Mock mockBufferSizeControl) { using (var memory = new MemoryPool()) - using (var socketInput = new SocketInput(memory, null, mockBufferLengthControl?.Object)) + using (var socketInput = new SocketInput(memory, null, mockBufferSizeControl?.Object)) { socketInput.IncomingComplete(5, null); - mockBufferLengthControl?.Verify(b => b.Add(5)); + mockBufferSizeControl?.Verify(b => b.Add(5)); } } [Theory] - [MemberData("MockBufferLengthControlData")] - public void ConsumingCompleteCallsBufferLengthControlSubtract(Mock mockBufferLengthControl) + [MemberData("MockBufferSizeControlData")] + public void ConsumingCompleteCallsBufferSizeControlSubtract(Mock mockBufferSizeControl) { using (var kestrelEngine = new KestrelEngine(new MockLibuv(), new TestServiceContext())) { kestrelEngine.Start(1); using (var memory = new MemoryPool()) - using (var socketInput = new SocketInput(memory, null, mockBufferLengthControl?.Object)) + using (var socketInput = new SocketInput(memory, null, mockBufferSizeControl?.Object)) { socketInput.IncomingData(new byte[20], 0, 20); var iterator = socketInput.ConsumingStart(); iterator.Skip(5); socketInput.ConsumingComplete(iterator, iterator); - mockBufferLengthControl?.Verify(b => b.Subtract(5)); + mockBufferSizeControl?.Verify(b => b.Subtract(5)); } } } From c9e7f9b09c2ddaf521c92fe66cf70eb0cc8a2cfd Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Mon, 13 Jun 2016 17:06:04 -0700 Subject: [PATCH 36/37] Make test more robust, by continuing to wait if client has not yet uploaded the expected number of bytes. - If test fails, it may now wait forever instead of throwing a nice assert. - However, this change should make the test more robust on slow machines. --- .../MaxInputBufferLengthTests.cs | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs index 0eb78ccfe..70bbf876f 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs @@ -114,24 +114,32 @@ public async Task LargeUpload(long? maxRequestBufferSize, bool sendContentLength if (expectPause) { - // Block until the send task has gone a while without writing bytes, which likely means - // the server input buffer is full. - while ((DateTime.Now - lastBytesWritten) < bytesWrittenTimeout) - { - await Task.Delay(bytesWrittenPollingInterval); - } - - // Verify the number of bytes written before the client was paused. - // // The minimum is (maxRequestBufferSize - maxSendSize + 1), since if bytesWritten is // (maxRequestBufferSize - maxSendSize) or smaller, the client should be able to // complete another send. - // + var minimumExpectedBytesWritten = maxRequestBufferSize.Value - maxSendSize + 1; + // The maximum is harder to determine, since there can be OS-level buffers in both the client // and server, which allow the client to send more than maxRequestBufferSize before getting // paused. We assume the combined buffers are smaller than the difference between // data.Length and maxRequestBufferSize. - Assert.InRange(bytesWritten, maxRequestBufferSize.Value - maxSendSize + 1, data.Length - 1); + var maximumExpectedBytesWritten = data.Length - 1; + + // Block until the send task has gone a while without writing bytes AND + // the bytes written exceeds the minimum expected. This indicates the server buffer + // is full. + // + // If the send task is paused before the expected number of bytes have been + // written, keep waiting since the pause may have been caused by something else + // like a slow machine. + while ((DateTime.Now - lastBytesWritten) < bytesWrittenTimeout || + bytesWritten < minimumExpectedBytesWritten) + { + await Task.Delay(bytesWrittenPollingInterval); + } + + // Verify the number of bytes written before the client was paused. + Assert.InRange(bytesWritten, minimumExpectedBytesWritten, maximumExpectedBytesWritten); // Tell server to start reading request body startReadingRequestBody.Set(); From ed94ef8263435a0be2aeff06335ea0b272c02305 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Mon, 13 Jun 2016 17:12:51 -0700 Subject: [PATCH 37/37] Rename file to match class name. --- ...{MaxInputBufferLengthTests.cs => MaxRequestBufferSizeTests.cs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/{MaxInputBufferLengthTests.cs => MaxRequestBufferSizeTests.cs} (100%) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestBufferSizeTests.cs similarity index 100% rename from test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxInputBufferLengthTests.cs rename to test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestBufferSizeTests.cs