Verify read/write buffers, use offset parameter when reading. (#652)

src/Microsoft.AspNetCore.Server.IISIntegration/Server/IISHttpContext.ReadWrite.cs

@@ -23,6 +23,17 @@ namespace Microsoft.AspNetCore.Server.IISIntegration
         public async Task<int> ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
         {
             // Start a task which will continuously call ReadFromIISAsync and WriteToIISAsync
+            if (buffer == null)
+            {
+                throw new ArgumentNullException(nameof(buffer));
+            }
+            if (count == 0)
+            {
+                throw new ArgumentOutOfRangeException(nameof(count));
+            }
+
+            var memory = new Memory<byte>(buffer, offset, count);
+
             StartProcessingRequestAndResponseBody();
 
             while (true)
@@ -35,7 +46,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration
                     {
                         var actual = Math.Min(readableBuffer.Length, count);
                         readableBuffer = readableBuffer.Slice(0, actual);
-                        readableBuffer.CopyTo(buffer);
+                        readableBuffer.CopyTo(memory.Span);
                         return (int)actual;
                     }
                     else if (result.IsCompleted)
@@ -53,20 +64,22 @@ namespace Microsoft.AspNetCore.Server.IISIntegration
         /// <summary>
         /// Writes data to the output pipe.
         /// </summary>
-        /// <param name="data"></param>
+        /// <param name="memory"></param>
         /// <param name="cancellationToken"></param>
         /// <returns></returns>
-        public Task WriteAsync(ArraySegment<byte> data, CancellationToken cancellationToken = default(CancellationToken))
+        public Task WriteAsync(ReadOnlyMemory<byte> memory, CancellationToken cancellationToken = default(CancellationToken))
         {
+
+            // Want to keep exceptions consistent, 
             if (!_hasResponseStarted)
             {
-                return WriteAsyncAwaited(data, cancellationToken);
+                return WriteAsyncAwaited(memory, cancellationToken);
             }
 
             lock (_stateSync)
             {
                 DisableReads();
-                return Output.WriteAsync(data, cancellationToken: cancellationToken);
+                return Output.WriteAsync(memory, cancellationToken);
             }
         }
 
@@ -88,7 +101,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration
             }
         }
 
-        public void StartProcessingRequestAndResponseBody()
+        private void StartProcessingRequestAndResponseBody()
         {
             if (_processBodiesTask == null)
             {
@@ -117,7 +130,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration
             await flushTask;
         }
 
-        private async Task WriteAsyncAwaited(ArraySegment<byte> data, CancellationToken cancellationToken)
+        private async Task WriteAsyncAwaited(ReadOnlyMemory<byte> data, CancellationToken cancellationToken)
         {
             // WriteAsyncAwaited is only called for the first write to the body.
             // Ensure headers are flushed if Write(Chunked)Async isn't called.
@@ -288,7 +301,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration
         /// Await reading from IIS, which will be cancelled if application code calls Write/FlushAsync.
         /// </summary>
         /// <returns>The Reading and Writing task.</returns>
-        public async Task ReadAndWriteLoopAsync()
+        private async Task ReadAndWriteLoopAsync()
         {
             try
             {

src/Microsoft.AspNetCore.Server.IISIntegration/Server/IISHttpResponseBody.cs

@@ -54,7 +54,12 @@ namespace Microsoft.AspNetCore.Server.IISIntegration
 
         public override unsafe Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
         {
-            return _httpContext.WriteAsync(new ArraySegment<byte>(buffer, offset, count), cancellationToken);
+            if (buffer == null)
+            {
+                throw new ArgumentNullException(nameof(buffer));
+            }
+
+            return _httpContext.WriteAsync(new ReadOnlyMemory<byte>(buffer, offset, count), cancellationToken);
         }
 
         public override Task FlushAsync(CancellationToken cancellationToken)

src/Microsoft.AspNetCore.Server.IISIntegration/Server/OutputProducer.cs

@@ -72,7 +72,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration
         }
 
         public Task WriteAsync(
-            ArraySegment<byte> buffer,
+            ReadOnlyMemory<byte> buffer,
             CancellationToken cancellationToken)
         {
             lock (_contextLock)
@@ -82,7 +82,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration
                     throw new ObjectDisposedException("Response is already completed");
                 }
 
-                _pipe.Writer.Write(new ReadOnlySpan<byte>(buffer.Array, buffer.Offset, buffer.Count));
+                _pipe.Writer.Write(buffer.Span);
             }
 
             return FlushAsync(_pipe.Writer, cancellationToken);

test/IISIntegration.FunctionalTests/Inprocess/InvalidReadWriteOperationTests.cs

@@ -0,0 +1,55 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Net.Http;
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.Testing.xunit;
+using Xunit;
+
+namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests
+{
+    [Collection(IISTestSiteCollection.Name)]
+    public class InvalidReadWriteOperationTests
+    {
+        private readonly IISTestSiteFixture _fixture;
+
+        public InvalidReadWriteOperationTests(IISTestSiteFixture fixture)
+        {
+            _fixture = fixture;
+        }
+
+        [ConditionalFact]
+        public async Task TestReadOffsetWorks()
+        {
+            var result = await _fixture.Client.PostAsync($"/TestReadOffsetWorks", new StringContent("Hello World"));
+            Assert.Equal("Hello World", await result.Content.ReadAsStringAsync());
+        }
+
+        [ConditionalTheory]
+        [InlineData("/NullBuffer")]
+        [InlineData("/InvalidOffsetSmall")]
+        [InlineData("/InvalidOffsetLarge")]
+        [InlineData("/InvalidCountSmall")]
+        [InlineData("/InvalidCountLarge")]
+        [InlineData("/InvalidCountWithOffset")]
+        [InlineData("/InvalidCountZeroRead")]
+        public async Task TestInvalidReadOperations(string operation)
+        {
+            var result = await _fixture.Client.GetStringAsync($"/TestInvalidReadOperations{operation}");
+            Assert.Equal("Success", result);
+        }
+
+        [ConditionalTheory]
+        [InlineData("/NullBuffer")]
+        [InlineData("/InvalidOffsetSmall")]
+        [InlineData("/InvalidOffsetLarge")]
+        [InlineData("/InvalidCountSmall")]
+        [InlineData("/InvalidCountLarge")]
+        [InlineData("/InvalidCountWithOffset")]
+        public async Task TestInvalidWriteOperations(string operation)
+        {
+            var result = await _fixture.Client.GetStringAsync($"/TestInvalidWriteOperations{operation}");
+            Assert.Equal("Success", result);
+        }
+    }
+}

test/IISTestSite/Startup.cs

@@ -2,9 +2,11 @@
 // 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.IO;
 using System.Net;
 using System.Text;
+using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Builder;
@@ -44,6 +46,9 @@ namespace IISTestSite
             app.Map("/ReadAndWriteSlowConnection", ReadAndWriteSlowConnection);
             app.Map("/WebsocketRequest", WebsocketRequest);
             app.Map("/UpgradeFeatureDetection", UpgradeFeatureDetection);
+            app.Map("/TestInvalidReadOperations", TestInvalidReadOperations);
+            app.Map("/TestInvalidWriteOperations", TestInvalidWriteOperations);
+            app.Map("/TestReadOffsetWorks", TestReadOffsetWorks);
         }
 
         private void ServerVariable(IApplicationBuilder app)
@@ -441,5 +446,187 @@ namespace IISTestSite
                 }
             });
         }
+
+        private void TestReadOffsetWorks(IApplicationBuilder app)
+        {
+            app.Run(async ctx =>
+            {
+                var buffer = new byte[11];
+                ctx.Request.Body.Read(buffer, 0, 6);
+                ctx.Request.Body.Read(buffer, 6, 5);
+
+                await ctx.Response.WriteAsync(Encoding.UTF8.GetString(buffer));
+            });
+        }
+
+        private void TestInvalidReadOperations(IApplicationBuilder app)
+        {
+            app.Run(async context =>
+            {
+                var success = false;
+                if (context.Request.Path.StartsWithSegments("/NullBuffer"))
+                {
+                    try
+                    {
+                        await context.Request.Body.ReadAsync(null, 0, 0);
+                    }
+                    catch (ArgumentNullException)
+                    {
+                        success = true;
+                    }
+                    catch (Exception)
+                    {
+                        success = true;
+                    }
+                }
+                else if (context.Request.Path.StartsWithSegments("/InvalidOffsetSmall"))
+                {
+                    try
+                    {
+                        await context.Request.Body.ReadAsync(new byte[1], -1, 0);
+                    }
+                    catch (ArgumentOutOfRangeException)
+                    {
+                        success = true;
+                    }
+                }
+                else if (context.Request.Path.StartsWithSegments("/InvalidOffsetLarge"))
+                {
+                    try
+                    {
+                        await context.Request.Body.ReadAsync(new byte[1], 2, 0);
+                    }
+                    catch (ArgumentOutOfRangeException)
+                    {
+                        success = true;
+                    }
+                }
+                else if (context.Request.Path.StartsWithSegments("/InvalidCountSmall"))
+                {
+                    try
+                    {
+                        await context.Request.Body.ReadAsync(new byte[1], 0, -1);
+                    }
+                    catch (ArgumentOutOfRangeException)
+                    {
+                        success = true;
+                    }
+                }
+                else if (context.Request.Path.StartsWithSegments("/InvalidCountLarge"))
+                {
+                    try
+                    {
+                        await context.Request.Body.ReadAsync(new byte[1], 0, -1);
+                    }
+                    catch (ArgumentOutOfRangeException)
+                    {
+                        success = true;
+                    }
+                }
+                else if (context.Request.Path.StartsWithSegments("/InvalidCountWithOffset"))
+                {
+                    try
+                    {
+                        await context.Request.Body.ReadAsync(new byte[3], 1, 3);
+                    }
+                    catch (ArgumentOutOfRangeException)
+                    {
+                        success = true;
+                    }
+                }
+                else if (context.Request.Path.StartsWithSegments("/InvalidCountZeroRead"))
+                {
+                    try
+                    {
+                        await context.Request.Body.ReadAsync(new byte[1], 0, 0);
+                    }
+                    catch (ArgumentOutOfRangeException)
+                    {
+                        success = true;
+                    }
+                    catch (Exception)
+                    {
+                        success = true;
+                    }
+                }
+
+                await context.Response.WriteAsync(success ? "Success" : "Failure");
+            });
+        }
+        private void TestInvalidWriteOperations(IApplicationBuilder app)
+        {
+            app.Run(async context =>
+            {
+                var success = false;
+                if (context.Request.Path.StartsWithSegments("/NullBuffer"))
+                {
+                    try
+                    {
+                        await context.Response.Body.WriteAsync(null, 0, 0);
+                    }
+                    catch (ArgumentNullException)
+                    {
+                        success = true;
+                    }
+                }
+                else if (context.Request.Path.StartsWithSegments("/InvalidOffsetSmall"))
+                {
+                    try
+                    {
+                        await context.Response.Body.WriteAsync(new byte[1], -1, 0);
+                    }
+                    catch (ArgumentOutOfRangeException)
+                    {
+                        success = true;
+                    }
+                }
+                else if (context.Request.Path.StartsWithSegments("/InvalidOffsetLarge"))
+                {
+                    try
+                    {
+                        await context.Response.Body.WriteAsync(new byte[1], 2, 0);
+                    }
+                    catch (ArgumentOutOfRangeException)
+                    {
+                        success = true;
+                    }
+                }
+                else if (context.Request.Path.StartsWithSegments("/InvalidCountSmall"))
+                {
+                    try
+                    {
+                        await context.Response.Body.WriteAsync(new byte[1], 0, -1);
+                    }
+                    catch (ArgumentOutOfRangeException)
+                    {
+                        success = true;
+                    }
+                }
+                else if (context.Request.Path.StartsWithSegments("/InvalidCountLarge"))
+                {
+                    try
+                    {
+                        await context.Response.Body.WriteAsync(new byte[1], 0, -1);
+                    }
+                    catch (ArgumentOutOfRangeException)
+                    {
+                        success = true;
+                    }
+                }
+                else if (context.Request.Path.StartsWithSegments("/InvalidCountWithOffset"))
+                {
+                    try
+                    {
+                        await context.Response.Body.WriteAsync(new byte[3], 1, 3);
+                    }
+                    catch (ArgumentOutOfRangeException)
+                    {
+                        success = true;
+                    }
+                }
+
+                await context.Response.WriteAsync(success ? "Success" : "Failure");
+            });
+        }
     }
 }