CA2202, how to solve this case

Question

Can anybody tell me how to remove all CA2202 warnings from the following code?

public static byte[] Encrypt(string data, byte[] key, byte[] iv)
{
    using(MemoryStream memoryStream = new MemoryStream())
    {
        using (DESCryptoServiceProvider cryptograph = new DESCryptoServiceProvider())
        {
            using (CryptoStream cryptoStream = new CryptoStream(memoryStream, cryptograph.CreateEncryptor(key, iv), CryptoStreamMode.Write))
            {
                using(StreamWriter streamWriter = new StreamWriter(cryptoStream))
                {
                    streamWriter.Write(data);
                }
            }
        }
        return memoryStream.ToArray();
    }
}

Warning 7 CA2202 : Microsoft.Usage : Object 'cryptoStream' can be disposed more than once in method 'CryptoServices.Encrypt(string, byte[], byte[])'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.: Lines: 34

Warning 8 CA2202 : Microsoft.Usage : Object 'memoryStream' can be disposed more than once in method 'CryptoServices.Encrypt(string, byte[], byte[])'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.: Lines: 34, 37

You need Visual Studio Code Analysis to see these warnings (these are not c# compiler warnings).

Answer 1

The cryptostream is based on the memorystream.

What appears to be happening is that when the crypostream is disposed (at end of using) the memorystream is also disposed, then the memorystream is disposed again.

Answer 2

Off-topic but I would suggest you to use a different formatting technique for grouping usings:

using (var memoryStream = new MemoryStream())
{
    using (var cryptograph = new DESCryptoServiceProvider())
    using (var encryptor = cryptograph.CreateEncryptor(key, iv))
    using (var cryptoStream = new CryptoStream(memoryStream, encryptor, CryptoStreamMode.Write))
    using (var streamWriter = new StreamWriter(cryptoStream))
    {
        streamWriter.Write(data);
    }

    return memoryStream.ToArray();
}

I also advocate using vars here to avoid repetitions of really long class names.

P.S. Thanks to @ShellShock for pointing out I can't omit braces for first using as it would make memoryStream in return statement out of the scope.

Answer 3

When a StreamWriter is disposed, it will automatically dispose the wrapped Stream (here: the CryptoStream). CryptoStream also automatically disposes the wrapped Stream (here: the MemoryStream).

So your MemoryStream is disposed both by the CryptoStream and the using statement. And your CryptoStream is disposed by the StreamWriter and the outer using statement.

---

After some experimentation, it seems to be impossible to get rid of warnings completely. Theorectically, the MemoryStream needs to be disposed, but then you theoretically couldn't access its ToArray method anymore. Practically, a MemoryStream does not need to be disposed, so I'd go with this solution and suppress the CA2000 warning.

var memoryStream = new MemoryStream();

using (var cryptograph = new DESCryptoServiceProvider())
using (var writer = new StreamWriter(new CryptoStream(memoryStream, ...)))
{
    writer.Write(data);
}

return memoryStream.ToArray();

Answer 4

Well, it is accurate, the Dispose() method on these streams will be called more than one. The StreamReader class will take 'ownership' of the cryptoStream so disposing streamWriter will also dispose cryptoStream. Similarly, the CryptoStream class takes over responsibility for the memoryStream.

These are not exactly real bugs, these .NET classes are resilient to multiple Dispose() calls. But if you want to get rid of the warning then you should drop the using statement for these objects. And pain yourself a bit when reasoning what will happen if the code throws an exception. Or shut-up the warning with an attribute. Or just ignore the warning since it is silly.

Answer 5

Try something like this per Microsoft documentation about this warning :

public static byte[] Encrypt(string data, byte[] key, byte[] iv)
  {
     MemoryStream memoryStream = null;
     CryptoStream cryptoStream = null;

     try
     {
        memoryStream = new MemoryStream();
        using (DESCryptoServiceProvider cryptograph = new DESCryptoServiceProvider())
        {
           cryptoStream = new CryptoStream(memoryStream, cryptograph.CreateEncryptor(key, iv), CryptoStreamMode.Write);
           using (StreamWriter streamWriter = new StreamWriter(cryptoStream))
           {
              memoryStream = null;
              cryptoStream = null;
              streamWriter.Write(data);
              return // Have to recover the memoryStream from the StreamWriter ?
           }
        }
     }
     finally
     {
        if (cryptoStream != null)
        {
           cryptoStream.Dispose();
        }
        else
        {
           if (memoryStream != null)
              memoryStream.Dispose();
        }
     }
  }

Answer 6

I used this kind of code that takes byte[] and return byte[] without using streams

public static byte[] Encrypt(byte[] data, byte[] key, byte[] iv)
{
  DES des = new DES();
  des.BlockSize = 128;
  des.Mode = CipherMode.CBC;
  des.Padding = PaddingMode.Zeros;
  des.IV = IV
  des.Key = key
  ICryptoTransform encryptor = des.CreateEncryptor();

  //and finaly operations on bytes[] insted of streams
  return encryptor.TransformFinalBlock(plaintextarray,0,plaintextarray.Length);
}

This way all you have to do is conversion from string to byte[] using encodings.

Answer 7

I would do this using #pragma warning disable.

The .NET Framework Guidelines recommend to implement IDisposable.Dispose in such a way that it can be called multiple times. From the MSDN description of IDisposable.Dispose:

The object must not throw an exception if its Dispose method is called multiple times

Therefore the warning seems to be almost meaningless:

To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object

I guess it could be argued that the warning may be helpful if you're using a badly-implemented IDisposable object that does not follow the standard implementation guidelines. But when using classes from the .NET Framework like you are doing, I'd say it's safe to suppress the warning using a #pragma. And IMHO this is preferable to going through hoops as suggested in the MSDN documentation for this warning.

Answer 8

You should suppress the warnings in this case. Code that deals with disposables should be consistent, and you shouldn't have to know that other classes take ownership of the disposables you created:

[SuppressMessage("Microsoft.Usage", "CA2202:Do not dispose objects multiple times")]
public static byte[] Encrypt(string data, byte[] key, byte[] iv) {
  using (var memoryStream = new MemoryStream()) {
    using (var cryptograph = new DESCryptoServiceProvider())
    using (var cryptoStream = new CryptoStream(memoryStream, cryptograph.CreateEncryptor(key, iv), CryptoStreamMode.Write))
    using (var streamWriter = new StreamWriter(cryptoStream)) {
      streamWriter.Write(data);
    }
    return memoryStream.ToArray();
  }
}

Answer 9

This compiles without warning:

    public static byte[] Encrypt(string data, byte[] key, byte[] iv)
    {
        MemoryStream memoryStream = null;
        DESCryptoServiceProvider cryptograph = null;
        CryptoStream cryptoStream = null;
        StreamWriter streamWriter = null;
        try
        {
            memoryStream = new MemoryStream();
            cryptograph = new DESCryptoServiceProvider();
            cryptoStream = new CryptoStream(memoryStream, cryptograph.CreateEncryptor(key, iv), CryptoStreamMode.Write);
            var result = memoryStream;              
            memoryStream = null;
            streamWriter = new StreamWriter(cryptoStream);
            cryptoStream = null;
            streamWriter.Write(data);
            return result.ToArray();
        }
        finally
        {
            if (memoryStream != null)
                memoryStream.Dispose();
            if (cryptograph != null)
                cryptograph.Dispose();
            if (cryptoStream != null)
                cryptoStream.Dispose();
            if (streamWriter != null)
                streamWriter.Dispose();
        }
    }