From 8f7c0ff0412b3c2d3acfe333992154989363d834 Mon Sep 17 00:00:00 2001 From: Justin Van Patten Date: Mon, 25 Apr 2016 16:59:16 -0700 Subject: [PATCH] Minor Stream improvements - Unsupported members should throw NotSupportedException instead of NotImplementedException per MSDN docs for CanRead/CanSeek/CanWrite. - Position should throw NotSupportedException when CanSeek is false. - FrameRequestStream.Flush/FlushAsync should not throw NotImplementedException. - Use expression-bodied members for CanRead/CanSeek/CanWrite on FrameRequestStream to match FrameResponseStream. - Provide no-op override of LibuvStream.FlushAsync to match Flush. --- .../Filter/LibuvStream.cs | 7 ++ .../Http/FrameRequestStream.cs | 34 ++++-- .../Http/FrameResponseStream.cs | 20 +++- .../FrameRequestStreamTests.cs | 108 ++++++++++++++++++ .../FrameResponseStreamTests.cs | 94 +++++++++++++++ 5 files changed, 249 insertions(+), 14 deletions(-) create mode 100644 test/Microsoft.AspNetCore.Server.KestrelTests/FrameRequestStreamTests.cs create mode 100644 test/Microsoft.AspNetCore.Server.KestrelTests/FrameResponseStreamTests.cs diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Filter/LibuvStream.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Filter/LibuvStream.cs index 081861a9ca..bf2c2c65e7 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Filter/LibuvStream.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Filter/LibuvStream.cs @@ -6,6 +6,7 @@ using System.IO; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Server.Kestrel.Http; +using Microsoft.AspNetCore.Server.Kestrel.Infrastructure; namespace Microsoft.AspNetCore.Server.Kestrel.Filter { @@ -121,6 +122,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Filter // No-op since writes are immediate. } + public override Task FlushAsync(CancellationToken cancellationToken) + { + // No-op since writes are immediate. + return TaskUtilities.CompletedTask; + } + private ValueTask ReadAsync(ArraySegment buffer) { return _input.ReadAsync(buffer.Array, buffer.Offset, buffer.Count); diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Http/FrameRequestStream.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Http/FrameRequestStream.cs index f16cf244cb..09c8957edd 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Http/FrameRequestStream.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Http/FrameRequestStream.cs @@ -19,35 +19,51 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http _state = FrameStreamState.Closed; } - public override bool CanRead { get { return true; } } + public override bool CanRead => true; - public override bool CanSeek { get { return false; } } + public override bool CanSeek => false; - public override bool CanWrite { get { return false; } } + public override bool CanWrite => false; public override long Length { get { - throw new NotImplementedException(); + throw new NotSupportedException(); } } - public override long Position { get; set; } + public override long Position + { + get + { + throw new NotSupportedException(); + } + set + { + throw new NotSupportedException(); + } + } public override void Flush() { - throw new NotImplementedException(); + // No-op. + } + + public override Task FlushAsync(CancellationToken cancellationToken) + { + // No-op. + return TaskUtilities.CompletedTask; } public override long Seek(long offset, SeekOrigin origin) { - throw new NotImplementedException(); + throw new NotSupportedException(); } public override void SetLength(long value) { - throw new NotImplementedException(); + throw new NotSupportedException(); } public override int Read(byte[] buffer, int offset, int count) @@ -113,7 +129,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http public override void Write(byte[] buffer, int offset, int count) { - throw new NotImplementedException(); + throw new NotSupportedException(); } public Stream StartAcceptingReads(MessageBody body) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Http/FrameResponseStream.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Http/FrameResponseStream.cs index 145710af24..c7568bed36 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Http/FrameResponseStream.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Http/FrameResponseStream.cs @@ -29,11 +29,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http { get { - throw new NotImplementedException(); + throw new NotSupportedException(); } } - public override long Position { get; set; } + public override long Position + { + get + { + throw new NotSupportedException(); + } + set + { + throw new NotSupportedException(); + } + } public override void Flush() { @@ -54,17 +64,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http public override long Seek(long offset, SeekOrigin origin) { - throw new NotImplementedException(); + throw new NotSupportedException(); } public override void SetLength(long value) { - throw new NotImplementedException(); + throw new NotSupportedException(); } public override int Read(byte[] buffer, int offset, int count) { - throw new NotImplementedException(); + throw new NotSupportedException(); } public override void Write(byte[] buffer, int offset, int count) diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameRequestStreamTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameRequestStreamTests.cs new file mode 100644 index 0000000000..10bdbf452c --- /dev/null +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameRequestStreamTests.cs @@ -0,0 +1,108 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.IO; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Server.Kestrel.Http; +using Xunit; + +namespace Microsoft.AspNetCore.Server.KestrelTests +{ + public class FrameRequestStreamTests + { + [Fact] + public void CanReadReturnsTrue() + { + var stream = new FrameRequestStream(); + Assert.True(stream.CanRead); + } + + [Fact] + public void CanSeekReturnsFalse() + { + var stream = new FrameRequestStream(); + Assert.False(stream.CanSeek); + } + + [Fact] + public void CanWriteReturnsFalse() + { + var stream = new FrameRequestStream(); + Assert.False(stream.CanWrite); + } + + [Fact] + public void SeekThrows() + { + var stream = new FrameRequestStream(); + Assert.Throws(() => stream.Seek(0, SeekOrigin.Begin)); + } + + [Fact] + public void LengthThrows() + { + var stream = new FrameRequestStream(); + Assert.Throws(() => stream.Length); + } + + [Fact] + public void SetLengthThrows() + { + var stream = new FrameRequestStream(); + Assert.Throws(() => stream.SetLength(0)); + } + + [Fact] + public void PositionThrows() + { + var stream = new FrameRequestStream(); + Assert.Throws(() => stream.Position); + Assert.Throws(() => stream.Position = 0); + } + + [Fact] + public void WriteThrows() + { + var stream = new FrameRequestStream(); + Assert.Throws(() => stream.Write(new byte[1], 0, 1)); + } + + [Fact] + public void WriteByteThrows() + { + var stream = new FrameRequestStream(); + Assert.Throws(() => stream.WriteByte(0)); + } + + [Fact] + public async Task WriteAsyncThrows() + { + var stream = new FrameRequestStream(); + await Assert.ThrowsAsync(() => stream.WriteAsync(new byte[1], 0, 1)); + } + +#if NET451 + [Fact] + public void BeginWriteThrows() + { + var stream = new FrameRequestStream(); + Assert.Throws(() => stream.BeginWrite(new byte[1], 0, 1, null, null)); + } +#endif + + [Fact] + public void FlushDoesNotThrow() + { + var stream = new FrameRequestStream(); + stream.Flush(); + } + + [Fact] + public async Task FlushAsyncDoesNotThrow() + { + var stream = new FrameRequestStream(); + await stream.FlushAsync(); + } + } +} diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameResponseStreamTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameResponseStreamTests.cs new file mode 100644 index 0000000000..d8d1dbcad4 --- /dev/null +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameResponseStreamTests.cs @@ -0,0 +1,94 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.IO; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Server.Kestrel.Http; +using Xunit; + +namespace Microsoft.AspNetCore.Server.KestrelTests +{ + public class FrameResponseStreamTests + { + [Fact] + public void CanReadReturnsFalse() + { + var stream = new FrameResponseStream(); + Assert.False(stream.CanRead); + } + + [Fact] + public void CanSeekReturnsFalse() + { + var stream = new FrameResponseStream(); + Assert.False(stream.CanSeek); + } + + [Fact] + public void CanWriteReturnsTrue() + { + var stream = new FrameResponseStream(); + Assert.True(stream.CanWrite); + } + + [Fact] + public void ReadThrows() + { + var stream = new FrameResponseStream(); + Assert.Throws(() => stream.Read(new byte[1], 0, 1)); + } + + [Fact] + public void ReadByteThrows() + { + var stream = new FrameResponseStream(); + Assert.Throws(() => stream.ReadByte()); + } + + [Fact] + public async Task ReadAsyncThrows() + { + var stream = new FrameResponseStream(); + await Assert.ThrowsAsync(() => stream.ReadAsync(new byte[1], 0, 1)); + } + +#if NET451 + [Fact] + public void BeginReadThrows() + { + var stream = new FrameResponseStream(); + Assert.Throws(() => stream.BeginRead(new byte[1], 0, 1, null, null)); + } +#endif + + [Fact] + public void SeekThrows() + { + var stream = new FrameResponseStream(); + Assert.Throws(() => stream.Seek(0, SeekOrigin.Begin)); + } + + [Fact] + public void LengthThrows() + { + var stream = new FrameResponseStream(); + Assert.Throws(() => stream.Length); + } + + [Fact] + public void SetLengthThrows() + { + var stream = new FrameResponseStream(); + Assert.Throws(() => stream.SetLength(0)); + } + + [Fact] + public void PositionThrows() + { + var stream = new FrameResponseStream(); + Assert.Throws(() => stream.Position); + Assert.Throws(() => stream.Position = 0); + } + } +}