Geekly articles weekly

Mencari kesalahan dalam kode sumber SDK Layanan Web Amazon untuk .NET

Gambar 1


Selamat datang di semua penggemar yang merusak kode orang lain. :) Hari ini di laboratorium kami, kami memiliki bahan baru untuk penelitian - kode sumber AWS SDK untuk proyek .NET. Pada saat itu, kami menulis artikel tentang memeriksa AWS SDK untuk C ++. Maka tidak ada sesuatu yang sangat menarik. Mari kita lihat apa yang layak. NET dari versi AWS SDK. Sekali lagi, ini adalah kesempatan besar untuk menunjukkan kemampuan penganalisa PVS-Studio dan membuat dunia sedikit lebih baik.

Amazon Web Services (AWS) SDK untuk .NET adalah seperangkat alat pengembang, dimaksudkan untuk membuat aplikasi berdasarkan .NET dalam infrastruktur AWS. Set ini memungkinkan untuk menyederhanakan proses penulisan kode secara signifikan. SDK mencakup set API .NET untuk berbagai layanan AWS, seperti Amazon S3, Amazon EC2, DynamoDB, dan lainnya. Kode sumber SDK tersedia di GitHub .

Seperti yang saya sebutkan, pada saat itu kami telah menulis artikel tentang memeriksa AWS SDK untuk C ++. Artikel itu ternyata kecil - hanya beberapa kesalahan yang ditemukan per 512 ribu baris kode. Kali ini kita berurusan dengan ukuran kode yang jauh lebih besar, yang mencakup sekitar 34 ribu file cs, dan jumlah total baris kode (tidak termasuk yang kosong) sangat mengesankan 5 juta. Sebagian kecil kode (200 ribu baris dalam file 664-cs) dikumpulkan untuk pengujian, saya belum mempertimbangkannya.

Jika kualitas kode .NET dari versi SDK kurang lebih sama dengan salah satu dari C ++ (dua kesalahan per 512 KLOC), maka kita harus mendapatkan jumlah kesalahan sekitar 10 kali lebih besar. Tentu saja, ini adalah metodologi perhitungan yang sangat tidak akurat, yang tidak memperhitungkan kekhasan linguistik dan banyak faktor lainnya, tetapi saya tidak berpikir pembaca sekarang ingin masuk ke penalaran yang membosankan. Sebagai gantinya, saya sarankan beralih ke hasil.

Pemeriksaan dilakukan menggunakan PVS-Studio 6.27. Ini hanya luar biasa, tetapi masih faktanya adalah bahwa dalam AWS SDK untuk .NET, penganalisa berhasil mendeteksi 40 kesalahan, yang layak untuk dibicarakan. Ini menunjukkan tidak hanya kualitas tinggi dari kode SDK (sekitar 4 kesalahan per 512 KLOC), tetapi juga kualitas yang sebanding dari penganalisis C # PVS-Studio dibandingkan dengan C ++. Hasil yang bagus!

Penulis AWS SDK untuk .NET, Anda benar-benar juara! Dengan setiap proyek, Anda menunjukkan kualitas kode yang luar biasa. Ini bisa menjadi contoh yang bagus untuk tim lain. Namun, tentu saja, saya tidak akan menjadi pengembang analisa statis, jika saya tidak memberikan 2 sen saya. :) Kami sudah bekerja dengan tim Lumberyard dari Amazon tentang penggunaan PVS-Studio. Karena ini adalah perusahaan yang sangat besar dengan banyak unit di seluruh dunia, sangat mungkin bahwa tim AWS SDK untuk .NET belum pernah mendengar tentang PVS-Studio. Lagi pula, saya belum menemukan tanda-tanda menggunakan penganalisis kami dalam kode SDK, meskipun tidak mengatakan apa-apa. Namun, setidaknya, tim menggunakan alat analisa yang dibangun ke dalam Visual Studio. Sangat bagus, tetapi ulasan kode selalu dapat ditingkatkan :).

Akibatnya, saya berhasil menemukan beberapa bug 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; } }

Penganalisis memperingatkan tentang penetapan nilai berulang untuk variabel yang sama. Dari kode menjadi jelas bahwa ini disebabkan oleh kesalahan yang melanggar logika program kerja: nilai variabel this.linker.s3.region akan selalu sama dengan nilai nilai variabel, terlepas dari kondisi if (String.IsNullOrEmpty (value)) . pernyataan kembali terlewatkan di badan if block. 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

Peringatan PVS-Studio: 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 klasik kesalahan ketik, yang mengarah pada rekursi tak terbatas di dapatkan aksesor properti OnFailure . Alih-alih mengembalikan nilai bidang pribadi pada Kegagalan, akses ke properti OnFailure terjadi. Varian yang benar:

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

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

Komentar lain untuk konstruksi ini adalah menggunakan pengidentifikasi, yang sepenuhnya cocok dengan nama tipe OnFailure . Dari sudut pandang kompiler, itu cukup dapat diterima, tetapi ini mempersulit persepsi kode untuk seseorang.

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. Namun, di sini rekursi tak terbatas akan terjadi ketika mengakses properti SSES3 baik untuk membaca dan menugaskan. Varian yang benar:

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

Tes dengan pertimbangan

Sekarang saya ingin mengutip tugas dari pengembang, diambil dengan menggunakan metode Copy-Paste. Lihatlah cara kode terlihat di editor Visual Studio, dan cobalah untuk menemukan kesalahan.

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 , setelah menghapus semua yang tidak diperlukan. Sekarang Anda dapat melihat bahwa pemeriksaan identik saling mengikuti:

 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 bug itu tidak kasar - hanya pemeriksaan tambahan. Namun demikian, sering kali pola seperti itu mengindikasikan masalah yang lebih serius dalam kode, ketika pemeriksaan yang diperlukan tidak akan dilakukan.

Dalam kode tersebut, ada beberapa kesalahan serupa.

Peringatan PVS-Studio:

  • V3029 Ekspresi kondisional dari pernyataan 'jika' yang terletak berdampingan adalah identik. Periksa baris: 75, 79. AWSSDK.CloudDirectory.Net45 Buat SkemaResponseUnmarshaller.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 itu

Peringatan PVS-Studio: V3062 Objek 'atributName' 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 telah mendeteksi kesalahan dalam metode GetAttribute : string diperiksa apakah mengandung itu sendiri. Dari uraian metode, dapat diketahui bahwa jika nama atribut (kunci atributName ) ditemukan (dalam kamus _attributes ), nilai atribut harus dikembalikan, jika tidak - null . Faktanya, karena condition attributeName.Contains (atributName) selalu benar, upaya dilakukan untuk mengembalikan nilai dengan kunci yang mungkin tidak ditemukan dalam kamus. Kemudian, alih-alih mengembalikan nol, pengecualian KeyNotFoundException akan dibuang.

Mari kita coba perbaiki kode ini. Untuk memahami lebih baik bagaimana melakukan ini, Anda harus melihat 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 lagi 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.

Satu lagi komentar kecil untuk fragmen kode ini. Saya perhatikan bahwa penulis menggunakan kunci ketika bekerja dengan kamus _attributes . Jelas bahwa ini diperlukan ketika memiliki akses multithreaded, tetapi konstruksi kunci agak lambat dan rumit. Alih-alih Kamus , dalam hal ini, mungkin, akan lebih mudah untuk menggunakan kamus versi thread-safe - ConcurrentDictionary . Dengan cara ini, tidak perlu terkunci. Meskipun, mungkin saya tidak tahu tentang spesifik proyek.

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 khawatir tentang string cek. IsNullOrEmpty (disimpulkanIndexName) . Memang, string inferredIndexName ditugaskan nol , maka nilai variabel ini tidak berubah di mana pun, maka karena alasan tertentu diperiksa untuk null atau string kosong. Terlihat mencurigakan. Mari kita lihat lebih dekat fragmen kode di atas. Saya sengaja tidak menguranginya untuk memahami situasi dengan lebih baik. Jadi, dalam pernyataan if pertama (dan juga yang berikutnya), variabel yang ditentukanIndexName entah bagaimana diperiksa. Bergantung pada hasil pemeriksaan, variabel inferredIndexName mendapatkan nilai baru. Sekarang mari kita lihat pernyataan if ketiga. Badan pernyataan ini (melempar pengecualian) akan dilakukan jika indexNames.Count> 0, sebagai bagian pertama dari seluruh kondisi, yaitu string. IsNullOrEmpty (inferredIndexName) selalu benar. Mungkin, variabel yang ditentukanIndexName dan disimpulkanIndexName digabungkan atau cek ketiga harus tanpa yang lain , mewakili pernyataan mandiri 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. Bagaimanapun, penulis perlu memeriksanya.

NullReferenceException

Peringatan PVS-Studio: V3095 [CWE-476] Objek 'conditionValues' digunakan sebelum diverifikasi terhadap nol. 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) { .... } .... } }

Ini klasik. Variabel conditionValues digunakan tanpa pemeriksaan awal untuk null . Sementara nanti dalam kode pemeriksaan ini dilakukan. Kode perlu 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) { .... } .... } }

Saya menemukan beberapa kesalahan serupa 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 kasusnya berlawanan dengan yang dibahas di atas.

Peringatan PVS-Studio: 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 termasuk memeriksa nilai variabel keadaan untuk null . Dalam kode di bawah ini, variabel digunakan untuk berhenti berlangganan dari acara PreemptExpiryTime , namun, pemeriksaan untuk null tidak lagi dilakukan dan melempar pengecualian NullReferenceException menjadi mungkin. Versi 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; .... }

Dalam kode, ada kesalahan serupa lainnya:

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 PutCORSConfigurationRequestMarshaller.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 non-alternatif

Peringatan PVS-Studio: V3009 [CWE-393] Aneh bahwa metode ini selalu mengembalikan satu dan nilai yang sama dari 'benar'. 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 kode panggilan. Saya memeriksa kasus-kasus menggunakan metode State19 . Ini terlibat dalam mengisi array handler fsm_handler_table sama dengan metode serupa lainnya (masing-masing ada 28 di antaranya dengan nama, mulai dari State1 ke State28 ). Di sini penting untuk dicatat bahwa, selain State19 , untuk beberapa penangan lainnya peringatan V3009 [CWE-393] juga dikeluarkan. Ini adalah penangan: State23, State26, State27, State28 . Peringatan, yang dikeluarkan oleh penganalisa 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 adalah cara deklarasi dan inisialisasi array dari penangan terlihat seperti:

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

Untuk melengkapi gambar, mari kita lihat kode salah satu penangan yang belum dianalisis oleh penganalisa , misalnya, State2 :

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

Inilah cara terjadinya panggilan penangan:

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

Seperti yang bisa kita lihat, pengecualian akan dilemparkan untuk mengembalikan kesalahan . Dalam kasus kami, untuk penangan State19, State23, State26 State27 dan State28 ini tidak akan pernah terjadi. Terlihat mencurigakan. Di sisi lain, lima penangan memiliki perilaku yang sama (akan selalu kembali benar ), jadi mungkin itu sangat dibuat-buat dan bukan hasil kesalahan ketik.

Mengapa saya begitu terlibat dalam semua ini? Situasi ini sangat signifikan dalam arti bahwa analisa statis sering hanya dapat mengindikasikan konstruksi yang mencurigakan. Dan bahkan seseorang (bukan mesin), yang tidak memiliki pengetahuan yang cukup tentang proyek tersebut, masih belum dapat memberikan jawaban penuh tentang adanya kesalahan, bahkan setelah menghabiskan waktu mempelajari kode. Pengembang harus meninjau kode ini.

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 nilai palsu , variabel ini akan mendapatkan nilai sebenarnya dalam semua kasus lebih lanjut di sepanjang kode. Oleh karena itu, periksa apakah (doLog) selalu benar. Mungkin, sebelumnya dalam metode ada cabang, di mana variabel doLog tidak diberi nilai apa pun. Pada saat pengecekan dapat berisi nilai palsu , yang diterima saat 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. Mungkin hanya dalam kasus 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 semua kondisi. Selain itu, penganalisa telah mengeluarkan peringatan untuk metode ini. Peringatan PVS-Studio: V3009 [CWE-393] Aneh bahwa metode ini selalu mengembalikan satu dan nilai yang sama dari 'benar'. SQLiteLocalStorage.cs 1016

Saya sengaja tidak mengutip peringatan ini sebelumnya ketika saya menanyakan bug V3009 lainnya dan menyimpannya untuk kasus ini. Dengan demikian, alat itu benar untuk menunjukkan kesalahan V3022 dalam kode panggilan.

Salin-tempel. Lagi

Peringatan PVS-Studio: V3001 Ada sub-ekspresi 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 // <= )) { .... } .... }

Bidang this.token dibandingkan dua kali dengan nilai JsonToken.String dari enumerasi JsonToken . Mungkin, salah satu perbandingan harus berisi nilai enumerasi lain. Jika demikian, kesalahan serius telah dibuat di sini.

Refactoring + kekurangan perhatian?

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, format string untuk metode string.Format sebelumnya berisi item format {0}, yang mana argumen AWSConfigs.AWSRegionKey ditetapkan. Kemudian string diubah, item format hilang, tetapi pengembang lupa untuk menghapus argumen. Contoh kode yang diberikan bekerja tanpa kesalahan (pengecualian dilemparkan dalam kasus yang berlawanan - item format tanpa argumen), tetapi terlihat tidak bagus. 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 umum dari panggilan yang tidak aman dari pengendali acara. Seorang pengguna dapat berhenti berlangganan antara pemeriksaan variabel mOnSyncSuccess untuk null dan pemanggilan seorang penangan, sehingga 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)); }

Dalam kode, ada kesalahan serupa lainnya:

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 kasar

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 cukup banyak kode, jadi saya tidak memberikannya secara keseluruhan, hanya mengutip deklarasi. Kelas ini benar-benar tidak mengandung metode yang diganti GetHashCode, 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 di masa depan jenis strategi ini mungkin berubah. Kesalahan ini dijelaskan dalam dokumentasi lebih terinci.

Kesimpulan

Ini semua adalah bug menarik yang saya dapat mendeteksi dalam kode AWS SDK untuk .NET menggunakan penganalisa statis PVS-Studio. Saya ingin menyoroti sekali lagi kualitas proyek. Saya menemukan sejumlah kecil kesalahan untuk 5 juta baris kode. Meskipun mungkin analisis yang lebih menyeluruh dari peringatan yang dikeluarkan akan membiarkan saya menambahkan beberapa kesalahan lagi ke daftar ini. Namun demikian, sangat mungkin bahwa saya menambahkan beberapa peringatan untuk kesalahan tanpa biaya. Kesimpulan yang tidak ambigu dalam hal ini selalu dibuat hanya oleh pengembang yang berada dalam konteks kode yang diperiksa.

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

More articles:


All Articles