Geekly articles weekly

Mencari bug di Amazon Web Services SDK untuk kode sumber .NET

Gambar 1


Salam semua penggemar mengkritik kode orang lain. :) Hari ini, di laboratorium kami, bahan penelitian baru adalah kode sumber proyek AWS SDK untuk .NET. Pada suatu waktu, kami menulis artikel tentang memeriksa AWS SDK untuk C ++. Kemudian tidak ada yang menarik. Mari kita lihat bagaimana versi .NET dari AWS SDK akan menyenangkan kita. Peluang yang baik untuk sekali lagi menunjukkan kemampuan penganalisa PVS-Studio, serta membuat dunia sedikit lebih sempurna.

Amazon Web Services (AWS) .NET SDK adalah toolkit pengembang yang dirancang untuk membuat aplikasi berbasis .NET di infrastruktur AWS dan sangat menyederhanakan proses penulisan kode. SDK termasuk suite .NET API untuk berbagai layanan AWS seperti Amazon S3, Amazon EC2, DynamoDB, dan lainnya. Kode sumber untuk SDK di-host di GitHub .

Seperti yang saya katakan, pada suatu waktu kami menulis artikel tentang memeriksa AWS SDK untuk C ++. Artikel itu ternyata kecil - hanya ada beberapa kesalahan pada 512 ribu baris kode. Kali ini kita berurusan dengan jumlah kode yang jauh lebih besar, termasuk sekitar 34 ribu file cs, dan jumlah total baris kode (tidak termasuk kosong) adalah 5 juta yang mengesankan. Sebagian kecil dari kode (200 ribu baris dalam 664 file cs) jatuh pada tes, saya tidak menganggapnya.

Jika kualitas kode .NET dari versi SDK kurang lebih sama dengan C ++ (dua kesalahan per 512 KLOC), maka kita akan mendapatkan kesalahan 10 kali lebih banyak. Ini, tentu saja, adalah metode perhitungan yang sangat tidak akurat yang tidak mempertimbangkan fitur bahasa akun dan banyak faktor lainnya, tetapi saya tidak berpikir bahwa pembaca sekarang ingin mempelajari diskusi yang membosankan. Sebagai gantinya, saya mengusulkan untuk langsung menuju hasil.

Verifikasi dilakukan menggunakan PVS-Studio versi 6.27. Luar biasanya, penganalisa mampu mendeteksi sekitar 40 kesalahan dalam AWS SDK untuk .NET code yang layak disebut. Ini menunjukkan tidak hanya kualitas tinggi dari kode SDK (sekitar 4 kesalahan per 512 KLOC), tetapi juga kualitas yang sebanding dari karya C # dari analisa PVS-Studio dibandingkan dengan C ++. Hasil yang bagus!

Penulis AWS SDK untuk .NET sangat bagus. Dari proyek ke proyek, mereka menunjukkan kualitas kode yang luar biasa. Ini bisa menjadi contoh yang baik untuk tim lain. Tapi, tentu saja, saya tidak akan menjadi pengembang penganalisa statis jika saya belum memasukkan 5 kopecks saya. :) Kami sudah bekerja dengan tim Amazon Lumberyard untuk menggunakan PVS-Studio. Tetapi karena ini adalah perusahaan yang sangat besar dengan banyak divisi di seluruh dunia, sangat mungkin bahwa AWS SDK untuk tim .NET tidak pernah mendengar tentang PVS-Studio sama sekali. Bagaimanapun, dalam kode SDK saya tidak menemukan tanda-tanda menggunakan penganalisa kami, meskipun ini tidak berarti apa-apa. Namun, tim, setidaknya, menggunakan alat analisa yang dibangun ke dalam Visual Studio. Ini bagus, tetapi pemeriksaan kode dapat ditingkatkan :).

Akibatnya, saya masih berhasil menemukan beberapa kesalahan dalam kode SDK, dan akhirnya, saatnya untuk membaginya.

Kesalahan dalam logika

PVS-Studio Warning: V3008 [CWE-563] Variabel 'this.linker.s3.region' diberi nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 116, 114. AWSSDK.DynamoDBv2.Net45 S3Link.cs 116

public string Region { get { .... } set { if (String.IsNullOrEmpty(value)) { this.linker.s3.region = "us-east-1"; } this.linker.s3.region = value; } }

Penganalisa memperingatkan penugasan kembali nilai variabel yang sama. Dari kode itu menjadi jelas bahwa ini disebabkan oleh kesalahan yang melanggar logika kerja: nilai variabel this.linker.s3.region akan selalu sama dengan nilai , terlepas dari kondisi if (String.IsNullOrEmpty (value)) . Di badan blok if , return dilewati. Kode perlu diperbaiki sebagai berikut:

 public string Region { get { .... } set { if (String.IsNullOrEmpty(value)) { this.linker.s3.region = "us-east-1"; return; } this.linker.s3.region = value; } }

Rekursi tak terbatas

PVS-Studio Warning: V3110 [CWE-674] Kemungkinan rekursi tak terbatas di dalam properti 'OnFailure'. AWSSDK.ElasticMapReduce.Net45 ResizeJobFlowStep.cs 171

 OnFailure? onFailure = null; public OnFailure? OnFailure { get { return this.OnFailure; } // <= set { this.onFailure = value; } }

Contoh kesalahan ketik klasik yang menyebabkan rekursi tak terbatas di dapatkan accessor dari properti OnFailure . Alih-alih mengembalikan nilai bidang pribadi, onFailure merujuk ke properti OnFailure . Opsi yang diperbaiki:

 public OnFailure? OnFailure { get { return this.onFailure; } set { this.onFailure = value; } }

Anda bertanya: "Bagaimana cara kerjanya?" Sejauh ini, tidak mungkin. Properti tidak digunakan di mana pun, tetapi bersifat sementara. Pada satu titik, seseorang akan mulai menggunakannya dan pasti akan mendapatkan hasil yang tidak terduga. Untuk mencegah kesalahan ketik seperti itu, disarankan untuk tidak menggunakan pengidentifikasi yang hanya berbeda dalam huruf pertama.

Catatan lain untuk desain ini adalah penggunaan pengenal yang sepenuhnya cocok dengan nama tipe OnFailure . Dari sudut pandang kompiler, ini cukup dapat diterima, tetapi itu membuat persepsi manusia tentang kode lebih sulit.

Kesalahan serupa lainnya:

Peringatan PVS-Studio: V3110 [CWE-674] Kemungkinan rekursi tak terbatas di dalam properti 'SSES3'. AWSSDK.S3.Net45 InventoryEncryption.cs 37

 private SSES3 sSES3; public SSES3 SSES3 { get { return this.SSES3; } set { this.SSES3 = value; } }

Situasinya identik dengan yang dijelaskan di atas. Hanya di sini, rekursi tak terbatas akan terjadi ketika mengakses properti SSES3 untuk membaca dan menulis. Opsi yang diperbaiki:

 public SSES3 SSES3 { get { return this.sSES3; } set { this.sSES3 = value; } }

Tes kesadaran

Berikut ini adalah tugas dari seorang programmer yang tertarik menggunakan teknik Copy-Paste. Lihat bagaimana kode terlihat di editor Visual Studio dan coba temukan kesalahannya.

Gambar 3


PVS-Studio Warning: V3029 Ekspresi kondisional dari pernyataan 'jika' yang terletak bersebelahan adalah identik. Periksa baris: 91, 95. AWSSDK.AppSync.Net45 BuatApiKeyResponseUnmarshaller.cs 91

Saya mengurangi tubuh metode UnmarshallException , menghapus semua yang tidak perlu. Sekarang Anda dapat melihat bahwa pemeriksaan identik mengikuti satu demi satu:

 public override AmazonServiceException UnmarshallException(....) { .... if (errorResponse.Code != null && errorResponse.Code.Equals("LimitExceededException")) { return new LimitExceededException(errorResponse.Message, innerException, errorResponse.Type, errorResponse.Code, errorResponse.RequestId, statusCode); } if (errorResponse.Code != null && errorResponse.Code.Equals("LimitExceededException")) { return new LimitExceededException(errorResponse.Message, innerException, errorResponse.Type, errorResponse.Code, errorResponse.RequestId, statusCode); } .... }

Tampaknya kesalahan itu tidak kotor - hanya pemeriksaan tambahan. Namun seringkali, pola seperti itu dapat mengindikasikan masalah yang lebih serius dalam kode ketika beberapa pemeriksaan yang diperlukan tidak dilakukan.

Ada beberapa kesalahan serupa dalam kode.

Peringatan PVS-Studio:

  • V3029 Ekspresi kondisional dari pernyataan 'jika' yang terletak berdampingan adalah identik. Periksa baris: 75, 79. AWSSDK.CloudDirectory.Net45 CreateSchemaResponseUnmarshaller.cs 75
  • V3029 Ekspresi kondisional dari pernyataan 'jika' yang terletak berdampingan adalah identik. Periksa baris: 105, 109. AWSSDK.CloudDirectory.Net45 GetSchemaAsJsonResponseUnmarshaller.cs 105
  • V3029 Ekspresi kondisional dari pernyataan 'jika' yang terletak berdampingan adalah identik. Periksa baris: 201, 205. AWSSDK.CodeCommit.Net45 PostCommentForPullRequestResponseUnmarshaller.cs 201
  • V3029 Ekspresi kondisional dari pernyataan 'jika' yang terletak berdampingan adalah identik. Periksa baris: 101, 105. AWSSDK.CognitoIdentityProvider.Net45 VerifikasiSoftwareTokenResponseUnmarshaller.cs 101
  • V3029 Ekspresi kondisional dari pernyataan 'jika' yang terletak berdampingan adalah identik. Periksa baris: 72, 76. Pembaruan AWSSDK.Glue.Net45ResponseUnmarshaller.cs 72
  • V3029 Ekspresi kondisional dari pernyataan 'jika' yang terletak berdampingan adalah identik. Periksa baris: 123, 127. AWSSDK.Neptune.Net45 PulihkanDBClusterDariSnapshotResponseUnmarshaller.cs 123
  • V3029 Ekspresi kondisional dari pernyataan 'jika' yang terletak berdampingan adalah identik. Periksa baris: 167, 171. AWSSDK.Neptune.Net45 PulihkanDBClusterDariSnapshotResponseUnmarshaller.cs 167
  • V3029 Ekspresi kondisional dari pernyataan 'jika' yang terletak berdampingan adalah identik. Periksa baris: 127, 131. AWSSDK.RDS.Net45 PulihkanDBClusterDariSnapshotResponseUnmarshaller.cs 127
  • V3029 Ekspresi kondisional dari pernyataan 'jika' yang terletak berdampingan adalah identik. Periksa baris: 171, 175. AWSSDK.RDS.Net45 PulihkanDBClusterDariSnapshotResponseUnmarshaller.cs 171
  • V3029 Ekspresi kondisional dari pernyataan 'jika' yang terletak berdampingan adalah identik. Periksa baris: 99, 103. AWSSDK.Rekognition.Net45 Kenali SelebrasiResponseUnmarshaller.cs 99

Apa kamu

PVS-Studio Warning: V3062 Objek 'attributeName' digunakan sebagai argumen untuk metode sendiri. Pertimbangkan untuk memeriksa argumen aktual pertama dari metode 'Berisi'. AWSSDK.MobileAnalytics.Net45 CustomEvent.cs 261

 /// <summary> /// Dictionary that stores attribute for this event only. /// </summary> private Dictionary<string,string> _attributes = new Dictionary<string,string>(); /// <summary> /// Gets the attribute. /// </summary> /// <param name="attributeName">Attribute name.</param> /// <returns>The attribute. Return null of attribute doesn't /// exist.</returns> public string GetAttribute(string attributeName) { if(string.IsNullOrEmpty(attributeName)) { throw new ArgumentNullException("attributeName"); } string ret = null; lock(_lock) { if(attributeName.Contains(attributeName)) // <= ret = _attributes[attributeName]; } return ret; }

Penganalisis mendeteksi kesalahan dalam metode GetAttribute : string diperiksa untuk fakta bahwa itu berisi dirinya sendiri. Dari uraian metode, berikut bahwa jika nama atribut (kunci atributName ) ditemukan (dalam kamus _attributes ), maka nilai atribut harus dikembalikan, jika tidak, null . Pada kenyataannya, karena kondisi attributeName.Contains (atributName) selalu benar, upaya dilakukan untuk mengembalikan nilai dengan kunci yang mungkin tidak ditemukan dalam kamus. Kemudian, alih-alih mengembalikan nol, KeyNotFoundException akan dibuang.

Mari kita coba perbaiki kode ini. Untuk lebih memahami bagaimana melakukan ini, lihat metode lain:

 /// <summary> /// Determines whether this instance has attribute the specified /// attributeName. /// </summary> /// <param name="attributeName">Attribute name.</param> /// <returns>Return true if the event has the attribute, else /// false.</returns> public bool HasAttribute(string attributeName) { if(string.IsNullOrEmpty(attributeName)) { throw new ArgumentNullException("attributeName"); } bool ret = false; lock(_lock) { ret = _attributes.ContainsKey(attributeName); } return ret; }

Metode ini memeriksa apakah nama atribut (kunci atributName ) ada di kamus _attributes . Mari kita kembali ke metode GetAttribute dan memperbaiki kesalahan:

 public string GetAttribute(string attributeName) { if(string.IsNullOrEmpty(attributeName)) { throw new ArgumentNullException("attributeName"); } string ret = null; lock(_lock) { if(_attributes.ContainsKey(attributeName)) ret = _attributes[attributeName]; } return ret; }

Sekarang metode melakukan apa yang dinyatakan dalam deskripsi.

Dan satu lagi komentar kecil untuk fragmen kode ini. Saya perhatikan bahwa penulis menggunakan kunci ketika bekerja dengan kamus _attributes . Jelas, ini diperlukan dengan akses multi-utas, tetapi konstruk kunci agak lambat dan rumit. Alih-alih Kamus dalam hal ini, mungkin lebih nyaman untuk menggunakan versi kamus yang aman - ConcurrentDictionary . Maka kebutuhan akan kunci hilang. Meskipun, mungkin saya tidak tahu tentang fitur dari proyek ini.

Perilaku mencurigakan

Peringatan PVS-Studio: V3063 [CWE-571] Bagian dari ekspresi kondisional selalu benar jika dievaluasi: string.IsNullOrEmpty (inferredIndexName). AWSSDK.DynamoDBv2.PCL ContextInternal.cs 802

 private static string GetQueryIndexName(....) { .... string inferredIndexName = null; if (string.IsNullOrEmpty(specifiedIndexName) && indexNames.Count == 1) { inferredIndexName = indexNames[0]; } else if (indexNames.Contains(specifiedIndexName, StringComparer.Ordinal)) { inferredIndexName = specifiedIndexName; } else if (string.IsNullOrEmpty(inferredIndexName) && // <= indexNames.Count > 0) throw new InvalidOperationException("Local Secondary Index range key conditions are used but no index could be inferred from model. Specified index name = " + specifiedIndexName); .... }

Penganalisa memberi tanda pada string.IsNullOrEmpty (inferredIndexName) memeriksa. Memang, string inferredIndexName ditugaskan nol , maka nilai variabel ini tidak berubah di mana pun, dan kemudian karena alasan tertentu diperiksa untuk null atau string kosong. Itu terlihat mencurigakan. Mari kita perhatikan potongan kode yang diberikan. Saya sengaja tidak menguranginya untuk lebih memahami situasi. Jadi, dalam pernyataan if pertama (dan juga yang berikutnya), variabel yang ditentukanIndexName diperiksa dengan cara tertentu. Bergantung pada hasil pemeriksaan, variabel inferredIndexName menerima nilai baru. Sekarang mari kita perhatikan pernyataan if ketiga. Badan pernyataan ini (melempar pengecualian) akan dieksekusi jika indexNames.Count> 0 , karena bagian pertama dari kondisinya adalah string.IsNullOrEmpty (inferredIndexName) selalu benar. Mungkin variabel yang ditentukanIndexName dan inferredIndexName hanya bingung. Atau cek ketiga harus tanpa yang lain , mewakili pernyataan yang berdiri sendiri jika :

 if (string.IsNullOrEmpty(specifiedIndexName) && indexNames.Count == 1) { inferredIndexName = indexNames[0]; } else if (indexNames.Contains(specifiedIndexName, StringComparer.Ordinal)) { inferredIndexName = specifiedIndexName; } if (string.IsNullOrEmpty(inferredIndexName) && indexNames.Count > 0) throw new InvalidOperationException(....);

Dalam hal ini, sulit untuk memberikan jawaban yang pasti tentang opsi untuk memperbaiki kode ini. Tetapi sangat penting bagi penulis untuk memeriksanya.

NullReferenceException

Peringatan PVS-Studio: V3095 [CWE-476] Objek 'conditionValues' digunakan sebelum diverifikasi dengan null. Periksa baris: 228, 238. AWSSDK.Core.Net45 JsonPolicyWriter.cs 228

 private static void writeConditions(....) { .... foreach (....) { IList<string> conditionValues = keyEntry.Value; if (conditionValues.Count == 0) // <= continue; .... if (conditionValues != null && conditionValues.Count != 0) { .... } .... } }

Klasik genre. Variabel conditionValues digunakan tanpa terlebih dahulu memeriksa nol . Selain itu, lebih lanjut dalam kode verifikasi seperti itu dilakukan, tetapi sudah terlambat. :)

Kode dapat diperbaiki sebagai berikut:

 private static void writeConditions(....) { .... foreach (....) { IList<string> conditionValues = keyEntry.Value; if (conditionValues != null && conditionValues.Count == 0) continue; .... if (conditionValues != null && conditionValues.Count != 0) { .... } .... } }

Beberapa kesalahan serupa ditemukan dalam kode.

Peringatan PVS-Studio:

  • V3095 [CWE-476] Objek 'ts.Listeners' digunakan sebelum diverifikasi terhadap nol. Periksa baris: 140, 143. AWSSDK.Core.Net45 Logger.Diagnostic.cs 140
  • V3095 [CWE-476] Objek 'obj' digunakan sebelum diverifikasi terhadap null. Periksa baris: 743, 745. AWSSDK.Core.Net45 JsonMapper.cs 743
  • V3095 [CWE-476] Objek 'multipartUploadMultipartUploadpartsList' digunakan sebelum diverifikasi terhadap nol. Periksa baris: 65, 67. AWSSDK.S3.Net45 LengkapMultipartUploadRequestMarshaller.cs 65

Peringatan berikut sangat mirip artinya, tetapi situasinya adalah kebalikan dari yang dibahas di atas.

PVS-Studio Warning: V3125 [CWE-476] Objek 'state' digunakan setelah diverifikasi terhadap null. Periksa baris: 139, 127. AWSSDK.Core.Net45 RefreshingAWSCredentials.cs 139

 private void UpdateToGeneratedCredentials( CredentialsRefreshState state) { string errorMessage; if (ShouldUpdate) { .... if (state == null) errorMessage = "Unable to generate temporary credentials"; else .... throw new AmazonClientException(errorMessage); } state.Expiration -= PreemptExpiryTime; // <= .... }

Salah satu fragmen kode berisi cek variabel keadaan untuk null . Lebih jauh di sepanjang kode, variabel keadaan digunakan untuk berhenti berlangganan dari acara PreemptExpiryTime , sementara pemeriksaan untuk null tidak lagi dilakukan, dan NullReferenceException dimungkinkan . Opsi kode yang lebih aman:

 private void UpdateToGeneratedCredentials( CredentialsRefreshState state) { string errorMessage; if (ShouldUpdate) { .... if (state == null) errorMessage = "Unable to generate temporary credentials"; else .... throw new AmazonClientException(errorMessage); } if (state != null) state.Expiration -= PreemptExpiryTime; .... }

Ada kesalahan serupa lainnya dalam kode.

Peringatan PVS-Studio:

  • V3125 [CWE-476] Objek 'wrapRequest.Content' digunakan setelah diverifikasi terhadap nol. Periksa baris: 395, 383. AWSSDK.Core.Net45 HttpHandler.cs 395
  • V3125 [CWE-476] Objek 'datasetUpdates' digunakan setelah diverifikasi terhadap nol. Periksa baris: 477, 437. AWSSDK.CognitoSync.Net45 Dataset.cs 477
  • V3125 [CWE-476] Objek 'cORSConfigurationCORSConfigurationcORSRulesListValue' digunakan setelah diverifikasi terhadap nol. Periksa baris: 125, 111. AWSSDK.S3.Net45 PutCORSKonfigurasi KonfigurasiRequestMarshaller.cs 125
  • V3125 [CWE-476] Objek 'lifecycleConfigurationLifecycleConfigurationrulesListValue' digunakan setelah diverifikasi terhadap nol. Periksa baris: 157, 68. AWSSDK.S3.Net45 PutLifecycleConfigurationRequestMarshaller.cs 157
  • V3125 [CWE-476] Objek 'this.Key' digunakan setelah diverifikasi terhadap null. Periksa baris: 199, 183. AWSSDK.S3.Net45 S3PostUploadRequest.cs 199

Realitas yang tidak terbantahkan

PVS-Studio Warning: V3009 [CWE-393] Aneh bahwa metode ini selalu mengembalikan satu dan nilai yang sama dari 'true'. AWSSDK.Core.Net45 Lexer.cs 651

 private static bool State19 (....) { while (....) { switch (....) { case '"': .... return true; case '\\': .... return true; default: .... continue; } } return true; }

Metode ini selalu mengembalikan true . Mari kita lihat betapa pentingnya ini untuk kode panggilan. Saya melacak penggunaan metode State19 . Dia berpartisipasi dalam mengisi array dari penangan fsm_handler_table bersama dengan metode serupa lainnya (masing-masing ada 28 dari mereka dengan nama, dari State1 ke State28 ). Penting untuk dicatat di sini bahwa selain State19 , peringatan V3009 [CWE-393] juga dikeluarkan untuk beberapa penangan lainnya. Ini adalah penangannya: State23, State26, State27, State28 . Lansiran yang dikeluarkan oleh analis untuk mereka:

  • V3009 [CWE-393] Aneh bahwa metode ini selalu mengembalikan satu dan nilai 'benar' yang sama. AWSSDK.Core.Net45 Lexer.cs 752
  • V3009 [CWE-393] Aneh bahwa metode ini selalu mengembalikan satu dan nilai 'benar' yang sama. AWSSDK.Core.Net45 Lexer.cs 810
  • V3009 [CWE-393] Aneh bahwa metode ini selalu mengembalikan satu dan nilai 'benar' yang sama. AWSSDK.Core.Net45 Lexer.cs 822
  • V3009 [CWE-393] Aneh bahwa metode ini selalu mengembalikan satu dan nilai 'benar' yang sama. AWSSDK.Core.Net45 Lexer.cs 834

Berikut ini deklarasi dan inisialisasi array handler:

 private static StateHandler[] fsm_handler_table; .... private static void PopulateFsmTables () { fsm_handler_table = new StateHandler[28] { State1, State2, .... State19, .... State23, .... State26, State27, State28 };

Untuk kelengkapan, mari kita lihat kode salah satu penangan, yang tidak ada keluhan penganalisa, misalnya, State2 :

 private static bool State2 (....) { .... if (....) { return true; } switch (....) { .... default: return false; } }

Dan ini adalah bagaimana penangan dipanggil:

 public bool NextToken () { .... while (true) { handler = fsm_handler_table[state - 1]; if (! handler (fsm_context)) // <= throw new JsonException (input_char); .... } .... }

Seperti yang Anda lihat, pengecualian akan dilemparkan jika false dikembalikan. Dalam kasus kami, untuk penangan State19, State23, State26, State27 dan State28 ini tidak akan pernah terjadi. Itu terlihat mencurigakan. Di sisi lain, sebanyak lima penangan memiliki perilaku yang sama (selalu kembali benar ), jadi mungkin ini dimaksudkan dan bukan hasil kesalahan ketik.

Mengapa saya memikirkan semua ini dengan sangat detail? Situasi ini sangat indikatif dalam arti bahwa analisa statis sering hanya mampu menunjukkan desain yang mencurigakan. Dan bahkan seseorang (bukan mesin) yang tidak memiliki pengetahuan yang cukup tentang proyek, setelah menghabiskan waktu mempelajari kode, masih tidak dapat memberikan jawaban yang lengkap tentang adanya kesalahan. Kode harus dipelajari oleh pengembang.

Cek tidak berarti

Peringatan PVS-Studio: V3022 [CWE-571] Ekspresi 'doLog' selalu benar. AWSSDK.Core.Net45 StoredProfileAWSCredentials.cs 235

 private static bool ValidCredentialsExistInSharedFile(....) { .... var doLog = false; try { if (....) { return true; } else { doLog = true; } } catch (InvalidDataException) { doLog = true; } if (doLog) // <= { .... } .... }

Perhatikan variabel doLog . Setelah inisialisasi dengan false , maka dalam kode variabel ini akan menjadi benar dalam semua kasus. Jadi pemeriksaan if (doLog) selalu benar. Mungkin, sebelumnya dalam metode ini ada cabang di mana variabel doLog tidak diberi nilai apa pun, maka pada saat verifikasi itu bisa berisi nilai palsu yang diperoleh selama inisialisasi. Tetapi sekarang tidak ada cabang seperti itu.

Kesalahan serupa lainnya:

Peringatan PVS-Studio: Ekspresi V3022 '! Hasil' selalu salah. AWSSDK.CognitoSync.PCL SQLiteLocalStorage.cs 353

 public void PutValue(....) { .... bool result = PutValueHelper(....); if (!result) <= { _logger.DebugFormat("{0}", @"Cognito Sync - SQLiteStorage - Put Value Failed"); } else { UpdateLastModifiedTimestamp(....); } .... }

Penganalisa mengklaim bahwa nilai variabel hasil selalu benar . Ini hanya mungkin jika metode PutValueHelper akan selalu mengembalikan true . Lihatlah metode ini:

 private bool PutValueHelper(....) { .... if (....)) { return true; } if (record == null) { .... return true; } else { .... return true; } }

Memang, metode ini akan mengembalikan true dalam kondisi apa pun. Selain itu, penganalisa mengeluarkan peringatan untuk metode ini. PVS-Studio Warning: V3009 [CWE-393] Aneh bahwa metode ini selalu mengembalikan satu dan nilai yang sama dari 'true'. SQLiteLocalStorage.cs 1016

Saya sengaja tidak mengutip peringatan ini sebelumnya ketika saya mempertimbangkan kesalahan lain dari V3009 , tetapi saya menyimpannya untuk kasus ini. Dengan demikian, penganalisis itu benar untuk menunjukkan kesalahan V3022 dalam kode panggilan.

Salin-tempel. Lagi

Peringatan PVS-Studio: V3001 Ada sub-ekspresi yang identik 'this.token == JsonToken.String' di sebelah kiri dan di sebelah kanan '||' operator. AWSSDK.Core.Net45 JsonReader.cs 343

 public bool Read() { .... if ( (this.token == JsonToken.ObjectEnd || this.token == JsonToken.ArrayEnd || this.token == JsonToken.String || // <= this.token == JsonToken.Boolean || this.token == JsonToken.Double || this.token == JsonToken.Int || this.token == JsonToken.UInt || this.token == JsonToken.Long || this.token == JsonToken.ULong || this.token == JsonToken.Null || this.token == JsonToken.String // <= )) { .... } .... }

Bandingkan dua kali bidang this.token dengan nilai JsonToken.String dari enumerasi JsonToken . Mungkin salah satu perbandingan harus berisi nilai enumerasi yang berbeda. Jika demikian, maka kesalahan serius telah dibuat.

Refactoring + kecerobohan?

Peringatan PVS-Studio: V3025 [CWE-685] Format salah. Jumlah item format yang berbeda diharapkan saat memanggil fungsi 'Format'. Argumen tidak digunakan: AWSConfigs.AWSRegionKey. AWSSDK.Core.Net45 AWSRegion.cs 116

 public InstanceProfileAWSRegion() { .... if (region == null) { throw new InvalidOperationException( string.Format(CultureInfo.InvariantCulture, "EC2 instance metadata was not available or did not contain region information.", AWSConfigs.AWSRegionKey)); } .... }

Mungkin, string format untuk metode string.Format sebelumnya berisi specifier output {0} , yang argumennya AWSConfigs.AWSRegionKey ditetapkan. Kemudian baris diubah, specifier hilang, tetapi mereka lupa menghapus argumen. Fragmen kode di atas bekerja tanpa kesalahan (pengecualian akan dilemparkan dalam kasus sebaliknya - specifier tanpa argumen), tetapi terlihat jelek. Kode harus diperbaiki sebagai berikut:

 if (region == null) { throw new InvalidOperationException( "EC2 instance metadata was not available or did not contain region information."); }

Tidak aman

Peringatan PVS-Studio: V3083 [CWE-367] Doa yang tidak aman dari acara 'mOnSyncSuccess', NullReferenceException dimungkinkan. Pertimbangkan menugaskan acara ke variabel lokal sebelum menjalankannya. AWSSDK.CognitoSync.PCL Dataset.cs 827

 protected void FireSyncSuccessEvent(List<Record> records) { if (mOnSyncSuccess != null) { mOnSyncSuccess(this, new SyncSuccessEventArgs(records)); } }

Situasi yang cukup umum dari panggilan tidak aman ke pengendali acara. Antara memeriksa variabel mOnSyncSuccess untuk null dan memanggil handler, suatu peristiwa dapat berhenti berlangganan, dan nilainya akan menjadi nol. Kemungkinan skenario seperti itu kecil, tetapi masih lebih baik untuk membuat kode lebih aman:

 protected void FireSyncSuccessEvent(List<Record> records) { mOnSyncSuccess?.Invoke(this, new SyncSuccessEventArgs(records)); }

Ada kesalahan serupa lainnya dalam kode.

Peringatan PVS-Studio:

  • V3083 [CWE-367] Doa yang tidak aman dari acara 'mOnSyncFailure', NullReferenceException dimungkinkan. Pertimbangkan menugaskan acara ke variabel lokal sebelum menjalankannya. AWSSDK.CognitoSync.PCL Dataset.cs 839
  • V3083 [CWE-367] Doa acara tidak aman, NullReferenceException dimungkinkan. Pertimbangkan menugaskan acara ke variabel lokal sebelum menjalankannya. AWSSDK.Core.PCL AmazonServiceClient.cs 332
  • V3083 [CWE-367] Doa acara tidak aman, NullReferenceException dimungkinkan. Pertimbangkan menugaskan acara ke variabel lokal sebelum menjalankannya. AWSSDK.Core.PCL AmazonServiceClient.cs 344
  • V3083 [CWE-367] Doa acara tidak aman, NullReferenceException dimungkinkan. Pertimbangkan menugaskan acara ke variabel lokal sebelum menjalankannya. AWSSDK.Core.PCL AmazonServiceClient.cs 357
  • V3083 [CWE-367] Doa yang tidak aman dari acara 'mExceptionEvent', NullReferenceException dimungkinkan. Pertimbangkan menugaskan acara ke variabel lokal sebelum menjalankannya. AWSSDK.Core.PCL AmazonServiceClient.cs 366
  • V3083 [CWE-367] Doa acara tidak aman, NullReferenceException dimungkinkan. Pertimbangkan menugaskan acara ke variabel lokal sebelum menjalankannya. AWSSDK.Core.PCL AmazonWebServiceRequest.cs 78
  • V3083 [CWE-367] Doa yang tidak aman dari acara 'OnRead', NullReferenceException dimungkinkan. Pertimbangkan menugaskan acara ke variabel lokal sebelum menjalankannya. AWSSDK.Core.PCL EventStream.cs 97
  • V3083 [CWE-367] Doa acara tidak aman, NullReferenceException dimungkinkan. Pertimbangkan menugaskan acara ke variabel lokal sebelum menjalankannya. AWSSDK.Core.Android NetworkReachability.cs 57
  • V3083 [CWE-367] Doa acara tidak aman, NullReferenceException dimungkinkan. Pertimbangkan menugaskan acara ke variabel lokal sebelum menjalankannya. AWSSDK.Core.Android NetworkReachability.cs 94
  • V3083 [CWE-367] Doa acara tidak aman, NullReferenceException dimungkinkan. Pertimbangkan menugaskan acara ke variabel lokal sebelum menjalankannya. AWSSDK.Core.iOS NetworkReachability.cs 54

Kelas yang belum selesai

Peringatan PVS-Studio: V3126 Ketik 'JsonData' yang mengimplementasikan antarmuka <qu> IEquatable tidak menggantikan metode 'GetHashCode'. AWSSDK.Core.Net45 JsonData.cs 26

 public class JsonData : IJsonWrapper, IEquatable<JsonData> { .... }

Kelas JsonData berisi banyak kode, jadi saya tidak memberikannya secara keseluruhan, membatasi diri saya untuk hanya mendeklarasikannya. Kelas ini benar-benar tidak mengandung metode GetHashCode yang ditimpa, yang tidak aman karena dapat menyebabkan perilaku yang salah ketika menggunakan tipe JsonData untuk bekerja, misalnya, dengan koleksi. Mungkin tidak ada masalah saat ini, tetapi strategi untuk menggunakan tipe ini dapat berubah di masa depan. Kesalahan ini dijelaskan lebih rinci dalam dokumentasi .

Kesimpulan

Itu semua kesalahan menarik yang berhasil saya deteksi di AWS SDK untuk .NET code menggunakan analisa statis PVS-Studio. Sekali lagi, saya menekankan kualitas proyek. Saya menemukan sejumlah kecil kesalahan untuk 5 juta baris kode. Meskipun, mungkin, analisis yang lebih menyeluruh dari peringatan yang dikeluarkan akan memungkinkan saya untuk menambahkan beberapa kesalahan lagi ke daftar ini. Tetapi juga sangat mungkin bahwa saya sia-sia mengaitkan beberapa peringatan yang dijelaskan sebagai kesalahan. Dalam hal ini, kesimpulan pasti selalu dibuat hanya oleh pengembang, yang berada dalam konteks kode yang sedang diperiksa.



Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Sergey Khrenov. Mencari kesalahan dalam kode sumber SDK Layanan Web Amazon untuk .NET

Source: https://habr.com/ru/post/id437516/

More articles:


All Articles