Geekly articles weekly

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

الصورة 1


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

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

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

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

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

مؤلفو AWS SDK لـ .NET رائعون. من مشروع إلى آخر ، فإنها تظهر جودة رمز مذهلة. هذا يمكن أن يكون مثالا جيدا للفرق الأخرى. لكن ، بالطبع ، لن أكون مطورًا لمحلل ثابت إذا لم أقم بإدخال 5 أكواب. :) نحن نعمل بالفعل مع فريق Amazon Lumberyard على استخدام PVS-Studio. ولكن نظرًا لأن هذه شركة كبيرة جدًا تضم ​​مجموعة من الأقسام في جميع أنحاء العالم ، فمن المحتمل جدًا أن فريق AWS SDK for .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 دائمًا مساوية للقيمة ، بغض النظر عن حالة الشرط (String.IsNullOrEmpty (value)) . في جسم الكتلة if ، تم تخطي العودة . يجب إصلاح الكود كما يلي:

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

نبه المحلل في السلسلة . التحقق من صحة (inferredIndexName) . بالفعل ، يتم تعيين السلسلة inferredIndexName فارغة ، ثم لا يتم تغيير قيمة هذا المتغير في أي مكان ، ثم لسبب ما يتم التحقق من أنه فارغ أو سلسلة فارغة. يبدو مشبوهة. دعونا نلقي نظرة فاحصة على مقتطف الشفرة المحدد. أنا عمدا لم يقلل من ذلك لفهم أفضل للوضع. لذلك ، في أول عبارة (وكذلك في العبارة التالية) ، يتم تحديد المتغير المحددإندكسنام بطريقة ما. اعتمادًا على نتيجة الاختبارات ، يتلقى المتغير inferredIndexName قيمة جديدة. الآن دعنا ننتبه إلى البيان الثالث إذا . سيتم تنفيذ نص هذا البيان (رمي استثناء) إذا كان indexNames.Count> 0 ، لأن الجزء الأول من الشرط هو string.IsNullOrEmpty (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 و State26 و 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 . بعد التهيئة مع false ، سيصبح هذا المتغير صحيحًا في التعليمات البرمجية في جميع الحالات. إذا كان الاختيار (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)); } .... }

ربما ، كانت سلسلة التنسيق لأسلوب string.Format تحتوي في السابق على محدد الإخراج {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 لإلغاء null واستدعاء المعالج ، يمكن إلغاء اشتراك الحدث ، وستصبح قيمته صفرًا. احتمال حدوث مثل هذا السيناريو ضئيل ، لكن لا يزال من الأفضل جعل الكود أكثر أمانًا:

 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 ملايين سطر من التعليمات البرمجية. رغم أنه من المحتمل أن يسمح لي تحليل أكثر شمولاً للتحذيرات الصادرة بإضافة المزيد من الأخطاء إلى هذه القائمة. لكن من المحتمل جدًا أن أكون دون جدوى قد صنفت بعض التحذيرات الموصوفة بأنها أخطاء. في هذه الحالة ، دائمًا ما يقوم المطور ، الذي يكون في سياق الكود الجاري فحصه ، دائمًا بعمل استنتاجات لا لبس فيها.



إذا كنت ترغب في مشاركة هذه المقالة مع جمهور يتحدث الإنجليزية ، فالرجاء استخدام الرابط الخاص بترجمة: سيرجي خرينوف. البحث عن الأخطاء في التعليمات البرمجية المصدر Amazon Web Services SDK لـ .NET

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

More articles:


All Articles