1

Don't Take Microsoft's Code for Granted

by Alexandru Lungu 5. November 2010 12:09

 

Take a look at the code below. This code is from Microsoft WCF-WF samples. Or, if you don’t want to download all samples, this exact piece of code can be found here.
Do you notice something strange?

//Helper method to compress an array of bytes
static ArraySegment<byte> CompressBuffer(ArraySegment<byte> buffer, BufferManager bufferManager, int messageOffset)
{
    byte[] bufferedBytes;
    using (MemoryStream memoryStream = new MemoryStream())
    {
        memoryStream.Write(buffer.Array, 0, messageOffset);

        using (GZipStream gzStream = new GZipStream(memoryStream, CompressionMode.Compress, true))
        {
            gzStream.Write(buffer.Array, messageOffset, buffer.Count);
        }


        byte[] compressedBytes = memoryStream.ToArray();
        bufferedBytes = bufferManager.TakeBuffer(compressedBytes.Length);

        Array.Copy(compressedBytes, 0, bufferedBytes, 0, compressedBytes.Length);

        bufferManager.ReturnBuffer(buffer.Array);
    }

    ArraySegment<byte> byteArray = new ArraySegment<byte>(bufferedBytes, messageOffset, bufferedBytes.Length - messageOffset);
    return byteArray;
}

 

A guy actually has written an article based on this code (withoud crediting Microsoft) and he noticed nothing; and this code has spread on the internet in the above form.

I didn’t noticed also, or more exactly I didn’t look at the code from the beginning. But I did look at the size of the original and the compressed message. And the size was almost the same (actually sometimes it was bigger when compressed). Initial I suspected Microsoft’s gzip implementation; I knew that they implemented the Deflate algorithm which isn’t the most effective but is free of patents.

So I replaced .Net implementation with DotNetZip. (which means just to add a reference to the DotNetZip.dll and change using System.IO.Compression; with using Ionic.Zlib;) And I was surprised to find that an error occurred at decompression.

And after that, I looked at the compression code. The line:

ArraySegment<byte> byteArray = new ArraySegment<byte>(bufferedBytes, messageOffset, bufferedBytes.Length - messageOffset);

was obvious wrong. The 3rd parameter of the ArraySegment<> must be the size of the segment, in our case compressedBytes.Length (and the compressedBytes must be declared outside of the using statement). 

Therefore the correct code is:

public static ArraySegment<byte> CompressBuffer(ArraySegment<byte> buffer, BufferManager bufferManager, int messageOffset)
{
    byte[] bufferedBytes, compressedBytes;
    using (MemoryStream memoryStream = new MemoryStream())
    {
        memoryStream.Write(buffer.Array, 0, messageOffset);

        using (GZipStream gzStream = new GZipStream(memoryStream, CompressionMode.Compress, true))
        {
            gzStream.Write(buffer.Array, messageOffset, buffer.Count);
        }

        compressedBytes = memoryStream.ToArray();
        bufferedBytes = bufferManager.TakeBuffer(compressedBytes.Length);

        Array.Copy(compressedBytes, 0, bufferedBytes, 0, compressedBytes.Length);
        bufferManager.ReturnBuffer(buffer.Array);
    }

    //ArraySegment<byte> byteArray = new ArraySegment<byte>(bufferedBytes, messageOffset, bufferedBytes.Length - messageOffset); //bad here
    ArraySegment<byte> byteArray = new ArraySegment<byte>(bufferedBytes, messageOffset, compressedBytes.Length);
    return byteArray;
}

 

You may think that it is not much of a deal; a small bug that slipped unnoticed; but it was in a code that has spread over the internet as the main example on how to use gzip compression with WCF.

The effects for those that took that code for granted:
- time wasted to implement compression
- server resources wasted to compress/decompress
- client resources wasted to compress/decompress
- bandwidth wasted for times when the compressed message was larger than the uncompressed.

In the end the result was much worse than if no improvement (gzip compression) would’ve been implemented.

So, if you use someone else’s code (even if it’s Microsoft’s) be sure you test it properly.

Tags: , , ,

Programming

Comments (1) -

Mihai T
Mihai T Romania
11/6/2010 12:05:24 AM #

But you have to admit: situations like this (where everything seems to work as expected and more harm happens)are extremely, extremely rare.
But this doesn't excuse the lack of testing.

Add comment




  Country flag
biuquote
  • Comment
  • Preview
Loading






Powered by BlogEngine.NET 2.0.0.36
Original Design by Laptop Geek, Adapted by onesoft