From 4dae9d45193ad41501f9e5c38ab297365370bc47 Mon Sep 17 00:00:00 2001 From: Chris Ross <chrross@microsoft.com> Date: Thu, 5 May 2022 10:46:58 -0700 Subject: [PATCH] Allow HttpSys zero-byte reads #41305 (#41500) --- .../src/RequestProcessing/RequestStream.cs | 18 ++++++-- .../RequestStreamAsyncResult.cs | 28 ++++------- .../test/FunctionalTests/RequestBodyTests.cs | 46 ++++++++++++++++++- 3 files changed, 68 insertions(+), 24 deletions(-) diff --git a/src/Servers/HttpSys/src/RequestProcessing/RequestStream.cs b/src/Servers/HttpSys/src/RequestProcessing/RequestStream.cs index b4de879f24b..d1971c53b02 100644 --- a/src/Servers/HttpSys/src/RequestProcessing/RequestStream.cs +++ b/src/Servers/HttpSys/src/RequestProcessing/RequestStream.cs @@ -102,11 +102,11 @@ namespace Microsoft.AspNetCore.Server.HttpSys { throw new ArgumentNullException(nameof(buffer)); } - if (offset < 0 || offset > buffer.Length) + if ((uint)offset > (uint)buffer.Length) { throw new ArgumentOutOfRangeException(nameof(offset), offset, string.Empty); } - if (size <= 0 || size > buffer.Length - offset) + if ((uint)size > (uint)(buffer.Length - offset)) { throw new ArgumentOutOfRangeException(nameof(size), size, string.Empty); } @@ -163,7 +163,14 @@ namespace Microsoft.AspNetCore.Server.HttpSys dataRead += extraDataRead; } - if (statusCode != UnsafeNclNativeMethods.ErrorCodes.ERROR_SUCCESS && statusCode != UnsafeNclNativeMethods.ErrorCodes.ERROR_HANDLE_EOF) + + // Zero-byte reads + if (statusCode == UnsafeNclNativeMethods.ErrorCodes.ERROR_MORE_DATA && size == 0) + { + // extraDataRead returns 1 to let us know there's data available. Don't count it against the request body size yet. + dataRead = 0; + } + else if (statusCode != UnsafeNclNativeMethods.ErrorCodes.ERROR_SUCCESS && statusCode != UnsafeNclNativeMethods.ErrorCodes.ERROR_HANDLE_EOF) { Exception exception = new IOException(string.Empty, new HttpSysException((int)statusCode)); Log.ErrorWhileRead(Logger, exception); @@ -183,7 +190,8 @@ namespace Microsoft.AspNetCore.Server.HttpSys internal void UpdateAfterRead(uint statusCode, uint dataRead) { - if (statusCode == UnsafeNclNativeMethods.ErrorCodes.ERROR_HANDLE_EOF || dataRead == 0) + if (statusCode == UnsafeNclNativeMethods.ErrorCodes.ERROR_HANDLE_EOF + || statusCode != UnsafeNclNativeMethods.ErrorCodes.ERROR_MORE_DATA && dataRead == 0) { Dispose(); } @@ -244,7 +252,7 @@ namespace Microsoft.AspNetCore.Server.HttpSys cancellationRegistration = RequestContext.RegisterForCancellation(cancellationToken); } - asyncResult = new RequestStreamAsyncResult(this, null, null, buffer, offset, dataRead, cancellationRegistration); + asyncResult = new RequestStreamAsyncResult(this, null, null, buffer, offset, size, dataRead, cancellationRegistration); uint bytesReturned; try diff --git a/src/Servers/HttpSys/src/RequestProcessing/RequestStreamAsyncResult.cs b/src/Servers/HttpSys/src/RequestProcessing/RequestStreamAsyncResult.cs index e5fbc69ba72..e51128e9989 100644 --- a/src/Servers/HttpSys/src/RequestProcessing/RequestStreamAsyncResult.cs +++ b/src/Servers/HttpSys/src/RequestProcessing/RequestStreamAsyncResult.cs @@ -17,38 +17,24 @@ namespace Microsoft.AspNetCore.Server.HttpSys private readonly SafeNativeOverlapped? _overlapped; private readonly IntPtr _pinnedBuffer; + private readonly int _size; private readonly uint _dataAlreadyRead; private readonly TaskCompletionSource<int> _tcs; private readonly RequestStream _requestStream; private readonly AsyncCallback? _callback; private readonly CancellationTokenRegistration _cancellationRegistration; - internal RequestStreamAsyncResult(RequestStream requestStream, object? userState, AsyncCallback? callback) + internal RequestStreamAsyncResult(RequestStream requestStream, object? userState, AsyncCallback? callback, byte[] buffer, int offset, int size, uint dataAlreadyRead, CancellationTokenRegistration cancellationRegistration) { _requestStream = requestStream; _tcs = new TaskCompletionSource<int>(userState); _callback = callback; - } - - internal RequestStreamAsyncResult(RequestStream requestStream, object? userState, AsyncCallback? callback, uint dataAlreadyRead) - : this(requestStream, userState, callback) - { - _dataAlreadyRead = dataAlreadyRead; - } - - internal RequestStreamAsyncResult(RequestStream requestStream, object? userState, AsyncCallback? callback, byte[] buffer, int offset, uint dataAlreadyRead) - : this(requestStream, userState, callback, buffer, offset, dataAlreadyRead, new CancellationTokenRegistration()) - { - } - - internal RequestStreamAsyncResult(RequestStream requestStream, object? userState, AsyncCallback? callback, byte[] buffer, int offset, uint dataAlreadyRead, CancellationTokenRegistration cancellationRegistration) - : this(requestStream, userState, callback) - { _dataAlreadyRead = dataAlreadyRead; var boundHandle = requestStream.RequestContext.Server.RequestQueue.BoundHandle; _overlapped = new SafeNativeOverlapped(boundHandle, boundHandle.AllocateNativeOverlapped(IOCallback, this, buffer)); _pinnedBuffer = (Marshal.UnsafeAddrOfPinnedArrayElement(buffer, offset)); + _size = size; _cancellationRegistration = cancellationRegistration; } @@ -87,7 +73,13 @@ namespace Microsoft.AspNetCore.Server.HttpSys { try { - if (errorCode != UnsafeNclNativeMethods.ErrorCodes.ERROR_SUCCESS && errorCode != UnsafeNclNativeMethods.ErrorCodes.ERROR_HANDLE_EOF) + // Zero-byte reads + if (errorCode == UnsafeNclNativeMethods.ErrorCodes.ERROR_MORE_DATA && asyncResult._size == 0) + { + // numBytes returns 1 to let us know there's data available. Don't count it against the request body size yet. + asyncResult.Complete(0, errorCode); + } + else if (errorCode != UnsafeNclNativeMethods.ErrorCodes.ERROR_SUCCESS && errorCode != UnsafeNclNativeMethods.ErrorCodes.ERROR_HANDLE_EOF) { asyncResult.Fail(new IOException(string.Empty, new HttpSysException((int)errorCode))); } diff --git a/src/Servers/HttpSys/test/FunctionalTests/RequestBodyTests.cs b/src/Servers/HttpSys/test/FunctionalTests/RequestBodyTests.cs index f9817310f4c..ba1bb65da67 100644 --- a/src/Servers/HttpSys/test/FunctionalTests/RequestBodyTests.cs +++ b/src/Servers/HttpSys/test/FunctionalTests/RequestBodyTests.cs @@ -39,6 +39,28 @@ namespace Microsoft.AspNetCore.Server.HttpSys } } + [ConditionalFact] + public async Task RequestBody_Read0ByteSync_Success() + { + string address; + using (Utilities.CreateHttpServer(out address, httpContext => + { + Assert.True(httpContext.Request.CanHaveBody()); + byte[] input = new byte[100]; + httpContext.Features.Get<IHttpBodyControlFeature>().AllowSynchronousIO = true; + int read = httpContext.Request.Body.Read(input, 0, 0); + Assert.Equal(0, read); + read = httpContext.Request.Body.Read(input, 0, input.Length); + httpContext.Response.ContentLength = read; + httpContext.Response.Body.Write(input, 0, read); + return Task.FromResult(0); + })) + { + string response = await SendRequestAsync(address, "Hello World"); + Assert.Equal("Hello World", response); + } + } + [ConditionalFact] public async Task RequestBody_ReadAsync_Success() { @@ -57,6 +79,29 @@ namespace Microsoft.AspNetCore.Server.HttpSys } } + [ConditionalFact] + public async Task RequestBody_Read0ByteAsync_Success() + { + string address; + using (Utilities.CreateHttpServer(out address, async httpContext => + { + Assert.True(httpContext.Request.CanHaveBody()); + byte[] input = new byte[100]; + await Task.Delay(1000); + int read = await httpContext.Request.Body.ReadAsync(input, 0, 1); + Assert.Equal(1, read); + read = await httpContext.Request.Body.ReadAsync(input, 0, 0); + Assert.Equal(0, read); + read = await httpContext.Request.Body.ReadAsync(input, 1, input.Length - 1); + httpContext.Response.ContentLength = read + 1; + await httpContext.Response.Body.WriteAsync(input, 0, read + 1); + })) + { + string response = await SendRequestAsync(address, "Hello World"); + Assert.Equal("Hello World", response); + } + } + [ConditionalFact] public async Task RequestBody_ReadBeginEnd_Success() { @@ -87,7 +132,6 @@ namespace Microsoft.AspNetCore.Server.HttpSys Assert.Throws<ArgumentOutOfRangeException>("offset", () => httpContext.Request.Body.Read(input, -1, 1)); Assert.Throws<ArgumentOutOfRangeException>("offset", () => httpContext.Request.Body.Read(input, input.Length + 1, 1)); Assert.Throws<ArgumentOutOfRangeException>("size", () => httpContext.Request.Body.Read(input, 10, -1)); - Assert.Throws<ArgumentOutOfRangeException>("size", () => httpContext.Request.Body.Read(input, 0, 0)); Assert.Throws<ArgumentOutOfRangeException>("size", () => httpContext.Request.Body.Read(input, 1, input.Length)); Assert.Throws<ArgumentOutOfRangeException>("size", () => httpContext.Request.Body.Read(input, 0, input.Length + 1)); return Task.FromResult(0); -- GitLab