1

I am trying to utilize the INFLATE compression stream in .NET using a DeflateStream. My code throws an InvalidDataException although I know that the data I am passing has been correctly processed by the DEFLATE algorithm (it has been tested). Am I using the DeflateStream incorrectly? My code is as follows:

public byte[] Inflate(byte[] deflateArr)
    {
        MemoryStream ms;

        // try to create a MemoryStream from a byte array
        try
        {
            ms = new MemoryStream(deflateArr);
        }
        catch (ArgumentNullException)
        {
            return null;
        }

        // create a deflatestream and pass it the memory stream
        DeflateStream ds;
        try
        {
            ds = new DeflateStream(ms, CompressionMode.Decompress);
        }
        catch (ArgumentNullException)
        {
            return null;
        }
        catch (ArgumentException)
        {
            return null;
        }

        // create a bytes array and read into it
        byte[] bytes = new byte[4096];

        try
        {
            ds.Read(bytes, 0, 4096);
        }
        catch (ArgumentNullException)
        {
            return null;
        }
        catch (InvalidOperationException)
        {
            return null;
        }
        catch (ArgumentOutOfRangeException)
        {
            return null;
        }
        catch (InvalidDataException)
        {
            return null;
        }

        // close the memory stream
        ms.Close();

        // close the deflate stream
        ds.Close();

        return bytes;
    }
cytinus
  • 5,467
  • 8
  • 36
  • 47

1 Answers1

5

No, you're not.

Things wrong with this code:

  • Explicitly calling Close() instead of using a using statement. Probably not harmful here, but a bad idea.
  • Catching various exceptions which you really shouldn't be catching at all, as they indicate programming bugs
  • Catching exceptions on a per-statement basis, even though you're treating them the same way throughout the code (so could catch the exceptions for a much larger block)
  • Ignoring the return value of Stream.Read

Here's a better version, assuming you're using .NET 4 (for Stream.CopyTo)

public static byte[] Inflate(byte[] inputData)
{
    using (Stream input = new DeflateStream(new MemoryStream(inputData),
                                            CompressionMode.Decompress))
    {
        using (MemoryStream output = new MemoryStream())
        {
            input.CopyTo(output);
            return output.ToArray();
        }
    }
}

Now you may want to catch InvalidDataException - personally I wouldn't at this point, but it may make sense to do so. (I'd catch it at the calling side, if necessary. You can always wrap this method in another one if necessary.)

OregonGhost
  • 23,359
  • 7
  • 71
  • 108
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Sorry if this is a dumb question, but what is the downside of catching exceptions? – cytinus Jul 02 '12 at 14:12
  • 1
    @cytinus: It's hiding a bug. If your method returns null, you have *no idea why* and your program will keep running - whereas if the exception bubbles up, you can tell that you've got an error, and you don't try to keep going with bad data. Something like `ArgumentNullException` should always be due to a bug somewhere - so you want to find that bug ASAP. – Jon Skeet Jul 02 '12 at 14:17
  • Oh, my actual code contains a bunch of logging, and if that method returns null the calling method kills the program with an error message. Everything is logged to the Windows Event logger, I just deleted that code from the question because it wasn't pertinent to my question. – cytinus Jul 02 '12 at 14:19
  • Also by `DefaultStream` do you mean `DeflateStream`? – cytinus Jul 02 '12 at 14:21
  • @cytinus: Yes, `DefaultStream` was a typo. But adding all those catch blocks is still a really bad idea. Let the exception bubble up to the top of the stack *then* log it in a central location and kill the program if necessary. – Jon Skeet Jul 02 '12 at 14:26
  • Oh I get it now. Thanks a lot. Also your code works, so I must have been doing something wrong. – cytinus Jul 02 '12 at 14:27