Merge pull request #2048 from stephentoub/compression_cc

Improve test coverage of System.IO.Compression

Author: stephentoub

src/System.IO.Compression/src/System/IO/Compression/DeflateStream.cs

@@ -42,39 +42,32 @@ public DeflateStream(Stream stream, CompressionMode mode, bool leaveOpen)
         {
             if (stream == null)
                 throw new ArgumentNullException("stream");
-
-            if (CompressionMode.Compress != mode && CompressionMode.Decompress != mode)
-                throw new ArgumentException(SR.ArgumentOutOfRange_Enum, "mode");
-
-            _stream = stream;
-            _mode = mode;
-            _leaveOpen = leaveOpen;
-
-            switch (_mode)
+            
+            switch (mode)
             {
                 case CompressionMode.Decompress:
-
-                    if (!_stream.CanRead)
+                    if (!stream.CanRead)
                     {
                         throw new ArgumentException(SR.NotReadableStream, "stream");
                     }
-
                     _inflater = new Inflater();
-
                     break;
 
                 case CompressionMode.Compress:
-
-                    if (!_stream.CanWrite)
+                    if (!stream.CanWrite)
                     {
                         throw new ArgumentException(SR.NotWriteableStream, "stream");
                     }
-
                     _deflater = CreateDeflater(null);
-
                     break;
-            }  // switch (_mode)
 
+                default:
+                    throw new ArgumentException(SR.ArgumentOutOfRange_Enum, "mode");
+            }
+
+            _stream = stream;
+            _mode = mode;
+            _leaveOpen = leaveOpen;
             _buffer = new byte[DefaultBufferSize];
         }
 
@@ -120,22 +113,16 @@ private static IDeflater CreateDeflater(CompressionLevel? compressionLevel)
                 deflatorType = s_forcedTestingDeflaterType;
 #endif
 
-            switch (deflatorType)
+            if (deflatorType == WorkerType.ZLib)
             {
-                case WorkerType.Managed:
-                    return new DeflaterManaged();
-
-                case WorkerType.ZLib:
-                    if (compressionLevel.HasValue)
-                        return new DeflaterZLib(compressionLevel.Value);
-                    else
-                        return new DeflaterZLib();
-
-                default:
-                    // We do not expect this to ever be thrown.
-                    // But this is better practice than returning null.
-                    Environment.FailFast("Program entered an unexpected state.");
-                    return null; // we'll not reach here
+                return compressionLevel.HasValue ?
+                    new DeflaterZLib(compressionLevel.Value) :
+                    new DeflaterZLib();
+            }
+            else
+            {
+                Debug.Assert(deflatorType == WorkerType.Managed);
+                return new DeflaterManaged();
             }
         }
 
@@ -221,7 +208,6 @@ public override long Position
         public override void Flush()
         {
             EnsureNotDisposed();
-            return;
         }
 
         public override Task FlushAsync(CancellationToken cancellationToken)
@@ -273,9 +259,15 @@ public override int Read(byte[] array, int offset, int count)
                 Debug.Assert(_inflater.NeedsInput(), "We can only run into this case if we are short of input");
 
                 int bytes = _stream.Read(_buffer, 0, _buffer.Length);
-                if (bytes == 0)
+                if (bytes <= 0)
                 {
-                    break;      //Do we want to throw an exception here?
+                    break;
+                }
+                else if (bytes > _buffer.Length)
+                {
+                    // The stream is either malicious or poorly implemented and returned a number of
+                    // bytes larger than the buffer supplied to it.
+                    throw new InvalidDataException(SR.GenericInvalidData);
                 }
 
                 _inflater.SetInput(_buffer, 0, bytes);
@@ -386,6 +378,12 @@ private async Task<int> ReadAsyncCore(Task<int> readTask, byte[] array, int offs
                         // This indicates the base stream has received EOF
                         return 0;
                     }
+                    else if (bytesRead > _buffer.Length)
+                    {
+                        // The stream is either malicious or poorly implemented and returned a number of
+                        // bytes larger than the buffer supplied to it.
+                        throw new InvalidDataException(SR.GenericInvalidData);
+                    }
 
                     cancellationToken.ThrowIfCancellationRequested();
 

src/System.IO.Compression/src/System/IO/Compression/ZLibNative.cs

@@ -1,10 +1,10 @@
 // Copyright (c) Microsoft. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using System.Diagnostics;
 using System.Diagnostics.Contracts;
 using System.Runtime.InteropServices;
 using System.Security;
-using System.Text;
 
 namespace System.IO.Compression
 {
@@ -221,7 +221,6 @@ public enum State { NotInitialized, InitializedForDeflate, InitializedForInflate
 
 
             public ZLibStreamHandle()
-
                 : base(new IntPtr(-1), true)
             {
                 _zStream = new ZStream();
@@ -236,7 +235,7 @@ public ZLibStreamHandle()
             public override bool IsInvalid
             {
                 [SecurityCritical]
-                get { return DangerousGetHandle() == new IntPtr(-1); }
+                get { return handle == new IntPtr(-1); }
             }
 
             public State InitializationState
@@ -250,9 +249,6 @@ public State InitializationState
             [SecurityCritical]
             protected override bool ReleaseHandle()
             {
-                // We are in a finalizer thread at the end of the App and the finalization of the dynamically loaded ZLib happend
-                // to be scheduled first. In such case we have no hope of properly freeing zStream. If the process is dying - we
-                // do not care. In other cases somethign went badly wrong anyway:
                 switch (InitializationState)
                 {
                     case State.NotInitialized: return true;
@@ -412,33 +408,11 @@ public ErrorCode InflateEnd()
             public string GetErrorMessage()
             {
                 // This can work even after XxflateEnd().
-
-                if (ZNullPtr.Equals(_zStream.msg))
-                    return String.Empty;
-
-                unsafe
-                {
-                    StringBuilder sb = new StringBuilder();
-                    SByte* pMessage = (SByte*)_zStream.msg;
-                    char c;
-                    do
-                    {
-                        c = (char)*pMessage;
-                        pMessage++;
-                        sb.Append(c);
-                    } while ((sbyte)c != 0);
-
-                    return sb.ToString();
-                }
+                return _zStream.msg != ZNullPtr ? Marshal.PtrToStringAnsi(_zStream.msg) : string.Empty;
             }
 
             #endregion  // Expose ZLib functions for use by user / Fx code (add more as required)
 
-            [SecurityCritical]
-            internal static Int32 ZLibCompileFlags()
-            {
-                return Interop.zlib.zlibCompileFlags();
-            }
         }  // class ZLibStreamHandle
 
         #endregion  // ZLib Stream Handle type
@@ -447,15 +421,6 @@ internal static Int32 ZLibCompileFlags()
         #region public factory methods for ZLibStreamHandle
 
 
-        [SecurityCritical]
-        public static ErrorCode CreateZLibStreamForDeflate(out ZLibStreamHandle zLibStreamHandle)
-        {
-            return CreateZLibStreamForDeflate(out zLibStreamHandle,
-                                              CompressionLevel.DefaultCompression, Deflate_DefaultWindowBits,
-                                              Deflate_DefaultMemLevel, CompressionStrategy.DefaultStrategy);
-        }
-
-
         [SecurityCritical]
         public static ErrorCode CreateZLibStreamForDeflate(out ZLibStreamHandle zLibStreamHandle,
                                                            CompressionLevel level, int windowBits, int memLevel, CompressionStrategy strategy)
@@ -465,13 +430,6 @@ public static ErrorCode CreateZLibStreamForDeflate(out ZLibStreamHandle zLibStre
         }
 
 
-        [SecurityCritical]
-        public static ErrorCode CreateZLibStreamForInflate(out ZLibStreamHandle zLibStreamHandle)
-        {
-            return CreateZLibStreamForInflate(out zLibStreamHandle, Deflate_DefaultWindowBits);
-        }
-
-
         [SecurityCritical]
         public static ErrorCode CreateZLibStreamForInflate(out ZLibStreamHandle zLibStreamHandle, int windowBits)
         {
@@ -481,15 +439,5 @@ public static ErrorCode CreateZLibStreamForInflate(out ZLibStreamHandle zLibStre
 
         #endregion  // public factory methods for ZLibStreamHandle
 
-
-        #region public utility APIs
-
-        [SecurityCritical]
-        public static Int32 ZLibCompileFlags()
-        {
-            return ZLibStreamHandle.ZLibCompileFlags();
-        }
-        #endregion  // public utility APIs
-
     }  // internal class ZLibNative
 }  // namespace System.IO.Compression