👩🏼‍🤝‍👨🏾 🚨 😻 دراسة جودة كود Microsoft Open XML SDK 🐺 🌋 👳

بدأت معرفتي بـ Open XML SDK عندما كنت بحاجة إلى مكتبة لإنشاء مستندات Word مع بعض التقارير. بعد العمل مع Word API لأكثر من 7 سنوات ، أردت تجربة شيء جديد وأكثر ملاءمة. هذه هي الطريقة التي اكتشفت بها أن Microsoft لديها حل بديل. حسب التقاليد ، نتحقق مسبقًا من البرامج والمكتبات المستخدمة في الشركة باستخدام محلل PVS-Studio.

المقدمة

Office Open XML ، المعروف أيضًا باسم OpenXML أو OOXML ، هو تنسيق مستند إلى XML للمستندات المكتبية ، بما في ذلك مستندات معالجة الكلمات وجداول البيانات والعروض التقديمية والمخططات والأشكال والرسومات الأخرى. تم تطوير المواصفات بواسطة Microsoft واعتمدتها ECMA International في عام 2006. في يونيو 2014 ، أصدرت Microsoft Open XML SDK في مفتوحة المصدر. المصدر متاح الآن على GitHub بموجب ترخيص MIT.

للعثور على أخطاء في الكود المصدري للمكتبة ، استخدمنا PVS-Studio . إنها أداة لاكتشاف الأخطاء ونقاط الضعف المحتملة في الكود المصدري للبرامج المكتوبة بلغة C و C ++ و C # و Java. يعمل في أنظمة 64 بت على Windows و Linux و macOS.

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

لماذا Word API وليس Open XML SDK

كما قد تكون خمنت من العنوان ، واصلت استخدام Word API. هذه الطريقة لها الكثير من العيوب:

واجهة برمجة تطبيقات قديمة محرجة ؛
يجب تثبيت Microsoft Office ؛
الحاجة إلى توزيع مجموعة أدوات توزيع مع مكتبات Office ؛
اعتماد Word API على إعدادات لغة النظام ؛
سرعة منخفضة في العمل.

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

سرد كل هذا ، تساءلت مرة أخرى لماذا ما زلت أستخدمه ...

لكن لا ، ما زلت أحب Word API بشكل أفضل ، وإليك السبب.

يبدو OOXML كما يلي:

<?xml version="1.0" encoding="utf-8" standalone="yes"?>
<w:document ....>
  <w:body>
    <w:p w:rsidR="00E22EB6"
         w:rsidRDefault="00E22EB6">
      <w:r>
        <w:t>This is a paragraph.</w:t>
      </w:r>
    </w:p>
    <w:p w:rsidR="00E22EB6"
         w:rsidRDefault="00E22EB6">
      <w:r>
        <w:t>This is another paragraph.</w:t>
      </w:r>
    </w:p>
  </w:body>
</w:document>

حيث <w: r> (Word Run) ليس جملة ، ولا حتى كلمة ، ولكنه أي جزء من النص له سمات تختلف عن أجزاء النص المجاورة.

تمت برمجته بشيء مثل هذا:

Paragraph para = body.AppendChild(new Paragraph());
Run run = para.AppendChild(new Run());
run.AppendChild(new Text(txt));

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

في المقابل ، تقوم Open XML SDK بحل مجموعة واسعة من المهام. باستخدامه ، يمكنك إنشاء مستندات ليس فقط لبرنامج Word ، ولكن أيضًا لبرنامج Excel و PowerPoint. ربما تكون هذه المكتبة أكثر ملاءمة لبعض المهام ، لكنني قررت البقاء على Word API في الوقت الحالي. على أي حال ، لن ينجح التخلي عنها تمامًا ، لأن للاحتياجات الداخلية ، نقوم بتطوير مكون إضافي لبرنامج Word ، ولا يمكن استخدام سوى Word API.

قيمتان للسلسلة

V3008 يتم تخصيص قيم لمتغير "_rawOuterXml" مرتين على التوالي. ربما هذا خطأ. فحص السطور: 164 ، 161. OpenXmlElement.cs 164

internal string RawOuterXml
{
    get => _rawOuterXml;

    set
    {
        if (string.IsNullOrEmpty(value))
        {
            _rawOuterXml = string.Empty;
        }

        _rawOuterXml = value;
    }
}

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

V3022 التعبير "مساحة الاسم Uri ! = Null" يكون دائمًا صحيحًا. OpenXmlElement.cs 497

public OpenXmlAttribute GetAttribute(string localName, string namespaceUri)
{
    ....
    if (namespaceUri == null)
    {
        // treat null string as empty.
        namespaceUri = string.Empty;
    }
    ....
    if (HasAttributes)
    {
        if (namespaceUri != null)  // <=
        {
            ....
        }
        ....
    }
    ....
}

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

حول تماسك الكود

V3009 من الغريب أن تقوم هذه الطريقة دائمًا بإرجاع قيمة واحدة ونفس قيمة '".xml"'. CustomXmlPartTypeInfo.cs 31

internal static string GetTargetExtension(CustomXmlPartType partType)
{
    switch (partType)
    {
        case CustomXmlPartType.AdditionalCharacteristics:
            return ".xml";

        case CustomXmlPartType.Bibliography:
            return ".xml";

        case CustomXmlPartType.CustomXml:
            return ".xml";

        case CustomXmlPartType.InkContent:
            return ".xml";

        default:
            return ".xml";
    }
}

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

هذا ليس المكان الوحيد من هذا القبيل. إليك المزيد من هذه التحذيرات:

V3009 من الغريب أن هذه الطريقة تقوم دائمًا بإرجاع قيمة واحدة ونفس قيمة '".xml"'. CustomPropertyPartTypeInfo.cs 25
V3009 من الغريب أن هذه الطريقة تقوم دائمًا بإرجاع قيمة واحدة ونفس قيمة '".bin"'. EmbeddedControlPersistanceBinaryDataPartTypeInfo.cs 22

سيكون من المثير للاهتمام معرفة سبب الكتابة بهذه الطريقة.

V3139 يقوم فرعي حالة أو أكثر بتنفيذ نفس التصرفات. 560

private void InnerSkip()
{
    Debug.Assert(_xmlReader != null);

    switch (_elementState)
    {
        case ElementState.Null:
            ThrowIfNull();
            break;

        case ElementState.EOF:
            return;

        case ElementState.Start:
            _xmlReader.Skip();
            _elementStack.Pop();
            GetElementInformation();
            return;

        case ElementState.End:
        case ElementState.MiscNode:
            // cursor is end element, pop stack
            _xmlReader.Skip();
            _elementStack.Pop();
            GetElementInformation();
            return;
        ....
    }
    ....
}

هناك عدد أقل من الأسئلة حول هذا الرمز. على الأرجح ، يمكن دمج الحالات المتطابقة وسيصبح الرمز أقصر وأكثر وضوحًا.

عدد قليل من هذه الأماكن:

V3139 يقوم فرعي حالة أو أكثر بتنفيذ نفس التصرفات. 312- مكتبه
V3139 يقوم فرعي حالة أو أكثر بتنفيذ نفس التصرفات. CustomPropertyPartTypeInfo.cs 30
V3139 يقوم فرعي حالة أو أكثر بتنفيذ نفس التصرفات. CustomXmlPartTypeInfo.cs 15
V3139 يقوم فرعي حالة أو أكثر بتنفيذ نفس التصرفات. OpenXmlElement.cs 1803

تلك دائما صحيحة / خطأ

حان الوقت الآن لتقديم بعض الأمثلة البرمجية التي حددت اختياري لصورة العنوان.

تحذير 1

V3022 التعبير "Complete ()" خاطئ دائمًا. 243- علي عبدالله

private bool IsComplete => Current is null ||
                           Current == _collection._element.FirstChild;

public bool MoveNext()
{
    ....
    if (IsComplete)
    {
        return Complete();
    }

    if (....)
    {
        return Complete();
    }

    return IsComplete ? Complete() : true;
}

يتم استخدام خاصية IsComplete مرتين ، ومن السهل أن نفهم من الكود أن قيمتها لن تتغير. وهكذا ، في نهاية الوظيفة ، يمكنك ببساطة إرجاع القيمة الثانية للعامل الثلاثي - صواب .

تحذير 2

V3022 التعبير "_elementStack.Count> 0" يكون دائمًا صحيحًا. 501

private readonly Stack<OpenXmlElement> _elementStack;

private bool MoveToNextSibling()
{
    ....
    if (_elementStack.Count == 0)
    {
        _elementState = ElementState.EOF;
        return false;
    }
    ....
    if (_elementStack.Count > 0) // <=
    {
        _elementState = ElementState.End;
    }
    else
    {
        // no more element, EOF
        _elementState = ElementState.EOF;
    }
    ....
}

من الواضح أنه إذا لم يكن هناك 0 عناصر في _elementStack ، فهناك المزيد منها. يمكن اختصار الرمز بـ 8 أسطر على الأقل.

تحذير 3

V3022 التعبير 'rootElement == null' خاطئ دائمًا. 746

private static OpenXmlElement CreateElement(string namespaceUri, string name)
{
    if (string.IsNullOrEmpty(name))
    {
        throw new ArgumentException(....);
    }

    if (NamespaceIdMap.TryGetNamespaceId(namespaceUri, out byte nsId)
        && ElementLookup.Parts.Create(nsId, name) is OpenXmlElement element)
    {
        return element;
    }

    return new OpenXmlUnknownElement();
}

private bool ReadRoot()
{
  ....
  var rootElement = CreateElement(....);

  if (rootElement == null) // <=
  {
      throw new InvalidDataException(....);
  }
  ....
}

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

تحذير 4

V3022 تعبير "nameProvider" ليس دائمًا فارغًا. عامل التشغيل "؟." مفرط. OpenXmlSimpleTypeExtensions.cs 50

public static XmlQualifiedName GetSimpleTypeQualifiedName(....)
{
    foreach (var validator in validators)
    {
        if (validator is INameProvider nameProvider &&
            nameProvider?.QName is XmlQualifiedName qname) // <=
        {
            return qname;
        }
    }

    return type.GetSimpleTypeQualifiedName();
}

عامل التشغيل هو النمط التالي:

expr is type varname

إذا تم تقييم expr إلى true ، فسيكون كائن varname صالحًا ولا يلزم مقارنته مع null مرة أخرى ، كما هو الحال في مقتطف الشفرة هذا.

تحذير 5

V3022 امتداد 'التعبير == ".xlsx" || الامتداد == ".xlsm" "خطأ دائمًا. PresentationDocument.cs 246

public static PresentationDocument CreateFromTemplate(string path)
{
    ....
    string extension = Path.GetExtension(path);
    if (extension != ".pptx" && extension != ".pptm" &&
        extension != ".potx" && extension != ".potm")
    {
        throw new ArgumentException("...." + path, nameof(path));
    }

    using (PresentationDocument template = PresentationDocument.Open(....)
    {
        PresentationDocument document = (PresentationDocument)template.Clone();

        if (extension == ".xlsx" || extension == ".xlsm")
        {
            return document;
        }
        ....
    }
    ....
}

تحول رمز مثير للاهتمام. استبعد المؤلف الأول جميع المستندات ذات الامتدادات التالية ليست .pptx ، .pptm ،. بوتكس و. potm ، ثم قرر التحقق من عدم وجود .xlsx و. xlsm بينهم . تعتبر وظيفة PresentationDocument بالتأكيد ضحية لإعادة البناء.

تحذير 7

V3022 التعبير "OpenSettings.MarkupCompatibilityProcessSettings == فارغ" دائمًا خطأ. OpenXmlPackage.cs 661

public MarkupCompatibilityProcessSettings MarkupCompatibilityProcessSettings
{
    get
    {
        if (_mcSettings is null)
        {
            _mcSettings = new MarkupCompatibilityProcessSettings(....);
        }

        return _mcSettings;
    }

    set
    {
        _mcSettings = value;
    }
}

public MarkupCompatibilityProcessSettings MarkupCompatibilityProcessSettings
{
    get
    {
        if (OpenSettings.MarkupCompatibilityProcessSettings == null) // <=
        {
            return new MarkupCompatibilityProcessSettings(....);
        }
        else
        {
            return OpenSettings.MarkupCompatibilityProcessSettings;
        }
    }
}

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

تحذيرات أخرى

تحذير 1

V3080 احتمال وجود مرجع فارغ. ضع في اعتبارك فحص "الشقيق السابق". 380

public OpenXmlElement PreviousSibling()
{
    if (!(Parent is OpenXmlCompositeElement parent))
    {
        return null;
    }
    ....
}

public override T InsertBefore<T>(T newChild, OpenXmlElement referenceChild)
{
    ....
    OpenXmlElement previousSibling = nextNode.PreviousSibling();
    prevNode.Next = nextNode;
    previousSibling.Next = prevNode;    // <=
    ....
}

وهنا مثال حيث لا يكفي التحقق الإضافي. يمكن أن ترجع طريقة PreviousSibling قيمة خالية ، ويتم استخدام نتيجة هذه الوظيفة على الفور دون التحقق.

2 أماكن أكثر خطورة:

V3080 احتمال عدم وجود مرجع فارغ. ضع في اعتبارك فحص "prevNode". 489
V3080 احتمال عدم وجود مرجع فارغ. ضع في اعتبارك فحص "prevNode". 497

تحذير 2

V3093 يقوم عامل التشغيل "&" بتقييم كلا المعاملين. ربما يجب استخدام عامل تشغيل "&&" ماس كهربائى بدلاً من ذلك. UniqueAttributeValueConstraint.cs 60

public override ValidationErrorInfo ValidateCore(ValidationContext context)
{
    ....
    foreach (var e in root.Descendants(....))
    {
        if (e != element & e.GetType() == elementType) // <=
        {
            var eValue = e.ParsedState.Attributes[_attribute];

            if (eValue.HasValue && _comparer.Equals(....))
            {
                return true;
            }
        }
    }
    ....
}

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

تحذير 3

V3097 استثناء محتمل: النوع المحدد بواسطة [Serializable] يحتوي على أعضاء غير قابلة للتسلسل لم يتم وضع علامة عليها بواسطة [NonSerialized]. OpenXmlPackageValidationEventArgs.cs 15

[Serializable]
[Obsolete(ObsoleteAttributeMessages.ObsoleteV1ValidationFunctionality, false)]
[EditorBrowsable(EditorBrowsableState.Never)]
public sealed class OpenXmlPackageValidationEventArgs : EventArgs
{
    private string _message;

    [NonSerialized]
    private readonly object _sender;

    [NonSerialized]
    private OpenXmlPart _subPart;

    [NonSerialized]
    private OpenXmlPart _part;

    ....

    internal DataPartReferenceRelationship
        DataPartReferenceRelationship { get; set; } // <=
}

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

خاتمة

نحن في الشركة نحب مشاريع وتقنيات Microsoft. في القسم الذي ندرج فيه مشاريع المصدر المفتوح التي تم اختبارها باستخدام PVS-Studio ، قمنا حتى بتخصيص قسم منفصل لـ Microsoft. لقد تراكمت بالفعل 21 مشروعًا ، تمت كتابة 26 مقالة عنها. هذا هو السابع والعشرون.

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

المزيد من الأخبار من الأخبار للمهتمين بتحليل الكود في C ++ و C # و Java: لقد أضفنا مؤخرًا دعمًا لمعيار OWASP ونعمل على زيادة تغطيته بنشاط.

إذا كنت ترغب في مشاركة هذه المقالة مع جمهور يتحدث الإنجليزية ، فيرجى استخدام رابط الترجمة: Svyatoslav Razmyslov. تحليل جودة كود Microsoft Open XML SDK .

دراسة جودة كود Microsoft Open XML SDK