Geekly articles weekly

البحث عن الأخطاء في التعليمات البرمجية المصدر Amazon Web Services SDK لـ .NET

الصورة 1


مرحبًا بك في كل محبي تحطيم رمز شخص آخر. :) اليوم في مختبرنا ، لدينا مادة جديدة للبحث - الكود المصدري لمشروع AWS SDK for .NET. في ذلك الوقت ، كتبنا مقالة حول التحقق من AWS SDK لـ C ++. ثم لم يكن هناك شيء مثير للاهتمام بشكل خاص. دعونا نرى ما يستحق. NET من إصدار AWS SDK. مرة أخرى ، إنها فرصة رائعة لإظهار قدرات محلل PVS-Studio وجعل العالم أفضل قليلاً.

مجموعة خدمات أمازون Web Services (AWS) SDK for .NET هي مجموعة من أدوات المطور ، والتي تهدف إلى إنشاء تطبيقات قائمة على .NET في بنية AWS الأساسية. تمكن هذه المجموعة من تبسيط عملية كتابة التعليمات البرمجية بشكل كبير. يتضمن SDK مجموعات API .NET لمختلف خدمات AWS ، مثل Amazon S3 و Amazon EC2 و DynamoDB وغيرها. كود مصدر SDK متاح في جيثب .

كما ذكرت ، في الوقت الذي كتبنا فيه بالفعل مقالة حول التحقق من AWS SDK لـ C ++. تحولت المقالة إلى أنها صغيرة - تم العثور على أخطاء فقط لكل 512 ألف سطر من التعليمات البرمجية. هذه المرة نحن نتعامل مع حجم أكبر بكثير من الكود ، والذي يتضمن حوالي 34 ألف من ملفات cs ، ويبلغ إجمالي عدد سطور الكود (باستثناء الفراغات الفارغة) 5 ملايين. جزء صغير من الشفرة (200 ألف سطر في ملفات 664-cs) يعود إلى الاختبارات ، لم أكن قد نظرت فيها.

إذا كانت جودة كود .NET الخاصة بإصدار SDK هي نفسها تقريبا C ++ (خطأين لكل 512 KLOC) ، فيجب أن نحصل على عدد أكبر بنحو 10 أضعاف من الأخطاء. بالطبع ، هذه منهجية حسابية غير دقيقة للغاية ، والتي لا تأخذ في الاعتبار الخصائص اللغوية والعديد من العوامل الأخرى ، لكنني لا أعتقد أن القارئ يريد الآن الدخول في التفكير الممل. بدلاً من ذلك ، أقترح الانتقال إلى النتائج.

تم إجراء الفحص باستخدام PVS-Studio 6.27. إنه أمر لا يصدق ، ولكن الحقيقة هي أنه في AWS SDK لـ .NET ، تمكن المحلل من اكتشاف 40 خطأ ، الأمر الذي يستحق الحديث عنه. إنه لا يوضح الجودة العالية فقط لرمز SDK (حوالي 4 أخطاء لكل 512 KLOC) ، ولكنه يوضح أيضًا جودة مماثلة لمحلل C # PVS-Studio مقارنةً بـ C ++. نتيجة رائعة!

مؤلفو AWS SDK لـ .NET ، أنت أبطال حقيقيون! مع كل مشروع ، تظهر جودة هائلة للكود. يمكن أن يكون مثالا رائعا للفرق الأخرى. ومع ذلك ، بالطبع ، لن أكون مطورًا لمحلل ثابت ، إذا لم أعطي سنتي. :) نحن نعمل بالفعل مع فريق Lumberyard من Amazon على استخدام PVS-Studio. نظرًا لأنها شركة كبيرة جدًا تضم ​​مجموعة من الوحدات حول العالم ، فمن المحتمل جدًا أن فريق AWS SDK لـ .NET لم يسمع أبدًا عن PVS-Studio. على أي حال ، لم أجد أي علامات لاستخدام محللنا في رمز SDK ، على الرغم من أنه لا يقول أي شيء. ومع ذلك ، على الأقل ، يستخدم الفريق محلل مضمن في Visual Studio. إنه لأمر رائع ، ولكن يمكن دائمًا تعزيز مراجعات الكود :).

نتيجة لذلك ، تمكنت من العثور على بعض الأخطاء في رمز SDK ، وأخيرا ، حان الوقت لمشاركتها.

خطأ في المنطق

تحذير PVS-Studio: V3008 [CWE-563] تم تعيين متغير "this.linker.s3.region" لقيمتين متتاليتين. ربما هذا خطأ. خطوط التحقق: 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; } }

يحذر المحلل من تخصيص قيمة متكررة لنفس المتغير. يتضح من التعليمات البرمجية أن هذا يرجع إلى الخطأ الذي ينتهك منطق عمل البرنامج: ستكون قيمة المتغير this.linker.s3.region دائمًا مساوية لقيمة القيمة المتغيرة ، بغض النظر عن الحالة if (String.IsNullOrEmpty (value)) . غاب بيان العودة في الجسم إذا كتلة. يجب إصلاح الكود كما يلي:

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

العودية لانهائية

تحذير PVS-Studio: V3110 [CWE-674] عودية لانهائية محتملة داخل خاصية "OnFailure". AWSSDK.ElasticMapReduce.Net45 ResizeJobFlowStep.cs 171

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

مثال كلاسيكي على خطأ مطبعي ، مما يؤدي إلى تكرار غير محدود في أداة الحصول على خاصية OnFailure . بدلاً من إرجاع قيمة الحقل الخاص onFailure ، يتم الوصول إلى خاصية OnFailure . البديل الصحيح:

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

قد تسأل: "كيف عملت؟" حتى الآن - لا كيف. لا يتم استخدام الخاصية في أي مكان آخر ، ولكن هذا مؤقت. عند نقطة واحدة ، سيبدأ شخص ما في استخدامه وسيحصل بالتأكيد على نتيجة غير متوقعة. لمنع مثل هذه الأخطاء المطبعية ، يوصى بعدم استخدام المعرفات التي تختلف فقط في حالة الحرف الأول.

هناك تعليق آخر لهذا البناء يستخدم المعرف ، والذي يطابق تمامًا اسم نوع OnFailure . من وجهة نظر المترجم ، فإنه مقبول تمامًا ، لكن هذا يعقد تصور الكود للشخص.

خطأ مشابه آخر:

تحذير PVS-Studio: V3110 [CWE-674] احتمال تكرار غير محدود داخل خاصية "SSES3". AWSSDK.S3.Net45 InventoryEncryption.cs 37

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

الموقف مطابق لما ورد أعلاه. ومع ذلك ، هنا سوف تحدث العودية لانهائية عند الوصول إلى الخاصية SSES3 على حد سواء للقراءة والتخصيص . البديل الصحيح:

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

اختبار على النظر

الآن أود أن أذكر مهمة من مطور ، مأخوذة باستخدام طريقة نسخ اللصق. ألقِ نظرة على الطريقة التي يبدو بها الكود في محرر Visual Studio ، وحاول العثور على خطأ.

الصورة 3


تحذير PVS-Studio: V3029 التعبيرات الشرطية لعبارات "if" الموجودة بجانب بعضها البعض متطابقة. خطوط التحقق: 91 ، 95. AWSSDK.AppSync.Net45 CreateApiKeyResponseUnmarshaller.cs 91

لقد قمت بتقليل محتوى أسلوب UnmarshallException ، بعد إزالة كل ما هو غير مطلوب. الآن يمكنك أن ترى أن الاختبارات المتطابقة تتبع بعضها البعض:

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

قد يبدو أن الخطأ ليس تقريبيًا - إنه مجرد فحص إضافي. ومع ذلك ، غالبًا ما يشير هذا النمط إلى مشاكل أكثر خطورة في الشفرة ، عندما لن يتم إجراء الفحص المطلوب.

في الكود ، هناك عدة أخطاء مماثلة.

تحذيرات PVS-Studio:

  • V3029 التعبيرات الشرطية لعبارات "if" الموجودة بجانب بعضها البعض متطابقة. خطوط التحقق: 75 ، 79. AWSSDK.CloudDirectory.Net45 CreateSchemaResponseUnmarshaller.cs 75
  • V3029 التعبيرات الشرطية لعبارات "if" الموجودة بجانب بعضها البعض متطابقة. خطوط التحقق: 105 ، 109. AWSSDK.CloudDirectory.Net45 GetSchemaAsJsonResponseUnmarshaller.cs 105
  • V3029 التعبيرات الشرطية لعبارات "if" الموجودة بجانب بعضها البعض متطابقة. خطوط التحقق: 201 ، 205. AWSSDK.CodeCommit.Net45 PostCommentForPullRequestResponseUnmarshaller.cs 201
  • V3029 التعبيرات الشرطية لعبارات "if" الموجودة بجانب بعضها البعض متطابقة. خطوط التحقق: 101 ، 105. AWSSDK.CognitoIdentityProvider.Net45 VerifySoftwareTokenResponseUnmarshaller.cs 101
  • V3029 التعبيرات الشرطية لعبارات "if" الموجودة بجانب بعضها البعض متطابقة. خطوط التحقق: 72 ، 76. AWSSDK.Glue.Net45 UpdateConnectionResponseUnmarshaller.cs 72
  • V3029 التعبيرات الشرطية لعبارات "if" الموجودة بجانب بعضها البعض متطابقة. خطوط التحقق: 123 ، 127. AWSSDK.Neptune.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 123
  • V3029 التعبيرات الشرطية لعبارات "if" الموجودة بجانب بعضها البعض متطابقة. خطوط الاختيار: 167 ، 171. AWSSDK.Neptune.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 167
  • V3029 التعبيرات الشرطية لعبارات "if" الموجودة بجانب بعضها البعض متطابقة. خطوط التحقق: 127 ، 131. AWSSDK.RDS.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 127
  • V3029 التعبيرات الشرطية لعبارات "if" الموجودة بجانب بعضها البعض متطابقة. خطوط التحقق: 171 ، 175. AWSSDK.RDS.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 171
  • V3029 التعبيرات الشرطية لعبارات "if" الموجودة بجانب بعضها البعض متطابقة. سطور الاختيار: 99 ، 103. AWSSDK.Rekognition.Net45 التعرف على المشاهير ResponseUnmarshaller.cs 99

ما هذا؟

تحذير PVS-Studio: V3062 يتم استخدام كائن "attributeName" كوسيطة للأسلوب الخاص به. فكر في التحقق من الوسيطة الفعلية الأولى لأسلوب "يحتوي على". 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; }

اكتشف المحلل خطأ في أسلوب GetAttribute : يتم التحقق من السلسلة ما إذا كانت تحتوي على نفسها. من وصف الطريقة التي تتبعها ، إذا تم العثور على اسم السمة (مفتاح اسم السمة ) (في القاموس _attributes ) ، فيجب إرجاع قيمة السمة ، وإلا - تكون خالية . في الواقع ، نظرًا لأن الشرط attributeName.Contains (attributeName) صحيح دائمًا ، يتم إجراء محاولة لإرجاع القيمة بواسطة مفتاح قد لا يمكن العثور عليه في القاموس. ثم ، بدلاً من إرجاع خالية ، سيتم طرح استثناء KeyNotFoundException .

دعنا نحاول إصلاح هذا الرمز. لفهم كيفية القيام بذلك بشكل أفضل ، يجب أن تنظر إلى طريقة أخرى:

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

يتحقق هذا الأسلوب مما إذا كان اسم السمة (مفتاح attributeName ) موجودًا في القاموس _attributes . دعنا نعود إلى طريقة GetAttribute مرة أخرى وإصلاح الخطأ:

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

الآن الطريقة تفعل بالضبط ما هو مذكور في الوصف.

تعليق صغير آخر على هذا الجزء من الشفرة. لقد لاحظت أن المؤلفين يستخدمون القفل عند العمل مع قاموس _attributes . من الواضح أن هذا ضروري عند وجود وصول متعدد مؤشرات الترابط ، ولكن إنشاء القفل بطيء ومرهق إلى حد ما. بدلاً من القاموس ، في هذه الحالة ، ربما ، سيكون أكثر ملاءمة استخدام إصدار آمن لمؤشر ترابط القاموس - ConcurrentDictionary . بهذه الطريقة ، لن تكون هناك حاجة في القفل. ومع ذلك ، ربما لا أعرف عن تفاصيل المشروع.

سلوك مشبوه

تحذير PVS-Studio: V3063 [CWE-571] دائمًا ما يكون جزء من التعبير الشرطي صحيحًا إذا تم تقييمه: 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); .... }

كان محلل قلق حول سلسلة الاختيار. IsNullOrEmpty (inferredIndexName) . في الواقع ، يتم تعيين السلسلة inferredIndexName لاغية ، ثم لا تتغير قيمة هذا المتغير في أي مكان ، ثم لسبب ما يتم التحقق من أنه فارغ أو سلسلة فارغة. تبدو مشبوهة. دعونا نلقي نظرة فاحصة على جزء رمز أعلاه. أنا عمدا لم تقلل من ذلك لفهم الوضع بشكل أفضل. لذلك ، في أول عبارة (وكذلك في العبارة التالية) ، يتم تحديد المتغير المحددإندكسيم بطريقة أو بأخرى. اعتمادًا على نتائج الاختبارات ، يحصل المتغير inferredIndexName على قيمة جديدة. الآن دعونا ننظر إلى البيان الثالث إذا . سيتم تنفيذ نص هذا البيان (رمي الاستثناء) في حالة إذا كان indexNames.Count> 0 ، هو الجزء الأول من الشرط بالكامل ، وهو string.IsNullOrEmpty (inferredIndexName) يكون دائمًا صحيحًا. ربما ، يتم خلط المتغيرات المحددة IndexName و inferredIndexName أو يجب أن يكون الاختيار الثالث بدون آخر ، ويمثل قائمة بذاتها إذا كانت العبارة:

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

في هذه الحالة ، من الصعب إعطاء إجابة محددة عن الخيارات لإصلاح هذا الرمز. على أي حال ، يحتاج المؤلف إلى التحقق من ذلك.

NullReferenceException

تحذير PVS-Studio: V3095 [CWE-476] تم استخدام كائن "conditionValues" قبل أن يتم التحقق منه ضد قيمة خالية. خطوط الفحص: 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) { .... } .... } }

انها الكلاسيكية. يتم استخدام conditionValues المتغيرة بدون فحص أولي للقيمة الخالية . في وقت لاحق في رمز يتم تنفيذ هذا الاختيار. يحتاج الرمز إلى تصحيح كما يلي:

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

لقد وجدت عدة أخطاء مماثلة في التعليمات البرمجية.

تحذيرات PVS-Studio:

  • V3095 [CWE-476] تم استخدام كائن "ts.Listeners" قبل أن يتم التحقق منه مقابل لاغٍ. خطوط التحقق: 140 ، 143. AWSSDK.Core.Net45 Logger.Diagnostic.cs 140
  • V3095 [CWE-476] تم استخدام كائن "obj" قبل أن يتم التحقق منه ضد قيمة خالية. خطوط التحقق: 743 ، 745. AWSSDK.Core.Net45 JsonMapper.cs 743
  • V3095 [CWE-476] تم استخدام كائن 'multipartUploadMultipartUploadpartsList' قبل أن يتم التحقق منه مقابل لاغٍ. خطوط التحقق: 65 ، 67. AWSSDK.S3.Net45 CompleteMultipartUploadRequestMarshaller.cs 65

التحذير التالي مشابه جدًا في المعنى ، لكن الحالة معارضة للحالة الموضحة أعلاه.

تحذير PVS-Studio: V3125 [CWE-476] تم استخدام كائن "الحالة" بعد التحقق من أنه لاغٍ. خطوط التحقق: 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; // <= .... }

يتضمن أحد أجزاء التعليمات البرمجية التحقق من قيمة متغير الحالة للقيمة الخالية . في التعليمة البرمجية أدناه ، يتم استخدام المتغير لإلغاء الاشتراك من الحدث PreemptExpiryTime ، ومع ذلك ، لم يعد يتم إجراء التحقق من وجود قيمة خالية وأصبح الاستثناء NullReferenceException ممكنًا. إصدار أكثر أمانًا من الكود:

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

في التعليمات البرمجية ، هناك أخطاء أخرى مماثلة:

تحذيرات PVS-Studio:

  • V3125 [CWE-476] تم استخدام كائن "wrappedRequest.Content" بعد أن تم التحقق منه ضد قيمة خالية. خطوط التحقق: 395 ، 383. AWSSDK.Core.Net45 HttpHandler.cs 395
  • V3125 [CWE-476] تم استخدام كائن "datasetUpdates" بعد أن تم التحقق منه ضد قيمة خالية. خطوط التحقق: 477 ، 437. AWSSDK.CognitoSync.Net45 Dataset.cs 477
  • V3125 [CWE-476] تم استخدام كائن 'cORSConfigurationCORSConfigurationcORSRulesListValue' بعد أن تم التحقق منه ضد قيمة خالية. سطور الاختيار: 125 ، 111. AWSSDK.S3.Net45 PutCORSC التكوين - RequestMarshaller.cs 125
  • V3125 [CWE-476] تم استخدام كائن "دورة حياة التكوين - دورة حياة التكوين - قواعد ListValue" بعد أن تم التحقق منه ضد قيمة خالية. خطوط التحقق: 157 ، 68. AWSSDK.S3.Net45 PutLifecycleConfigurationRequestMarshaller.cs 157
  • V3125 [CWE-476] تم استخدام كائن "this.Key" بعد أن تم التحقق منه مقابل لاغٍ. خطوط التحقق: 199 ، 183. AWSSDK.S3.Net45 S3PostUploadRequest.cs 199

حقيقة غير بديلة

تحذير PVS-Studio: V3009 [CWE-393] من الغريب أن هذه الطريقة تُرجع دائمًا نفس القيمة "صواب". AWSSDK.Core.Net45 Lexer.cs 651

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

الأسلوب يعود دائما صحيح . دعونا نرى مدى أهمية رمز الاتصال. راجعت حالات استخدام طريقة State19 . وتشارك في ملء مجموعة من معالجات fsm_handler_table بالتساوي مع أساليب أخرى مماثلة (هناك 28 منهم مع الأسماء ، على التوالي ، ابتداء من State1 إلى State28 ). من المهم هنا ملاحظة أنه بالإضافة إلى State19 ، تم إصدار التحذيرات V3009 [CWE-393] بالنسبة لبعض معالجات أخرى. هذه هي معالجات: State23 ، State26 ، State27 ، State28 . التحذيرات الصادرة عن المحلل لهم:

  • V3009 [CWE-393] من الغريب أن هذه الطريقة تُرجع دائمًا نفس القيمة "صواب". AWSSDK.Core.Net45 Lexer.cs 752
  • V3009 [CWE-393] من الغريب أن هذه الطريقة تُرجع دائمًا نفس القيمة "صواب". AWSSDK.Core.Net45 Lexer.cs 810
  • V3009 [CWE-393] من الغريب أن هذه الطريقة تُرجع دائمًا نفس القيمة "صواب". AWSSDK.Core.Net45 Lexer.cs 822
  • V3009 [CWE-393] من الغريب أن هذه الطريقة تُرجع دائمًا نفس القيمة "صواب". AWSSDK.Core.Net45 Lexer.cs 834

إليك الطريقة التي يبدو بها الإعلان وتهيئة الصفيف للمعالجات:

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

لإكمال الصورة ، دعونا نرى رمزًا لأحد المعالجات التي لم يكن للمحلل أي مطالبات عليها ، على سبيل المثال ، الحالة 2 :

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

إليك الطريقة التي تحدث بها استدعاء المعالجين:

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

كما نرى ، سيتم طرح استثناء في حالة إرجاع false . في حالتنا ، لن يحدث هذا أبدًا بالنسبة إلى معالجات State 19 و State23 و State 26 State27 و State28 . تبدو مشبوهة. من ناحية أخرى ، هناك خمسة متعهدين لديهم سلوك مماثل (سيعودون دائمًا إلى حقيقة ) ، لذلك ربما كان الأمر مفتعلًا وليس نتيجة خطأ مطبعي.

لماذا أنا أعمق في كل هذا؟ هذا الموقف مهم للغاية بمعنى أن المحلل الثابت غالباً ما يمكن أن يشير فقط إلى بناء مشبوه. وحتى الشخص (وليس الآلة) ، الذي لا يملك معرفة كافية بالمشروع ، لا يزال غير قادر على تقديم إجابة كاملة عن وجود الخطأ ، حتى بعد قضاء الوقت في تعلم الكود. يجب على المطور مراجعة هذا الرمز.

الشيكات بلا معنى

تحذير PVS-Studio: V3022 [CWE-571] التعبير 'doLog' صحيح دائمًا. 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) // <= { .... } .... }

إيلاء الاهتمام لمتغير doLog . بعد التهيئة بالقيمة الخاطئة ، سيحصل هذا المتغير على القيمة الحقيقية في جميع الحالات على طول الكود. لذلك ، تحقق مما إذا كان (doLog) صحيحًا دائمًا. ربما ، في وقت سابق من الأسلوب كان هناك فرع ، حيث لم يتم تعيين متغير doLog أي قيمة. في لحظة التحقق من ذلك ، قد تحتوي على قيمة خاطئة ، يتم تلقيها عند التهيئة. ولكن الآن لا يوجد مثل هذا الفرع.

خطأ مشابه آخر:

تحذير PVS-Studio: التعبير V3022 '! النتيجة' غير صحيح دائمًا. 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(....); } .... }

يدعي المحلل أن قيمة متغير النتيجة صحيحة دائمًا. يكون ذلك ممكنًا فقط في حالة ما إذا كانت الطريقة PutValueHelper ستعود دائمًا إلى حقيقة . ألقِ نظرة على هذه الطريقة:

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

في الواقع ، سوف تعود الطريقة الحقيقية في جميع الظروف. علاوة على ذلك ، أصدر المحلل تحذيرا لهذه الطريقة. تحذير PVS-Studio: V3009 [CWE-393] من الغريب أن هذه الطريقة تُرجع دائمًا نفس القيمة "صواب". SQLiteLocalStorage.cs 1016

أنا عمدا لم أذكر هذا التحذير في وقت سابق عندما كنت أستفسر عن الأخطاء الأخرى V3009 وقمت بحفظه لهذه الحالة. وبالتالي ، كانت الأداة صحيحة للإشارة إلى الخطأ V3022 في رمز الاتصال.

نسخ لصق. مرة اخرى

تحذير PVS-Studio: V3001 هناك تعبيرات فرعية متطابقة 'this.token == JsonToken.String' إلى اليسار وإلى يمين '||' المشغل. 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 // <= )) { .... } .... }

تتم مقارنة الحقل this.token مرتين بالقيمة JsonToken.String الخاصة بالتعداد JsonToken . ربما ، يجب أن تحتوي إحدى المقارنات على قيمة تعداد أخرى. إذا كان الأمر كذلك ، فقد حدث خطأ خطير هنا.

إعادة بيع الديون + عدم الانتباه؟

تحذير PVS-Studio: V3025 [CWE-685] تنسيق غير صحيح. من المتوقع وجود عدد مختلف من عناصر التنسيق أثناء استدعاء وظيفة "التنسيق". الوسيطات غير المستخدمة: 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)); } .... }

ربما ، سلسلة التنسيق للأسلوب. التنسيق السابق احتوى سابقًا على عنصر التنسيق {0} ، والذي تم تعيين الوسيطة AWSConfigs.AWSRegionKey عليه. بعد ذلك تم تغيير السلسلة ، وذهب عنصر التنسيق ، ولكن نسي أحد المطورين إزالة الوسيطة. يعمل مثال التعليمة البرمجية المعطاة دون أخطاء (تم طرح الاستثناء في الحالة المعاكسة - عنصر التنسيق بدون الوسيطة) ، لكنه لا يبدو لطيفًا. يجب تصحيح الكود كما يلي:

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

غير آمن

تحذير PVS-Studio: V3083 [CWE-367] الاحتجاج غير الآمن للحدث "mOnSyncSuccess" ، NullReferenceException ممكن. النظر في تعيين الحدث إلى متغير محلي قبل استدعاء ذلك. AWSSDK.CognitoSync.PCL Dataset.cs 827

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

حالة شائعة لاستدعاء غير آمن لمعالج الأحداث. تمكن المستخدم من إلغاء الاشتراك بين التحقق من المتغير mOnSyncSuccess للإلغاء الخالي واستدعاء المعالج ، بحيث تصبح قيمته خالية . احتمال حدوث مثل هذا السيناريو ضئيل ، لكن لا يزال من الأفضل جعل الكود أكثر أمانًا:

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

في التعليمات البرمجية ، هناك أخطاء أخرى مماثلة:

تحذيرات PVS-Studio:

  • V3083 [CWE-367] الاحتجاج غير الآمن للحدث "mOnSyncFailure" ، NullReferenceException ممكن. النظر في تعيين الحدث إلى متغير محلي قبل استدعاء ذلك. AWSSDK.CognitoSync.PCL Dataset.cs 839
  • V3083 [CWE-367] الاحتجاج غير الآمن للحدث ، NullReferenceException ممكن. النظر في تعيين الحدث إلى متغير محلي قبل استدعاء ذلك. AWSSDK.Core.PCL AmazonServiceClient.cs 332
  • V3083 [CWE-367] الاحتجاج غير الآمن للحدث ، NullReferenceException ممكن. النظر في تعيين الحدث إلى متغير محلي قبل استدعاء ذلك. AWSSDK.Core.PCL AmazonServiceClient.cs 344
  • V3083 [CWE-367] الاحتجاج غير الآمن للحدث ، NullReferenceException ممكن. النظر في تعيين الحدث إلى متغير محلي قبل استدعاء ذلك. AWSSDK.Core.PCL AmazonServiceClient.cs 357
  • V3083 [CWE-367] الاحتجاج غير الآمن للحدث "mExceptionEvent" ، NullReferenceException ممكن. النظر في تعيين الحدث إلى متغير محلي قبل استدعاء ذلك. AWSSDK.Core.PCL AmazonServiceClient.cs 366
  • V3083 [CWE-367] الاحتجاج غير الآمن للحدث ، NullReferenceException ممكن. النظر في تعيين الحدث إلى متغير محلي قبل استدعاء ذلك. AWSSDK.Core.PCL AmazonWebServiceRequest.cs 78
  • V3083 [CWE-367] الاحتجاج غير الآمن للحدث "OnRead" ، NullReferenceException ممكن. النظر في تعيين الحدث إلى متغير محلي قبل استدعاء ذلك. AWSSDK.Core.PCL EventStream.cs 97
  • V3083 [CWE-367] الاحتجاج غير الآمن للحدث ، NullReferenceException ممكن. النظر في تعيين الحدث إلى متغير محلي قبل استدعاء ذلك. AWSSDK.Core.Android NetworkReachability.cs 57
  • V3083 [CWE-367] الاحتجاج غير الآمن للحدث ، NullReferenceException ممكن. النظر في تعيين الحدث إلى متغير محلي قبل استدعاء ذلك. AWSSDK.Core.Android NetworkReachability.cs 94
  • V3083 [CWE-367] الاحتجاج غير الآمن للحدث ، NullReferenceException ممكن. النظر في تعيين الحدث إلى متغير محلي قبل استدعاء ذلك. AWSSDK.Core.iOS NetworkReachability.cs 54

الطبقة الخام

تحذير PVS-Studio: V3126 اكتب "JsonData" الذي يقوم بتنفيذ واجهة <T> IEquatable لا يتجاوز أسلوب "GetHashCode". AWSSDK.Core.Net45 JsonData.cs 26

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

تحتوي فئة JsonData على الكثير من التعليمات البرمجية ، لذلك لم أعطها برمتها ، مستشهدة فقط بتصريحها . هذه الفئة حقًا لا تحتوي على الأسلوب المتجاوز GetHashCode ، وهو غير آمن ، حيث يمكن أن يؤدي إلى سلوك خاطئ عند استخدام نوع JsonData للعمل ، على سبيل المثال ، مع المجموعات. ربما ، لا توجد مشكلة في الوقت الحالي ، ولكن في المستقبل قد يتغير هذا النوع من الاستراتيجية. يوصف هذا الخطأ في الوثائق بمزيد من التفاصيل.

الخاتمة

هذه كلها أخطاء مثيرة للاهتمام تمكنت من اكتشافها في رمز AWS SDK لـ .NET باستخدام محلل ثابت PVS-Studio. أود تسليط الضوء مرة أخرى على جودة المشروع. لقد وجدت عددًا صغيرًا جدًا من الأخطاء لـ 5 ملايين سطر من التعليمات البرمجية. على الرغم من أن التحليل الأكثر شمولاً للتحذيرات الصادرة سيسمح لي بإضافة بعض الأخطاء الإضافية إلى هذه القائمة. ومع ذلك ، فمن المحتمل أيضًا أن أضفت بعض التحذيرات إلى أخطاء من أجل لا شيء. يتم دائمًا إجراء استنتاجات لا لبس فيها في هذه الحالة فقط عن طريق مطور موجود في سياق التعليمات البرمجية المحددة.

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

More articles:


All Articles