Reputation: 19147
I have seen several questions about this issue. For example:
"Object can be disposed of more than once" error
But I can't quite work out what I should be doing with my code. The warning I have from code analysis is:
1>D:\My Programs\2017\PTSTools\PTSTools\PTSTools.cs(81): warning CA2202: Microsoft.Usage : Object 'iCrypt' can be disposed more than once in method 'PTSToolsClass.Crypt(string, string, bool)'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.: Lines: 81
The method in question:
private string Crypt(string strData, string strPassword, bool bEncrypt)
{
byte[] u8Salt = new byte[] { ... };
PasswordDeriveBytes iPass = new PasswordDeriveBytes(strPassword, u8Salt);
Rijndael iAlg = Rijndael.Create();
iAlg.Key = iPass.GetBytes(32);
iAlg.IV = iPass.GetBytes(16);
ICryptoTransform iTrans = (bEncrypt) ? iAlg.CreateEncryptor() : iAlg.CreateDecryptor();
MemoryStream iMem = new MemoryStream();
CryptoStream iCrypt = new CryptoStream(iMem, iTrans, CryptoStreamMode.Write);
byte[] u8Data;
if (bEncrypt)
u8Data = Encoding.Unicode.GetBytes(strData);
else
u8Data = Convert.FromBase64String(strData);
try
{
iCrypt.Write(u8Data, 0, u8Data.Length);
iCrypt.Close();
if (bEncrypt)
return Convert.ToBase64String(iMem.ToArray());
else
return Encoding.Unicode.GetString(iMem.ToArray());
}
catch
{
return null;
}
finally
{
iCrypt.Close();
}
}
Can someone please explain why I get this warning raised for the finally
line of code?
Thank you.
Update:
Based on comments I changed it to:
private string Crypt(string strData, string strPassword, bool bEncrypt)
{
byte[] u8Salt = new byte[] { ... };
PasswordDeriveBytes iPass = new PasswordDeriveBytes(strPassword, u8Salt);
Rijndael iAlg = Rijndael.Create();
iAlg.Key = iPass.GetBytes(32);
iAlg.IV = iPass.GetBytes(16);
ICryptoTransform iTrans = (bEncrypt) ? iAlg.CreateEncryptor() : iAlg.CreateDecryptor();
MemoryStream iMem = new MemoryStream();
using (CryptoStream iCrypt = new CryptoStream(iMem, iTrans, CryptoStreamMode.Write))
{
byte[] u8Data;
if (bEncrypt)
u8Data = Encoding.Unicode.GetBytes(strData);
else
u8Data = Convert.FromBase64String(strData);
try
{
iCrypt.Write(u8Data, 0, u8Data.Length);
iCrypt.Close();
if (bEncrypt)
return Convert.ToBase64String(iMem.ToArray());
else
return Encoding.Unicode.GetString(iMem.ToArray());
}
catch
{
return null;
}
}
}
But I now have these warnings:
1>D:\My Programs\2017\PTSTools\PTSTools\PTSTools.cs(77): warning CA2202: Microsoft.Usage : Object 'iCrypt' can be disposed more than once in method 'PTSToolsClass.Crypt(string, string, bool)'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.: Lines: 77
1>D:\My Programs\2017\PTSTools\PTSTools\PTSTools.cs(77): warning CA2202: Microsoft.Usage : Object 'iMem' can be disposed more than once in method 'PTSToolsClass.Crypt(string, string, bool)'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.: Lines: 77
It now does not like the catch block.
Update:
Understood:
private string Crypt(string strData, string strPassword, bool bEncrypt)
{
byte[] u8Salt = new byte[] { 0x26, 0x19, 0x81, 0x4E, 0xA0, 0x6D, 0x95, 0x34, 0x26, 0x75, 0x64, 0x05, 0xF6 };
PasswordDeriveBytes iPass = new PasswordDeriveBytes(strPassword, u8Salt);
Rijndael iAlg = Rijndael.Create();
iAlg.Key = iPass.GetBytes(32);
iAlg.IV = iPass.GetBytes(16);
ICryptoTransform iTrans = (bEncrypt) ? iAlg.CreateEncryptor() : iAlg.CreateDecryptor();
MemoryStream iMem = new MemoryStream();
using (CryptoStream iCrypt = new CryptoStream(iMem, iTrans, CryptoStreamMode.Write))
{
byte[] u8Data;
if (bEncrypt)
u8Data = Encoding.Unicode.GetBytes(strData);
else
u8Data = Convert.FromBase64String(strData);
try
{
iCrypt.Write(u8Data, 0, u8Data.Length);
if (bEncrypt)
return Convert.ToBase64String(iMem.ToArray());
else
return Encoding.Unicode.GetString(iMem.ToArray());
}
catch
{
return null;
}
}
}
Upvotes: 0
Views: 175
Reputation: 151604
Because the finally
block of a try-(catch-)finally
block is always executed, the analyzer sees this:
try
{
iCrypt.Close();
}
finally
{
iCrypt.Close();
}
It then correctly infers that those calls can occur in succession, causing the ObjectDisposedException
to be thrown in the second call.
So: you don't need the iCrypt.Close()
in the try
block, because it'll always be closed in the finally
, whether an exception occurs or not.
Upvotes: 2
Reputation: 822
The message from code analysis is clear. Your code disposes (closes) iCrypt two times. One time inside the try block and one time in the finally block. Remove the line iCrypt.Close();
in the try block (between iCrypt.Write(...)
and if(...)
and the warning will disappear.
Even better approch is a using block:
using (CryptoStream iCrypt = new CryptoStream(...)) {
.... // your code without the finally block
}
The using will call iCrypt.Close() regardless whether and exception is thrown inside the block or not.
Upvotes: 1
Reputation: 1188
You are calling iCrypt.Close(); twice, remove the call from inside the try statement and you will be fine.
Upvotes: 1