CA2202, how to solve this case
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 Str开发者_如何转开发eamWriter(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).
You should suppress the warnings in this case. Code that deals with disposables should be consistent, and you shouldn't have to care that other classes take ownership of the disposables you created and also call Dispose
on them.
[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();
}
}
UPDATE: In the IDisposable.Dispose documentation you can read this:
If an object's Dispose method is called more than once, the object must ignore all calls after the first one. The object must not throw an exception if its Dispose method is called multiple times.
It can be argued that this rule exists so that developers can employ the using
statement sanely in a cascade of disposables, like I've shown above (or maybe this is just a nice side-effect). By the same token, then, CA2202 serves no useful purpose, and it should be suppressed project-wise. The real culprit would be a faulty implementation of Dispose
, and CA1065 should take care of that (if it's under your responsibility).
Well, it is accurate, the Dispose() method on these streams will be called more than once. 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.
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();
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.
I was faced with similar issues in my code.
Looks like the whole CA2202 thing is triggered because MemoryStream
can be disposed if exception occurs in constructor (CA2000).
This could be resolved like this:
1 public static byte[] Encrypt(string data, byte[] key, byte[] iv)
2 {
3 MemoryStream memoryStream = GetMemoryStream();
4 using (DESCryptoServiceProvider cryptograph = new DESCryptoServiceProvider())
5 {
6 CryptoStream cryptoStream = new CryptoStream(memoryStream, cryptograph.CreateEncryptor(key, iv), CryptoStreamMode.Write);
7 using (StreamWriter streamWriter = new StreamWriter(cryptoStream))
8 {
9 streamWriter.Write(data);
10 return memoryStream.ToArray();
11 }
12 }
13 }
14
15 /// <summary>
16 /// Gets the memory stream.
17 /// </summary>
18 /// <returns>A new memory stream</returns>
19 private static MemoryStream GetMemoryStream()
20 {
21 MemoryStream stream;
22 MemoryStream tempStream = null;
23 try
24 {
25 tempStream = new MemoryStream();
26
27 stream = tempStream;
28 tempStream = null;
29 }
30 finally
31 {
32 if (tempStream != null)
33 tempStream.Dispose();
34 }
35 return stream;
36 }
Notice that we have to return the memoryStream
inside the last using
statement (line 10) because cryptoStream
gets disposed at line 11 (because it's used in streamWriter
using
statement), which leads memoryStream
to get also disposed at line 11 (because memoryStream
is used to create the cryptoStream
).
At least this code worked for me.
EDIT:
Funny as it may sound, I discovered that if you replace the GetMemoryStream
method with the following code,
/// <summary>
/// Gets a memory stream.
/// </summary>
/// <returns>A new memory stream</returns>
private static MemoryStream GetMemoryStream()
{
return new MemoryStream();
}
you get the same result.
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.
I wanted to solve this the right way - that is without suppressing the warnings and rightly disposing all disposable objects.
I pulled out 2 of the 3 streams as fields and disposed them in the Dispose()
method of my class. Yes, implementing the IDisposable
interface might not necessarily be what you are looking for but the solution looks pretty clean as compared to dispose()
calls from all random places in the code.
public class SomeEncryption : IDisposable
{
private MemoryStream memoryStream;
private CryptoStream cryptoStream;
public static byte[] Encrypt(string data, byte[] key, byte[] iv)
{
// Do something
this.memoryStream = new MemoryStream();
this.cryptoStream = new CryptoStream(this.memoryStream, encryptor, CryptoStreamMode.Write);
using (var streamWriter = new StreamWriter(this.cryptoStream))
{
streamWriter.Write(plaintext);
}
return memoryStream.ToArray();
}
public void Dispose()
{
this.Dispose(true);
GC.SuppressFinalize(this);
}
protected virtual void Dispose(bool disposing)
{
if (disposing)
{
if (this.memoryStream != null)
{
this.memoryStream.Dispose();
}
if (this.cryptoStream != null)
{
this.cryptoStream.Dispose();
}
}
}
}
Off-topic but I would suggest you to use a different formatting technique for grouping using
s:
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 var
s 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.
I just wanted to unwrap the code so we can see multiple calls to Dispose
on the objects. The reality is that you are calling Dispose
on objects twice:
memoryStream = new MemoryStream()
cryptograph = new DESCryptoServiceProvider()
cryptoStream = new CryptoStream()
streamWriter = new StreamWriter()
memoryStream.Dispose(); //implicitly owned by cryptoStream
cryptoStream.Dispose(); //implicitly owned by streamWriter
streamWriter.Dispose(); //through a using
cryptoStream.Dispose(); //INVALID: second dispose through using
cryptograph.Dispose(); //through a using
memorySTream.Dipose(); //INVALID: second dispose through a using
return memoryStream.ToArray(); //INVALID: accessing disposed memoryStream
While most .NET class are (hopefully) resilient against the mistake of multiple calls to .Dispose
, not all classes are as defensive against programmer misuse.
Yes, the canonical documentation says that all class must be immune to programmer misuse from multiple calls to .Dispose
:
The object must not throw an exception if its Dispose method is called multiple times
But this is the real world - where we're trying to eliminate bugs; not cause them. FX Cop knows this, and warns you.
You have a few choices;
- only call
Dispose
once on any object; don't useusing
- keep calling dispose twice, and hope the code doesn't crash
- suppress the warning
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.
Avoid all usings and use nested Dispose-Calls!
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);
streamWriter = new StreamWriter(cryptoStream);
streamWriter.Write(data);
return memoryStream.ToArray();
}
finally
{
if(streamWriter != null)
streamWriter.Dispose();
else if(cryptoStream != null)
cryptoStream.Dispose();
else if(memoryStream != null)
memoryStream.Dispose();
if (cryptograph != null)
cryptograph.Dispose();
}
}
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();
}
}
Edit in response to the comments:
I just verified again that this code does not generate the warning, while the original one does.
In the original code, CryptoStream.Dispose()
and MemoryStream().Dispose(
) are actually called twice (which may or may not be a problem).
The modified code works as follows: references are set to null
, as soon as responsibilty for disposing is transferred to another object. E.g. memoryStream
is set to null
after the call to CryptoStream
constructor succeeded. cryptoStream
is set to null
, after the call to StreamWriter
constructor succeeded. If no exception occurs, streamWriter
is disposed in the finally
block and will in turn dispose CryptoStream
and MemoryStream
.
精彩评论