Azure SDK untuk .NET: Kisah Pencari Bug yang Sulit

Ketika kami memutuskan untuk mencari kesalahan dalam Azure SDK untuk proyek .NET, kami sangat terkejut dengan ukurannya. "Tiga setengah juta baris kode," kata kami, mempelajari statistik proyek. Ini adalah berapa banyak yang dapat Anda temukan di sana. Tapi sayang sekali, ah. Proyek itu ternyata rahasia. Apa kekhasan proyek dan bagaimana pengujiannya - baca di artikel ini.

Tentang proyek

Saya menulis artikel ini sebagai tindak lanjut dari artikel saya sebelumnya, yang juga tentang proyek yang berkaitan dengan Microsoft Azure: Azure PowerShell: "pada dasarnya tidak berbahaya . " Jadi, kali ini saya mengandalkan sejumlah kesalahan yang beragam dan menarik. Memang, untuk analisis statis, ukuran proyek biasanya penting, terutama dengan pemeriksaan satu kali seluruh proyek. Ya, dalam praktiknya mereka biasanya tidak melakukan itu. Dan jika mereka melakukannya, maka hanya pada tahap implementasi alat analisa. Pada saat yang sama, tidak ada yang segera memahami sejumlah besar operasi (sejumlah besar peringatan adalah norma ketika penganalisa dimulai dalam mode ini), tetapi hanya menempatkan mereka ke dalam utang teknis menggunakan mekanisme penindasan pesan dan penyimpanannya dalam database khusus (penindasan massal). Kami terlibat dalam inspeksi satu kali untuk tujuan penelitian. Oleh karena itu, proyek besar untuk studi selalu lebih disukai daripada yang kecil.

Namun, Azure SDK untuk proyek .NET segera menunjukkan kegagalannya sebagai bangku tes. Bahkan ukurannya yang mengesankan tidak membantu, tetapi malah mempersulit pekerjaan. Alasannya ditunjukkan dalam statistik proyek berikut:

• File Sumber .cs (tidak termasuk tes): 16.500
• Solusi Visual Studio (.sln): 163
• Baris kode yang tidak kosong: 3.462.000
• Dari yang dihasilkan secara otomatis: sekitar 3,3 juta
• Repositori proyek tersedia di GitHub .

Sekitar 95% kode dihasilkan secara otomatis, bagian penting dari kode ini diulang berkali-kali. Memeriksa proyek-proyek semacam itu dengan analisa statis biasanya memakan waktu dan tidak berguna, karena ada banyak pekerjaan, tetapi tidak logis (sekilas pandang) dan kode berlebihan. Ini mengarah ke sejumlah besar positif palsu.

Sejumlah besar solusi Visual Studio (163) bertindak sebagai ceri pada kue, yang menurutnya kumpulan kode ini “tersebar. Jadi untuk memeriksa kode yang tersisa (bukan yang dihasilkan secara otomatis) saya harus melakukan beberapa upaya. Ini membantu bahwa semua kode yang dihasilkan secara otomatis terletak di subfolder solusi di sepanjang jalur relatif "<folder Solusi> \ src \ Generated". Selain itu, setiap file .cs dari kode tersebut berisi komentar khusus dalam tag <auto-generated> :

// <auto-generated> // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. See License.txt in the project root for // license information. // // Code generated by Microsoft (R) AutoRest Code Generator. // Changes may cause incorrect behavior and will be lost if the code is // regenerated. // </auto-generated> 

Untuk kemurnian percobaan, saya secara acak memeriksa sekitar sepuluh solusi yang dipilih secara acak dengan kode yang dibuat secara otomatis. Hasilnya akan lebih rendah.

Jadi, meskipun ada sedikit kode “jujur” yang tersisa, saya masih berhasil menemukan sejumlah kesalahan di sana. Kali ini saya tidak akan memberikan perjalanan dalam urutan jumlah diagnosa PVS-Studio. Sebagai gantinya, saya akan mengelompokkan respons berdasarkan solusi yang terdeteksi.

Mari kita lihat apa yang saya temukan di Azure SDK untuk kode .NET.

Microsoft.Azure.Management.Advisor

Ini hanyalah salah satu dari banyak solusi yang berisi kode yang dibuat secara otomatis. Seperti yang saya katakan di atas, sekitar selusin solusi semacam itu diuji secara selektif. Dan di mana-mana pesannya sama dan, seperti yang diharapkan, tidak berguna. Izinkan saya memberi Anda beberapa contoh tanggapan semacam itu.

Ekspresi V3022 'Kredensial! = Null' selalu benar. AdvisorManagementClient.cs 204

 // Code generated by Microsoft (R) AutoRest Code Generator. .... public ServiceClientCredentials Credentials { get; private set; } .... public AdvisorManagementClient(ServiceClientCredentials credentials, params DelegatingHandler[] handlers) : this(handlers) { if (credentials == null) { throw new System.ArgumentNullException("credentials"); } Credentials = credentials; if (Credentials != null) // <= { Credentials.InitializeServiceClient(this); } } 

Jelas, kodenya berlebihan, dan memeriksa Kredensial! = Null tidak berguna. Tetapi kodenya bekerja. Dan dibuat otomatis. Oleh karena itu - tidak ada keluhan.

Ekspresi V3022 '_queryParameters.Count> 0' selalu salah. ConfigurationsOperations.cs 871

 // Code generated by Microsoft (R) AutoRest Code Generator. .... public async Task<AzureOperationResponse<IPage<ConfigData>>> ListBySubscriptionNextWithHttpMessagesAsync(....) { .... List<string> _queryParameters = new List<string>(); if (_queryParameters.Count > 0) { .... } .... } 

Dan lagi-lagi, desain logika yang tampaknya tanpa. Untuk beberapa alasan, mereka memeriksa ukuran daftar kosong yang baru saja dibuat. Bahkan - semuanya beres. Pemeriksaan ini tidak ada gunanya sekarang, tetapi jika generator mengatur pembuatan daftar, misalnya, berdasarkan koleksi lain, maka pemeriksaan sudah masuk akal. Jadi - lagi, tidak ada keluhan terhadap kode, mengingat asalnya, tentu saja.

Untuk setiap solusi dengan kode yang dibuat secara otomatis, ratusan peringatan serupa diterima. Mengingat kesia-siaan mereka, saya pikir tidak ada gunanya diskusi lebih lanjut tentang hal-hal positif semacam itu. Hanya kesalahan nyata dalam kode "normal" yang akan dipertimbangkan di bawah ini.

Azure. Skor

V3001 Ada sub-ekspresi identik 'buffer.Length' ke kiri dan ke kanan operator '<'. AzureBaseBuffersExtensions.cs 30

 public static async Task WriteAsync(...., ReadOnlyMemory<byte> buffer, ....) { byte[]? array = null; .... if (array == null || buffer.Length < buffer.Length) // <= { if (array != null) ArrayPool<byte>.Shared.Return(array); array = ArrayPool<byte>.Shared.Rent(buffer.Length); } if (!buffer.TryCopyTo(array)) throw new Exception("could not rent large enough buffer."); .... } 

Kesalahan dalam kondisi itu dibuat, mungkin sebagai akibat dari copy-paste. Dilihat oleh fakta bahwa selanjutnya dalam buffer kode disalin ke array , cek akan terlihat seperti:

 if (array == null || array.Length < buffer.Length) 

Tetapi, seperti yang selalu saya katakan, pembuat kode harus memperbaiki kesalahan tersebut.

V3083 Doa yang tidak aman dari acara '_onChange', NullReferenceException dimungkinkan. Pertimbangkan menugaskan acara ke variabel lokal sebelum menjalankannya. ClientOptionsMonitor.cs 44

 private event Action<TOptions, string> _onChange; .... private void InvokeChanged(....) { .... if (_onChange != null) { _onChange.Invoke(options, name); } } 

Tidak kritis, tapi kesalahan. Antara memeriksa acara untuk kesetaraan nol dan permintaannya, acara tersebut dapat berhenti berlangganan. Kemudian variabel _onChange akan mendapatkan nol dan pengecualian akan dibuang. Kode harus ditulis ulang dengan lebih aman. Misalnya, seperti ini:

 private void InvokeChanged(....) { .... _onChange?.Invoke(options, name); } 

Azure.Messaging.EventHubs

V3080 Kemungkinan null dereference. Pertimbangkan untuk memeriksa 'eventPropertyValue'. AmqpMessageConverter.cs 650

 private static bool TryCreateEventPropertyForAmqpProperty( object amqpPropertyValue, out object eventPropertyValue) { eventPropertyValue = null; .... switch (GetTypeIdentifier(amqpPropertyValue)) { case AmqpProperty.Type.Byte: .... case AmqpProperty.Type.String: eventPropertyValue = amqpPropertyValue; return true; .... } .... switch (amqpPropertyValue) { case AmqpSymbol symbol: eventPropertyValue = ....; break; case byte[] array: eventPropertyValue = ....; break; case ArraySegment<byte> segment when segment.Count == segment.Array.Length: eventPropertyValue = ....; break; case ArraySegment<byte> segment: .... eventPropertyValue = ....; break; case DescribedType described when (described.Descriptor is AmqpSymbol): eventPropertyValue = ....; break; default: var exception = new SerializationException( string.Format(...., eventPropertyValue.GetType().FullName)); // <= .... } return (eventPropertyValue != null); } 

Mari kita lacak apa yang terjadi pada nilai variabel eventPropertyValue dalam cuplikan kode di atas. Pada awal metode, variabelnya nol . Selanjutnya, dalam salah satu kondisi sakelar pertama, variabel diinisialisasi, setelah itu metode keluar. Blok kedua saklar berisi banyak kondisi, di mana masing-masing variabel juga menerima beberapa nilai baru. Tetapi dalam blok default , variabel eventPropertyValue hanya digunakan tanpa verifikasi, yang merupakan kesalahan, karena saat ini variabelnya nol .

V3066 Kemungkinan urutan argumen yang salah diteruskan ke konstruktor 'EventHubConsumer': 'partisiId' dan 'consumerGroup'. TrackOneEventHubClient.cs 394

 public override EventHubConsumer CreateConsumer(....) { return new EventHubConsumer ( new TrackOneEventHubConsumer(....), TrackOneClient.EventHubName, partitionId, // <= 3 consumerGroup, // <= 4 eventPosition, consumerOptions, initialRetryPolicy ); } 

Penganalisa menduga bahwa ketika memanggil konstruktor dari kelas EventHubConsumer , urutan argumen ketiga dan keempat tercampur aduk. Mari kita lihat deklarasi konstruktor:

 internal EventHubConsumer(TransportEventHubConsumer transportConsumer, string eventHubName, string consumerGroup, // <= 3 string partitionId, // <= 4 EventPosition eventPosition, EventHubConsumerOptions consumerOptions, EventHubRetryPolicy retryPolicy) { .... } 

Argumennya benar-benar campur aduk. Saya kira bagaimana kesalahan ini dibuat. Kesalahannya mungkin karena format kode yang buruk. Lihat lagi deklarasi konstruktor EventHubConsumer . Karena fakta bahwa parameter transportConsumer pertama terletak pada baris yang sama dengan nama kelas, sambil melihat kode secara singkat, mungkin tampak bahwa parameter partisiId di tempat ketiga dan tidak di tempat keempat (tidak ada komentar saya dengan nomor seri parameter dalam kode asli) )

Ini hanya dugaan, tetapi saya akan mengubah format kode deklarasi konstruktor menjadi ini:

 internal EventHubConsumer ( TransportEventHubConsumer transportConsumer, string eventHubName, string consumerGroup, string partitionId, EventPosition eventPosition, EventHubConsumerOptions consumerOptions, EventHubRetryPolicy retryPolicy) { .... } 

Azure. Penyimpanan

V3112 Kelainan dalam perbandingan serupa. Mungkin salah ketik ada di dalam ekspresi 'ContentLanguage == other.ContentEncoding'. BlobSasBuilder.cs 410

 public struct BlobSasBuilder : IEquatable<BlobSasBuilder> { .... public bool Equals(BlobSasBuilder other) => BlobName == other.BlobName && CacheControl == other.CacheControl && BlobContainerName == other.BlobContainerName && ContentDisposition == other.ContentDisposition && ContentEncoding == other.ContentEncoding && // <= ContentLanguage == other.ContentEncoding && // <= ContentType == other.ContentType && ExpiryTime == other.ExpiryTime && Identifier == other.Identifier && IPRange == other.IPRange && Permissions == other.Permissions && Protocol == other.Protocol && StartTime == other.StartTime && Version == other.Version; } 

Sebuah kesalahan yang dilakukan karena kurangnya perhatian. Menemukan kesalahan serupa dengan ulasan kode cukup sulit. Opsi centang yang benar:

  .... ContentEncoding == other.ContentEncoding && ContentLanguage == other.ContentLanguage && .... 

V3112 Kelainan dalam perbandingan serupa. Mungkin salah ketik ada di dalam ekspresi 'ContentLanguage == other.ContentEncoding'. FileSasBuilder.cs 265

 public struct FileSasBuilder : IEquatable<FileSasBuilder> { .... public bool Equals(FileSasBuilder other) => CacheControl == other.CacheControl && ContentDisposition == other.ContentDisposition && ContentEncoding == other.ContentEncoding // <= && ContentLanguage == other.ContentEncoding // <= && ContentType == other.ContentType && ExpiryTime == other.ExpiryTime && FilePath == other.FilePath && Identifier == other.Identifier && IPRange == other.IPRange && Permissions == other.Permissions && Protocol == other.Protocol && ShareName == other.ShareName && StartTime == other.StartTime && Version == other.Version ; 

Kesalahan yang persis sama dalam bagian kode yang sangat mirip. Kode mungkin disalin dan dimodifikasi sebagian. Namun kesalahan tetap ada.

Microsoft.Azure.Batch

V3053 Ekspresi yang berlebihan. Periksa substring 'IList' dan 'List'. PropertyData.cs 157

V3053 Ekspresi yang berlebihan. Periksa substring 'Daftar' dan 'IReadOnlyList'. PropertyData.cs 158

 public class PropertyData { .... public bool IsTypeCollection => this.Type.Contains("IList") || this.Type.Contains("IEnumerable") || this.Type.Contains("List") || // <= this.Type.Contains("IReadOnlyList"); // <= } 

Penganalisa mengeluarkan dua peringatan tentang cek tidak masuk akal atau salah. Dalam kasus pertama, pencarian untuk substring "Daftar" setelah mencari "IList" terlihat berlebihan. Memang kondisinya:

 this.Type.Contains("IList") || this.Type.Contains("List") 

dapat diganti dengan ini:

 this.Type.Contains("List") 

Dalam kasus kedua, pencarian untuk substring "IReadOnlyList" tidak ada artinya, karena sebelumnya pencarian untuk substring "List" yang lebih pendek dilakukan.

Ada juga kemungkinan bahwa substring sendiri untuk pencarian membuat kesalahan dan harus ada sesuatu yang lain. Dalam kasus apa pun, versi yang benar dari koreksi kondisi, dengan mempertimbangkan kedua pernyataan, hanya dapat ditawarkan oleh pembuat kode.

V3095 Objek 'httpRequest.Content.Headers' digunakan sebelum diverifikasi terhadap nol. Periksa baris: 76, 79. BatchSharedKeyCredential.cs 76

 public override Task ProcessHttpRequestAsync( HttpRequestMessage httpRequest, ....) { .... signature.Append(httpRequest.Content != null && httpRequest.Content.Headers.Contains("Content-Language") ? .... : ....; long? contentLength = httpRequest.Content?.Headers?.ContentLength; .... } 

Pada awalnya, variabel httpRequest.Content.Headers digunakan tanpa pemeriksaan, tetapi kemudian dalam kode variabel ini diakses menggunakan operator akses bersyarat.

V3125 Objek 'omPropertyData' digunakan setelah diverifikasi terhadap null. Periksa baris: 156, 148. CodeGenerationUtilities.cs 156

 private static string GetProtocolCollectionToObjectModelCollectionString( ...., PropertyData omPropertyData, ....) { if (IsMappedEnumPair(omPropertyData?.GenericTypeParameter, ....)) { .... } if (IsTypeComplex(omPropertyData.GenericTypeParameter)) .... } 

Situasi sebaliknya. Satu blok kode berisi opsi akses aman ke tautan omPropertyData yang berpotensi nol. Lebih lanjut dalam kode dengan tautan yang sama mereka bekerja tanpa pemeriksaan.

V3146 Kemungkinan nol dereferensi 'nilai'. 'FirstOrDefault' dapat mengembalikan nilai nol default. BatchSharedKeyCredential.cs 127

 public override Task ProcessHttpRequestAsync(HttpRequestMessage httpRequest, ....) { .... foreach (string canonicalHeader in customHeaders) { string value = httpRequest.Headers. GetValues(canonicalHeader).FirstOrDefault(); value = value.Replace('\n', ' ').Replace('\r', ' ').TrimStart(); .... } .... } 

Sebagai hasil dari metode FirstOrDefault , jika pencarian gagal, nilai default untuk tipe string akan dikembalikan, yaitu nol . Nilai akan ditetapkan ke variabel nilai , yang digunakan kemudian dalam kode dengan metode Ganti tanpa pemeriksaan apa pun. Kode harus dibuat lebih aman. Misalnya, seperti ini:

 foreach (string canonicalHeader in customHeaders) { string value = httpRequest.Headers. GetValues(canonicalHeader).FirstOrDefault(); value = value?.Replace('\n', ' ').Replace('\r', ' ').TrimStart(); .... } 

Microsoft.Azure.ServiceBus

V3121 Enumerasi 'BlocksUsing' dideklarasikan dengan atribut 'Flags', tetapi tidak mengatur inisialisasi apa pun untuk mengesampingkan nilai default. Fx.cs 69

 static class Fx { .... public static class Tag { .... [Flags] public enum BlocksUsing { MonitorEnter, MonitorWait, ManualResetEvent, AutoResetEvent, AsyncResult, IAsyncResult, PInvoke, InputQueue, ThreadNeutralSemaphore, PrivatePrimitive, OtherInternalPrimitive, OtherFrameworkPrimitive, OtherInterop, Other, NonBlocking, } .... } .... } 

Pencacahan dideklarasikan dengan atribut Flags . Dalam hal ini, nilai-nilai konstan dibiarkan secara default ( MonitorEnter = 0 , MonitorWait = 1 , ManualResetEvent = 2, dan seterusnya). Ini dapat mengarah pada fakta bahwa ketika Anda mencoba menggunakan kombinasi flag, misalnya, konstanta kedua dan ketiga MonitorWait (= 1) | ManualResetEvent (= 2) , bukan nilai unik yang akan diterima, tetapi sebuah konstanta dengan nilai 3 secara default ( AutoResetEvent ). Ini mungkin mengejutkan bagi kode panggilan. Jika enumerasi BlocksUsing benar - benar direncanakan untuk digunakan untuk menentukan kombinasi flag (bidang bit), maka Anda harus memberikan nilai konstanta yang sama dengan kekuatan dua:

 [Flags] public enum BlocksUsing { MonitorEnter = 1, MonitorWait = 2, ManualResetEvent = 4, AutoResetEvent = 8, AsyncResult = 16, IAsyncResult = 32, PInvoke = 64, InputQueue = 128, ThreadNeutralSemaphore = 256, PrivatePrimitive = 512, OtherInternalPrimitive = 1024, OtherFrameworkPrimitive = 2048, OtherInterop = 4096, Other = 8192, NonBlocking = 16384, } 

V3125 Objek 'sesi' digunakan setelah diverifikasi terhadap nol. Periksa baris: 69, 68. AmqpLinkCreator.cs 69

 public async Task<Tuple<AmqpObject, DateTime>> CreateAndOpenAmqpLinkAsync() { .... AmqpSession session = null; try { // Create Session .... } catch (Exception exception) { .... session?.Abort(); throw AmqpExceptionHelper.GetClientException(exception, null, session.GetInnerException(), amqpConnection.IsClosing()); } .... } 

Perhatikan pekerjaan dengan variabel sesi di blok tangkap . Metode Abort dipanggil dengan aman melalui pernyataan akses bersyarat. Tapi kemudian mereka membuat panggilan tidak aman ke metode GetInnerException . Dalam hal ini, alih-alih melemparkan pengecualian dari tipe yang diharapkan, NullReferenceException mungkin dilemparkan. Kode perlu diperbaiki. Metode AmqpExceptionHelper.GetClientException mendukung melewati nilai nol untuk parameter innerException :

 public static Exception GetClientException( Exception exception, string referenceId = null, Exception innerException = null, bool connectionError = false) { .... } 

Oleh karena itu, cukup menggunakan operator akses bersyarat saat memanggil sesi. DapatkanInnerException () :

 public async Task<Tuple<AmqpObject, DateTime>> CreateAndOpenAmqpLinkAsync() { .... AmqpSession session = null; try { // Create Session .... } catch (Exception exception) { .... session?.Abort(); throw AmqpExceptionHelper.GetClientException(exception, null, session?.GetInnerException(), amqpConnection.IsClosing()); } .... } 

Kesimpulan

Seperti yang Anda lihat, ukuran proyek yang besar tidak selalu menjamin sejumlah besar kesalahan. Tetapi tidak perlu rileks - Anda selalu dapat menemukan sesuatu. Bahkan dalam proyek yang kompleks seperti Azure SDK untuk .NET. Ya, ini membutuhkan upaya tambahan, tetapi hasilnya akan lebih menyenangkan. Dan agar Anda tidak perlu melakukan upaya berlebihan, kami sarankan menggunakan analisis statis, dan di tempat kerja pengembang saat menulis kode baru. Ini adalah pendekatan yang paling efektif. Unduh dan coba PVS-Studio dalam aksi. Semoga berhasil dalam perang melawan serangga!

Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Sergey Khrenov. Azure SDK untuk .NET: Kisah tentang Pencarian Kesalahan yang Sulit .