Azure SDK for .NET: قصة الباحث عن الأخطاء الصعبة

عندما قررنا البحث عن أخطاء في مشروع Azure SDK for .NET ، فوجئنا بسرورها بحجمه. "قلنا ثلاثة ملايين ونصف المليون سطر من التعليمات البرمجية" ، قلنا ، ندرس إحصائيات المشروع. هذا هو مقدار ما يمكنك أن تجد هناك. ولكن للأسف ، آه. تحول المشروع ليكون سرا. ما هي خصوصية المشروع وكيف تم اختباره - اقرأ في هذا المقال.

عن المشروع

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

ومع ذلك ، أظهر مشروع Azure SDK for .NET على الفور فشله كمقعد اختبار. حتى حجمها المثير للإعجاب لم يساعد ، بل بالأحرى تعقيد العمل. السبب موضح في إحصائيات المشروع التالية:

• ملفات المصدر .cs (لا تشمل الاختبارات): 16،500
• حلول Visual Studio (.sln): 163
• أسطر الكود غير الفارغة: 3،462،000
• التي ولدت تلقائيا: حوالي 3.3 مليون دولار
• مستودع المشروع متاح على جيثب .

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

تم استخدام عدد كبير من حلول Visual Studio (163) كرز على الكعكة ، وفقًا لذلك تم "تلطيخ" هذه الكتلة من الكود. حتى أتحقق من الكود المتبقي (لا يتم إنشاؤه تلقائيًا) اضطررت إلى بذل بعض الجهود. لقد ساعد ذلك في وضع جميع الشفرات التي تم إنشاؤها تلقائيًا في مجلدات الحلول الفرعية على طول المسار النسبي "<مجلد الحلول> \ src \ Generated". أيضًا ، يحتوي كل ملف .cs من هذا الرمز على تعليق خاص في علامة <auto-generated> :

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

من أجل نقاء التجربة ، راجعت عشوائيًا حوالي عشرة حلول تم اختيارها عشوائيًا باستخدام شفرة تم إنشاؤها تلقائيًا. ستكون النتائج أقل.

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

دعونا نرى ما وجدته في Azure SDK لرمز .NET.

Microsoft.Azure.Management.Advisor

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

V3022 التعبير "بيانات الاعتماد! = لاغ " صحيح دائمًا. AdvisorManagementClient.cs 204

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

من الواضح أن الكود لا لزوم له ، والتحقق من مؤهلات الاعتماد! = لا قيمة لها. لكن الرمز يعمل. و autogenerated. لذلك - لا شكاوى.

تعبير V3022 '_queryParameters.Count> 0' غير صحيح دائمًا. ConfigurationsOperations.cs 871

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

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

لكل حل برمز تم إنشاؤه تلقائيًا ، تم تلقي مئات التحذيرات المشابهة. بالنظر إلى عدم جدواها ، أعتقد أنه لا جدوى من النقاش الإضافي حول هذه الإيجابيات. الأخطاء الحقيقية فقط في الكود "العادي" سيتم النظر فيها أدناه.

Azure.Core

V3001 هناك تعبيرات فرعية مماثلة "buffer.Length" إلى اليسار وإلى يمين المشغل "<". AzureBaseBuffersExtensions.cs 30

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

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

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

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

V3083 الاحتجاج غير الآمن للحدث "_onChange" ، NullReferenceException ممكن. النظر في تعيين الحدث إلى متغير محلي قبل استدعاء ذلك. ClientOptionsMonitor.cs 44

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

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

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

Azure.Messaging.EventHubs

V3080 dereference ممكن. النظر في فحص "eventPropertyValue". AmqpMessageConverter.cs 650

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

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

V3066 تم ترتيب الوسيطات غير الصحيحة المحتملة إلى مُنشئ "EventHubConsumer": "partitionId" و "ConsumerGroup". TrackOneEventHubClient.cs 394

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

يشتبه المحلل أنه عند استدعاء مُنشئ فئة EventHubConsumer ، تم خلط ترتيب الوسيطتين الثالثة والرابعة. دعونا نلقي نظرة على إعلان المنشئ:

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

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

هذا مجرد تخمين ، لكنني أود تغيير تنسيق كود إعلان المنشئ إلى هذا:

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

Azure.Storage

V3112 خلل في مقارنات مماثلة. من الممكن أن يكون هناك خطأ مطبعي موجود داخل التعبير "ContentLanguage == other.ContentEncoding". BlobSasBuilder.cs 410

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

خطأ ارتكبه الغفلة. من الصعب جدًا العثور على خطأ مشابه في مراجعة الكود. خيار الاختيار الصحيح:

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

V3112 خلل في مقارنات مماثلة. من الممكن أن يكون هناك خطأ مطبعي موجود داخل التعبير "ContentLanguage == other.ContentEncoding". FileSasBuilder.cs 265

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

بالضبط نفس الخطأ في قطعة مماثلة جدا من التعليمات البرمجية. ربما تم نسخ الكود وتعديله جزئيًا. لكن الخطأ بقي.

Microsoft.Azure.Batch

V3053 تعبير مفرط. فحص سلاسل "IList" و "قائمة". PropertyData.cs 157

V3053 تعبير مفرط. فحص substrings "قائمة" و "IReadOnlyList". PropertyData.cs 158

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

أصدر المحلل تحذيرين حول فحوصات لا معنى لها أو خاطئة. في الحالة الأولى ، يبدو البحث عن السلسلة الفرعية "List" بعد البحث عن "IList" أمرًا ضروريًا. في الواقع ، الشرط:

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

يمكن استبداله بهذا:

 this.Type.Contains("List") 

في الحالة الثانية ، لا معنى للبحث عن السلسلة الفرعية "IReadOnlyList" ، حيث يتم في وقت سابق البحث عن السلسلة الفرعية الأقصر "قائمة".

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

V3095 تم استخدام عنصر 'httpRequest.Content.Headers' قبل أن يتم التحقق منه ضد قيمة خالية. خطوط التحقق: 76 ، 79. BatchSharedKeyCredential.cs 76

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

في البداية ، يتم استخدام المتغير httpRequest.Content.Headers دون أي عمليات تدقيق ، ولكن لاحقًا في الكود يتم الوصول إلى هذا المتغير باستخدام عامل الوصول المشروط.

V3125 تم استخدام كائن "omPropertyData" بعد أن تم التحقق منه مقابل خالية. خطوط التحقق: 156 ، 148. CodeGenerationUtilities.cs 156

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

الوضع العكسي. تحتوي كتلة واحدة من التعليمات البرمجية على خيار وصول آمن إلى ارتباط omPropertyData المحتمل أنه لاغٍ. علاوة على ذلك في الكود مع نفس الرابط يعملون دون أي شيكات.

V3146 التراجع المحتمل الفارغ لـ "القيمة". يمكن لـ 'FirstOrDefault' إرجاع القيمة الخالية الافتراضية. BatchSharedKeyCredential.cs 127

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

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

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

Microsoft.Azure.ServiceBus

V3121 تم إعلان التعداد "BlocksUsing" باستخدام سمة "الإشارات" ، لكنه لا يقوم بتعيين أي مُهيئات لتجاوز القيم الافتراضية. Fx.cs 69

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

يتم تعريف التعداد مع سمة العلامات . في هذه الحالة ، يتم ترك قيم الثوابت افتراضيًا ( MonitorEnter = 0 ، MonitorWait = 1 ، ManualResetEvent = 2 ، وهكذا). قد يؤدي ذلك إلى حقيقة أنه عند محاولة استخدام مجموعة من العلامات ، على سبيل المثال ، الثوابت الثانية والثالثة MonitorWait (= 1) | ManualResetEvent (= 2) ، لن يتم استلام قيمة فريدة ، ولكن ثابت بقيمة 3 افتراضيًا ( AutoResetEvent ). قد يكون هذا بمثابة مفاجأة لرمز الاتصال. إذا كان مخطط تعداد BlocksUsing مخططًا بالفعل لاستخدامه في تحديد مجموعات الأعلام (حقل بت) ، فعليك إعطاء قيم الثوابت مساوٍ لقوى اثنين:

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

V3125 تم استخدام عنصر "الجلسة" بعد أن تم التحقق منه ضد قيمة خالية. خطوط الفحص: 69 ، 68. AmqpLinkCreator.cs 69

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

انتبه للعمل مع متغير الجلسة في كتلة catch . يتم استدعاء أسلوب إحباط بأمان من خلال بيان الوصول الشرطي. ولكن بعد ذلك يقومون بإجراء مكالمة غير آمنة إلى الأسلوب GetInnerException . في هذه الحالة ، بدلاً من طرح استثناء من النوع المتوقع ، قد يتم طرح NullReferenceException . يحتاج الرمز إلى إصلاح. يعتمد أسلوب AmqpExceptionHelper.GetClientException تمرير قيمة فارغة للمعلمة innerException :

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

لذلك ، يكفي استخدام عامل الوصول المشروط عند استدعاء session.GetInnerException () :

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

استنتاج

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

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