CA2202, wie man diesen Fall löst

102

Kann mir jemand sagen, wie ich alle CA2202-Warnungen aus dem folgenden Code entfernen kann?

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();
    }
}

Warnung 7 CA2202: Microsoft.Usage: Das Objekt 'cryptoStream' kann in der Methode 'CryptoServices.Encrypt (Zeichenfolge, Byte [], Byte [])' mehrmals entsorgt werden. Um zu vermeiden, dass eine System.ObjectDisposedException generiert wird, sollten Sie Dispose nicht mehr als einmal für ein Objekt aufrufen: Zeilen: 34

Warnung 8 CA2202: Microsoft.Usage: Das Objekt 'memoryStream' kann in der Methode 'CryptoServices.Encrypt (Zeichenfolge, Byte [], Byte [])' mehrmals entsorgt werden. Um zu vermeiden, dass eine System.ObjectDisposedException generiert wird, sollten Sie Dispose nicht mehr als einmal für ein Objekt aufrufen: Zeilen: 34, 37

Sie benötigen Visual Studio Code Analysis, um diese Warnungen anzuzeigen (dies sind keine C # -Compiler-Warnungen).

Testalino
quelle
1
Dieser Code generiert diese Warnungen nicht.
Julien Hoarau
1
Ich erhalte dafür 0 Warnungen (Warnstufe 4, VS2010). Und für jemanden, der Probleme in diesem Bereich googelt, fügen Sie bitte auch den Text der Warnungen hinzu.
Henk Holterman
29
CAxxxx- Warnungen werden von Code Analysis und FxCop generiert .
dtb
Diese Warnung gilt nicht für den angezeigten Code. Warnungen können für genau dieses Szenario unterdrückt werden. Wenn Sie Ihren Code überprüft und dieser Einschätzung zugestimmt haben, platzieren Sie dies über Ihrer Methode: " [SuppressMessage("Microsoft.Usage", "CA2202:Do not dispose objects multiple times", Justification="BrainSlugs83 said so.")]" - using System.Diagnostics.CodeAnalysis;Stellen Sie sicher, dass Ihr Verwendungsblock eine " " -Anweisung enthält.
BrainSlugs83
2
Werfen
Adil Mammadov

Antworten:

-3

Dies wird ohne Vorwarnung kompiliert:

    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();
        }
    }

Bearbeiten als Antwort auf die Kommentare: Ich habe gerade erneut überprüft, dass dieser Code die Warnung nicht generiert, während der ursprüngliche Code dies tut. Im Originalcode werden CryptoStream.Dispose()und MemoryStream().Dispose() tatsächlich zweimal aufgerufen (was ein Problem sein kann oder nicht).

Der geänderte Code funktioniert wie folgt: Verweise werden auf gesetzt null, sobald die Verantwortung für die Entsorgung auf ein anderes Objekt übertragen wird. Wird memoryStreambeispielsweise auf gesetzt, nullnachdem der Aufruf des CryptoStreamKonstruktors erfolgreich war. cryptoStreamwird auf gesetzt null, nachdem der Aufruf des StreamWriterKonstruktors erfolgreich war. Wenn keine Ausnahme auftritt, streamWriterwird im finallyBlock entsorgt und wird wiederum CryptoStreamund entsorgen MemoryStream.

Henrik
quelle
85
-1 Es ist wirklich schlecht, hässlichen Code zu erstellen, um einer Warnung zu entsprechen, die unterdrückt werden sollte .
Jordão
4
Ich würde zustimmen, dass Sie Ihren Code nicht für etwas schlachten sollten, das irgendwann in der Zukunft repariert werden könnte, sondern nur unterdrücken.
Peteki
3
Wie löst dies das Problem? CA2202 wird weiterhin gemeldet, da memoryStream immer noch zweimal im finally-Block entsorgt werden kann.
Chris Gessler
3
Da CryptoStream Dispose intern im MemoryStream aufruft, kann es zweimal aufgerufen werden, was der Grund für die Warnung ist. Ich habe Ihre Lösung ausprobiert und bekomme trotzdem die Warnung.
Chris Gessler
2
Oh je, Sie haben Recht - ich habe nicht erwartet, dass es eine Bereinigungslogik gibt, die mit Ihrer ... Logiklogik ... gemischt ist - das ist nur bizarr und kryptisch - es ist sicherlich klug - aber auch hier beängstigend - bitte tun Sie dies nicht im Produktionscode; um klar zu sein: Sie bekommen, dass es kein tatsächliches Funktionsproblem gibt, das dadurch gelöst wird, richtig? (Es ist in Ordnung, diese Objekte mehrmals zu entsorgen.) - Ich würde die Abwärtsabstimmung entfernen, wenn ich könnte (SO verhindert, dass Sie die Antwort bearbeiten müssen) - aber ich würde dies nur ungern tun ... - und im Ernst, tu das niemals.
BrainSlugs83
142

Sie sollten in diesem Fall die Warnungen unterdrücken. Code, der sich mit Einwegartikeln befasst, sollte konsistent sein, und Sie sollten sich nicht darum kümmern müssen, dass andere Klassen das Eigentum an den von Ihnen erstellten Einwegartikeln übernehmen und diese auch aufrufen Dispose.

[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 der IDisposable.Dispose- Dokumentation können Sie Folgendes lesen:

Wenn die Dispose-Methode eines Objekts mehrmals aufgerufen wird, muss das Objekt alle Aufrufe nach dem ersten ignorieren. Das Objekt darf keine Ausnahme auslösen, wenn seine Dispose-Methode mehrmals aufgerufen wird.

Es kann argumentiert werden, dass diese Regel existiert, damit Entwickler die usingAussage vernünftig in einer Kaskade von Einwegartikeln verwenden können, wie ich oben gezeigt habe (oder vielleicht ist dies nur ein netter Nebeneffekt). Aus dem gleichen Grund erfüllt CA2202 keinen nützlichen Zweck und sollte projektbezogen unterdrückt werden. Der eigentliche Schuldige wäre eine fehlerhafte Implementierung von Dispose, und CA1065 sollte sich darum kümmern (wenn es in Ihrer Verantwortung liegt).

Jordão
quelle
14
Meiner Meinung nach ist dies ein Fehler in fxcop, diese Regel ist einfach falsch. Die dispose-Methode sollte niemals eine ObjectDisposedException auslösen. Wenn dies der Fall ist, sollten Sie sich zu diesem Zeitpunkt damit befassen, indem Sie dem Autor des Codes, der dispose auf diese Weise implementiert, einen Fehler melden.
justin.m.chase
14
Ich stimme @HansPassant im anderen Thread zu: Das Tool erledigt seine Aufgabe und warnt Sie vor unerwarteten Implementierungsdetails der Klassen. Persönlich denke ich, dass das eigentliche Problem das Design der APIs selbst ist. Wenn die verschachtelten Klassen standardmäßig davon ausgehen, dass es in Ordnung ist, das Eigentum an einem anderen Objekt zu übernehmen, das an anderer Stelle erstellt wurde, erscheint dies höchst fraglich. Ich kann sehen, wo dies nützlich sein könnte, wenn das resultierende Objekt zurückgegeben werden würde, aber die Standardeinstellung dieser Annahme scheint nicht intuitiv zu sein und verstößt gegen normale IDisposable-Muster.
BTJ
8
Msdn empfiehlt jedoch nicht, diese Art von Nachricht zu unterdrücken. Werfen
Adil Mammadov
2
Vielen Dank für den Link @AdilMammadov, nützliche Informationen, aber Microsoft hat in diesen Dingen nicht immer Recht.
Tim Abell
40

Nun, es ist richtig, die Dispose () -Methode für diese Streams wird mehrmals aufgerufen. Die StreamReader-Klasse übernimmt den Besitz des cryptoStream, sodass durch das Entsorgen von streamWriter auch cryptoStream entsorgt wird. Ebenso übernimmt die CryptoStream-Klasse die Verantwortung für den memoryStream.

Dies sind keine echten Fehler. Diese .NET-Klassen sind gegenüber mehreren Dispose () -Aufrufen stabil. Wenn Sie die Warnung jedoch entfernen möchten, sollten Sie die using-Anweisung für diese Objekte löschen. Und schmerzen Sie sich ein wenig, wenn Sie überlegen, was passieren wird, wenn der Code eine Ausnahme auslöst. Oder schließen Sie die Warnung mit einem Attribut. Oder ignorieren Sie einfach die Warnung, da sie albern ist.

Hans Passant
quelle
10
Spezielle Kenntnisse über das interne Verhalten von Klassen zu haben (wie ein Einwegartikel, der die Verantwortung für einen anderen übernimmt), ist zu viel, um zu fragen, ob man eine wiederverwendbare API entwerfen möchte. Ich denke also, dass die usingAussagen bleiben sollten. Diese Warnungen sind wirklich dumm.
Jordão
4
@ Jordão - ist das nicht was für ein Werkzeug? Um Sie vor internem Verhalten zu warnen, von dem Sie möglicherweise nichts gewusst haben?
Hans Passant
8
Genau. Aber ich würde die usingAussagen trotzdem nicht fallen lassen . Es fühlt sich einfach falsch an, sich auf ein anderes Objekt zu verlassen, um ein von mir erstelltes Objekt zu entsorgen. Für diesen Code ist es in Ordnung, aber es gibt viele Implementierungen von Streamund TextWriterda draußen (nicht nur auf der BCL). Der Code, um sie alle zu verwenden, sollte konsistent sein.
Jordão
3
Ja, stimme Jordão zu. Wenn Sie wirklich möchten, dass der Programmierer das interne Verhalten der API kennt, können Sie Ihre Funktion als DoSomethingAndDisposeStream (Stream-Stream, OtherData-Daten) benennen.
ZZZ
4
@HansPassant Können Sie darauf hinweisen, wo dokumentiert ist, dass die XmlDocument.Save()Methode Disposeden angegebenen Parameter aufruft ? Ich sehe es nicht in der Dokumentation von Save(XmlWriter)(wo ich den FxCop-Fehler habe), in der Save()Methode selbst oder in der Dokumentation von XmlDocumentsich.
Ian Boyd
9

Wenn ein StreamWriter entsorgt wird, wird der umschlossene Stream automatisch entsorgt (hier: der CryptoStream ). CryptoStream entsorgt auch automatisch den umschlossenen Stream (hier: den MemoryStream ).

Ihr MemoryStream wird also sowohl vom CryptoStream als auch von der using- Anweisung entsorgt . Und Ihr CryptoStream wird vom StreamWriter und der äußeren using- Anweisung entsorgt .


Nach einigen Experimenten scheint es unmöglich zu sein, Warnungen vollständig zu beseitigen. Theoretisch muss der MemoryStream entsorgt werden, aber theoretisch konnte man nicht mehr auf seine ToArray-Methode zugreifen. Praktisch muss ein MemoryStream nicht entsorgt werden, daher würde ich mich für diese Lösung entscheiden und die CA2000-Warnung unterdrücken.

var memoryStream = new MemoryStream();

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

return memoryStream.ToArray();
dtb
quelle
9

Ich würde das mit machen #pragma warning disable.

In den .NET Framework-Richtlinien wird empfohlen, IDisposable.Dispose so zu implementieren, dass es mehrmals aufgerufen werden kann. Aus der MSDN-Beschreibung von IDisposable.Dispose :

Das Objekt darf keine Ausnahme auslösen, wenn seine Dispose-Methode mehrmals aufgerufen wird

Daher scheint die Warnung fast bedeutungslos zu sein:

Um zu vermeiden, dass eine System.ObjectDisposedException generiert wird, sollten Sie Dispose nicht mehr als einmal für ein Objekt aufrufen

Ich denke, es könnte argumentiert werden, dass die Warnung hilfreich sein kann, wenn Sie ein schlecht implementiertes IDisposable-Objekt verwenden, das nicht den Standard-Implementierungsrichtlinien entspricht. Wenn Sie jedoch Klassen aus .NET Framework verwenden, wie Sie es tun, ist es sicher, die Warnung mit einem #pragma zu unterdrücken. Und meiner Meinung nach ist dies dem Durchlaufen von Rahmen vorzuziehen, wie in der MSDN-Dokumentation für diese Warnung vorgeschlagen .

Joe
quelle
4
CA2202 ist eine Code-Analyse-Warnung und keine Compiler-Warnung. #pragma warning disablekann nur zum Unterdrücken von Compiler-Warnungen verwendet werden. Um eine Code-Analyse-Warnung zu unterdrücken, müssen Sie ein Attribut verwenden.
Martin Liversage
2

Ich hatte ähnliche Probleme in meinem Code.

Es sieht so aus, als ob das gesamte CA2202-Ding ausgelöst wird, da MemoryStreames entsorgt werden kann, wenn im Konstruktor (CA2000) eine Ausnahme auftritt.

Dies könnte folgendermaßen gelöst werden:

 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 }

Beachten Sie, dass wir das memoryStreamInnere der letzten usingAnweisung (Zeile 10) zurückgeben cryptoStreammüssen, da es in Zeile 11 entsorgt wird (weil es in der streamWriter usingAnweisung verwendet wird), was dazu führt memoryStream, dass es auch in Zeile 11 entsorgt wird (weil memoryStreames zum Erstellen der verwendet wird cryptoStream).

Zumindest hat dieser Code bei mir funktioniert.

BEARBEITEN:

So komisch es auch klingen mag, ich habe festgestellt, dass, wenn Sie die GetMemoryStreamMethode durch den folgenden Code ersetzen ,

/// <summary>
/// Gets a memory stream.
/// </summary>
/// <returns>A new memory stream</returns>
private static MemoryStream GetMemoryStream()
{
    return new MemoryStream();
}

Sie erhalten das gleiche Ergebnis.

Jimi
quelle
1

Der Kryptostream basiert auf dem Memorystream.

Was zu passieren scheint, ist, dass, wenn der Kryostream (am Ende der Verwendung) auch der Memorystream entsorgt wird, der Memorystream erneut entsorgt wird.

Shiraz Bhaiji
quelle
1

Ich wollte das richtig lösen - das heißt, ohne die Warnungen zu unterdrücken und alle Einweggegenstände richtig zu entsorgen.

Ich habe 2 der 3 Streams als Felder herausgezogen und sie in der Dispose()Methode meiner Klasse angeordnet. Ja, die Implementierung der IDisposableSchnittstelle ist möglicherweise nicht unbedingt das, wonach Sie suchen, aber die Lösung sieht im Vergleich zu dispose()Aufrufen von allen zufälligen Stellen im Code ziemlich sauber aus.

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();
                }
            }
        }
   }
Divyanshm
quelle
0

Off-Topic, aber ich würde Ihnen empfehlen, eine andere Formatierungstechnik für die Gruppierung von usings zu verwenden:

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();
}

Ich empfehle auch, varhier s zu verwenden, um Wiederholungen von wirklich langen Klassennamen zu vermeiden.

PS Dank @ShellShock für den Hinweis auf Ich kann nicht auslassen Klammern für die ersten , usingda es machen würde , memoryStreamin returnAnweisung aus dem Rahmen.

Dan Abramov
quelle
5
Wird memoryStream.ToArray () nicht außerhalb des Gültigkeitsbereichs liegen?
Polyfun
Dies entspricht absolut dem ursprünglichen Code. Ich habe nur geschweifte Klammern entfernt, ähnlich wie Sie es mit ifs tun können (obwohl ich diese Technik für nichts anderes als usings empfehlen würde ).
Dan Abramov
2
Im ursprünglichen Code befand sich memoryStream.ToArray () im Bereich der ersten Verwendung. Sie haben es außerhalb des Anwendungsbereichs.
Polyfun
Vielen Dank, ich habe gerade festgestellt, dass Sie eine returnAussage gemeint haben . So wahr. Ich habe die Antwort bearbeitet, um dies widerzuspiegeln.
Dan Abramov
Ich persönlich denke, dass das usingohne Klammern den Code zerbrechlicher macht (denken Sie an jahrelange Unterschiede und Zusammenführungen). joelonsoftware.com/2005/05/11/making-wrong-code-look-wrong & imperialviolet.org/2014/02/22/applebug.html
Tim Abell
0

Vermeiden Sie alle Verwendungen und verwenden Sie verschachtelte Entsorgungsaufrufe!

    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();
        }
    }
Harry Saltzman
quelle
1
Bitte erläutern Sie, warum Sie usingdies in diesem Fall vermeiden sollten .
StuperUser
1
Sie könnten die using-Anweisung in der Mitte behalten, aber Sie müssen die anderen auflösen. Um eine logisch kohärente und in alle Richtungen aktualisierbare Lösung zu erhalten, habe ich beschlossen, alle Verwendungen zu entfernen!
Harry Saltzman
0

Ich wollte nur den Code auspacken, damit wir mehrere Aufrufe für Disposedie Objekte sehen können:

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

Während die meisten .NET-Klassen (hoffentlich) widerstandsfähig gegen den Fehler mehrerer Aufrufe sind, .Disposesind nicht alle Klassen so defensiv gegen den Missbrauch durch Programmierer.

FX Cop weiß das und warnt Sie.

Sie haben einige Möglichkeiten;

  • Rufen Sie Disposeein Objekt nur einmal auf. nicht benutzenusing
  • Rufen Sie dispose zweimal an und hoffen Sie, dass der Code nicht abstürzt
  • Unterdrücke die Warnung
Ian Boyd
quelle
-1

Ich habe diese Art von Code verwendet, der Byte [] nimmt und Byte [] zurückgibt, ohne Streams zu verwenden

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);
}

Auf diese Weise müssen Sie lediglich mithilfe von Codierungen von Zeichenfolge zu Byte [] konvertieren.

Luka Rahne
quelle