CA2202, jak rozwiązać ten przypadek

102

Czy ktoś może mi powiedzieć, jak usunąć wszystkie ostrzeżenia CA2202 z następującego kodu?

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

Ostrzeżenie 7 CA2202: Microsoft.Usage: Obiekt „cryptoStream” można usunąć więcej niż raz w metodzie „CryptoServices.Encrypt (string, byte [], byte [])”. Aby uniknąć generowania wyjątku System.ObjectDisposedException, nie należy wywoływać metody Dispose na obiekcie więcej niż jeden raz .: Wiersze: 34

Ostrzeżenie 8 CA2202: Microsoft.Usage: Obiekt „memoryStream” można usunąć więcej niż raz w metodzie „CryptoServices.Encrypt (string, byte [], byte [])”. Aby uniknąć generowania wyjątku System.ObjectDisposedException, nie należy wywoływać metody Dispose na obiekcie więcej niż jeden raz .: Lines: 34, 37

Potrzebujesz analizy kodu programu Visual Studio, aby wyświetlić te ostrzeżenia (nie są to ostrzeżenia kompilatora C #).

testalino
źródło
1
Ten kod nie generuje tych ostrzeżeń.
Julien Hoarau,
1
Otrzymuję za to 0 ostrzeżeń (poziom ostrzeżenia 4, VS2010). A jeśli ktoś szuka w Google problemów w tym obszarze, prosimy o dodanie również tekstu ostrzeżeń.
Henk Holterman
29
Ostrzeżenia CAxxxx są generowane przez Code Analysis i FxCop.
dtb
To ostrzeżenie nie dotyczy pokazanego kodu - ostrzeżenia można pominąć dokładnie w tym scenariuszu. Po przejrzeniu kodu i zgodzeniu się z tą oceną, umieść to nad metodą: „ [SuppressMessage("Microsoft.Usage", "CA2202:Do not dispose objects multiple times", Justification="BrainSlugs83 said so.")]” - upewnij się, że using System.Diagnostics.CodeAnalysis;w bloku użycia znajduje się wyrażenie „ ”.
BrainSlugs83
2
Spójrz na: msdn.microsoft.com/en-us/library/…
Adil Mammadov

Odpowiedzi:

-3

To kompiluje się bez ostrzeżenia:

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

Edytuj w odpowiedzi na komentarze: Właśnie ponownie zweryfikowałem, że ten kod nie generuje ostrzeżenia, podczas gdy oryginalny. W oryginalnym kodzie CryptoStream.Dispose()i MemoryStream().Dispose() są w rzeczywistości wywoływane dwukrotnie (co może, ale nie musi, stanowić problem).

Zmodyfikowany kod działa w następujący sposób: referencje są ustawiane na null, gdy tylko odpowiedzialność za usunięcie zostanie przeniesiona na inny obiekt. Eg memoryStreamjest ustawiony na nullpo pomyślnym wywołaniu CryptoStreamkonstruktora. cryptoStreamjest ustawiona na null, po pomyślnym wywołaniu StreamWriterkonstruktora. Jeśli nie wystąpi żaden wyjątek, streamWriterzostanie usunięty w finallybloku i z kolei usunie CryptoStreami MemoryStream.

Henrik
źródło
85
-1 Naprawdę źle jest tworzyć brzydki kod tylko po to, aby zastosować się do ostrzeżenia, które powinno zostać pominięte .
Jordão
4
Zgadzam się, że nie powinieneś marnować kodu na coś, co może zostać naprawione w pewnym momencie w przyszłości, po prostu stłum.
peteski
3
Jak to rozwiązuje problem? Raport CA2202 jest nadal zgłaszany, ponieważ memoryStream nadal można usunąć dwukrotnie w ostatnim bloku.
Chris Gessler
3
Ponieważ CryptoStream wewnętrznie wywołuje Dispose na MemoryStream, można go wywołać dwukrotnie, co jest przyczyną ostrzeżenia. Wypróbowałem twoje rozwiązanie i nadal otrzymuję ostrzeżenie.
Chris Gessler
2
O rany, masz rację - nie spodziewałem się, że logika oczyszczania będzie mieszana z twoją ... logiką-logiką ... - to po prostu dziwaczne i tajemnicze - z pewnością sprytne - ale znowu, straszne - nie rób tego w kodzie produkcyjnym; żeby było jasne: dostajesz, że nie ma rzeczywistego problemu funkcjonalnego, który można rozwiązać, prawda? (Dobrze jest pozbywać się tych obiektów wiele razy.) - Usunąłbym głos negatywny, gdybym mógł (SO uniemożliwia mi, mówi, że musisz edytować odpowiedź) - ale zrobiłbym to tylko niechętnie ... - i poważnie, nigdy tego nie rób.
BrainSlugs83
142

W takim przypadku należy wyłączyć ostrzeżenia. Kod, który dotyczy artykułów jednorazowego użytku, powinien być spójny i nie powinieneś martwić się, że inne klasy przejmują własność stworzonych przez Ciebie artykułów jednorazowego użytku, a także Disposeje wywołują .

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

AKTUALIZACJA: W dokumentacji IDisposable.Dispose można przeczytać to:

Jeśli metoda Dispose obiektu jest wywoływana więcej niż raz, obiekt musi zignorować wszystkie wywołania po pierwszym. Obiekt nie może zgłosić wyjątku, jeśli jego metoda Dispose jest wywoływana wiele razy.

Można argumentować, że ta reguła istnieje, aby programiści mogli usingrozsądnie zastosować instrukcję w kaskadzie urządzeń jednorazowego użytku, jak pokazałem powyżej (a może jest to po prostu fajny efekt uboczny). Z tego samego powodu CA2202 nie służy żadnemu pożytecznemu celowi i powinno być stłumione z punktu widzenia projektu. Prawdziwym winowajcą byłaby wadliwa implementacja Dispose, a CA1065 powinien się tym zająć (jeśli za to odpowiadasz).

Jordão
źródło
14
Moim zdaniem jest to błąd w fxcop, ta zasada jest po prostu błędna. Metoda dispose nigdy nie powinna zgłaszać wyjątku ObjectDisposedException, a jeśli tak, to powinieneś sobie z tym poradzić w tym czasie, zgłaszając błąd autorowi kodu, który implementuje metodę dispose.
justin.m.chase
14
Zgadzam się z @HansPassant w innym wątku: narzędzie wykonuje swoją pracę i ostrzega o nieoczekiwanych szczegółach implementacji klas. Osobiście uważam, że prawdziwym problemem jest projekt samych interfejsów API. Posiadanie klas zagnieżdżonych domyślnie zakładających, że przejęcie na własność innego obiektu utworzonego w innym miejscu jest w porządku, wydaje się wysoce wątpliwe. Widzę, gdzie mogłoby to być przydatne, gdyby wynikowy obiekt miał zostać zwrócony, ale domyślne ustawienie tego założenia wydaje się sprzeczne z intuicją, a także narusza normalne wzorce IDisposable.
BTJ
8
Ale msdn nie zaleca blokowania tego typu wiadomości. Zajrzyj na: msdn.microsoft.com/en-us/library/…
Adil Mammadov
2
Dzięki za link @AdilMammadov, przydatne informacje, ale Microsoft nie zawsze ma rację w tych sprawach.
Tim Abell
40

Cóż, to prawda, metoda Dispose () na tych strumieniach będzie wywoływana więcej niż raz. Klasa StreamReader przejmie „własność” cryptoStream, więc usunięcie streamWriter spowoduje również usunięcie cryptoStream. Podobnie klasa CryptoStream przejmuje odpowiedzialność za memoryStream.

Nie są to do końca prawdziwe błędy, te klasy .NET są odporne na wiele wywołań metody Dispose (). Ale jeśli chcesz pozbyć się ostrzeżenia, powinieneś porzucić instrukcję using dla tych obiektów. I zrób sobie trochę bólu, myśląc, co się stanie, jeśli kod wyrzuci wyjątek. Lub zamknij ostrzeżenie za pomocą atrybutu. Lub po prostu zignoruj ​​ostrzeżenie, ponieważ jest głupie.

Hans Passant
źródło
10
Konieczność posiadania specjalnej wiedzy na temat wewnętrznego zachowania klas (na przykład jednorazowego przejęcia na własność innej) to zbyt wiele, aby zapytać, czy chce się zaprojektować interfejs API wielokrotnego użytku. Myślę więc, że usingoświadczenia powinny pozostać. Te ostrzeżenia są naprawdę głupie.
Jordão,
4
@ Jordão - czy nie do tego służy narzędzie? Aby ostrzec Cię o wewnętrznym zachowaniu, o którym mógłbyś nie wiedzieć?
Hans Passant
8
Zgadzam się. Ale nadal nie zrezygnowałbym z usingoświadczeń. Po prostu czuję się źle, polegając na innym obiekcie, aby pozbyć się obiektu, który stworzyłem. W przypadku tego kodu jest to w porządku, ale istnieje wiele implementacji Streami TextWritertam (nie tylko w BCL). Kod użycia ich wszystkich powinien być spójny.
Jordão,
3
Tak, zgadzam się z Jordão. Jeśli naprawdę chcesz, aby programista był świadomy wewnętrznego zachowania interfejsu API, wypowiedz się, nazywając swoją funkcję DoSomethingAndDisposeStream (strumień Stream, dane OtherData).
ZZZ,
4
@HansPassant Czy możesz wskazać, gdzie jest udokumentowane, że XmlDocument.Save()metoda będzie wywoływać podany Disposeparametr? Nie widzę tego w dokumentacji Save(XmlWriter)(gdzie doświadczam błędu FxCop), w Save()samej metodzie ani w samej dokumentacji XmlDocument.
Ian Boyd
9

Kiedy StreamWriter zostanie usunięty, automatycznie usunie opakowany Stream (tutaj: CryptoStream ). CryptoStream również automatycznie usuwa opakowany strumień (tutaj: MemoryStream ).

Więc twój MemoryStream jest usuwany zarówno przez CryptoStream, jak i instrukcję using . Twój CryptoStream jest usuwany przez StreamWriter i zewnętrzną instrukcję using .


Po pewnych eksperymentach wydaje się, że całkowite pozbycie się ostrzeżeń wydaje się niemożliwe. Teoretycznie MemoryStream musi zostać usunięty, ale teoretycznie nie można już uzyskać dostępu do jego metody ToArray. Praktycznie MemoryStream nie trzeba pozbywać się, więc wybrałbym to rozwiązanie i pominął ostrzeżenie CA2000.

var memoryStream = new MemoryStream();

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

return memoryStream.ToArray();
dtb
źródło
9

Zrobiłbym to za pomocą #pragma warning disable.

Wytyczne .NET Framework zalecają zaimplementowanie IDisposable.Dispose w taki sposób, aby można było go wielokrotnie wywoływać. Z opisu IDisposable.Dispose w witrynie MSDN :

Obiekt nie może zgłosić wyjątku, jeśli jego metoda Dispose jest wywoływana wiele razy

Dlatego ostrzeżenie wydaje się prawie bez znaczenia:

Aby uniknąć generowania wyjątku System.ObjectDisposedException, nie należy wywoływać metody Dispose na obiekcie więcej niż jeden raz

Myślę, że można by argumentować, że ostrzeżenie może być pomocne, jeśli używasz źle zaimplementowanego obiektu IDisposable, który nie jest zgodny ze standardowymi wytycznymi dotyczącymi implementacji. Ale kiedy używasz klas z .NET Framework, tak jak robisz, powiedziałbym, że bezpiecznie jest zignorować ostrzeżenie za pomocą #pragma. I IMHO jest to lepsze od przechodzenia przez obręcze, jak sugeruje dokumentacja MSDN dla tego ostrzeżenia .

Joe
źródło
4
CA2202 to ostrzeżenie analizy kodu, a nie ostrzeżenie kompilatora. #pragma warning disablemoże służyć tylko do pomijania ostrzeżeń kompilatora. Aby pominąć ostrzeżenie analizy kodu, musisz użyć atrybutu.
Martin Liversage
2

Miałem podobne problemy w moim kodzie.

Wygląda na to, że wyzwalany jest cały CA2202, ponieważ MemoryStreammożna go usunąć, jeśli wystąpi wyjątek w konstruktorze (CA2000).

Można to rozwiązać w następujący sposób:

 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 }

Zauważ, że musimy zwrócić memoryStreamwnętrze ostatniej usinginstrukcji (linia 10), ponieważ cryptoStreamzostaje usunięta w linii 11 (ponieważ jest używana w streamWriter usinginstrukcji), co prowadzi memoryStreamdo memoryStreamusunięcia również w linii 11 (ponieważ jest używana do tworzenia cryptoStream).

Przynajmniej ten kod działał dla mnie.

EDYTOWAĆ:

Choć może to zabrzmieć zabawnie, odkryłem, że jeśli zastąpisz GetMemoryStreammetodę następującym kodem,

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

uzyskasz ten sam wynik.

Jimi
źródło
1

Cryptostream jest oparty na memorystream.

Wydaje się, że dzieje się tak, że kiedy strumień kryptograficzny jest usuwany (pod koniec używania), strumień pamięci jest również usuwany, a następnie strumień pamięci jest usuwany ponownie.

Shiraz Bhaiji
źródło
1

Chciałem rozwiązać ten problem we właściwy sposób - to znaczy bez tłumienia ostrzeżeń i właściwego usuwania wszystkich przedmiotów jednorazowego użytku.

Wyciągnąłem 2 z 3 strumieni jako pola i umieściłem je w Dispose()metodzie mojej klasy. Tak, implementacja IDisposableinterfejsu może niekoniecznie być tym, czego szukasz, ale rozwiązanie wygląda całkiem czysto w porównaniu do dispose()wywołań ze wszystkich losowych miejsc w kodzie.

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
źródło
0

Nie na temat, ale sugerowałbym użycie innej techniki formatowania do grupowania 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();
}

Zalecam również użycie vartutaj s, aby uniknąć powtórzeń naprawdę długich nazw klas.

PS Dziękuję @ShellShock za wskazanie, że nie mogę pominąć nawiasów klamrowych na początku, usingponieważ spowodowałoby to memoryStreamw returninstrukcji poza zakresem.

Dan Abramov
źródło
5
Czy memoryStream.ToArray () nie będzie poza zakresem?
Polyfun
Jest to absolutnie równoważne z oryginalnym fragmentem kodu. Po prostu ominąłem szelki kręcone, podobnie jak możesz to zrobić z ifs (chociaż nie radziłbym tej techniki dla niczego innego niż usings).
Dan Abramov
2
W oryginalnym kodzie memoryStream.ToArray () znajdował się w zakresie pierwszego użycia; masz to poza zakresem.
Polyfun
Dziękuję bardzo, właśnie sobie sprawę, że miałeś na myśli returnoświadczenie. Tak prawdziwe. Zredagowałem odpowiedź, aby to odzwierciedlić.
Dan Abramov
Osobiście uważam, że brak usingnawiasów klamrowych sprawia, że ​​kod jest bardziej kruchy (pomyśl o latach różnic i połączeń). joelonsoftware.com/2005/05/11/making-wrong-code-look-wrong & imperialviolet.org/2014/02/22/applebug.html
Tim Abell
0

Unikaj wszelkich zastosowań i używaj zagnieżdżonych wywołań Dispose!

    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
źródło
1
Proszę wyjaśnić, dlaczego usingw takim przypadku należy unikać .
StuperUser
1
Możesz zachować instrukcję using w środku, ale musisz rozwiązać pozostałe. Aby uzyskać logiczne, spójne i umożliwiające aktualizację we wszystkich kierunkach rozwiązanie, zdecydowałem się usunąć wszystkie zastosowania!
Harry Saltzman
0

Chciałem tylko rozpakować kod, abyśmy mogli zobaczyć wiele wywołań Disposeobiektów:

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

Chociaż większość klas .NET jest (miejmy nadzieję) odporna na błąd wielokrotnych wywołań .Dispose, nie wszystkie klasy są tak samo odporne na nadużycia programisty.

FX Cop wie o tym i ostrzega.

Masz kilka możliwości;

  • zadzwoń tylko Disposeraz na dowolny obiekt; nie używajusing
  • wołaj dispose dwa razy i miej nadzieję, że kod nie ulegnie awarii
  • pominąć ostrzeżenie
Ian Boyd
źródło
-1

Użyłem tego rodzaju kodu, który pobiera bajt [] i zwraca bajt [] bez użycia strumieni

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

W ten sposób wszystko, co musisz zrobić, to przekonwertować ciąg znaków na bajt [] za pomocą kodowania.

Luka Rahne
źródło