From a112379a677ab36e0b417086ba74f7cc547118f3 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Fri, 30 Oct 2020 05:50:49 -0400 Subject: [PATCH] Add Stream argument validation helpers and use throughout dotnet/runtime (#43968) * Add Stream argument validation helpers and use throughout dotnet/runtime * Delete unnecessary code in Stream Stream.Null was using BlockingBegin/EndRead/Write, but the operations are nops (they don't even do argument validation), so the functionality was unnecessary complication. We can just use TaskToApm instead, with already completed tasks, and then also delete the SynchronousAsyncResult that was used by these operations. Also, the Begin/EndRead/Write methods were checking CanRead/Write and throwing if they return false, but this is a private sealed type with CanRead/Write hardcoded to return true! Delete. Finally added consistency across Stream.Null to checking for cancellation. * Clean up some style on Stream Also consolidate a few resources and use a few existing ThrowHelpers where appropriate. * Fix a few issues, and remove duplicative tests missed in conformance tests rollout * Address PR feedback --- .../Common/src/System/IO/ReadOnlyMemoryStream.cs | 24 +- .../Common/tests/System/IO/ConnectedStreams.cs | 52 +- .../System.Console/src/System/IO/ConsoleStream.cs | 14 +- .../src/System/Data/SQLTypes/SQLBytes.cs | 14 +- .../src/System/IO/Compression/BrotliStream.cs | 15 - .../IO/Compression/dec/BrotliStream.Decompress.cs | 4 +- .../IO/Compression/enc/BrotliStream.Compress.cs | 4 +- .../src/System.IO.Compression.csproj | 4 +- .../DeflateManaged/DeflateManagedStream.cs | 19 +- .../IO/Compression/DeflateZLib/DeflateStream.cs | 35 +- .../src/System/IO/Compression/ZipArchiveEntry.cs | 10 +- .../src/System/IO/Compression/ZipCustomStreams.cs | 9 +- .../src/Resources/Strings.resx | 3 - .../src/System.IO.FileSystem.csproj | 2 - .../tests/FileStream/CopyToAsync.cs | 138 ---- .../tests/FileStream/EndRead.cs | 21 - .../tests/FileStream/EndWrite.cs | 21 - .../System.IO.FileSystem/tests/FileStream/Read.cs | 72 +- .../tests/FileStream/ReadAsync.cs | 123 +--- .../System.IO.FileSystem/tests/FileStream/Write.cs | 81 +-- .../tests/FileStream/WriteAsync.cs | 113 +--- .../tests/System.IO.FileSystem.Tests.csproj | 17 +- .../src/System/IO/Pipelines/PipeReaderStream.cs | 11 +- .../src/System/IO/Pipelines/PipeWriterStream.cs | 9 +- .../System.IO.Pipes/src/Resources/Strings.resx | 3 - .../System.IO.Pipes/src/System.IO.Pipes.csproj | 2 - .../src/System/IO/Pipes/PipeStream.cs | 20 +- .../tests/UmsReadWrite.cs | 12 +- .../BinaryWriter.WriteByteCharTests.cs | 4 +- .../tests/MemoryStream/MemoryStreamTests.cs | 4 +- .../System.IO/tests/Stream/Stream.Methods.cs | 4 +- .../System.IO/tests/Stream/Stream.NullTests.cs | 18 +- src/libraries/System.IO/tests/Stream/Stream.cs | 42 ++ .../src/System/Net/Connections/DuplexPipeStream.cs | 10 +- .../System.Net.Http/src/System.Net.Http.csproj | 2 - .../src/System/Net/Http/EmptyReadStream.cs | 2 +- .../src/System/Net/Http/HttpBaseStream.cs | 48 +- .../src/System/Net/Http/HttpContent.cs | 2 +- .../src/System/Net/Http/MultipartContent.cs | 24 +- .../ChunkedEncodingReadStream.cs | 2 +- .../ConnectionCloseReadStream.cs | 2 +- .../SocketsHttpHandler/ContentLengthReadStream.cs | 2 +- .../Net/Http/SocketsHttpHandler/Http2Stream.cs | 4 +- .../Http/SocketsHttpHandler/RawConnectionStream.cs | 2 +- .../tests/FunctionalTests/MultipartContentTest.cs | 4 +- .../UnitTests/System.Net.Http.Unit.Tests.csproj | 4 +- .../src/System/Net/HttpRequestStream.cs | 45 +- .../src/System/Net/HttpResponseStream.cs | 40 +- .../tests/HttpRequestStreamTests.cs | 8 +- .../tests/HttpResponseStreamTests.cs | 8 +- .../System.Net.Mail/src/System/Net/Base64Stream.cs | 49 +- .../src/System/Net/Mime/EightBitStream.cs | 26 +- .../src/System/Net/Mime/QEncodedStream.cs | 26 +- .../src/System/Net/Mime/QuotedPrintableStream.cs | 26 +- .../src/System/Net/Quic/QuicStream.cs | 26 +- .../src/System/Net/RequestStream.cs | 39 +- .../System.Net.Requests/tests/RequestStreamTest.cs | 6 +- .../System.Net.Security/src/Resources/Strings.resx | 3 - .../src/System/Net/Security/NegotiateStream.cs | 32 +- .../Net/Security/SslStream.Implementation.cs | 26 - .../src/System/Net/Security/SslStream.cs | 8 +- .../Fakes/FakeSslStream.Implementation.cs | 4 - .../src/System.Net.Sockets.csproj | 2 - .../src/System/Net/Sockets/NetworkStream.cs | 108 +-- .../src/Resources/Strings.resx | 6 - .../src/System.Private.CoreLib.Shared.projitems | 3 - .../src/System/IO/BufferedStream.cs | 47 +- .../src/System/IO/FileStream.Windows.cs | 18 +- .../src/System/IO/FileStream.cs | 47 +- .../src/System/IO/MemoryStream.cs | 49 +- .../System.Private.CoreLib/src/System/IO/Stream.cs | 747 ++++++++------------- .../src/System/IO/UnmanagedMemoryStream.cs | 36 +- .../src/System/Text/TranscodingStream.cs | 20 +- .../src/System/ThrowHelper.cs | 6 + src/libraries/System.Runtime/ref/System.Runtime.cs | 2 + .../System/Security/Cryptography/CryptoStream.cs | 18 +- .../tests/CryptoStream.cs | 22 - 77 files changed, 538 insertions(+), 1997 deletions(-) delete mode 100644 src/libraries/System.IO.FileSystem/tests/FileStream/EndRead.cs delete mode 100644 src/libraries/System.IO.FileSystem/tests/FileStream/EndWrite.cs diff --git a/src/libraries/Common/src/System/IO/ReadOnlyMemoryStream.cs b/src/libraries/Common/src/System/IO/ReadOnlyMemoryStream.cs index a400188..87b1531 100644 --- a/src/libraries/Common/src/System/IO/ReadOnlyMemoryStream.cs +++ b/src/libraries/Common/src/System/IO/ReadOnlyMemoryStream.cs @@ -65,7 +65,7 @@ namespace System.IO public override int Read(byte[] buffer, int offset, int count) { - ValidateReadArrayArguments(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); return Read(new Span(buffer, offset, count)); } @@ -93,7 +93,7 @@ namespace System.IO public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - ValidateReadArrayArguments(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); return cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : Task.FromResult(Read(new Span(buffer, offset, count))); @@ -112,7 +112,7 @@ namespace System.IO public override void CopyTo(Stream destination, int bufferSize) { - StreamHelpers.ValidateCopyToArgs(this, destination, bufferSize); + ValidateCopyToArguments(destination, bufferSize); if (_content.Length > _position) { destination.Write(_content.Span.Slice(_position)); @@ -121,7 +121,7 @@ namespace System.IO public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) { - StreamHelpers.ValidateCopyToArgs(this, destination, bufferSize); + ValidateCopyToArguments(destination, bufferSize); return _content.Length > _position ? destination.WriteAsync(_content.Slice(_position), cancellationToken).AsTask() : Task.CompletedTask; @@ -134,21 +134,5 @@ namespace System.IO public override void SetLength(long value) => throw new NotSupportedException(); public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); - - private static void ValidateReadArrayArguments(byte[] buffer, int offset, int count) - { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if (count < 0 || buffer.Length - offset < count) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } - } } } diff --git a/src/libraries/Common/tests/System/IO/ConnectedStreams.cs b/src/libraries/Common/tests/System/IO/ConnectedStreams.cs index 8d084e2..75aa34e 100644 --- a/src/libraries/Common/tests/System/IO/ConnectedStreams.cs +++ b/src/libraries/Common/tests/System/IO/ConnectedStreams.cs @@ -134,8 +134,8 @@ namespace System.IO public override int Read(byte[] buffer, int offset, int count) { + ValidateBufferArguments(buffer, offset, count); ThrowIfDisposed(); - ValidateArgs(buffer, offset, count); ThrowIfReadingNotSupported(); return _buffer.Read(new Span(buffer, offset, count)); @@ -172,8 +172,8 @@ namespace System.IO public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { + ValidateBufferArguments(buffer, offset, count); ThrowIfDisposed(); - ValidateArgs(buffer, offset, count); ThrowIfReadingNotSupported(); return _buffer.ReadAsync(new Memory(buffer, offset, count), cancellationToken).AsTask(); @@ -189,8 +189,8 @@ namespace System.IO public override void Write(byte[] buffer, int offset, int count) { + ValidateBufferArguments(buffer, offset, count); ThrowIfDisposed(); - ValidateArgs(buffer, offset, count); ThrowIfWritingNotSupported(); _buffer.Write(new ReadOnlySpan(buffer, offset, count)); @@ -214,8 +214,8 @@ namespace System.IO public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { + ValidateBufferArguments(buffer, offset, count); ThrowIfDisposed(); - ValidateArgs(buffer, offset, count); ThrowIfWritingNotSupported(); return _buffer.WriteAsync(new ReadOnlyMemory(buffer, offset, count), cancellationToken).AsTask(); @@ -245,24 +245,6 @@ namespace System.IO public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); public override void SetLength(long value) => throw new NotSupportedException(); - private void ValidateArgs(byte[] buffer, int offset, int count) - { - if (buffer is null) - { - throw new ArgumentNullException(nameof(buffer)); - } - - if ((uint)offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - - if ((uint)count > buffer.Length || offset > buffer.Length - count) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } - } - private void ThrowIfDisposed() { if (_disposed) @@ -345,8 +327,8 @@ namespace System.IO public override int Read(byte[] buffer, int offset, int count) { + ValidateBufferArguments(buffer, offset, count); ThrowIfDisposed(); - ValidateArgs(buffer, offset, count); return _readBuffer.Read(new Span(buffer, offset, count)); } @@ -377,8 +359,8 @@ namespace System.IO public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { + ValidateBufferArguments(buffer, offset, count); ThrowIfDisposed(); - ValidateArgs(buffer, offset, count); return _readBuffer.ReadAsync(new Memory(buffer, offset, count), cancellationToken).AsTask(); } @@ -390,8 +372,8 @@ namespace System.IO public override void Write(byte[] buffer, int offset, int count) { + ValidateBufferArguments(buffer, offset, count); ThrowIfDisposed(); - ValidateArgs(buffer, offset, count); _writeBuffer.Write(new ReadOnlySpan(buffer, offset, count)); } @@ -409,8 +391,8 @@ namespace System.IO public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { + ValidateBufferArguments(buffer, offset, count); ThrowIfDisposed(); - ValidateArgs(buffer, offset, count); return _writeBuffer.WriteAsync(new ReadOnlyMemory(buffer, offset, count), cancellationToken).AsTask(); } @@ -434,24 +416,6 @@ namespace System.IO public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); public override void SetLength(long value) => throw new NotSupportedException(); - private void ValidateArgs(byte[] buffer, int offset, int count) - { - if (buffer is null) - { - throw new ArgumentNullException(nameof(buffer)); - } - - if ((uint)offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - - if ((uint)count > buffer.Length || offset > buffer.Length - count) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } - } - private void ThrowIfDisposed() { if (_disposed) diff --git a/src/libraries/System.Console/src/System/IO/ConsoleStream.cs b/src/libraries/System.Console/src/System/IO/ConsoleStream.cs index 7b82be2..9fdc87d 100644 --- a/src/libraries/System.Console/src/System/IO/ConsoleStream.cs +++ b/src/libraries/System.Console/src/System/IO/ConsoleStream.cs @@ -71,24 +71,14 @@ namespace System.IO protected void ValidateRead(byte[] buffer, int offset, int count) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer)); - if (offset < 0 || count < 0) - throw new ArgumentOutOfRangeException(offset < 0 ? nameof(offset) : nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); + ValidateBufferArguments(buffer, offset, count); if (!_canRead) throw Error.GetReadNotSupported(); } protected void ValidateWrite(byte[] buffer, int offset, int count) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer)); - if (offset < 0 || count < 0) - throw new ArgumentOutOfRangeException(offset < 0 ? nameof(offset) : nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); + ValidateBufferArguments(buffer, offset, count); if (!_canWrite) throw Error.GetWriteNotSupported(); } diff --git a/src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLBytes.cs b/src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLBytes.cs index caeff12..b04d392 100644 --- a/src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLBytes.cs +++ b/src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLBytes.cs @@ -696,12 +696,7 @@ namespace System.Data.SqlTypes { CheckIfStreamClosed(); - if (buffer == null) - throw new ArgumentNullException(nameof(buffer)); - if (offset < 0 || offset > buffer.Length) - throw new ArgumentOutOfRangeException(nameof(offset)); - if (count < 0 || count > buffer.Length - offset) - throw new ArgumentOutOfRangeException(nameof(count)); + ValidateBufferArguments(buffer, offset, count); int iBytesRead = (int)_sb.Read(_lPosition, buffer, offset, count); _lPosition += iBytesRead; @@ -713,12 +708,7 @@ namespace System.Data.SqlTypes { CheckIfStreamClosed(); - if (buffer == null) - throw new ArgumentNullException(nameof(buffer)); - if (offset < 0 || offset > buffer.Length) - throw new ArgumentOutOfRangeException(nameof(offset)); - if (count < 0 || count > buffer.Length - offset) - throw new ArgumentOutOfRangeException(nameof(count)); + ValidateBufferArguments(buffer, offset, count); _sb.Write(_lPosition, buffer, offset, count); _lPosition += count; diff --git a/src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs b/src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs index e865158..7fc6d7b 100644 --- a/src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs +++ b/src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs @@ -112,21 +112,6 @@ namespace System.IO.Compression } } - private static void ValidateParameters(byte[] buffer, int offset, int count) - { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer)); - - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedPosNum); - - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedPosNum); - - if (buffer.Length - offset < count) - throw new ArgumentException(SR.InvalidArgumentOffsetCount); - } - public Stream BaseStream => _stream; public override bool CanRead => _mode == CompressionMode.Decompress && _stream != null && _stream.CanRead; public override bool CanWrite => _mode == CompressionMode.Compress && _stream != null && _stream.CanWrite; diff --git a/src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/dec/BrotliStream.Decompress.cs b/src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/dec/BrotliStream.Decompress.cs index b5c86a0..c4de386 100644 --- a/src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/dec/BrotliStream.Decompress.cs +++ b/src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/dec/BrotliStream.Decompress.cs @@ -16,7 +16,7 @@ namespace System.IO.Compression public override int Read(byte[] buffer, int offset, int count) { - ValidateParameters(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); return Read(new Span(buffer, offset, count)); } @@ -95,7 +95,7 @@ namespace System.IO.Compression public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - ValidateParameters(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); return ReadAsync(new Memory(buffer, offset, count), cancellationToken).AsTask(); } diff --git a/src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/enc/BrotliStream.Compress.cs b/src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/enc/BrotliStream.Compress.cs index 61a9483..1de181d 100644 --- a/src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/enc/BrotliStream.Compress.cs +++ b/src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/enc/BrotliStream.Compress.cs @@ -20,7 +20,7 @@ namespace System.IO.Compression public override void Write(byte[] buffer, int offset, int count) { - ValidateParameters(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); WriteCore(new ReadOnlySpan(buffer, offset, count)); } @@ -64,7 +64,7 @@ namespace System.IO.Compression public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - ValidateParameters(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); return WriteAsync(new ReadOnlyMemory(buffer, offset, count), cancellationToken).AsTask(); } diff --git a/src/libraries/System.IO.Compression/src/System.IO.Compression.csproj b/src/libraries/System.IO.Compression/src/System.IO.Compression.csproj index 1bf09a4..48daf8e 100644 --- a/src/libraries/System.IO.Compression/src/System.IO.Compression.csproj +++ b/src/libraries/System.IO.Compression/src/System.IO.Compression.csproj @@ -34,8 +34,6 @@ - @@ -65,4 +63,4 @@ - \ No newline at end of file + diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateManaged/DeflateManagedStream.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateManaged/DeflateManagedStream.cs index 0c2a6f0..a584771 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateManaged/DeflateManagedStream.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateManaged/DeflateManagedStream.cs @@ -96,7 +96,7 @@ namespace System.IO.Compression public override int Read(byte[] buffer, int offset, int count) { - ValidateParameters(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); EnsureNotDisposed(); int bytesRead; @@ -139,21 +139,6 @@ namespace System.IO.Compression return count - remainingCount; } - private void ValidateParameters(byte[] buffer, int offset, int count) - { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer)); - - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset)); - - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count)); - - if (buffer.Length - offset < count) - throw new ArgumentException(SR.InvalidArgumentOffsetCount); - } - private void EnsureNotDisposed() { if (_stream == null) @@ -177,7 +162,7 @@ namespace System.IO.Compression if (_asyncOperations != 0) throw new InvalidOperationException(SR.InvalidBeginCall); - ValidateParameters(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); EnsureNotDisposed(); if (cancellationToken.IsCancellationRequested) diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/DeflateStream.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/DeflateStream.cs index c2c7598..c8cd407 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/DeflateStream.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/DeflateStream.cs @@ -235,7 +235,7 @@ namespace System.IO.Compression public override int Read(byte[] buffer, int offset, int count) { - ValidateParameters(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); return ReadCore(new Span(buffer, offset, count)); } @@ -303,21 +303,6 @@ namespace System.IO.Compression return totalRead; } - private void ValidateParameters(byte[] buffer, int offset, int count) - { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer)); - - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset)); - - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count)); - - if (buffer.Length - offset < count) - throw new ArgumentOutOfRangeException(SR.InvalidArgumentOffsetCount); - } - private void EnsureNotDisposed() { if (_stream == null) @@ -363,7 +348,7 @@ namespace System.IO.Compression public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - ValidateParameters(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); return ReadAsyncMemory(new Memory(buffer, offset, count), cancellationToken).AsTask(); } @@ -453,7 +438,7 @@ namespace System.IO.Compression public override void Write(byte[] buffer, int offset, int count) { - ValidateParameters(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); WriteCore(new ReadOnlySpan(buffer, offset, count)); } @@ -741,7 +726,7 @@ namespace System.IO.Compression public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - ValidateParameters(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); return WriteAsyncMemory(new ReadOnlyMemory(buffer, offset, count), cancellationToken).AsTask(); } @@ -809,23 +794,21 @@ namespace System.IO.Compression public override void CopyTo(Stream destination, int bufferSize) { - StreamHelpers.ValidateCopyToArgs(this, destination, bufferSize); + ValidateCopyToArguments(destination, bufferSize); - EnsureDecompressionMode(); EnsureNotDisposed(); + if (!CanRead) throw new NotSupportedException(); new CopyToStream(this, destination, bufferSize).CopyFromSourceToDestination(); } public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) { - // Validation as base CopyToAsync would do - StreamHelpers.ValidateCopyToArgs(this, destination, bufferSize); + ValidateCopyToArguments(destination, bufferSize); - // Validation as ReadAsync would do - EnsureDecompressionMode(); - EnsureNoActiveAsyncOperation(); EnsureNotDisposed(); + if (!CanRead) throw new NotSupportedException(); + EnsureNoActiveAsyncOperation(); // Early check for cancellation if (cancellationToken.IsCancellationRequested) diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs index 2e78b22..100a61b 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs @@ -1184,15 +1184,7 @@ namespace System.IO.Compression // they must set _everWritten, etc. public override void Write(byte[] buffer, int offset, int count) { - //we can't pass the argument checking down a level - if (buffer == null) - throw new ArgumentNullException(nameof(buffer)); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentNeedNonNegative); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentNeedNonNegative); - if ((buffer.Length - offset) < count) - throw new ArgumentException(SR.OffsetLengthInvalid); + ValidateBufferArguments(buffer, offset, count); ThrowIfDisposed(); Debug.Assert(CanWrite); diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipCustomStreams.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipCustomStreams.cs index dd85b89..dbc4f56 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipCustomStreams.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipCustomStreams.cs @@ -397,14 +397,7 @@ namespace System.IO.Compression public override void Write(byte[] buffer, int offset, int count) { // we can't pass the argument checking down a level - if (buffer == null) - throw new ArgumentNullException(nameof(buffer)); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentNeedNonNegative); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentNeedNonNegative); - if ((buffer.Length - offset) < count) - throw new ArgumentException(SR.OffsetLengthInvalid); + ValidateBufferArguments(buffer, offset, count); // if we're not actually writing anything, we don't want to trigger as if we did write something ThrowIfDisposed(); diff --git a/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx b/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx index 637881c..683090b 100644 --- a/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx +++ b/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx @@ -176,9 +176,6 @@ Combining FileMode: {0} with FileAccess: {1} is invalid. - - Offset and length were out of bounds for the array or count is greater than the number of elements from index to the end of the source collection. - Illegal characters in path '{0}'. diff --git a/src/libraries/System.IO.FileSystem/src/System.IO.FileSystem.csproj b/src/libraries/System.IO.FileSystem/src/System.IO.FileSystem.csproj index f7d2d0c..28dfd46 100644 --- a/src/libraries/System.IO.FileSystem/src/System.IO.FileSystem.csproj +++ b/src/libraries/System.IO.FileSystem/src/System.IO.FileSystem.csproj @@ -16,8 +16,6 @@ - ("destination", () => { fs.CopyToAsync(null); }); - AssertExtensions.Throws("bufferSize", () => { fs.CopyToAsync(new MemoryStream(), 0); }); - Assert.Throws(() => { fs.CopyToAsync(new MemoryStream(new byte[1], writable: false)); }); - fs.Dispose(); - Assert.Throws(() => { fs.CopyToAsync(new MemoryStream()); }); - } - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create, FileAccess.Write)) - { - Assert.Throws(() => { fs.CopyToAsync(new MemoryStream()); }); - } - using (FileStream src = new FileStream(GetTestFilePath(), FileMode.Create, FileAccess.ReadWrite, FileShare.None, 0x100, useAsync)) - using (FileStream dst = new FileStream(GetTestFilePath(), FileMode.Create, FileAccess.ReadWrite, FileShare.None, 0x100, useAsync)) - { - dst.Dispose(); - Assert.Throws(() => { src.CopyToAsync(dst); }); - } - } - - [Theory] - [InlineData(false)] - [InlineData(true)] public void DisposeHandleThenUseFileStream_CopyToAsync(bool useAsync) { using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create, FileAccess.ReadWrite, FileShare.None, 0x100, useAsync)) @@ -58,17 +33,6 @@ namespace System.IO.Tests } } - [Theory] - [InlineData(false)] - [InlineData(true)] - public async Task AlreadyCanceled_ReturnsCanceledTask(bool useAsync) - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create, FileAccess.ReadWrite, FileShare.None, 0x100, useAsync)) - { - await Assert.ThrowsAnyAsync(() => fs.CopyToAsync(fs, 0x1000, new CancellationToken(canceled: true))); - } - } - [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] // inner loop, just a few cases [InlineData(false, false)] [InlineData(true, false)] @@ -175,108 +139,6 @@ namespace System.IO.Tests } } - [Theory] - [InlineData(10, 1024)] - [PlatformSpecific(~TestPlatforms.Browser)] // IO.Pipes not supported - public async Task AnonymousPipeViaFileStream_AllDataCopied(int writeSize, int numWrites) - { - long totalLength = writeSize * numWrites; - var expectedData = new byte[totalLength]; - new Random(42).NextBytes(expectedData); - - var results = new MemoryStream(); - - using (var server = new AnonymousPipeServerStream(PipeDirection.Out)) - { - Task serverTask = Task.Run(async () => - { - for (int i = 0; i < numWrites; i++) - { - await server.WriteAsync(expectedData, i * writeSize, writeSize); - } - }); - - using (var client = new FileStream(new SafeFileHandle(server.ClientSafePipeHandle.DangerousGetHandle(), false), FileAccess.Read, bufferSize: 3)) - { - Task copyTask = client.CopyToAsync(results, writeSize); - await await Task.WhenAny(serverTask, copyTask); - - server.Dispose(); - await copyTask; - } - } - - byte[] actualData = results.ToArray(); - Assert.Equal(expectedData.Length, actualData.Length); - Assert.Equal(expectedData, actualData); - } - - [PlatformSpecific(TestPlatforms.Windows)] // Uses P/Invokes to create async pipe handle - [Theory] - [InlineData(false, 10, 1024)] - [InlineData(true, 10, 1024)] - public async Task NamedPipeViaFileStream_AllDataCopied(bool useAsync, int writeSize, int numWrites) - { - long totalLength = writeSize * numWrites; - var expectedData = new byte[totalLength]; - new Random(42).NextBytes(expectedData); - - var results = new MemoryStream(); - var pipeOptions = useAsync ? PipeOptions.Asynchronous : PipeOptions.None; - - string name = GetNamedPipeServerStreamName(); - using (var server = new NamedPipeServerStream(name, PipeDirection.Out, 1, PipeTransmissionMode.Byte, pipeOptions)) - { - Task serverTask = Task.Run(async () => - { - await server.WaitForConnectionAsync(); - for (int i = 0; i < numWrites; i++) - { - await server.WriteAsync(expectedData, i * writeSize, writeSize); - } - server.Dispose(); - }); - - Assert.True(WaitNamedPipeW(@"\\.\pipe\" + name, -1)); - using (SafeFileHandle clientHandle = CreateFileW(@"\\.\pipe\" + name, Interop.Kernel32.GenericOperations.GENERIC_READ, FileShare.None, IntPtr.Zero, FileMode.Open, (int)pipeOptions, IntPtr.Zero)) - using (var client = new FileStream(clientHandle, FileAccess.Read, bufferSize: 3, isAsync: useAsync)) - { - Task copyTask = client.CopyToAsync(results, (int)totalLength); - await await Task.WhenAny(serverTask, copyTask); - await copyTask; - } - } - - byte[] actualData = results.ToArray(); - Assert.Equal(expectedData.Length, actualData.Length); - Assert.Equal(expectedData, actualData); - } - - [PlatformSpecific(TestPlatforms.Windows)] // Uses P/Invokes to create async pipe handle - [Fact] - public async Task NamedPipeViaFileStream_CancellationRequested_OperationCanceled() - { - string name = Guid.NewGuid().ToString("N"); - using (var server = new NamedPipeServerStream(name, PipeDirection.Out, 1, PipeTransmissionMode.Byte, PipeOptions.Asynchronous)) - { - Task serverTask = server.WaitForConnectionAsync(); - - Assert.True(WaitNamedPipeW(@"\\.\pipe\" + name, -1)); - using (SafeFileHandle clientHandle = CreateFileW(@"\\.\pipe\" + name, Interop.Kernel32.GenericOperations.GENERIC_READ, FileShare.None, IntPtr.Zero, FileMode.Open, (int)PipeOptions.Asynchronous, IntPtr.Zero)) - using (var client = new FileStream(clientHandle, FileAccess.Read, bufferSize: 3, isAsync: true)) - { - await serverTask; - - var cts = new CancellationTokenSource(); - Task clientTask = client.CopyToAsync(new MemoryStream(), 0x1000, cts.Token); - Assert.False(clientTask.IsCompleted); - - cts.Cancel(); - await Assert.ThrowsAsync(() => clientTask); - } - } - } - [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] [InlineData(false)] [InlineData(true)] diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/EndRead.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/EndRead.cs deleted file mode 100644 index 7779550..0000000 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/EndRead.cs +++ /dev/null @@ -1,21 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.IO; -using Xunit; - -namespace System.IO.Tests -{ - public class FileStream_EndRead : FileSystemTest - { - [Fact] - public void EndReadThrowsForNullAsyncResult() - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create, FileAccess.ReadWrite)) - { - AssertExtensions.Throws("asyncResult", () => fs.EndRead(null)); - } - } - } -} diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/EndWrite.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/EndWrite.cs deleted file mode 100644 index ecc1d97..0000000 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/EndWrite.cs +++ /dev/null @@ -1,21 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.IO; -using Xunit; - -namespace System.IO.Tests -{ - public class FileStream_EndWrite : FileSystemTest - { - [Fact] - public void EndWriteThrowsForNullAsyncResult() - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create, FileAccess.ReadWrite)) - { - AssertExtensions.Throws("asyncResult", () => fs.EndWrite(null)); - } - } - } -} diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/Read.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/Read.cs index 75ebf98..74b39ee 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/Read.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/Read.cs @@ -11,15 +11,6 @@ namespace System.IO.Tests public class FileStream_Read : FileSystemTest { [Fact] - public void NullArrayThrows() - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) - { - AssertExtensions.Throws("buffer", () => fs.Read(null, 0, 1)); - } - } - - [Fact] public void NegativeReadRootThrows() { Assert.Throws(() => @@ -27,56 +18,6 @@ namespace System.IO.Tests } [Fact] - public void NegativeOffsetThrows() - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) - { - AssertExtensions.Throws("offset", () => fs.Read(new byte[1], -1, 1)); - - // array is checked first - AssertExtensions.Throws("buffer", () => fs.Read(null, -1, 1)); - } - } - - [Fact] - public void NegativeCountThrows() - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) - { - AssertExtensions.Throws("count", () => fs.Read(new byte[1], 0, -1)); - - // offset is checked before count - AssertExtensions.Throws("offset", () => fs.Read(new byte[1], -1, -1)); - - // array is checked first - AssertExtensions.Throws("buffer", () => fs.Read(null, -1, -1)); - } - } - - [Fact] - public void ArrayOutOfBoundsThrows() - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) - { - // offset out of bounds - Assert.Throws(null, () => fs.Read(new byte[1], 1, 1)); - - // offset out of bounds for 0 count read - Assert.Throws(null, () => fs.Read(new byte[1], 2, 0)); - - // offset out of bounds even for 0 length buffer - Assert.Throws(null, () => fs.Read(new byte[0], 1, 0)); - - // combination offset and count out of bounds - Assert.Throws(null, () => fs.Read(new byte[2], 1, 2)); - - // edges - Assert.Throws(null, () => fs.Read(new byte[0], int.MaxValue, 0)); - Assert.Throws(null, () => fs.Read(new byte[0], int.MaxValue, int.MaxValue)); - } - } - - [Fact] public void ReadDisposedThrows() { using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) @@ -87,7 +28,7 @@ namespace System.IO.Tests Assert.Throws(() => fs.Read(new byte[1], 0, 0)); // out of bounds checking happens first - Assert.Throws(null, () => fs.Read(new byte[2], 1, 2)); + Assert.Throws(() => fs.Read(new byte[2], 1, 2)); // count is checked prior AssertExtensions.Throws("count", () => fs.Read(new byte[1], 0, -1)); @@ -112,16 +53,7 @@ namespace System.IO.Tests Assert.Throws(() => fs.Read(new byte[1], 0, 1)); // out of bounds checking happens first - Assert.Throws(null, () => fs.Read(new byte[2], 1, 2)); - - // count is checked prior - AssertExtensions.Throws("count", () => fs.Read(new byte[1], 0, -1)); - - // offset is checked prior - AssertExtensions.Throws("offset", () => fs.Read(new byte[1], -1, -1)); - - // array is checked first - AssertExtensions.Throws("buffer", () => fs.Read(null, -1, -1)); + Assert.Throws(() => fs.Read(new byte[2], 1, 2)); } } diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/ReadAsync.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/ReadAsync.cs index 5cadaad..6075cfb 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/ReadAsync.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/ReadAsync.cs @@ -10,85 +10,11 @@ namespace System.IO.Tests { public abstract class FileStream_AsyncReads : FileSystemTest { - protected virtual string BufferParamName => "buffer"; - protected virtual string OffsetParamName => "offset"; - protected virtual string CountParamName => "count"; protected abstract Task ReadAsync(FileStream stream, byte[] buffer, int offset, int count, CancellationToken cancellationToken); private Task ReadAsync(FileStream stream, byte[] buffer, int offset, int count) => ReadAsync(stream, buffer, offset, count, CancellationToken.None); [Fact] - public void NullBufferThrows() - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) - { - AssertExtensions.Throws(BufferParamName, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, null, 0, 1))); - } - } - - [Fact] - public void NegativeOffsetThrows() - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) - { - AssertExtensions.Throws(OffsetParamName, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[1], -1, 1))); - - // buffer is checked first - AssertExtensions.Throws(BufferParamName, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, null, -1, 1))); - } - } - - [Fact] - public void NegativeCountThrows() - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) - { - AssertExtensions.Throws(CountParamName, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[1], 0, -1))); - - // offset is checked before count - AssertExtensions.Throws(OffsetParamName, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[1], -1, -1))); - - // buffer is checked first - AssertExtensions.Throws(BufferParamName, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, null, -1, -1))); - } - } - - [Fact] - public void BufferOutOfBoundsThrows() - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) - { - // offset out of bounds - Assert.Throws(null, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[1], 1, 1))); - - // offset out of bounds for 0 count ReadAsync - Assert.Throws(null, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[1], 2, 0))); - - // offset out of bounds even for 0 length buffer - Assert.Throws(null, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[0], 1, 0))); - - // combination offset and count out of bounds - Assert.Throws(null, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[2], 1, 2))); - - // edges - Assert.Throws(null, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[0], int.MaxValue, 0))); - Assert.Throws(null, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[0], int.MaxValue, int.MaxValue))); - } - } - - [Fact] public void ReadAsyncDisposedThrows() { using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) @@ -96,25 +22,14 @@ namespace System.IO.Tests fs.Dispose(); Assert.Throws(() => FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[1], 0, 1))); + // even for noop ReadAsync Assert.Throws(() => FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[1], 0, 0))); // out of bounds checking happens first - Assert.Throws(null, () => + Assert.Throws(() => FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[2], 1, 2))); - - // count is checked prior - AssertExtensions.Throws(CountParamName, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[1], 0, -1))); - - // offset is checked prior - AssertExtensions.Throws(OffsetParamName, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[1], -1, -1))); - - // buffer is checked first - AssertExtensions.Throws(BufferParamName, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, null, -1, -1))); } } @@ -132,20 +47,8 @@ namespace System.IO.Tests FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[1], 0, 1))); // out of bounds checking happens first - Assert.Throws(null, () => + Assert.Throws(() => FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[2], 1, 2))); - - // count is checked prior - AssertExtensions.Throws(CountParamName, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[1], 0, -1))); - - // offset is checked prior - AssertExtensions.Throws(OffsetParamName, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[1], -1, -1))); - - // buffer is checked first - AssertExtensions.Throws(BufferParamName, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, null, -1, -1))); } } @@ -252,7 +155,7 @@ namespace System.IO.Tests } [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] - public async Task ReadAsyncCancelledFile() + public async Task ReadAsyncCanceledFile() { string fileName = GetTestFilePath(); using (FileStream fs = new FileStream(fileName, FileMode.Create)) @@ -381,24 +284,13 @@ namespace System.IO.Tests FSAssert.IsCancelled(ReadAsync(fs, new byte[1], 0, 1, cancelledToken), cancelledToken); fs.Dispose(); + // before disposed check FSAssert.IsCancelled(ReadAsync(fs, new byte[1], 0, 1, cancelledToken), cancelledToken); // out of bounds checking happens first - Assert.Throws(null, () => + Assert.Throws(() => FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[2], 1, 2, cancelledToken))); - - // count is checked prior - AssertExtensions.Throws(CountParamName, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[1], 0, -1, cancelledToken))); - - // offset is checked prior - AssertExtensions.Throws(OffsetParamName, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, new byte[1], -1, -1, cancelledToken))); - - // buffer is checked first - AssertExtensions.Throws(BufferParamName, () => - FSAssert.CompletesSynchronously(ReadAsync(fs, null, -1, -1, cancelledToken))); } } } @@ -411,8 +303,5 @@ namespace System.IO.Tests (callback, state) => stream.BeginRead(buffer, offset, count, callback, state), iar => stream.EndRead(iar), null); - - protected override string BufferParamName => "buffer"; - protected override string CountParamName => "count"; } } diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/Write.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/Write.cs index a51959e..475ecfc 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/Write.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/Write.cs @@ -11,65 +11,6 @@ namespace System.IO.Tests public class FileStream_Write : FileSystemTest { [Fact] - public void NullArrayThrows() - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) - { - AssertExtensions.Throws("buffer", () => fs.Write(null, 0, 1)); - } - } - - [Fact] - public void NegativeOffsetThrows() - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) - { - AssertExtensions.Throws("offset", () => fs.Write(new byte[1], -1, 1)); - - // array is checked first - AssertExtensions.Throws("buffer", () => fs.Write(null, -1, 1)); - } - } - - [Fact] - public void NegativeCountThrows() - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) - { - AssertExtensions.Throws("count", () => fs.Write(new byte[1], 0, -1)); - - // offset is checked before count - AssertExtensions.Throws("offset", () => fs.Write(new byte[1], -1, -1)); - - // array is checked first - AssertExtensions.Throws("buffer", () => fs.Write(null, -1, -1)); - } - } - - [Fact] - public void ArrayOutOfBoundsThrows() - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) - { - // offset out of bounds - Assert.Throws(null, () => fs.Write(new byte[1], 1, 1)); - - // offset out of bounds for 0 count Write - Assert.Throws(null, () => fs.Write(new byte[1], 2, 0)); - - // offset out of bounds even for 0 length buffer - Assert.Throws(null, () => fs.Write(new byte[0], 1, 0)); - - // combination offset and count out of bounds - Assert.Throws(null, () => fs.Write(new byte[2], 1, 2)); - - // edges - Assert.Throws(null, () => fs.Write(new byte[0], int.MaxValue, 0)); - Assert.Throws(null, () => fs.Write(new byte[0], int.MaxValue, int.MaxValue)); - } - } - - [Fact] public void WriteDisposedThrows() { using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) @@ -80,16 +21,7 @@ namespace System.IO.Tests Assert.Throws(() => fs.Write(new byte[1], 0, 0)); // out of bounds checking happens first - Assert.Throws(null, () => fs.Write(new byte[2], 1, 2)); - - // count is checked prior - AssertExtensions.Throws("count", () => fs.Write(new byte[1], 0, -1)); - - // offset is checked prior - AssertExtensions.Throws("offset", () => fs.Write(new byte[1], -1, -1)); - - // array is checked first - AssertExtensions.Throws("buffer", () => fs.Write(null, -1, -1)); + Assert.Throws(() => fs.Write(new byte[2], 1, 2)); } } @@ -111,16 +43,7 @@ namespace System.IO.Tests Assert.Throws(() => fs.Write(new byte[1], 0, 1)); // out of bounds checking happens first - Assert.Throws(null, () => fs.Write(new byte[2], 1, 2)); - - // count is checked prior - AssertExtensions.Throws("count", () => fs.Write(new byte[1], 0, -1)); - - // offset is checked prior - AssertExtensions.Throws("offset", () => fs.Write(new byte[1], -1, -1)); - - // array is checked first - AssertExtensions.Throws("buffer", () => fs.Write(null, -1, -1)); + Assert.Throws(() => fs.Write(new byte[2], 1, 2)); } } diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/WriteAsync.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/WriteAsync.cs index 6637c6c..170a890 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/WriteAsync.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/WriteAsync.cs @@ -10,85 +10,11 @@ namespace System.IO.Tests { public abstract class FileStream_AsyncWrites : FileSystemTest { - protected virtual string BufferParamName => "buffer"; - protected virtual string OffsetParamName => "offset"; - protected virtual string CountParamName => "count"; private Task WriteAsync(FileStream stream, byte[] buffer, int offset, int count) => WriteAsync(stream, buffer, offset, count, CancellationToken.None); protected abstract Task WriteAsync(FileStream stream, byte[] buffer, int offset, int count, CancellationToken cancellationToken); [Fact] - public void NullBufferThrows() - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) - { - AssertExtensions.Throws(BufferParamName, () => - FSAssert.CompletesSynchronously(WriteAsync(fs, null, 0, 1))); - } - } - - [Fact] - public void NegativeOffsetThrows() - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) - { - AssertExtensions.Throws(OffsetParamName, () => - FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[1], -1, 1))); - - // buffer is checked first - AssertExtensions.Throws(BufferParamName, () => - FSAssert.CompletesSynchronously(WriteAsync(fs, null, -1, 1))); - } - } - - [Fact] - public void NegativeCountThrows() - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) - { - AssertExtensions.Throws(CountParamName, () => - FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[1], 0, -1))); - - // offset is checked before count - AssertExtensions.Throws(OffsetParamName, () => - FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[1], -1, -1))); - - // buffer is checked first - AssertExtensions.Throws(BufferParamName, () => - FSAssert.CompletesSynchronously(WriteAsync(fs, null, -1, -1))); - } - } - - [Fact] - public void BufferOutOfBoundsThrows() - { - using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) - { - // offset out of bounds - Assert.Throws(null, () => - FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[1], 1, 1))); - - // offset out of bounds for 0 count WriteAsync - Assert.Throws(null, () => - FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[1], 2, 0))); - - // offset out of bounds even for 0 length buffer - Assert.Throws(null, () => - FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[0], 1, 0))); - - // combination offset and count out of bounds - Assert.Throws(null, () => - FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[2], 1, 2))); - - // edges - Assert.Throws(null, () => - FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[0], int.MaxValue, 0))); - Assert.Throws(null, () => - FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[0], int.MaxValue, int.MaxValue))); - } - } - - [Fact] public void WriteAsyncDisposedThrows() { using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create)) @@ -101,20 +27,8 @@ namespace System.IO.Tests FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[1], 0, 0))); // out of bounds checking happens first - Assert.Throws(null, () => + Assert.Throws(() => FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[2], 1, 2))); - - // count is checked prior - AssertExtensions.Throws(CountParamName, () => - FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[1], 0, -1))); - - // offset is checked prior - AssertExtensions.Throws(OffsetParamName, () => - FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[1], -1, -1))); - - // buffer is checked first - AssertExtensions.Throws(BufferParamName, () => - FSAssert.CompletesSynchronously(WriteAsync(fs, null, -1, -1))); } } @@ -136,20 +50,8 @@ namespace System.IO.Tests FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[1], 0, 1))); // out of bounds checking happens first - Assert.Throws(null, () => + Assert.Throws(() => FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[2], 1, 2))); - - // count is checked prior - AssertExtensions.Throws(CountParamName, () => - FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[1], 0, -1))); - - // offset is checked prior - AssertExtensions.Throws(OffsetParamName, () => - FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[1], -1, -1))); - - // buffer is checked first - AssertExtensions.Throws(BufferParamName, () => - FSAssert.CompletesSynchronously(WriteAsync(fs, null, -1, -1))); } } @@ -443,19 +345,19 @@ namespace System.IO.Tests FSAssert.IsCancelled(WriteAsync(fs, new byte[1], 0, 1, cancelledToken), cancelledToken); // out of bounds checking happens first - Assert.Throws(null, () => + Assert.Throws(() => FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[2], 1, 2, cancelledToken))); // count is checked prior - AssertExtensions.Throws(CountParamName, () => + AssertExtensions.Throws("count", () => FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[1], 0, -1, cancelledToken))); // offset is checked prior - AssertExtensions.Throws(OffsetParamName, () => + AssertExtensions.Throws("offset", () => FSAssert.CompletesSynchronously(WriteAsync(fs, new byte[1], -1, -1, cancelledToken))); // buffer is checked first - AssertExtensions.Throws(BufferParamName, () => + AssertExtensions.Throws("buffer", () => FSAssert.CompletesSynchronously(WriteAsync(fs, null, -1, -1, cancelledToken))); } } @@ -469,8 +371,5 @@ namespace System.IO.Tests (callback, state) => stream.BeginWrite(buffer, offset, count, callback, state), iar => stream.EndWrite(iar), null); - - protected override string BufferParamName => "buffer"; - protected override string CountParamName => "count"; } } diff --git a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj index c99be14..8505c8d 100644 --- a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj +++ b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj @@ -20,8 +20,6 @@ - - @@ -166,17 +164,12 @@ - - - - + + + + - + diff --git a/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/PipeReaderStream.cs b/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/PipeReaderStream.cs index e87c44c..8bc68d9 100644 --- a/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/PipeReaderStream.cs +++ b/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/PipeReaderStream.cs @@ -45,15 +45,8 @@ namespace System.IO.Pipelines { } - public override int Read(byte[] buffer, int offset, int count) - { - if (buffer is null) - { - throw new ArgumentNullException(nameof(buffer)); - } - - return ReadAsync(buffer, offset, count).GetAwaiter().GetResult(); - } + public override int Read(byte[] buffer, int offset, int count) => + ReadAsync(buffer, offset, count).GetAwaiter().GetResult(); public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); diff --git a/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/PipeWriterStream.cs b/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/PipeWriterStream.cs index 6ef2067..67aeadc 100644 --- a/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/PipeWriterStream.cs +++ b/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/PipeWriterStream.cs @@ -56,15 +56,8 @@ namespace System.IO.Pipelines public sealed override void EndWrite(IAsyncResult asyncResult) => TaskToApm.End(asyncResult); - public override void Write(byte[] buffer, int offset, int count) - { - if (buffer is null) - { - throw new ArgumentNullException(nameof(buffer)); - } - + public override void Write(byte[] buffer, int offset, int count) => WriteAsync(buffer, offset, count).GetAwaiter().GetResult(); - } public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { diff --git a/src/libraries/System.IO.Pipes/src/Resources/Strings.resx b/src/libraries/System.IO.Pipes/src/Resources/Strings.resx index 5b1a648..dfde861 100644 --- a/src/libraries/System.IO.Pipes/src/Resources/Strings.resx +++ b/src/libraries/System.IO.Pipes/src/Resources/Strings.resx @@ -123,9 +123,6 @@ Invalid PipeAccessRights value. - - Offset and length were out of bounds for the array or count is greater than the number of elements from index to the end of the source collection. - pipeName cannot be an empty string. diff --git a/src/libraries/System.IO.Pipes/src/System.IO.Pipes.csproj b/src/libraries/System.IO.Pipes/src/System.IO.Pipes.csproj index 324ce07..c3744e2 100644 --- a/src/libraries/System.IO.Pipes/src/System.IO.Pipes.csproj +++ b/src/libraries/System.IO.Pipes/src/System.IO.Pipes.csproj @@ -21,8 +21,6 @@ - diff --git a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.cs b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.cs index 01c5f19..4dad917 100644 --- a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.cs +++ b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.cs @@ -114,7 +114,7 @@ namespace System.IO.Pipes return ReadAsync(buffer, offset, count, CancellationToken.None).GetAwaiter().GetResult(); } - CheckReadWriteArgs(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); if (!CanRead) { throw Error.GetReadNotSupported(); @@ -142,7 +142,7 @@ namespace System.IO.Pipes public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - CheckReadWriteArgs(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); if (!CanRead) { throw Error.GetReadNotSupported(); @@ -221,7 +221,7 @@ namespace System.IO.Pipes return; } - CheckReadWriteArgs(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); if (!CanWrite) { throw Error.GetWriteNotSupported(); @@ -250,7 +250,7 @@ namespace System.IO.Pipes public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - CheckReadWriteArgs(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); if (!CanWrite) { throw Error.GetWriteNotSupported(); @@ -319,18 +319,6 @@ namespace System.IO.Pipes base.EndWrite(asyncResult); } - private void CheckReadWriteArgs(byte[] buffer, int offset, int count) - { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer), SR.ArgumentNull_Buffer); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); - } - [Conditional("DEBUG")] private static void DebugAssertHandleValid(SafePipeHandle handle) { diff --git a/src/libraries/System.IO.UnmanagedMemoryStream/tests/UmsReadWrite.cs b/src/libraries/System.IO.UnmanagedMemoryStream/tests/UmsReadWrite.cs index 833ad54..fe31b2e 100644 --- a/src/libraries/System.IO.UnmanagedMemoryStream/tests/UmsReadWrite.cs +++ b/src/libraries/System.IO.UnmanagedMemoryStream/tests/UmsReadWrite.cs @@ -41,14 +41,14 @@ namespace System.IO.Tests Assert.Throws(() => stream.WriteAsync(new byte[] { }, 1, -2).GetAwaiter().GetResult()); //case#6: call Read with count > ums.Length-startIndex, ArgumentOutOfRangeException should be thrown. - AssertExtensions.Throws(null, () => stream.Read(new byte[10], 0, 11)); // "Offset and length were out of bounds for the array or count is greater than the number of elements from index to the end of the source collection." - AssertExtensions.Throws(null, () => stream.ReadAsync(new byte[10], 0, 11).GetAwaiter().GetResult()); - AssertExtensions.Throws(null, () => stream.Write(new byte[3], 0, 4)); - AssertExtensions.Throws(null, () => stream.WriteAsync(new byte[3], 0, 4).GetAwaiter().GetResult()); + AssertExtensions.Throws("count", () => stream.Read(new byte[10], 0, 11)); // "Offset and length were out of bounds for the array or count is greater than the number of elements from index to the end of the source collection." + AssertExtensions.Throws("count", () => stream.ReadAsync(new byte[10], 0, 11).GetAwaiter().GetResult()); + AssertExtensions.Throws("count", () => stream.Write(new byte[3], 0, 4)); + AssertExtensions.Throws("count", () => stream.WriteAsync(new byte[3], 0, 4).GetAwaiter().GetResult()); //case#10: Call Read on a n length stream, (Capacity is implicitly n), position is set to end, call it, should throw ArgumentException. - AssertExtensions.Throws(null, () => stream.Read(new byte[] { }, 0, 1)); - AssertExtensions.Throws(null, () => stream.Write(new byte[] { }, 0, 1)); + AssertExtensions.Throws("count", () => stream.Read(new byte[] { }, 0, 1)); + AssertExtensions.Throws("count", () => stream.Write(new byte[] { }, 0, 1)); } } } diff --git a/src/libraries/System.IO/tests/BinaryWriter/BinaryWriter.WriteByteCharTests.cs b/src/libraries/System.IO/tests/BinaryWriter/BinaryWriter.WriteByteCharTests.cs index f008fcd..d1a8530 100644 --- a/src/libraries/System.IO/tests/BinaryWriter/BinaryWriter.WriteByteCharTests.cs +++ b/src/libraries/System.IO/tests/BinaryWriter/BinaryWriter.WriteByteCharTests.cs @@ -302,9 +302,9 @@ namespace System.IO.Tests for (int iLoop = 0; iLoop < iArrLargeValues.Length; iLoop++) { // [] Offset out of range - AssertExtensions.Throws(null, () => dw2.Write(bArr, iArrLargeValues[iLoop], 0)); + Assert.Throws(() => dw2.Write(bArr, iArrLargeValues[iLoop], 0)); // [] Invalid count value - AssertExtensions.Throws(null, () => dw2.Write(bArr, 0, iArrLargeValues[iLoop])); + Assert.Throws(() => dw2.Write(bArr, 0, iArrLargeValues[iLoop])); } dw2.Dispose(); mstr.Dispose(); diff --git a/src/libraries/System.IO/tests/MemoryStream/MemoryStreamTests.cs b/src/libraries/System.IO/tests/MemoryStream/MemoryStreamTests.cs index f6b95fd..919af5d 100644 --- a/src/libraries/System.IO/tests/MemoryStream/MemoryStreamTests.cs +++ b/src/libraries/System.IO/tests/MemoryStream/MemoryStreamTests.cs @@ -126,8 +126,8 @@ namespace System.IO.Tests Assert.Throws(() => ms2.Read(null, 0, 0)); Assert.Throws(() => ms2.Read(new byte[] { 1 }, -1, 0)); Assert.Throws(() => ms2.Read(new byte[] { 1 }, 0, -1)); - AssertExtensions.Throws(null, () => ms2.Read(new byte[] { 1 }, 2, 0)); - AssertExtensions.Throws(null, () => ms2.Read(new byte[] { 1 }, 0, 2)); + Assert.Throws(() => ms2.Read(new byte[] { 1 }, 2, 0)); + Assert.Throws(() => ms2.Read(new byte[] { 1 }, 0, 2)); ms2.Dispose(); diff --git a/src/libraries/System.IO/tests/Stream/Stream.Methods.cs b/src/libraries/System.IO/tests/Stream/Stream.Methods.cs index e44c645..08daafc 100644 --- a/src/libraries/System.IO/tests/Stream/Stream.Methods.cs +++ b/src/libraries/System.IO/tests/Stream/Stream.Methods.cs @@ -281,8 +281,8 @@ namespace System.IO.Tests AssertExtensions.Throws("count", () => { stream.ReadAsync(new byte[1], 0, -1); }); AssertExtensions.Throws("count", () => { stream.WriteAsync(new byte[1], 0, -1); }); - AssertExtensions.Throws(null, () => { stream.ReadAsync(new byte[1], 0, 2); }); - AssertExtensions.Throws(null, () => { stream.WriteAsync(new byte[1], 0, 2); }); + Assert.Throws(() => { stream.ReadAsync(new byte[1], 0, 2); }); + Assert.Throws(() => { stream.WriteAsync(new byte[1], 0, 2); }); } } } diff --git a/src/libraries/System.IO/tests/Stream/Stream.NullTests.cs b/src/libraries/System.IO/tests/Stream/Stream.NullTests.cs index a7311be..5b2ab1f 100644 --- a/src/libraries/System.IO/tests/Stream/Stream.NullTests.cs +++ b/src/libraries/System.IO/tests/Stream/Stream.NullTests.cs @@ -56,22 +56,18 @@ namespace System.IO.Tests [Fact] public static async Task TestNullStream_CopyToAsyncValidation() { - // Since Stream.Null previously inherited its CopyToAsync - // implementation from the base class, which did check its - // arguments, we have to make sure it validates them as - // well for compat. - var disposedStream = new MemoryStream(); disposedStream.Dispose(); + // Stream.Null doesn't do any validation of state or arguments var readOnlyStream = new MemoryStream(new byte[1], writable: false); - await AssertExtensions.ThrowsAsync("destination", () => Stream.Null.CopyToAsync(null)); - await AssertExtensions.ThrowsAsync("destination", () => Stream.Null.CopyToAsync(null, -123)); // Should check if destination == null first - await AssertExtensions.ThrowsAsync("bufferSize", () => Stream.Null.CopyToAsync(Stream.Null, 0)); // 0 shouldn't be a valid buffer size - await AssertExtensions.ThrowsAsync("bufferSize", () => Stream.Null.CopyToAsync(Stream.Null, -123)); - await Assert.ThrowsAsync(() => Stream.Null.CopyToAsync(disposedStream)); - await Assert.ThrowsAsync(() => Stream.Null.CopyToAsync(readOnlyStream)); + await Stream.Null.CopyToAsync(null); + await Stream.Null.CopyToAsync(null, -123); + await Stream.Null.CopyToAsync(Stream.Null, 0); // 0 shouldn't be a valid buffer size + await Stream.Null.CopyToAsync(Stream.Null, -123); + await Stream.Null.CopyToAsync(disposedStream); + await Stream.Null.CopyToAsync(readOnlyStream); } [Theory] diff --git a/src/libraries/System.IO/tests/Stream/Stream.cs b/src/libraries/System.IO/tests/Stream/Stream.cs index 2acf4e6..03d547a 100644 --- a/src/libraries/System.IO/tests/Stream/Stream.cs +++ b/src/libraries/System.IO/tests/Stream/Stream.cs @@ -44,4 +44,46 @@ namespace System.IO.Tests } } } + + public class StreamArgumentValidation + { + [Fact] + public void ValidateBufferArguments() + { + AssertExtensions.Throws("buffer", () => ExposeProtectedStream.ValidateBufferArguments(null, 0, 0)); + AssertExtensions.Throws("offset", () => ExposeProtectedStream.ValidateBufferArguments(new byte[3], -1, 0)); + AssertExtensions.Throws("count", () => ExposeProtectedStream.ValidateBufferArguments(new byte[3], 4, 0)); + AssertExtensions.Throws("count", () => ExposeProtectedStream.ValidateBufferArguments(new byte[3], 0, -1)); + AssertExtensions.Throws("count", () => ExposeProtectedStream.ValidateBufferArguments(new byte[3], 0, 4)); + AssertExtensions.Throws("count", () => ExposeProtectedStream.ValidateBufferArguments(new byte[3], 3, 1)); + } + + [Fact] + public void ValidateCopyToArguments() + { + AssertExtensions.Throws("destination", () => ExposeProtectedStream.ValidateCopyToArguments(null, 0)); + AssertExtensions.Throws("bufferSize", () => ExposeProtectedStream.ValidateCopyToArguments(new MemoryStream(), 0)); + AssertExtensions.Throws("bufferSize", () => ExposeProtectedStream.ValidateCopyToArguments(new MemoryStream(), -1)); + + var srcDisposed = new MemoryStream(); + srcDisposed.Dispose(); + + var dstDisposed = new MemoryStream(); + dstDisposed.Dispose(); + + Assert.Throws(() => ExposeProtectedStream.ValidateCopyToArguments(dstDisposed, 1)); + Assert.Throws(() => ExposeProtectedStream.ValidateCopyToArguments(dstDisposed, 1)); + + Assert.Throws(() => ExposeProtectedStream.ValidateCopyToArguments(new MemoryStream(new byte[1], writable: false), 1)); + } + + private abstract class ExposeProtectedStream : Stream + { + public static new void ValidateBufferArguments(byte[] buffer, int offset, int count) => + Stream.ValidateBufferArguments(buffer, offset, count); + + public static new void ValidateCopyToArguments(Stream destination, int bufferSize) => + Stream.ValidateCopyToArguments(destination, bufferSize); + } + } } diff --git a/src/libraries/System.Net.Connections/src/System/Net/Connections/DuplexPipeStream.cs b/src/libraries/System.Net.Connections/src/System/Net/Connections/DuplexPipeStream.cs index 3319c97..825f4fe 100644 --- a/src/libraries/System.Net.Connections/src/System/Net/Connections/DuplexPipeStream.cs +++ b/src/libraries/System.Net.Connections/src/System/Net/Connections/DuplexPipeStream.cs @@ -5,7 +5,6 @@ using System; using System.Buffers; using System.IO; using System.IO.Pipelines; -using System.Runtime.ExceptionServices; using System.Threading; using System.Threading.Tasks; @@ -57,7 +56,7 @@ namespace System.Net.Connections public override int Read(byte[] buffer, int offset, int count) { - if (buffer == null) throw new ArgumentNullException(nameof(buffer)); + ValidateBufferArguments(buffer, offset, count); ValueTask t = ReadAsync(buffer.AsMemory(offset, count)); return @@ -67,7 +66,8 @@ namespace System.Net.Connections public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - if (buffer == null) return Task.FromException(ExceptionDispatchInfo.SetCurrentStackTrace(new ArgumentNullException(nameof(buffer)))); + ValidateBufferArguments(buffer, offset, count); + return ReadAsync(buffer.AsMemory(offset, count), cancellationToken).AsTask(); } @@ -139,6 +139,8 @@ namespace System.Net.Connections public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { + ValidateBufferArguments(buffer, offset, count); + return WriteAsync(buffer.AsMemory(offset, count), cancellationToken).AsTask(); } @@ -160,6 +162,8 @@ namespace System.Net.Connections public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) { + ValidateCopyToArguments(destination, bufferSize); + return _reader.CopyToAsync(destination, cancellationToken); } } diff --git a/src/libraries/System.Net.Http/src/System.Net.Http.csproj b/src/libraries/System.Net.Http/src/System.Net.Http.csproj index bfc8a69..e38d9a0 100644 --- a/src/libraries/System.Net.Http/src/System.Net.Http.csproj +++ b/src/libraries/System.Net.Http/src/System.Net.Http.csproj @@ -106,8 +106,6 @@ - buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - - if ((uint)count > buffer.Length - offset) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } - } - - /// - /// Validate the arguments to CopyTo, as would Stream.CopyTo, but with knowledge that - /// the source stream is always readable and so only checking the destination. - /// - protected static void ValidateCopyToArgs(Stream source, Stream destination, int bufferSize) - { - if (destination == null) - { - throw new ArgumentNullException(nameof(destination)); - } - - if (bufferSize <= 0) - { - throw new ArgumentOutOfRangeException(nameof(bufferSize), bufferSize, SR.ArgumentOutOfRange_NeedPosNum); - } - - if (!destination.CanWrite) - { - throw destination.CanRead ? - new NotSupportedException(SR.NotSupported_UnwritableStream) : - (Exception)new ObjectDisposedException(nameof(destination), SR.ObjectDisposed_StreamClosed); - } - } - public sealed override int ReadByte() { byte b = 0; @@ -86,13 +44,13 @@ namespace System.Net.Http public sealed override int Read(byte[] buffer, int offset, int count) { - ValidateBufferArgs(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); return Read(buffer.AsSpan(offset, count)); } public sealed override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - ValidateBufferArgs(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); return ReadAsync(new Memory(buffer, offset, count), cancellationToken).AsTask(); } @@ -110,7 +68,7 @@ namespace System.Net.Http public sealed override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - ValidateBufferArgs(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); return WriteAsync(new ReadOnlyMemory(buffer, offset, count), cancellationToken).AsTask(); } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpContent.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpContent.cs index 7ceae4e..1ecfeed 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpContent.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpContent.cs @@ -910,7 +910,7 @@ namespace System.Net.Http ArraySegment buffer; if (TryGetBuffer(out buffer)) { - StreamHelpers.ValidateCopyToArgs(this, destination, bufferSize); + ValidateCopyToArguments(destination, bufferSize); long pos = Position; long length = Length; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/MultipartContent.cs b/src/libraries/System.Net.Http/src/System/Net/Http/MultipartContent.cs index 7ea8eb2..8227b4b 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/MultipartContent.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/MultipartContent.cs @@ -471,7 +471,7 @@ namespace System.Net.Http public override int Read(byte[] buffer, int offset, int count) { - ValidateReadArgs(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); if (count == 0) { return 0; @@ -532,7 +532,7 @@ namespace System.Net.Http public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - ValidateReadArgs(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); return ReadAsyncPrivate(new Memory(buffer, offset, count), cancellationToken).AsTask(); } @@ -641,26 +641,6 @@ namespace System.Net.Http public override long Length => _length; - private static void ValidateReadArgs(byte[] buffer, int offset, int count) - { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if (count < 0) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } - if (offset > buffer.Length - count) - { - throw new ArgumentException(SR.net_http_buffer_insufficient_length, nameof(buffer)); - } - } - public override void Flush() { } public override void SetLength(long value) { throw new NotSupportedException(); } public override void Write(byte[] buffer, int offset, int count) { throw new NotSupportedException(); } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ChunkedEncodingReadStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ChunkedEncodingReadStream.cs index b9ccbce..a0f211a 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ChunkedEncodingReadStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ChunkedEncodingReadStream.cs @@ -191,7 +191,7 @@ namespace System.Net.Http public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) { - ValidateCopyToArgs(this, destination, bufferSize); + ValidateCopyToArguments(destination, bufferSize); return cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionCloseReadStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionCloseReadStream.cs index 28ef24f..d061ae0 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionCloseReadStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionCloseReadStream.cs @@ -89,7 +89,7 @@ namespace System.Net.Http public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) { - ValidateCopyToArgs(this, destination, bufferSize); + ValidateCopyToArguments(destination, bufferSize); if (cancellationToken.IsCancellationRequested) { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ContentLengthReadStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ContentLengthReadStream.cs index a946a7a..d1c7389 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ContentLengthReadStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ContentLengthReadStream.cs @@ -118,7 +118,7 @@ namespace System.Net.Http public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) { - ValidateCopyToArgs(this, destination, bufferSize); + ValidateCopyToArguments(destination, bufferSize); if (cancellationToken.IsCancellationRequested) { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs index 2005672..3eed838 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs @@ -1444,14 +1444,14 @@ namespace System.Net.Http public override void CopyTo(Stream destination, int bufferSize) { - StreamHelpers.ValidateCopyToArgs(this, destination, bufferSize); + ValidateCopyToArguments(destination, bufferSize); Http2Stream http2Stream = _http2Stream ?? throw ExceptionDispatchInfo.SetCurrentStackTrace(new ObjectDisposedException(nameof(Http2ReadStream))); http2Stream.CopyTo(_responseMessage, destination, bufferSize); } public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) { - StreamHelpers.ValidateCopyToArgs(this, destination, bufferSize); + ValidateCopyToArguments(destination, bufferSize); Http2Stream? http2Stream = _http2Stream; return http2Stream is null ? Task.FromException(ExceptionDispatchInfo.SetCurrentStackTrace(new ObjectDisposedException(nameof(Http2ReadStream)))) : diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RawConnectionStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RawConnectionStream.cs index 31b2408..4459b2e 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RawConnectionStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RawConnectionStream.cs @@ -89,7 +89,7 @@ namespace System.Net.Http public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) { - ValidateCopyToArgs(this, destination, bufferSize); + ValidateCopyToArguments(destination, bufferSize); if (cancellationToken.IsCancellationRequested) { diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/MultipartContentTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/MultipartContentTest.cs index a945cc3..1ac9ae9 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/MultipartContentTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/MultipartContentTest.cs @@ -331,12 +331,12 @@ namespace System.Net.Http.Functional.Tests AssertExtensions.Throws("buffer", null, () => s.Read(null, 0, 0)); AssertExtensions.Throws("offset", () => s.Read(new byte[1], -1, 0)); AssertExtensions.Throws("count", () => s.Read(new byte[1], 0, -1)); - AssertExtensions.Throws("buffer", null, () => s.Read(new byte[1], 1, 1)); + AssertExtensions.Throws("count", null, () => s.Read(new byte[1], 1, 1)); AssertExtensions.Throws("buffer", null, () => { s.ReadAsync(null, 0, 0); }); AssertExtensions.Throws("offset", () => { s.ReadAsync(new byte[1], -1, 0); }); AssertExtensions.Throws("count", () => { s.ReadAsync(new byte[1], 0, -1); }); - AssertExtensions.Throws("buffer", null, () => { s.ReadAsync(new byte[1], 1, 1); }); + AssertExtensions.Throws("count", null, () => { s.ReadAsync(new byte[1], 1, 1); }); AssertExtensions.Throws("value", () => s.Position = -1); AssertExtensions.Throws("value", () => s.Seek(-1, SeekOrigin.Begin)); diff --git a/src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj b/src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj index 72fb0d8..ddcd41c 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj +++ b/src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj @@ -1,4 +1,4 @@ - + ../../src/Resources/Strings.resx true @@ -22,8 +22,6 @@ Link="ProductionCode\Common\System\StringExtensions.cs" /> - false; public override bool CanRead => true; - public override int Read(byte[] buffer, int offset, int size) + public override int Read(byte[] buffer, int offset, int count) { - if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, "buffer.Length:" + buffer?.Length + " size:" + size + " offset:" + offset); + if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, "buffer.Length:" + buffer.Length + " count:" + count + " offset:" + offset); - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0 || offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if (size < 0 || size > buffer.Length - offset) - { - throw new ArgumentOutOfRangeException(nameof(size)); - } - if (size == 0 || _closed) + ValidateBufferArguments(buffer, offset, count); + + if (count == 0 || _closed) { return 0; } - return ReadCore(buffer, offset, size); + return ReadCore(buffer, offset, count); } - public override IAsyncResult BeginRead(byte[] buffer, int offset, int size, AsyncCallback? callback, object? state) + public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) { - if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, "buffer.Length:" + buffer?.Length + " size:" + size + " offset:" + offset); + if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, "buffer.Length:" + buffer.Length + " count:" + count + " offset:" + offset); - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0 || offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if (size < 0 || size > buffer.Length - offset) - { - throw new ArgumentOutOfRangeException(nameof(size)); - } + ValidateBufferArguments(buffer, offset, count); - return BeginReadCore(buffer, offset, size, callback, state)!; + return BeginReadCore(buffer, offset, count, callback, state)!; } public override void Flush() { } @@ -72,9 +51,9 @@ namespace System.Net public override void SetLength(long value) => throw new NotSupportedException(SR.net_noseek); - public override void Write(byte[] buffer, int offset, int size) => throw new InvalidOperationException(SR.net_readonlystream); + public override void Write(byte[] buffer, int offset, int count) => throw new InvalidOperationException(SR.net_readonlystream); - public override IAsyncResult BeginWrite(byte[] buffer, int offset, int size, AsyncCallback? callback, object? state) + public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) { throw new InvalidOperationException(SR.net_readonlystream); } diff --git a/src/libraries/System.Net.HttpListener/src/System/Net/HttpResponseStream.cs b/src/libraries/System.Net.HttpListener/src/System/Net/HttpResponseStream.cs index e51f802..f082d7c 100644 --- a/src/libraries/System.Net.HttpListener/src/System/Net/HttpResponseStream.cs +++ b/src/libraries/System.Net.HttpListener/src/System/Net/HttpResponseStream.cs @@ -40,47 +40,27 @@ namespace System.Net public override int EndRead(IAsyncResult asyncResult) => throw new InvalidOperationException(SR.net_writeonlystream); - public override void Write(byte[] buffer, int offset, int size) + public override void Write(byte[] buffer, int offset, int count) { - if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, "buffer.Length:" + buffer?.Length + " size:" + size + " offset:" + offset); + ValidateBufferArguments(buffer, offset, count); + + if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, "buffer.Length:" + buffer.Length + " count:" + count + " offset:" + offset); - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0 || offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if (size < 0 || size > buffer.Length - offset) - { - throw new ArgumentOutOfRangeException(nameof(size)); - } if (_closed) { return; } - WriteCore(buffer, offset, size); + WriteCore(buffer, offset, count); } - public override IAsyncResult BeginWrite(byte[] buffer, int offset, int size, AsyncCallback? callback, object? state) + public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) { - if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, "buffer.Length:" + buffer?.Length + " size:" + size + " offset:" + offset); - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0 || offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if (size < 0 || size > buffer.Length - offset) - { - throw new ArgumentOutOfRangeException(nameof(size)); - } + ValidateBufferArguments(buffer, offset, count); + + if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, "buffer.Length:" + buffer.Length + " count:" + count + " offset:" + offset); - return BeginWriteCore(buffer, offset, size, callback, state); + return BeginWriteCore(buffer, offset, count, callback, state); } public override void EndWrite(IAsyncResult asyncResult) diff --git a/src/libraries/System.Net.HttpListener/tests/HttpRequestStreamTests.cs b/src/libraries/System.Net.HttpListener/tests/HttpRequestStreamTests.cs index a451d51..ba1f6b5 100644 --- a/src/libraries/System.Net.HttpListener/tests/HttpRequestStreamTests.cs +++ b/src/libraries/System.Net.HttpListener/tests/HttpRequestStreamTests.cs @@ -531,8 +531,8 @@ namespace System.Net.Tests using (Stream inputStream = request.InputStream) { - AssertExtensions.Throws("offset", () => inputStream.Read(new byte[2], offset, 0)); - await AssertExtensions.ThrowsAsync("offset", () => inputStream.ReadAsync(new byte[2], offset, 0)); + Assert.Throws(() => inputStream.Read(new byte[2], offset, 0)); + await Assert.ThrowsAsync(() => inputStream.ReadAsync(new byte[2], offset, 0)); } context.Response.Close(); @@ -560,8 +560,8 @@ namespace System.Net.Tests using (Stream inputStream = request.InputStream) { - AssertExtensions.Throws("size", () => inputStream.Read(new byte[2], offset, size)); - await AssertExtensions.ThrowsAsync("size", () => inputStream.ReadAsync(new byte[2], offset, size)); + Assert.Throws(() => inputStream.Read(new byte[2], offset, size)); + await Assert.ThrowsAsync(() => inputStream.ReadAsync(new byte[2], offset, size)); } context.Response.Close(); diff --git a/src/libraries/System.Net.HttpListener/tests/HttpResponseStreamTests.cs b/src/libraries/System.Net.HttpListener/tests/HttpResponseStreamTests.cs index d347820..21cfaac 100644 --- a/src/libraries/System.Net.HttpListener/tests/HttpResponseStreamTests.cs +++ b/src/libraries/System.Net.HttpListener/tests/HttpResponseStreamTests.cs @@ -278,8 +278,8 @@ namespace System.Net.Tests using (HttpListenerResponse response = await _helper.GetResponse()) using (Stream outputStream = response.OutputStream) { - AssertExtensions.Throws("offset", () => outputStream.Write(new byte[2], offset, 0)); - await AssertExtensions.ThrowsAsync("offset", () => outputStream.WriteAsync(new byte[2], offset, 0)); + Assert.Throws(() => outputStream.Write(new byte[2], offset, 0)); + await Assert.ThrowsAsync(() => outputStream.WriteAsync(new byte[2], offset, 0)); } } @@ -292,8 +292,8 @@ namespace System.Net.Tests using (HttpListenerResponse response = await _helper.GetResponse()) using (Stream outputStream = response.OutputStream) { - AssertExtensions.Throws("size", () => outputStream.Write(new byte[2], offset, size)); - await AssertExtensions.ThrowsAsync("size", () => outputStream.WriteAsync(new byte[2], offset, size)); + Assert.Throws(() => outputStream.Write(new byte[2], offset, size)); + await Assert.ThrowsAsync(() => outputStream.WriteAsync(new byte[2], offset, size)); } } diff --git a/src/libraries/System.Net.Mail/src/System/Net/Base64Stream.cs b/src/libraries/System.Net.Mail/src/System/Net/Base64Stream.cs index 69cf08e..5cc84a5 100644 --- a/src/libraries/System.Net.Mail/src/System/Net/Base64Stream.cs +++ b/src/libraries/System.Net.Mail/src/System/Net/Base64Stream.cs @@ -63,18 +63,7 @@ namespace System.Net public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0 || offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if (offset + count > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } + ValidateBufferArguments(buffer, offset, count); var result = new ReadAsyncResult(this, buffer, offset, count, callback, state); result.Read(); @@ -83,18 +72,7 @@ namespace System.Net public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0 || offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if (offset + count > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } + ValidateBufferArguments(buffer, offset, count); var result = new WriteAsyncResult(this, buffer, offset, count, callback, state); result.Write(); @@ -216,15 +194,7 @@ namespace System.Net public override int Read(byte[] buffer, int offset, int count) { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0 || offset > buffer.Length) - throw new ArgumentOutOfRangeException(nameof(offset)); - - if (offset + count > buffer.Length) - throw new ArgumentOutOfRangeException(nameof(count)); + ValidateBufferArguments(buffer, offset, count); while (true) { @@ -251,18 +221,7 @@ namespace System.Net public override void Write(byte[] buffer, int offset, int count) { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0 || offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if (offset + count > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } + ValidateBufferArguments(buffer, offset, count); int written = 0; diff --git a/src/libraries/System.Net.Mail/src/System/Net/Mime/EightBitStream.cs b/src/libraries/System.Net.Mail/src/System/Net/Mime/EightBitStream.cs index 0b65f2c..16fa22c 100644 --- a/src/libraries/System.Net.Mail/src/System/Net/Mime/EightBitStream.cs +++ b/src/libraries/System.Net.Mail/src/System/Net/Mime/EightBitStream.cs @@ -53,18 +53,7 @@ namespace System.Net.Mime /// State to pass to callback public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0 || offset >= buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if (offset + count > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } + ValidateBufferArguments(buffer, offset, count); IAsyncResult result; if (_shouldEncodeLeadingDots) @@ -95,18 +84,7 @@ namespace System.Net.Mime /// Count of bytes to write public override void Write(byte[] buffer, int offset, int count) { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0 || offset >= buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if (offset + count > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } + ValidateBufferArguments(buffer, offset, count); if (_shouldEncodeLeadingDots) { diff --git a/src/libraries/System.Net.Mail/src/System/Net/Mime/QEncodedStream.cs b/src/libraries/System.Net.Mail/src/System/Net/Mime/QEncodedStream.cs index d5499c0..21e2624 100644 --- a/src/libraries/System.Net.Mail/src/System/Net/Mime/QEncodedStream.cs +++ b/src/libraries/System.Net.Mail/src/System/Net/Mime/QEncodedStream.cs @@ -53,18 +53,7 @@ namespace System.Net.Mime public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0 || offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if (offset + count > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } + ValidateBufferArguments(buffer, offset, count); WriteAsyncResult result = new WriteAsyncResult(this, buffer, offset, count, callback, state); result.Write(); @@ -220,18 +209,7 @@ namespace System.Net.Mime public override void Write(byte[] buffer, int offset, int count) { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0 || offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if (offset + count > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } + ValidateBufferArguments(buffer, offset, count); int written = 0; while (true) diff --git a/src/libraries/System.Net.Mail/src/System/Net/Mime/QuotedPrintableStream.cs b/src/libraries/System.Net.Mail/src/System/Net/Mime/QuotedPrintableStream.cs index 60f85e4..eadab7b 100644 --- a/src/libraries/System.Net.Mail/src/System/Net/Mime/QuotedPrintableStream.cs +++ b/src/libraries/System.Net.Mail/src/System/Net/Mime/QuotedPrintableStream.cs @@ -86,18 +86,7 @@ namespace System.Net.Mime public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0 || offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if (offset + count > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } + ValidateBufferArguments(buffer, offset, count); WriteAsyncResult result = new WriteAsyncResult(this, buffer, offset, count, callback, state); result.Write(); @@ -338,18 +327,7 @@ namespace System.Net.Mime public override void Write(byte[] buffer, int offset, int count) { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0 || offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if (offset + count > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } + ValidateBufferArguments(buffer, offset, count); int written = 0; while (true) diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs index 2fe5d79..717b5fd 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs @@ -41,45 +41,27 @@ namespace System.Net.Quic public override void EndWrite(IAsyncResult asyncResult) => TaskToApm.End(asyncResult); - private static void ValidateBufferArgs(byte[] buffer, int offset, int count) - { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - - if ((uint)offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - - if ((uint)count > buffer.Length - offset) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } - } - public override int Read(byte[] buffer, int offset, int count) { - ValidateBufferArgs(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); return Read(buffer.AsSpan(offset, count)); } public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - ValidateBufferArgs(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); return ReadAsync(new Memory(buffer, offset, count), cancellationToken).AsTask(); } public override void Write(byte[] buffer, int offset, int count) { - ValidateBufferArgs(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); Write(buffer.AsSpan(offset, count)); } public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - ValidateBufferArgs(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); return WriteAsync(new ReadOnlyMemory(buffer, offset, count), cancellationToken).AsTask(); } diff --git a/src/libraries/System.Net.Requests/src/System/Net/RequestStream.cs b/src/libraries/System.Net.Requests/src/System/Net/RequestStream.cs index 41ca7f6..d5f5a26 100644 --- a/src/libraries/System.Net.Requests/src/System/Net/RequestStream.cs +++ b/src/libraries/System.Net.Requests/src/System/Net/RequestStream.cs @@ -95,52 +95,19 @@ namespace System.Net public override void Write(byte[] buffer, int offset, int count) { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0 || offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if (count < 0 || count > (buffer.Length - offset)) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } + ValidateBufferArguments(buffer, offset, count); _buffer.Write(buffer, offset, count); } public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0 || offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if (count < 0 || count > (buffer.Length - offset)) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } + ValidateBufferArguments(buffer, offset, count); return _buffer.WriteAsync(buffer, offset, count, cancellationToken); } public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback? asyncCallback, object? asyncState) { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if (offset < 0 || offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if (count < 0 || count > (buffer.Length - offset)) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } + ValidateBufferArguments(buffer, offset, count); return _buffer.BeginWrite(buffer, offset, count, asyncCallback, asyncState); } diff --git a/src/libraries/System.Net.Requests/tests/RequestStreamTest.cs b/src/libraries/System.Net.Requests/tests/RequestStreamTest.cs index f4f48ff..c642396 100644 --- a/src/libraries/System.Net.Requests/tests/RequestStreamTest.cs +++ b/src/libraries/System.Net.Requests/tests/RequestStreamTest.cs @@ -57,7 +57,7 @@ namespace System.Net.Tests { await GetRequestStream((stream) => { - AssertExtensions.Throws("offset", () => stream.Write(buffer, int.MaxValue, int.MaxValue)); + AssertExtensions.Throws("count", () => stream.Write(buffer, int.MaxValue, int.MaxValue)); }); } @@ -106,7 +106,7 @@ namespace System.Net.Tests { await GetRequestStream((stream) => { - AssertExtensions.Throws("offset", () => { stream.WriteAsync(buffer, int.MaxValue, int.MaxValue); }); + AssertExtensions.Throws("count", () => { stream.WriteAsync(buffer, int.MaxValue, int.MaxValue); }); }); } @@ -177,7 +177,7 @@ namespace System.Net.Tests { await GetRequestStream((stream) => { - AssertExtensions.Throws("offset", () => stream.BeginWrite(buffer, int.MaxValue, int.MaxValue, null, null)); + AssertExtensions.Throws("count", () => stream.BeginWrite(buffer, int.MaxValue, int.MaxValue, null, null)); }); } diff --git a/src/libraries/System.Net.Security/src/Resources/Strings.resx b/src/libraries/System.Net.Security/src/Resources/Strings.resx index e518415..74c7555 100644 --- a/src/libraries/System.Net.Security/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Security/src/Resources/Strings.resx @@ -242,9 +242,6 @@ The payload size is limited to {0}, attempted set it to {1}. - - Sum of offset and count cannot be greater than the length of the buffer. - The specified value is not valid in the '{0}' enumeration. diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateStream.cs b/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateStream.cs index 2cab7ef..e84f1ea 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateStream.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateStream.cs @@ -302,7 +302,7 @@ namespace System.Net.Security public override int Read(byte[] buffer, int offset, int count) { - ValidateParameters(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); ThrowIfFailed(authSuccessCheck: true); if (!CanGetSecureStream) @@ -317,7 +317,7 @@ namespace System.Net.Security public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - ValidateParameters(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); ThrowIfFailed(authSuccessCheck: true); if (!CanGetSecureStream) @@ -446,7 +446,7 @@ namespace System.Net.Security public override void Write(byte[] buffer, int offset, int count) { - ValidateParameters(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); ThrowIfFailed(authSuccessCheck: true); if (!CanGetSecureStream) @@ -460,7 +460,7 @@ namespace System.Net.Security public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - ValidateParameters(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); ThrowIfFailed(authSuccessCheck: true); if (!CanGetSecureStream) @@ -553,30 +553,6 @@ namespace System.Net.Security } } - /// Validates user parameters for all Read/Write methods. - private static void ValidateParameters(byte[] buffer, int offset, int count) - { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - - if (offset < 0) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - - if (count < 0) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } - - if (count > buffer.Length - offset) - { - throw new ArgumentOutOfRangeException(nameof(count), SR.net_offset_plus_count); - } - } - private void ValidateCreateContext( string package, NetworkCredential credential, diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs index b43c831..df02d7d 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs @@ -731,32 +731,6 @@ namespace System.Net.Security } } - // - // Validates user parameters for all Read/Write methods. - // - private void ValidateParameters(byte[] buffer, int offset, int count) - { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - - if (offset < 0) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - - if (count < 0) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } - - if (count > buffer.Length - offset) - { - throw new ArgumentOutOfRangeException(nameof(count), SR.net_offset_plus_count); - } - } - ~SslStream() { Dispose(disposing: false); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs index 03cf0a8..db0883a 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs @@ -753,7 +753,7 @@ namespace System.Net.Security public override int Read(byte[] buffer, int offset, int count) { ThrowIfExceptionalOrNotAuthenticated(); - ValidateParameters(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); ValueTask vt = ReadAsyncInternal(new SyncReadWriteAdapter(InnerStream), new Memory(buffer, offset, count)); Debug.Assert(vt.IsCompleted, "Sync operation must have completed synchronously"); return vt.GetAwaiter().GetResult(); @@ -764,7 +764,7 @@ namespace System.Net.Security public override void Write(byte[] buffer, int offset, int count) { ThrowIfExceptionalOrNotAuthenticated(); - ValidateParameters(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); ValueTask vt = WriteAsyncInternal(new SyncReadWriteAdapter(InnerStream), new ReadOnlyMemory(buffer, offset, count)); Debug.Assert(vt.IsCompleted, "Sync operation must have completed synchronously"); @@ -798,7 +798,7 @@ namespace System.Net.Security public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { ThrowIfExceptionalOrNotAuthenticated(); - ValidateParameters(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); return WriteAsync(new ReadOnlyMemory(buffer, offset, count), cancellationToken).AsTask(); } @@ -811,7 +811,7 @@ namespace System.Net.Security public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { ThrowIfExceptionalOrNotAuthenticated(); - ValidateParameters(buffer, offset, count); + ValidateBufferArguments(buffer, offset, count); return ReadAsyncInternal(new AsyncReadWriteAdapter(InnerStream, cancellationToken), new Memory(buffer, offset, count)).AsTask(); } diff --git a/src/libraries/System.Net.Security/tests/UnitTests/Fakes/FakeSslStream.Implementation.cs b/src/libraries/System.Net.Security/tests/UnitTests/Fakes/FakeSslStream.Implementation.cs index 9b74d71..666ca16 100644 --- a/src/libraries/System.Net.Security/tests/UnitTests/Fakes/FakeSslStream.Implementation.cs +++ b/src/libraries/System.Net.Security/tests/UnitTests/Fakes/FakeSslStream.Implementation.cs @@ -35,10 +35,6 @@ namespace System.Net.Security _handshakeCompleted = false; } - private void ValidateParameters(byte[] buffer, int offset, int count) - { - } - private void ValidateCreateContext(SslAuthenticationOptions sslAuthenticationOptions) { _sslAuthenticationOptions = new FakeOptions() { TargetHost = sslAuthenticationOptions.TargetHost }; diff --git a/src/libraries/System.Net.Sockets/src/System.Net.Sockets.csproj b/src/libraries/System.Net.Sockets/src/System.Net.Sockets.csproj index 1311c49..9d398e8 100644 --- a/src/libraries/System.Net.Sockets/src/System.Net.Sockets.csproj +++ b/src/libraries/System.Net.Sockets/src/System.Net.Sockets.csproj @@ -49,8 +49,6 @@ - diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs index bd9613d..0cff6c8 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs @@ -216,27 +216,13 @@ namespace System.Net.Sockets // Number of bytes we read, or 0 if the socket is closed. public override int Read(byte[] buffer, int offset, int count) { - bool canRead = CanRead; // Prevent race with Dispose. + ValidateBufferArguments(buffer, offset, count); ThrowIfDisposed(); - if (!canRead) + if (!CanRead) { throw new InvalidOperationException(SR.net_writeonlystream); } - // Validate input parameters. - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if ((uint)offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if ((uint)count > buffer.Length - offset) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } - try { return _streamSocket.Receive(buffer, offset, count, 0); @@ -297,27 +283,13 @@ namespace System.Net.Sockets // way to indicate an error. public override void Write(byte[] buffer, int offset, int count) { - bool canWrite = CanWrite; // Prevent race with Dispose. + ValidateBufferArguments(buffer, offset, count); ThrowIfDisposed(); - if (!canWrite) + if (!CanWrite) { throw new InvalidOperationException(SR.net_readonlystream); } - // Validate input parameters. - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if ((uint)offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if ((uint)count > buffer.Length - offset) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } - try { // Since the socket is in blocking mode this will always complete @@ -416,27 +388,13 @@ namespace System.Net.Sockets // An IASyncResult, representing the read. public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) { - bool canRead = CanRead; // Prevent race with Dispose. + ValidateBufferArguments(buffer, offset, count); ThrowIfDisposed(); - if (!canRead) + if (!CanRead) { throw new InvalidOperationException(SR.net_writeonlystream); } - // Validate input parameters. - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if ((uint)offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if ((uint)count > buffer.Length - offset) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } - try { return _streamSocket.BeginReceive( @@ -505,27 +463,13 @@ namespace System.Net.Sockets // An IASyncResult, representing the write. public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) { - bool canWrite = CanWrite; // Prevent race with Dispose. + ValidateBufferArguments(buffer, offset, count); ThrowIfDisposed(); - if (!canWrite) + if (!CanWrite) { throw new InvalidOperationException(SR.net_readonlystream); } - // Validate input parameters. - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if ((uint)offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if ((uint)count > buffer.Length - offset) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } - try { // Call BeginSend on the Socket. @@ -592,27 +536,13 @@ namespace System.Net.Sockets // A Task representing the read. public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - bool canRead = CanRead; // Prevent race with Dispose. + ValidateBufferArguments(buffer, offset, count); ThrowIfDisposed(); - if (!canRead) + if (!CanRead) { throw new InvalidOperationException(SR.net_writeonlystream); } - // Validate input parameters. - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if ((uint)offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if ((uint)count > buffer.Length - offset) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } - try { return _streamSocket.ReceiveAsync( @@ -675,27 +605,13 @@ namespace System.Net.Sockets // A Task representing the write. public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - bool canWrite = CanWrite; // Prevent race with Dispose. + ValidateBufferArguments(buffer, offset, count); ThrowIfDisposed(); - if (!canWrite) + if (!CanWrite) { throw new InvalidOperationException(SR.net_readonlystream); } - // Validate input parameters. - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - if ((uint)offset > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } - if ((uint)count > buffer.Length - offset) - { - throw new ArgumentOutOfRangeException(nameof(count)); - } - try { return _streamSocket.SendAsyncForNetworkStream( diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index 79a510f..5b12e47 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -2644,12 +2644,6 @@ Either the IAsyncResult object did not come from the corresponding async method on this type, or the End method was called multiple times with the same IAsyncResult. - - Either the IAsyncResult object did not come from the corresponding async method on this type, or EndRead was called multiple times with the same IAsyncResult. - - - Either the IAsyncResult object did not come from the corresponding async method on this type, or EndWrite was called multiple times with the same IAsyncResult. - Common Language Runtime detected an invalid program. diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 9be6be6..5e50a10 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -1123,9 +1123,6 @@ Common\System\Diagnostics\CodeAnalysis\ExcludeFromCodeCoverageAttribute.cs - - Common\System\IO\StreamHelpers.CopyValidation.cs - Common\System\Runtime\Versioning\NonVersionableAttribute.cs diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/BufferedStream.cs b/src/libraries/System.Private.CoreLib/src/System/IO/BufferedStream.cs index 804a5d3..fac7f93 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/BufferedStream.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/BufferedStream.cs @@ -487,15 +487,7 @@ namespace System.IO public override int Read(byte[] buffer, int offset, int count) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer), SR.ArgumentNull_Buffer); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); - + ValidateBufferArguments(buffer, offset, count); EnsureNotClosed(); EnsureCanRead(); Debug.Assert(_stream != null); @@ -611,14 +603,7 @@ namespace System.IO public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer), SR.ArgumentNull_Buffer); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); + ValidateBufferArguments(buffer, offset, count); // Fast path check for cancellation already requested if (cancellationToken.IsCancellationRequested) @@ -838,15 +823,7 @@ namespace System.IO public override void Write(byte[] buffer, int offset, int count) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer), SR.ArgumentNull_Buffer); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); - + ValidateBufferArguments(buffer, offset, count); EnsureNotClosed(); EnsureCanWrite(); Debug.Assert(_stream != null); @@ -1048,14 +1025,7 @@ namespace System.IO public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer), SR.ArgumentNull_Buffer); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); + ValidateBufferArguments(buffer, offset, count); return WriteAsync(new ReadOnlyMemory(buffer, offset, count), cancellationToken).AsTask(); } @@ -1288,7 +1258,9 @@ namespace System.IO public override void CopyTo(Stream destination, int bufferSize) { - StreamHelpers.ValidateCopyToArgs(this, destination, bufferSize); + ValidateCopyToArguments(destination, bufferSize); + EnsureNotClosed(); + EnsureCanRead(); Debug.Assert(_stream != null); int readBytes = _readLen - _readPos; @@ -1313,7 +1285,10 @@ namespace System.IO public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) { - StreamHelpers.ValidateCopyToArgs(this, destination, bufferSize); + ValidateCopyToArguments(destination, bufferSize); + EnsureNotClosed(); + EnsureCanRead(); + return cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : CopyToAsyncCore(destination, bufferSize, cancellationToken); diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.Windows.cs index 3cfdf8d..5db8e43 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.Windows.cs @@ -1249,19 +1249,23 @@ namespace System.IO return base.CopyToAsync(destination, bufferSize, cancellationToken); } - StreamHelpers.ValidateCopyToArgs(this, destination, bufferSize); - - // Bail early for cancellation if cancellation has been requested - if (cancellationToken.IsCancellationRequested) - { - return Task.FromCanceled(cancellationToken); - } + ValidateCopyToArguments(destination, bufferSize); // Fail if the file was closed if (_fileHandle.IsClosed) { throw Error.GetFileNotOpen(); } + if (!CanRead) + { + throw Error.GetReadNotSupported(); + } + + // Bail early for cancellation if cancellation has been requested + if (cancellationToken.IsCancellationRequested) + { + return Task.FromCanceled(cancellationToken); + } // Do the async copy, with differing implementations based on whether the FileStream was opened as async or sync Debug.Assert((_readPos == 0 && _readLength == 0 && _writePos >= 0) || (_writePos == 0 && _readPos <= _readLength), "We're either reading or writing, but not both."); diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs index d451abc..4604965 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs @@ -330,14 +330,7 @@ namespace System.IO public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer), SR.ArgumentNull_Buffer); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen /*, no good single parameter name to pass*/); + ValidateBufferArguments(buffer, offset, count); if (GetType() != typeof(FileStream)) { @@ -457,14 +450,7 @@ namespace System.IO public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer), SR.ArgumentNull_Buffer); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen /*, no good single parameter name to pass*/); + ValidateBufferArguments(buffer, offset, count); if (GetType() != typeof(FileStream)) { @@ -563,14 +549,7 @@ namespace System.IO /// The maximum number of bytes to read or write. private void ValidateReadWriteArgs(byte[] buffer, int offset, int count) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer), SR.ArgumentNull_Buffer); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen /*, no good single parameter name to pass*/); + ValidateBufferArguments(buffer, offset, count); if (_fileHandle.IsClosed) throw Error.GetFileNotOpen(); } @@ -851,15 +830,7 @@ namespace System.IO public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer)); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); - + ValidateBufferArguments(buffer, offset, count); if (IsClosed) throw new ObjectDisposedException(SR.ObjectDisposed_FileClosed); if (!CanRead) throw new NotSupportedException(SR.NotSupported_UnreadableStream); @@ -871,15 +842,7 @@ namespace System.IO public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer)); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); - + ValidateBufferArguments(buffer, offset, count); if (IsClosed) throw new ObjectDisposedException(SR.ObjectDisposed_FileClosed); if (!CanWrite) throw new NotSupportedException(SR.NotSupported_UnwritableStream); diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs b/src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs index dab8f01..59316e7 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs @@ -332,15 +332,7 @@ namespace System.IO public override int Read(byte[] buffer, int offset, int count) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer), SR.ArgumentNull_Buffer); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); - + ValidateBufferArguments(buffer, offset, count); EnsureNotClosed(); int n = _length - _position; @@ -388,14 +380,7 @@ namespace System.IO public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer), SR.ArgumentNull_Buffer); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); + ValidateBufferArguments(buffer, offset, count); // If cancellation was requested, bail early if (cancellationToken.IsCancellationRequested) @@ -467,10 +452,6 @@ namespace System.IO public override void CopyTo(Stream destination, int bufferSize) { - // Since we did not originally override this method, validate the arguments - // the same way Stream does for back-compat. - StreamHelpers.ValidateCopyToArgs(this, destination, bufferSize); - // If we have been inherited into a subclass, the following implementation could be incorrect // since it does not call through to Read() which a subclass might have overridden. // To be safe we will only use this implementation in cases where we know it is safe to do so, @@ -481,6 +462,10 @@ namespace System.IO return; } + // Validate the arguments the same way Stream does for back-compat. + ValidateCopyToArguments(destination, bufferSize); + EnsureNotClosed(); + int originalPosition = _position; // Seek to the end of the MemoryStream. @@ -499,7 +484,8 @@ namespace System.IO { // This implementation offers better performance compared to the base class version. - StreamHelpers.ValidateCopyToArgs(this, destination, bufferSize); + ValidateCopyToArguments(destination, bufferSize); + EnsureNotClosed(); // If we have been inherited into a subclass, the following implementation could be incorrect // since it does not call through to ReadAsync() which a subclass might have overridden. @@ -623,15 +609,7 @@ namespace System.IO public override void Write(byte[] buffer, int offset, int count) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer), SR.ArgumentNull_Buffer); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); - + ValidateBufferArguments(buffer, offset, count); EnsureNotClosed(); EnsureWriteable(); @@ -715,14 +693,7 @@ namespace System.IO public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer), SR.ArgumentNull_Buffer); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); + ValidateBufferArguments(buffer, offset, count); // If cancellation is already requested, bail early if (cancellationToken.IsCancellationRequested) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Stream.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Stream.cs index 68e4a63..93236dd 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Stream.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Stream.cs @@ -1,22 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -/*============================================================ -** -** -** -** -** -** Purpose: Abstract base class for all Streams. Provides -** default implementations of asynchronous reads & writes, in -** terms of the synchronous reads & writes (and vice versa). -** -** -===========================================================*/ - using System.Buffers; using System.Diagnostics; -using System.Runtime.ExceptionServices; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; @@ -27,50 +14,21 @@ namespace System.IO { public static readonly Stream Null = new NullStream(); - // We pick a value that is the largest multiple of 4096 that is still smaller than the large object heap threshold (85K). - // The CopyTo/CopyToAsync buffer is short-lived and is likely to be collected at Gen0, and it offers a significant - // improvement in Copy performance. - private const int DefaultCopyBufferSize = 81920; - - // To implement Async IO operations on streams that don't support async IO - + /// To implement Async IO operations on streams that don't support async IO private SemaphoreSlim? _asyncActiveSemaphore; - internal SemaphoreSlim EnsureAsyncActiveSemaphoreInitialized() - { + internal SemaphoreSlim EnsureAsyncActiveSemaphoreInitialized() => // Lazily-initialize _asyncActiveSemaphore. As we're never accessing the SemaphoreSlim's - // WaitHandle, we don't need to worry about Disposing it. - return LazyInitializer.EnsureInitialized(ref _asyncActiveSemaphore, () => new SemaphoreSlim(1, 1)); - } - - public abstract bool CanRead - { - get; - } - - // If CanSeek is false, Position, Seek, Length, and SetLength should throw. - public abstract bool CanSeek - { - get; - } + // WaitHandle, we don't need to worry about Disposing it in the case of a race condition. + LazyInitializer.EnsureInitialized(ref _asyncActiveSemaphore, () => new SemaphoreSlim(1, 1)); + public abstract bool CanRead { get; } + public abstract bool CanWrite { get; } + public abstract bool CanSeek { get; } public virtual bool CanTimeout => false; - public abstract bool CanWrite - { - get; - } - - public abstract long Length - { - get; - } - - public abstract long Position - { - get; - set; - } + public abstract long Length { get; } + public abstract long Position { get; set; } public virtual int ReadTimeout { @@ -84,42 +42,25 @@ namespace System.IO set => throw new InvalidOperationException(SR.InvalidOperation_TimeoutsNotSupported); } - public Task CopyToAsync(Stream destination) - { - int bufferSize = GetCopyBufferSize(); - - return CopyToAsync(destination, bufferSize); - } - - public Task CopyToAsync(Stream destination, int bufferSize) - { - return CopyToAsync(destination, bufferSize, CancellationToken.None); - } - - public Task CopyToAsync(Stream destination, CancellationToken cancellationToken) - { - int bufferSize = GetCopyBufferSize(); - - return CopyToAsync(destination, bufferSize, cancellationToken); - } + public void CopyTo(Stream destination) => CopyTo(destination, GetCopyBufferSize()); - public virtual Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) + public virtual void CopyTo(Stream destination, int bufferSize) { - StreamHelpers.ValidateCopyToArgs(this, destination, bufferSize); - - return CopyToAsyncInternal(destination, bufferSize, cancellationToken); - } + ValidateCopyToArguments(destination, bufferSize); + if (!CanRead) + { + throw CanWrite ? (Exception) + new NotSupportedException(SR.NotSupported_UnreadableStream) : + new ObjectDisposedException(GetType().Name, SR.ObjectDisposed_StreamClosed); + } - private async Task CopyToAsyncInternal(Stream destination, int bufferSize, CancellationToken cancellationToken) - { byte[] buffer = ArrayPool.Shared.Rent(bufferSize); try { - while (true) + int bytesRead; + while ((bytesRead = Read(buffer, 0, buffer.Length)) != 0) { - int bytesRead = await ReadAsync(new Memory(buffer), cancellationToken).ConfigureAwait(false); - if (bytesRead == 0) break; - await destination.WriteAsync(new ReadOnlyMemory(buffer, 0, bytesRead), cancellationToken).ConfigureAwait(false); + destination.Write(buffer, 0, bytesRead); } } finally @@ -128,37 +69,52 @@ namespace System.IO } } - // Reads the bytes from the current stream and writes the bytes to - // the destination stream until all bytes are read, starting at - // the current position. - public void CopyTo(Stream destination) - { - int bufferSize = GetCopyBufferSize(); + public Task CopyToAsync(Stream destination) => CopyToAsync(destination, GetCopyBufferSize()); - CopyTo(destination, bufferSize); - } + public Task CopyToAsync(Stream destination, int bufferSize) => CopyToAsync(destination, bufferSize, CancellationToken.None); - public virtual void CopyTo(Stream destination, int bufferSize) + public Task CopyToAsync(Stream destination, CancellationToken cancellationToken) => CopyToAsync(destination, GetCopyBufferSize(), cancellationToken); + + public virtual Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) { - StreamHelpers.ValidateCopyToArgs(this, destination, bufferSize); + ValidateCopyToArguments(destination, bufferSize); + if (!CanRead) + { + throw CanWrite ? (Exception) + new NotSupportedException(SR.NotSupported_UnreadableStream) : + new ObjectDisposedException(GetType().Name, SR.ObjectDisposed_StreamClosed); + } - byte[] buffer = ArrayPool.Shared.Rent(bufferSize); - try + return Core(this, destination, bufferSize, cancellationToken); + + static async Task Core(Stream source, Stream destination, int bufferSize, CancellationToken cancellationToken) { - int read; - while ((read = Read(buffer, 0, buffer.Length)) != 0) + byte[] buffer = ArrayPool.Shared.Rent(bufferSize); + try { - destination.Write(buffer, 0, read); + int bytesRead; + while ((bytesRead = await source.ReadAsync(new Memory(buffer), cancellationToken).ConfigureAwait(false)) != 0) + { + await destination.WriteAsync(new ReadOnlyMemory(buffer, 0, bytesRead), cancellationToken).ConfigureAwait(false); + } + } + finally + { + ArrayPool.Shared.Return(buffer); } - } - finally - { - ArrayPool.Shared.Return(buffer); } } private int GetCopyBufferSize() { + // This value was originally picked to be the largest multiple of 4096 that is still smaller than the large object heap threshold (85K). + // The CopyTo{Async} buffer is short-lived and is likely to be collected at Gen0, and it offers a significant improvement in Copy + // performance. Since then, the base implementations of CopyTo{Async} have been updated to use ArrayPool, which will end up rounding + // this size up to the next power of two (131,072), which will by default be on the large object heap. However, most of the time + // the buffer should be pooled, the LOH threshold is now configurable and thus may be different than 85K, and there are measurable + // benefits to using the larger buffer size. So, for now, this value remains. + const int DefaultCopyBufferSize = 81920; + int bufferSize = DefaultCopyBufferSize; if (CanSeek) @@ -187,25 +143,17 @@ namespace System.IO return bufferSize; } - // Stream used to require that all cleanup logic went into Close(), - // which was thought up before we invented IDisposable. However, we - // need to follow the IDisposable pattern so that users can write - // sensible subclasses without needing to inspect all their base - // classes, and without worrying about version brittleness, from a - // base class switching to the Dispose pattern. We're moving - // Stream to the Dispose(bool) pattern - that's where all subclasses - // should put their cleanup now. + public void Dispose() => Close(); + public virtual void Close() { + // When initially designed, Stream required that all cleanup logic went into Close(), + // but this was thought up before IDisposable was added and never revisited. All subclasses + // should put their cleanup now in Dispose(bool). Dispose(true); GC.SuppressFinalize(this); } - public void Dispose() - { - Close(); - } - protected virtual void Dispose(bool disposing) { // Note: Never change this to call other virtual methods on Stream @@ -228,27 +176,18 @@ namespace System.IO public abstract void Flush(); - public Task FlushAsync() - { - return FlushAsync(CancellationToken.None); - } + public Task FlushAsync() => FlushAsync(CancellationToken.None); - public virtual Task FlushAsync(CancellationToken cancellationToken) - { - return Task.Factory.StartNew(static state => ((Stream)state!).Flush(), this, + public virtual Task FlushAsync(CancellationToken cancellationToken) => + Task.Factory.StartNew( + static state => ((Stream)state!).Flush(), this, cancellationToken, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default); - } [Obsolete("CreateWaitHandle will be removed eventually. Please use \"new ManualResetEvent(false)\" instead.")] - protected virtual WaitHandle CreateWaitHandle() - { - return new ManualResetEvent(false); - } + protected virtual WaitHandle CreateWaitHandle() => new ManualResetEvent(false); - public virtual IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) - { - return BeginReadInternal(buffer, offset, count, callback, state, serializeAsynchronously: false, apm: true); - } + public virtual IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) => + BeginReadInternal(buffer, offset, count, callback, state, serializeAsynchronously: false, apm: true); internal IAsyncResult BeginReadInternal( byte[] buffer, int offset, int count, AsyncCallback? callback, object? state, @@ -304,32 +243,33 @@ namespace System.IO // Schedule it if (semaphoreTask != null) + { RunReadWriteTaskWhenReady(semaphoreTask, asyncResult); + } else + { RunReadWriteTask(asyncResult); - + } return asyncResult; // return it } public virtual int EndRead(IAsyncResult asyncResult) { - if (asyncResult == null) - throw new ArgumentNullException(nameof(asyncResult)); + if (asyncResult is null) + { + ThrowHelper.ThrowArgumentNullException(ExceptionArgument.asyncResult); + } ReadWriteTask? readTask = asyncResult as ReadWriteTask; - if (readTask == null) + if (readTask is null || !readTask._isRead) { - throw new ArgumentException(SR.InvalidOperation_WrongAsyncResultOrEndReadCalledMultiple); + ThrowHelper.ThrowArgumentException(ExceptionResource.InvalidOperation_WrongAsyncResultOrEndCalledMultiple); } else if (readTask._endCalled) { - throw new InvalidOperationException(SR.InvalidOperation_WrongAsyncResultOrEndReadCalledMultiple); - } - else if (!readTask._isRead) - { - throw new ArgumentException(SR.InvalidOperation_WrongAsyncResultOrEndReadCalledMultiple); + ThrowHelper.ThrowInvalidOperationException(ExceptionResource.InvalidOperation_WrongAsyncResultOrEndCalledMultiple); } try @@ -342,19 +282,12 @@ namespace System.IO } } - public Task ReadAsync(byte[] buffer, int offset, int count) - { - return ReadAsync(buffer, offset, count, CancellationToken.None); - } + public Task ReadAsync(byte[] buffer, int offset, int count) => ReadAsync(buffer, offset, count, CancellationToken.None); - public virtual Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) - { - // If cancellation was requested, bail early with an already completed task. - // Otherwise, return a task that represents the Begin/End methods. - return cancellationToken.IsCancellationRequested - ? Task.FromCanceled(cancellationToken) - : BeginEndReadAsync(buffer, offset, count); - } + public virtual Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) => + cancellationToken.IsCancellationRequested ? + Task.FromCanceled(cancellationToken) : + BeginEndReadAsync(buffer, offset, count); public virtual ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) { @@ -362,23 +295,21 @@ namespace System.IO { return new ValueTask(ReadAsync(array.Array!, array.Offset, array.Count, cancellationToken)); } - else - { - byte[] sharedBuffer = ArrayPool.Shared.Rent(buffer.Length); - return FinishReadAsync(ReadAsync(sharedBuffer, 0, buffer.Length, cancellationToken), sharedBuffer, buffer); - static async ValueTask FinishReadAsync(Task readTask, byte[] localBuffer, Memory localDestination) + byte[] sharedBuffer = ArrayPool.Shared.Rent(buffer.Length); + return FinishReadAsync(ReadAsync(sharedBuffer, 0, buffer.Length, cancellationToken), sharedBuffer, buffer); + + static async ValueTask FinishReadAsync(Task readTask, byte[] localBuffer, Memory localDestination) + { + try { - try - { - int result = await readTask.ConfigureAwait(false); - new Span(localBuffer, 0, result).CopyTo(localDestination.Span); - return result; - } - finally - { - ArrayPool.Shared.Return(localBuffer); - } + int result = await readTask.ConfigureAwait(false); + new ReadOnlySpan(localBuffer, 0, result).CopyTo(localDestination.Span); + return result; + } + finally + { + ArrayPool.Shared.Return(localBuffer); } } } @@ -394,9 +325,9 @@ namespace System.IO // Otherwise, we need to wrap calls to Begin/EndWrite to ensure we use the derived type's functionality. return TaskFactory.FromAsyncTrim( - this, new ReadWriteParameters { Buffer = buffer, Offset = offset, Count = count }, - (stream, args, callback, state) => stream.BeginRead(args.Buffer, args.Offset, args.Count, callback, state), // cached by compiler - (stream, asyncResult) => stream.EndRead(asyncResult)); // cached by compiler + this, new ReadWriteParameters { Buffer = buffer, Offset = offset, Count = count }, + (stream, args, callback, state) => stream.BeginRead(args.Buffer, args.Offset, args.Count, callback, state), // cached by compiler + (stream, asyncResult) => stream.EndRead(asyncResult)); // cached by compiler } private struct ReadWriteParameters // struct for arguments to Read and Write calls @@ -406,18 +337,17 @@ namespace System.IO internal int Count; } - - - public virtual IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) - { - return BeginWriteInternal(buffer, offset, count, callback, state, serializeAsynchronously: false, apm: true); - } + public virtual IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) => + BeginWriteInternal(buffer, offset, count, callback, state, serializeAsynchronously: false, apm: true); internal IAsyncResult BeginWriteInternal( byte[] buffer, int offset, int count, AsyncCallback? callback, object? state, bool serializeAsynchronously, bool apm) { - if (!CanWrite) throw Error.GetWriteNotSupported(); + if (!CanWrite) + { + throw Error.GetWriteNotSupported(); + } // To avoid a race condition with a stream's position pointer & generating conditions // with internal buffer indexes in our own streams that @@ -468,9 +398,13 @@ namespace System.IO // Schedule it if (semaphoreTask != null) + { RunReadWriteTaskWhenReady(semaphoreTask, asyncResult); + } else + { RunReadWriteTask(asyncResult); + } return asyncResult; // return it } @@ -519,21 +453,19 @@ namespace System.IO public virtual void EndWrite(IAsyncResult asyncResult) { - if (asyncResult == null) - throw new ArgumentNullException(nameof(asyncResult)); + if (asyncResult is null) + { + ThrowHelper.ThrowArgumentNullException(ExceptionArgument.asyncResult); + } ReadWriteTask? writeTask = asyncResult as ReadWriteTask; - if (writeTask == null) + if (writeTask is null || writeTask._isRead) { - throw new ArgumentException(SR.InvalidOperation_WrongAsyncResultOrEndWriteCalledMultiple); + ThrowHelper.ThrowArgumentException(ExceptionResource.InvalidOperation_WrongAsyncResultOrEndCalledMultiple); } else if (writeTask._endCalled) { - throw new InvalidOperationException(SR.InvalidOperation_WrongAsyncResultOrEndWriteCalledMultiple); - } - else if (writeTask._isRead) - { - throw new ArgumentException(SR.InvalidOperation_WrongAsyncResultOrEndWriteCalledMultiple); + ThrowHelper.ThrowInvalidOperationException(ExceptionResource.InvalidOperation_WrongAsyncResultOrEndCalledMultiple); } try @@ -630,7 +562,7 @@ namespace System.IO // directly, passing in the completed task as the IAsyncResult. // If there is one, process it with ExecutionContext.Run. ExecutionContext? context = _context; - if (context == null) + if (context is null) { AsyncCallback? callback = _callback; Debug.Assert(callback != null); @@ -650,19 +582,14 @@ namespace System.IO bool ITaskCompletionAction.InvokeMayRunArbitraryCode => true; } - public Task WriteAsync(byte[] buffer, int offset, int count) - { - return WriteAsync(buffer, offset, count, CancellationToken.None); - } + public Task WriteAsync(byte[] buffer, int offset, int count) => WriteAsync(buffer, offset, count, CancellationToken.None); - public virtual Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) - { + public virtual Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) => // If cancellation was requested, bail early with an already completed task. // Otherwise, return a task that represents the Begin/End methods. - return cancellationToken.IsCancellationRequested - ? Task.FromCanceled(cancellationToken) - : BeginEndWriteAsync(buffer, offset, count); - } + cancellationToken.IsCancellationRequested ? + Task.FromCanceled(cancellationToken) : + BeginEndWriteAsync(buffer, offset, count); public virtual ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken = default) { @@ -670,12 +597,10 @@ namespace System.IO { return new ValueTask(WriteAsync(array.Array!, array.Offset, array.Count, cancellationToken)); } - else - { - byte[] sharedBuffer = ArrayPool.Shared.Rent(buffer.Length); - buffer.Span.CopyTo(sharedBuffer); - return new ValueTask(FinishWriteAsync(WriteAsync(sharedBuffer, 0, buffer.Length, cancellationToken), sharedBuffer)); - } + + byte[] sharedBuffer = ArrayPool.Shared.Rent(buffer.Length); + buffer.Span.CopyTo(sharedBuffer); + return new ValueTask(FinishWriteAsync(WriteAsync(sharedBuffer, 0, buffer.Length, cancellationToken), sharedBuffer)); } private static async Task FinishWriteAsync(Task writeTask, byte[] localBuffer) @@ -701,13 +626,13 @@ namespace System.IO // Otherwise, we need to wrap calls to Begin/EndWrite to ensure we use the derived type's functionality. return TaskFactory.FromAsyncTrim( - this, new ReadWriteParameters { Buffer = buffer, Offset = offset, Count = count }, - (stream, args, callback, state) => stream.BeginWrite(args.Buffer, args.Offset, args.Count, callback, state), // cached by compiler - (stream, asyncResult) => // cached by compiler - { - stream.EndWrite(asyncResult); - return default; - }); + this, new ReadWriteParameters { Buffer = buffer, Offset = offset, Count = count }, + (stream, args, callback, state) => stream.BeginWrite(args.Buffer, args.Offset, args.Count, callback, state), // cached by compiler + (stream, asyncResult) => // cached by compiler + { + stream.EndWrite(asyncResult); + return default; + }); } public abstract long Seek(long offset, SeekOrigin origin); @@ -726,25 +651,21 @@ namespace System.IO { throw new IOException(SR.IO_StreamTooLong); } - new Span(sharedBuffer, 0, numRead).CopyTo(buffer); + + new ReadOnlySpan(sharedBuffer, 0, numRead).CopyTo(buffer); return numRead; } - finally { ArrayPool.Shared.Return(sharedBuffer); } + finally + { + ArrayPool.Shared.Return(sharedBuffer); + } } - // Reads one byte from the stream by calling Read(byte[], int, int). - // Will return an unsigned byte cast to an int or -1 on end of stream. - // This implementation does not perform well because it allocates a new - // byte[] each time you call it, and should be overridden by any - // subclass that maintains an internal buffer. Then, it can help perf - // significantly for people who are reading one byte at a time. public virtual int ReadByte() { - byte[] oneByteArray = new byte[1]; + var oneByteArray = new byte[1]; int r = Read(oneByteArray, 0, 1); - if (r == 0) - return -1; - return oneByteArray[0]; + return r == 0 ? -1 : oneByteArray[0]; } public abstract void Write(byte[] buffer, int offset, int count); @@ -757,331 +678,167 @@ namespace System.IO buffer.CopyTo(sharedBuffer); Write(sharedBuffer, 0, buffer.Length); } - finally { ArrayPool.Shared.Return(sharedBuffer); } - } - - // Writes one byte from the stream by calling Write(byte[], int, int). - // This implementation does not perform well because it allocates a new - // byte[] each time you call it, and should be overridden by any - // subclass that maintains an internal buffer. Then, it can help perf - // significantly for people who are writing one byte at a time. - public virtual void WriteByte(byte value) - { - byte[] oneByteArray = new byte[1]; - oneByteArray[0] = value; - Write(oneByteArray, 0, 1); + finally + { + ArrayPool.Shared.Return(sharedBuffer); + } } - public static Stream Synchronized(Stream stream) - { - if (stream == null) - throw new ArgumentNullException(nameof(stream)); - if (stream is SyncStream) - return stream; + public virtual void WriteByte(byte value) => Write(new byte[1] { value }, 0, 1); - return new SyncStream(stream); - } + public static Stream Synchronized(Stream stream) => + stream is null ? throw new ArgumentNullException(nameof(stream)) : + stream is SyncStream ? stream : + new SyncStream(stream); [Obsolete("Do not call or override this method.")] - protected virtual void ObjectInvariant() - { - } - - internal IAsyncResult BlockingBeginRead(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) + protected virtual void ObjectInvariant() { } + + /// Validates arguments provided to reading and writing methods on . + /// The array "buffer" argument passed to the reading or writing method. + /// The integer "offset" argument passed to the reading or writing method. + /// The integer "count" argument passed to the reading or writing method. + /// was null. + /// + /// was outside the bounds of , or + /// was negative, or the range specified by the combination of + /// and exceed the length of . + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + protected static void ValidateBufferArguments(byte[] buffer, int offset, int count) { - // To avoid a race with a stream's position pointer & generating conditions - // with internal buffer indexes in our own streams that - // don't natively support async IO operations when there are multiple - // async requests outstanding, we will block the application's main - // thread and do the IO synchronously. - // This can't perform well - use a different approach. - SynchronousAsyncResult asyncResult; - try + if (buffer is null) { - int numRead = Read(buffer, offset, count); - asyncResult = new SynchronousAsyncResult(numRead, state); + ThrowHelper.ThrowArgumentNullException(ExceptionArgument.buffer); } - catch (IOException ex) + + if (offset < 0) { - asyncResult = new SynchronousAsyncResult(ex, state, isWrite: false); + ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.offset, ExceptionResource.ArgumentOutOfRange_NeedNonNegNum); } - callback?.Invoke(asyncResult); - - return asyncResult; - } - - internal static int BlockingEndRead(IAsyncResult asyncResult) - { - return SynchronousAsyncResult.EndRead(asyncResult); + if ((uint)count > buffer.Length - offset) + { + ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.count, ExceptionResource.Argument_InvalidOffLen); + } } - internal IAsyncResult BlockingBeginWrite(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) + /// Validates arguments provided to the or methods. + /// The "destination" argument passed to the copy method. + /// The integer "bufferSize" argument passed to the copy method. + /// was null. + /// was not a positive value. + /// does not support writing. + /// does not support writing or reading. + protected static void ValidateCopyToArguments(Stream destination, int bufferSize) { - // To avoid a race condition with a stream's position pointer & generating conditions - // with internal buffer indexes in our own streams that - // don't natively support async IO operations when there are multiple - // async requests outstanding, we will block the application's main - // thread and do the IO synchronously. - // This can't perform well - use a different approach. - SynchronousAsyncResult asyncResult; - try + if (destination is null) { - Write(buffer, offset, count); - asyncResult = new SynchronousAsyncResult(state); + throw new ArgumentNullException(nameof(destination)); } - catch (IOException ex) + + if (bufferSize <= 0) { - asyncResult = new SynchronousAsyncResult(ex, state, isWrite: true); + throw new ArgumentOutOfRangeException(nameof(bufferSize), bufferSize, SR.ArgumentOutOfRange_NeedPosNum); } - callback?.Invoke(asyncResult); - - return asyncResult; - } - - internal static void BlockingEndWrite(IAsyncResult asyncResult) - { - SynchronousAsyncResult.EndWrite(asyncResult); + if (!destination.CanWrite) + { + throw destination.CanRead ? (Exception) + new NotSupportedException(SR.NotSupported_UnwritableStream) : + new ObjectDisposedException(destination.GetType().Name, SR.ObjectDisposed_StreamClosed); + } } + /// Provides a nop stream. private sealed class NullStream : Stream { internal NullStream() { } public override bool CanRead => true; - public override bool CanWrite => true; - public override bool CanSeek => true; - public override long Length => 0; + public override long Position { get => 0; set { } } - public override long Position - { - get => 0; - set { } - } + public override void CopyTo(Stream destination, int bufferSize) { } - public override void CopyTo(Stream destination, int bufferSize) - { - StreamHelpers.ValidateCopyToArgs(this, destination, bufferSize); - - // After we validate arguments this is a nop. - } - - public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) - { - // Validate arguments here for compat, since previously this method - // was inherited from Stream (which did check its arguments). - StreamHelpers.ValidateCopyToArgs(this, destination, bufferSize); - - return cancellationToken.IsCancellationRequested ? + public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) => + cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : Task.CompletedTask; - } protected override void Dispose(bool disposing) { // Do nothing - we don't want NullStream singleton (static) to be closable } - public override void Flush() - { - } + public override void Flush() { } - public override Task FlushAsync(CancellationToken cancellationToken) - { - return cancellationToken.IsCancellationRequested ? + public override Task FlushAsync(CancellationToken cancellationToken) => + cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : Task.CompletedTask; - } - - public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) - { - if (!CanRead) throw Error.GetReadNotSupported(); - - return BlockingBeginRead(buffer, offset, count, callback, state); - } - public override int EndRead(IAsyncResult asyncResult) - { - if (asyncResult == null) - throw new ArgumentNullException(nameof(asyncResult)); + public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) => + TaskToApm.Begin(Task.s_defaultResultTask, callback, state); - return BlockingEndRead(asyncResult); - } + public override int EndRead(IAsyncResult asyncResult) => + TaskToApm.End(asyncResult); - public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) - { - if (!CanWrite) throw Error.GetWriteNotSupported(); + public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) => + TaskToApm.Begin(Task.CompletedTask, callback, state); - return BlockingBeginWrite(buffer, offset, count, callback, state); - } + public override void EndWrite(IAsyncResult asyncResult) => + TaskToApm.End(asyncResult); - public override void EndWrite(IAsyncResult asyncResult) - { - if (asyncResult == null) - throw new ArgumentNullException(nameof(asyncResult)); + public override int Read(byte[] buffer, int offset, int count) => 0; - BlockingEndWrite(asyncResult); - } + public override int Read(Span buffer) => 0; - public override int Read(byte[] buffer, int offset, int count) - { - return 0; - } - - public override int Read(Span buffer) - { - return 0; - } - - public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) - { - return Task.FromResult(0); - } + public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) => + cancellationToken.IsCancellationRequested ? + Task.FromCanceled(cancellationToken) : + Task.FromResult(0); - public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) - { - return new ValueTask(0); - } + public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken) => + cancellationToken.IsCancellationRequested ? + ValueTask.FromCanceled(cancellationToken) : + default; - public override int ReadByte() - { - return -1; - } + public override int ReadByte() => -1; - public override void Write(byte[] buffer, int offset, int count) - { - } + public override void Write(byte[] buffer, int offset, int count) { } - public override void Write(ReadOnlySpan buffer) - { - } + public override void Write(ReadOnlySpan buffer) { } - public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) - { - return cancellationToken.IsCancellationRequested ? + public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) => + cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : Task.CompletedTask; - } - public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken = default) - { - return cancellationToken.IsCancellationRequested ? + public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken = default) => + cancellationToken.IsCancellationRequested ? ValueTask.FromCanceled(cancellationToken) : default; - } - public override void WriteByte(byte value) - { - } + public override void WriteByte(byte value) { } - public override long Seek(long offset, SeekOrigin origin) - { - return 0; - } + public override long Seek(long offset, SeekOrigin origin) => 0; - public override void SetLength(long length) - { - } + public override void SetLength(long length) { } } - - /// Used as the IAsyncResult object when using asynchronous IO methods on the base Stream class. - private sealed class SynchronousAsyncResult : IAsyncResult - { - private readonly object? _stateObject; - private readonly bool _isWrite; - private ManualResetEvent? _waitHandle; - private readonly ExceptionDispatchInfo? _exceptionInfo; - - private bool _endXxxCalled; - private readonly int _bytesRead; - - internal SynchronousAsyncResult(int bytesRead, object? asyncStateObject) - { - _bytesRead = bytesRead; - _stateObject = asyncStateObject; - } - - internal SynchronousAsyncResult(object? asyncStateObject) - { - _stateObject = asyncStateObject; - _isWrite = true; - } - - internal SynchronousAsyncResult(Exception ex, object? asyncStateObject, bool isWrite) - { - _exceptionInfo = ExceptionDispatchInfo.Capture(ex); - _stateObject = asyncStateObject; - _isWrite = isWrite; - } - - public bool IsCompleted => true; - - public WaitHandle AsyncWaitHandle => - LazyInitializer.EnsureInitialized(ref _waitHandle, () => new ManualResetEvent(true)); - - public object? AsyncState => _stateObject; - - public bool CompletedSynchronously => true; - - internal void ThrowIfError() - { - if (_exceptionInfo != null) - _exceptionInfo.Throw(); - } - - internal static int EndRead(IAsyncResult asyncResult) - { - if (!(asyncResult is SynchronousAsyncResult ar) || ar._isWrite) - throw new ArgumentException(SR.Arg_WrongAsyncResult); - - if (ar._endXxxCalled) - throw new ArgumentException(SR.InvalidOperation_EndReadCalledMultiple); - - ar._endXxxCalled = true; - - ar.ThrowIfError(); - return ar._bytesRead; - } - - internal static void EndWrite(IAsyncResult asyncResult) - { - if (!(asyncResult is SynchronousAsyncResult ar) || !ar._isWrite) - throw new ArgumentException(SR.Arg_WrongAsyncResult); - - if (ar._endXxxCalled) - throw new ArgumentException(SR.InvalidOperation_EndWriteCalledMultiple); - - ar._endXxxCalled = true; - - ar.ThrowIfError(); - } - } // class SynchronousAsyncResult - - - // SyncStream is a wrapper around a stream that takes - // a lock for every operation making it thread safe. + /// Provides a wrapper around a stream that takes a lock for every operation. private sealed class SyncStream : Stream, IDisposable { private readonly Stream _stream; - internal SyncStream(Stream stream) - { - if (stream == null) - throw new ArgumentNullException(nameof(stream)); - _stream = stream; - } + internal SyncStream(Stream stream) => _stream = stream ?? throw new ArgumentNullException(nameof(stream)); public override bool CanRead => _stream.CanRead; - public override bool CanWrite => _stream.CanWrite; - public override bool CanSeek => _stream.CanSeek; - public override bool CanTimeout => _stream.CanTimeout; public override long Length @@ -1125,12 +882,12 @@ namespace System.IO set => _stream.WriteTimeout = value; } - // In the off chance that some wrapped stream has different - // semantics for Close vs. Dispose, let's preserve that. public override void Close() { lock (_stream) { + // On the off chance that some wrapped stream has different + // semantics for Close vs. Dispose, let's preserve that. try { _stream.Close(); @@ -1150,7 +907,9 @@ namespace System.IO { // Explicitly pick up a potentially methodimpl'ed Dispose if (disposing) + { ((IDisposable)_stream).Dispose(); + } } finally { @@ -1162,31 +921,41 @@ namespace System.IO public override ValueTask DisposeAsync() { lock (_stream) + { return _stream.DisposeAsync(); + } } public override void Flush() { lock (_stream) + { _stream.Flush(); + } } public override int Read(byte[] bytes, int offset, int count) { lock (_stream) + { return _stream.Read(bytes, offset, count); + } } public override int Read(Span buffer) { lock (_stream) + { return _stream.Read(buffer); + } } public override int ReadByte() { lock (_stream) + { return _stream.ReadByte(); + } } public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) @@ -1213,41 +982,55 @@ namespace System.IO public override int EndRead(IAsyncResult asyncResult) { - if (asyncResult == null) - throw new ArgumentNullException(nameof(asyncResult)); + if (asyncResult is null) + { + ThrowHelper.ThrowArgumentNullException(ExceptionArgument.asyncResult); + } lock (_stream) + { return _stream.EndRead(asyncResult); + } } public override long Seek(long offset, SeekOrigin origin) { lock (_stream) + { return _stream.Seek(offset, origin); + } } public override void SetLength(long length) { lock (_stream) + { _stream.SetLength(length); + } } public override void Write(byte[] bytes, int offset, int count) { lock (_stream) + { _stream.Write(bytes, offset, count); + } } public override void Write(ReadOnlySpan buffer) { lock (_stream) + { _stream.Write(buffer); + } } public override void WriteByte(byte b) { lock (_stream) + { _stream.WriteByte(b); + } } public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) @@ -1274,11 +1057,15 @@ namespace System.IO public override void EndWrite(IAsyncResult asyncResult) { - if (asyncResult == null) - throw new ArgumentNullException(nameof(asyncResult)); + if (asyncResult is null) + { + ThrowHelper.ThrowArgumentNullException(ExceptionArgument.asyncResult); + } lock (_stream) + { _stream.EndWrite(asyncResult); + } } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/UnmanagedMemoryStream.cs b/src/libraries/System.Private.CoreLib/src/System/IO/UnmanagedMemoryStream.cs index be1ed72..316c9b8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/UnmanagedMemoryStream.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/UnmanagedMemoryStream.cs @@ -348,14 +348,7 @@ namespace System.IO /// Number of bytes actually read. public override int Read(byte[] buffer, int offset, int count) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer), SR.ArgumentNull_Buffer); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); + ValidateBufferArguments(buffer, offset, count); return ReadCore(new Span(buffer, offset, count)); } @@ -438,14 +431,7 @@ namespace System.IO /// Task that can be used to access the number of bytes actually read. public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer), SR.ArgumentNull_Buffer); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); + ValidateBufferArguments(buffer, offset, count); if (cancellationToken.IsCancellationRequested) return Task.FromCanceled(cancellationToken); @@ -626,14 +612,7 @@ namespace System.IO /// Number of bytes to write. public override void Write(byte[] buffer, int offset, int count) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer), SR.ArgumentNull_Buffer); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); + ValidateBufferArguments(buffer, offset, count); WriteCore(new ReadOnlySpan(buffer, offset, count)); } @@ -732,14 +711,7 @@ namespace System.IO /// Task that can be awaited public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - if (buffer == null) - throw new ArgumentNullException(nameof(buffer), SR.ArgumentNull_Buffer); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); + ValidateBufferArguments(buffer, offset, count); if (cancellationToken.IsCancellationRequested) return Task.FromCanceled(cancellationToken); diff --git a/src/libraries/System.Private.CoreLib/src/System/Text/TranscodingStream.cs b/src/libraries/System.Private.CoreLib/src/System/Text/TranscodingStream.cs index 930d7df..4bc5f5f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Text/TranscodingStream.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Text/TranscodingStream.cs @@ -288,10 +288,7 @@ namespace System.Text public override int Read(byte[] buffer, int offset, int count) { - if (buffer is null) - { - throw new ArgumentNullException(nameof(buffer)); - } + ValidateBufferArguments(buffer, offset, count); return Read(new Span(buffer, offset, count)); } @@ -352,10 +349,7 @@ namespace System.Text public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - if (buffer is null) - { - throw new ArgumentNullException(nameof(buffer)); - } + ValidateBufferArguments(buffer, offset, count); return ReadAsync(new Memory(buffer, offset, count), cancellationToken).AsTask(); } @@ -456,10 +450,7 @@ namespace System.Text public override void Write(byte[] buffer, int offset, int count) { - if (buffer is null) - { - throw new ArgumentNullException(nameof(buffer)); - } + ValidateBufferArguments(buffer, offset, count); Write(new ReadOnlySpan(buffer, offset, count)); } @@ -522,10 +513,7 @@ namespace System.Text public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - if (buffer is null) - { - throw new ArgumentNullException(nameof(buffer)); - } + ValidateBufferArguments(buffer, offset, count); return WriteAsync(new ReadOnlyMemory(buffer, offset, count), cancellationToken).AsTask(); } diff --git a/src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs b/src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs index 52f52d5..271028c 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs @@ -705,6 +705,10 @@ namespace System return "prefix"; case ExceptionArgument.suffix: return "suffix"; + case ExceptionArgument.buffer: + return "buffer"; + case ExceptionArgument.offset: + return "offset"; default: Debug.Fail("The enum value is not defined, please check the ExceptionArgument Enum."); return ""; @@ -963,6 +967,8 @@ namespace System options, prefix, suffix, + buffer, + offset, } // diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 873e224..c954f9b 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -7283,6 +7283,8 @@ namespace System.IO public abstract long Seek(long offset, System.IO.SeekOrigin origin); public abstract void SetLength(long value); public static System.IO.Stream Synchronized(System.IO.Stream stream) { throw null; } + protected static void ValidateBufferArguments(byte[] buffer, int offset, int count) { } + protected static void ValidateCopyToArguments(System.IO.Stream destination, int bufferSize) { } public abstract void Write(byte[] buffer, int offset, int count); public virtual void Write(System.ReadOnlySpan buffer) { } public System.Threading.Tasks.Task WriteAsync(byte[] buffer, int offset, int count) { throw null; } diff --git a/src/libraries/System.Security.Cryptography.Primitives/src/System/Security/Cryptography/CryptoStream.cs b/src/libraries/System.Security.Cryptography.Primitives/src/System/Security/Cryptography/CryptoStream.cs index 6fc21ad..74dca0f 100644 --- a/src/libraries/System.Security.Cryptography.Primitives/src/System/Security/Cryptography/CryptoStream.cs +++ b/src/libraries/System.Security.Cryptography.Primitives/src/System/Security/Cryptography/CryptoStream.cs @@ -273,14 +273,7 @@ namespace System.Security.Cryptography private void CheckReadArguments(byte[] buffer, int offset, int count) { - if (buffer is null) - throw new ArgumentNullException(nameof(buffer)); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); + ValidateBufferArguments(buffer, offset, count); if (!CanRead) throw new NotSupportedException(SR.NotSupported_UnreadableStream); } @@ -522,14 +515,7 @@ namespace System.Security.Cryptography private void CheckWriteArguments(byte[] buffer, int offset, int count) { - if (buffer is null) - throw new ArgumentNullException(nameof(buffer)); - if (offset < 0) - throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_NeedNonNegNum); - if (count < 0) - throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_NeedNonNegNum); - if (buffer.Length - offset < count) - throw new ArgumentException(SR.Argument_InvalidOffLen); + ValidateBufferArguments(buffer, offset, count); if (!CanWrite) throw new NotSupportedException(SR.NotSupported_UnwritableStream); } diff --git a/src/libraries/System.Security.Cryptography.Primitives/tests/CryptoStream.cs b/src/libraries/System.Security.Cryptography.Primitives/tests/CryptoStream.cs index 2da7664..bb23983 100644 --- a/src/libraries/System.Security.Cryptography.Primitives/tests/CryptoStream.cs +++ b/src/libraries/System.Security.Cryptography.Primitives/tests/CryptoStream.cs @@ -58,17 +58,6 @@ namespace System.Security.Cryptography.Encryption.Tests.Asymmetric Assert.False(encryptStream.CanRead); Assert.False(encryptStream.CanSeek); Assert.False(encryptStream.HasFlushedFinalBlock); - Assert.Throws(() => encryptStream.SetLength(1)); - Assert.Throws(() => encryptStream.Length); - Assert.Throws(() => encryptStream.Position); - Assert.Throws(() => encryptStream.Position = 0); - Assert.Throws(() => encryptStream.Seek(0, SeekOrigin.Begin)); - Assert.Throws(() => encryptStream.Read(new byte[0], 0, 0)); - Assert.Throws(() => encryptStream.Write(null, 0, 0)); - Assert.Throws(() => encryptStream.Write(new byte[0], -1, 0)); - Assert.Throws(() => encryptStream.Write(new byte[0], 0, -1)); - Assert.Throws(() => encryptStream.Write(new byte[0], 0, -1)); - AssertExtensions.Throws(null, () => encryptStream.Write(new byte[3], 1, 4)); byte[] toWrite = Encoding.UTF8.GetBytes(LoremText); @@ -111,17 +100,6 @@ namespace System.Security.Cryptography.Encryption.Tests.Asymmetric Assert.True(decryptStream.CanRead); Assert.False(decryptStream.CanSeek); Assert.False(decryptStream.HasFlushedFinalBlock); - Assert.Throws(() => decryptStream.SetLength(1)); - Assert.Throws(() => decryptStream.Length); - Assert.Throws(() => decryptStream.Position); - Assert.Throws(() => decryptStream.Position = 0); - Assert.Throws(() => decryptStream.Seek(0, SeekOrigin.Begin)); - Assert.Throws(() => decryptStream.Write(new byte[0], 0, 0)); - Assert.Throws(() => decryptStream.Read(null, 0, 0)); - Assert.Throws(() => decryptStream.Read(new byte[0], -1, 0)); - Assert.Throws(() => decryptStream.Read(new byte[0], 0, -1)); - Assert.Throws(() => decryptStream.Read(new byte[0], 0, -1)); - AssertExtensions.Throws(null, () => decryptStream.Read(new byte[3], 1, 4)); using (StreamReader reader = new StreamReader(decryptStream)) { -- 2.7.4