Work on Stream classes

- consistent naming, ordering
- CanRead/Seek/Write fixes
- argument validation
- some cases of blocking zero-byte Read calls

4 files changed, 75 insertions, 117 deletions

diff --git a/crypto/src/bcpg/ArmoredInputStream.cs b/crypto/src/bcpg/ArmoredInputStream.cs
index 2895c379a..cb5c2f91f 100644
--- a/crypto/src/bcpg/ArmoredInputStream.cs
+++ b/crypto/src/bcpg/ArmoredInputStream.cs
@@ -311,6 +311,26 @@ namespace Org.BouncyCastle.Bcpg
             return c;
         }
 
+        public override int Read(byte[] buffer, int offset, int count)
+        {
+            Streams.ValidateBufferArguments(buffer, offset, count);
+
+            /*
+             * TODO Currently can't return partial data when exception thrown (breaking test case), so we don't inherit
+             * the base class implementation. Probably the reason is that throws don't mark this instance as 'failed'.
+             */
+            int pos = 0;
+            while (pos < count)
+            {
+                int b = ReadByte();
+                if (b < 0)
+                    break;
+
+                buffer[offset + pos++] = (byte)b;
+            }
+            return pos;
+        }
+
         public override int ReadByte()
         {
             if (start)
@@ -463,52 +483,6 @@ namespace Org.BouncyCastle.Bcpg
             return c;
         }
 
-        /**
-         * Reads up to <code>len</code> bytes of data from the input stream into
-         * an array of bytes.  An attempt is made to read as many as
-         * <code>len</code> bytes, but a smaller number may be read.
-         * The number of bytes actually read is returned as an integer.
-         *
-         * The first byte read is stored into element <code>b[off]</code>, the
-         * next one into <code>b[off+1]</code>, and so on. The number of bytes read
-         * is, at most, equal to <code>len</code>.
-         *
-         * NOTE: We need to override the custom behavior of Java's {@link InputStream#read(byte[], int, int)},
-         * as the upstream method silently swallows {@link IOException IOExceptions}.
-         * This would cause CRC checksum errors to go unnoticed.
-         *
-         * @see <a href="https://github.com/bcgit/bc-java/issues/998">Related BC bug report</a>
-         * @param b byte array
-         * @param off offset at which we start writing data to the array
-         * @param len number of bytes we write into the array
-         * @return total number of bytes read into the buffer
-         *
-         * @throws IOException if an exception happens AT ANY POINT
-         */
-        public override int Read(byte[] b, int off, int len)
-        {
-            CheckIndexSize(b.Length, off, len);
-
-            int pos = 0;
-            while (pos < len)
-            {
-                int c = ReadByte();
-                if (c < 0)
-                    break;
-
-                b[off + pos++] = (byte)c;
-            }
-            return pos;
-        }
-
-        private void CheckIndexSize(int size, int off, int len)
-        {
-            if (off < 0 || len < 0)
-                throw new IndexOutOfRangeException("Offset and length cannot be negative.");
-            if (off > size - len)
-                throw new IndexOutOfRangeException("Invalid offset and length.");
-        }
-
 #if PORTABLE
         protected override void Dispose(bool disposing)
         {
diff --git a/crypto/src/bcpg/ArmoredOutputStream.cs b/crypto/src/bcpg/ArmoredOutputStream.cs
index 0df5d1141..97bcbde51 100644
--- a/crypto/src/bcpg/ArmoredOutputStream.cs
+++ b/crypto/src/bcpg/ArmoredOutputStream.cs
@@ -239,45 +239,44 @@ namespace Org.BouncyCastle.Bcpg
             clearText = false;
         }
 
-        public override void WriteByte(
-            byte b)
+        public override void WriteByte(byte value)
         {
             if (clearText)
             {
-                outStream.WriteByte(b);
+                outStream.WriteByte(value);
 
                 if (newLine)
                 {
-                    if (!(b == '\n' && lastb == '\r'))
+                    if (!(value == '\n' && lastb == '\r'))
                     {
                         newLine = false;
                     }
-                    if (b == '-')
+                    if (value == '-')
                     {
                         outStream.WriteByte((byte)' ');
                         outStream.WriteByte((byte)'-');      // dash escape
                     }
                 }
-                if (b == '\r' || (b == '\n' && lastb != '\r'))
+                if (value == '\r' || (value == '\n' && lastb != '\r'))
                 {
                     newLine = true;
                 }
-                lastb = b;
+                lastb = value;
                 return;
             }
 
             if (start)
             {
-                bool newPacket = (b & 0x40) != 0;
+                bool newPacket = (value & 0x40) != 0;
 
                 int tag;
                 if (newPacket)
                 {
-                    tag = b & 0x3f;
+                    tag = value & 0x3f;
                 }
                 else
                 {
-                    tag = (b & 0x3f) >> 2;
+                    tag = (value & 0x3f) >> 2;
                 }
 
                 switch ((PacketTag)tag)
@@ -334,8 +333,8 @@ namespace Org.BouncyCastle.Bcpg
                 }
             }
 
-            crc.Update(b);
-            buf[bufPtr++] = b & 0xff;
+            crc.Update(value);
+            buf[bufPtr++] = value & 0xff;
         }
 
         /**
diff --git a/crypto/src/bcpg/BcpgInputStream.cs b/crypto/src/bcpg/BcpgInputStream.cs
index 5efef193a..87c3f3812 100644
--- a/crypto/src/bcpg/BcpgInputStream.cs
+++ b/crypto/src/bcpg/BcpgInputStream.cs
@@ -1,7 +1,6 @@
 using System;
 using System.IO;
 
-using Org.BouncyCastle.Asn1;
 using Org.BouncyCastle.Utilities;
 using Org.BouncyCastle.Utilities.IO;
 
@@ -43,33 +42,22 @@ namespace Org.BouncyCastle.Bcpg
             return m_in.ReadByte();
         }
 
-        public override int Read(
-			byte[]	buffer,
-			int		offset,
-			int		count)
+        public override int Read(byte[] buffer, int offset, int count)
         {
-			// Strangely, when count == 0, we should still attempt to read a byte
-//			if (count == 0)
-//				return 0;
-
 			if (!next)
 				return m_in.Read(buffer, offset, count);
 
-			// We have next byte waiting, so return it
+            Streams.ValidateBufferArguments(buffer, offset, count);
 
 			if (nextB < 0)
-				return 0; // EndOfStream
-
-			if (buffer == null)
-				throw new ArgumentNullException("buffer");
-
-			buffer[offset] = (byte) nextB;
-			next = false;
+				return 0;
 
-			return 1;
+            buffer[offset] = (byte)nextB;
+            next = false;
+            return 1;
         }
 
-		public byte[] ReadAll()
+        public byte[] ReadAll()
         {
 			return Streams.ReadAll(this);
 		}
@@ -323,6 +311,8 @@ namespace Org.BouncyCastle.Bcpg
 
 			public override int Read(byte[] buffer, int offset, int count)
 			{
+                Streams.ValidateBufferArguments(buffer, offset, count);
+
 				do
 				{
 					if (dataLength != 0)
diff --git a/crypto/src/bcpg/BcpgOutputStream.cs b/crypto/src/bcpg/BcpgOutputStream.cs
index 738c28211..fbce0820e 100644
--- a/crypto/src/bcpg/BcpgOutputStream.cs
+++ b/crypto/src/bcpg/BcpgOutputStream.cs
@@ -215,8 +215,7 @@ namespace Org.BouncyCastle.Bcpg
             }
         }
 
-        private void PartialFlush(
-            bool isLast)
+        private void PartialFlush(bool isLast)
         {
             if (isLast)
             {
@@ -232,79 +231,75 @@ namespace Org.BouncyCastle.Bcpg
             partialOffset = 0;
         }
 
-		private void WritePartial(
-            byte b)
+        private void PartialWrite(byte[] buffer, int offset, int count)
         {
-            if (partialOffset == partialBufferLength)
-            {
-                PartialFlush(false);
-            }
-
-			partialBuffer[partialOffset++] = b;
-        }
+            Streams.ValidateBufferArguments(buffer, offset, count);
 
-		private void WritePartial(
-            byte[]	buffer,
-            int		off,
-            int		len)
-        {
             if (partialOffset == partialBufferLength)
             {
                 PartialFlush(false);
             }
 
-            if (len <= (partialBufferLength - partialOffset))
+            if (count <= (partialBufferLength - partialOffset))
             {
-                Array.Copy(buffer, off, partialBuffer, partialOffset, len);
-                partialOffset += len;
+                Array.Copy(buffer, offset, partialBuffer, partialOffset, count);
+                partialOffset += count;
             }
             else
             {
                 int diff = partialBufferLength - partialOffset;
-                Array.Copy(buffer, off, partialBuffer, partialOffset, diff);
-                off += diff;
-                len -= diff;
+                Array.Copy(buffer, offset, partialBuffer, partialOffset, diff);
+                offset += diff;
+                count -= diff;
                 PartialFlush(false);
-                while (len > partialBufferLength)
+                while (count > partialBufferLength)
                 {
-                    Array.Copy(buffer, off, partialBuffer, 0, partialBufferLength);
-                    off += partialBufferLength;
-                    len -= partialBufferLength;
+                    Array.Copy(buffer, offset, partialBuffer, 0, partialBufferLength);
+                    offset += partialBufferLength;
+                    count -= partialBufferLength;
                     PartialFlush(false);
                 }
-                Array.Copy(buffer, off, partialBuffer, 0, len);
-                partialOffset += len;
+                Array.Copy(buffer, offset, partialBuffer, 0, count);
+                partialOffset += count;
             }
         }
-        public override void WriteByte(
-			byte value)
+
+        private void PartialWriteByte(byte value)
+        {
+            if (partialOffset == partialBufferLength)
+            {
+                PartialFlush(false);
+            }
+
+            partialBuffer[partialOffset++] = value;
+        }
+
+        public override void Write(byte[] buffer, int offset, int count)
         {
             if (partialBuffer != null)
             {
-                WritePartial(value);
+                PartialWrite(buffer, offset, count);
             }
             else
             {
-                outStr.WriteByte(value);
+                outStr.Write(buffer, offset, count);
             }
         }
-        public override void Write(
-            byte[]	buffer,
-            int		offset,
-            int		count)
+
+        public override void WriteByte(byte value)
         {
             if (partialBuffer != null)
             {
-                WritePartial(buffer, offset, count);
+                PartialWriteByte(value);
             }
             else
             {
-                outStr.Write(buffer, offset, count);
+                outStr.WriteByte(value);
             }
         }
 
-		// Additional helper methods to write primitive types
-		internal virtual void WriteShort(
+        // Additional helper methods to write primitive types
+        internal virtual void WriteShort(
 			short n)
 		{
 			this.Write(