CODEWITHSUNDEEP
c#code-analysis
Rafael Herscovici
Rafael Herscovici

Reputation: 17134

Code analysis - Do not dispose object multiple times

i have tried following the rules of code analysis on this method:

public static string Encrypt(string password)
{
    string myPassword = string.Empty;
    if (!string.IsNullOrEmpty(password))
    {
        myPassword = password;
        byte[] Value = System.Text.Encoding.UTF8.GetBytes(myPassword);
        SymmetricAlgorithm mCSP = new RijndaelManaged();
        mCSP.Key = _key;
        mCSP.IV = _initVector;
        using (ICryptoTransform ct = mCSP.CreateEncryptor(mCSP.Key, mCSP.IV))
        {
            using (System.IO.MemoryStream ms = new System.IO.MemoryStream())
            {
                using (CryptoStream cs = new CryptoStream(ms, ct, CryptoStreamMode.Write))
                {
                    cs.Write(Value, 0, Value.Length);
                    cs.FlushFinalBlock();
                    cs.Close();
                    myPassword = Convert.ToBase64String(ms.ToArray());
                }
            }
        }
    }
    return myPassword;
}

added all the Try {} Finaly{} blocks, but it was still yelling at me that i dont respect rule 2202. anyone can give me a hand with this?

yes, i have read other posts about this subject and tried applying it, but at the end i still get the same message.

Upvotes: 8

Views: 5894

Answers (4)

NP83
NP83

Reputation: 182

The documentation on this analysis warning (http://msdn.microsoft.com/en-us/library/ms182334.aspx) gives this example, similar to yours in that it's manipulating streams:

Stream stream = null;
try
{
    stream = new FileStream("file.txt", FileMode.OpenOrCreate);
    using (StreamWriter writer = new StreamWriter(stream))
    {
        stream = null;
        // Use the writer object...
    }
}
finally
{
    if(stream != null)
        stream.Dispose();
}

but this still gives the error. The following will solve the error:

Stream stream = null;
StreamWriter writer = null;
try
{
   stream = new FileStream("file.txt", FileMode.OpenOrCreate);
   writer = new StreamWriter(stream))

   // Do some stuff on the stream writer..
}
finally
{
   if(writer != null)
       writer.Dispose();
   else if(stream != null)
       stream.Dispose();
}

The reason is simple; if the writer will always dispose the stream for you. Only in the scenario the writer is not successfully created should you dispose the stream yourself. But I must admit I like the following syntax a lot more, and if you create a MemoryStream instead of a FileStream the chance of an exception occurring is small and I would prefer suppressing the CA. Please notice that you can stack using statements, so a extra 'nesting level' is often not required.

using (Stream stream = new FileStream("file.txt", FileMode.OpenOrCreate))
using (StreamWriter writer = new StreamWriter(stream))
{
    // Use the writer object...
}

Upvotes: 2

Nicole Calinoiu
Nicole Calinoiu

Reputation: 21002

To get rid of the CA2202 warning for cs, simply remove the call to its Close method.

The CA2202 problem for ms is a wee bit more complex. The warning is cropping up because CryptoStream has the effrontery to dispose the stream it received via is constructor, which means that there's one inappropriate call to ms.Close() that you can't avoid. The good news is that this untimely disposition has no side-effects in your case, and the same goes for the double disposition, so you can safely slap on a SuppressMessageAttribute and ignore the problem. (For cases where you actually need to passed stream to survive its unpreventable disposition by something like CryptoStream, the usual technique is to use a stream subclass whose disposition can be prevented by its instantiating code.)

Upvotes: 13

Daniel Hilgarth
Daniel Hilgarth

Reputation: 174477

Following the documentation on this topic should lead to this code:

public static string Encrypt(string password)
{
    string myPassword = string.Empty;
    if (!string.IsNullOrEmpty(password))
    {
        myPassword = password;
        byte[] Value = System.Text.Encoding.UTF8.GetBytes(myPassword);
        SymmetricAlgorithm mCSP = new RijndaelManaged();
        mCSP.Key = _key;
        mCSP.IV = _initVector;
        using (ICryptoTransform ct = mCSP.CreateEncryptor(mCSP.Key, mCSP.IV))
        {
            System.IO.MemoryStream ms = null;
            try
            {
                ms = new System.IO.MemoryStream()
                var tmp = ms;
                using (CryptoStream cs = new CryptoStream(ms, ct, 
                                                       CryptoStreamMode.Write))
                {
                    ms = null;

                    cs.Write(Value, 0, Value.Length);
                    cs.FlushFinalBlock();
                    cs.Close();
                    myPassword = Convert.ToBase64String(tmp.ToArray());
                }
            }
            finally
            {
                if(ms != null)
                    ms.Dispose();
            }
        }
    }
    return myPassword;
}

Upvotes: 2

Ray
Ray

Reputation: 46605

Get rid of these two lines, they aren't needed:

cs.FlushFinalBlock();
cs.Close();

Upvotes: 2

Related Questions