Clever Geek Handbook

Studi Kualitas Kode Microsoft Open XML SDK

gambar1.png


Kenalan saya dengan Open XML SDK dimulai ketika saya membutuhkan perpustakaan untuk membuat dokumen Word dengan beberapa pelaporan. Setelah bekerja dengan Word API selama lebih dari 7 tahun, saya ingin mencoba sesuatu yang baru dan lebih nyaman. Beginilah cara saya mengetahui bahwa Microsoft memiliki solusi alternatif. Biasanya, kami memeriksa program dan pustaka yang digunakan di perusahaan menggunakan penganalisis PVS-Studio.



pengantar



Office Open XML, juga dikenal sebagai OpenXML atau OOXML, adalah format berbasis XML untuk dokumen office, termasuk dokumen pengolah kata, spreadsheet, presentasi, dan diagram, bentuk, dan grafik lainnya. Spesifikasi tersebut dikembangkan oleh Microsoft dan diadopsi oleh ECMA International pada tahun 2006. Pada bulan Juni 2014, Microsoft merilis Open XML SDK di open source. Sumbernya sekarang tersedia di GitHub di bawah lisensi MIT.



Untuk menemukan kesalahan dalam kode sumber pustaka, kami menggunakan PVS-Studio . Ini adalah alat untuk mendeteksi bug dan potensi kerentanan dalam kode sumber program yang ditulis dalam C, C ++, C # dan Java. Bekerja pada sistem 64-bit di Windows, Linux, dan macOS.



Proyek ini cukup kecil dan hanya ada sedikit peringatan. Namun pemilihan gambar judul justru didasarkan pada hasil. Ada banyak operator bersyarat yang tidak berguna di dalam kode. Menurut saya, jika Anda merefaktor ulang semua tempat seperti itu dalam kode, maka volumenya akan berkurang secara nyata. Alhasil, pembacaan kode juga akan meningkat.



Mengapa Word API dan bukan Open XML SDK



Seperti yang bisa Anda tebak dari judulnya, saya terus menggunakan Word API. Metode ini memiliki banyak kekurangan:



  • API lama yang canggung;
  • Microsoft Office harus diinstal;
  • Kebutuhan untuk mendistribusikan kit distribusi dengan perpustakaan Office;
  • Ketergantungan API Word pada pengaturan lokal sistem;
  • Kecepatan kerja rendah.


Dengan lokasi pada umumnya, insiden lucu terjadi. Windows memiliki selusin pengaturan regional. Di salah satu komputer server ada kekacauan lokal AS dan Inggris. Karena itu, dokumen Word dibuat, di mana alih-alih simbol dolar ada rubel, dan pound tidak ditampilkan sama sekali. Memperbaiki pengaturan sistem operasi memperbaiki masalah.



Mendaftar semua ini, saya bertanya-tanya lagi mengapa saya masih menggunakannya ...



Tapi tidak, saya masih lebih menyukai Word API, dan inilah alasannya.



OOXML terlihat seperti ini: 



<?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>


Di mana <w: r> (Word Run) bukanlah sebuah kalimat, dan bahkan bukan sebuah kata, tetapi bagian teks apa pun yang memiliki atribut yang berbeda dari fragmen teks yang berdekatan.



Itu diprogram dengan sesuatu seperti ini: 



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


Dokumen tersebut memiliki struktur internal tertentu, dan dalam kode Anda perlu membuat elemen yang sama. Open XML SDK, menurut saya, tidak memiliki lapisan akses data abstrak yang cukup. Membuat dokumen menggunakan Word API akan menjadi lebih jelas dan lebih pendek. Terutama jika menyangkut tabel dan struktur data kompleks lainnya.



Pada gilirannya, Open XML SDK menyelesaikan berbagai tugas. Dengannya, Anda dapat membuat dokumen tidak hanya untuk Word, tetapi juga untuk Excel dan PowerPoint. Pustaka ini mungkin lebih cocok untuk beberapa tugas, tetapi saya memutuskan untuk tetap menggunakan Word API untuk saat ini. Bagaimanapun, itu tidak akan berhasil untuk sepenuhnya meninggalkannya, karena untuk kebutuhan internal, kami sedang mengembangkan plugin untuk Word, dan di sana dimungkinkan hanya menggunakan Word API.



Dua nilai untuk string



V3008 Variabel '_rawOuterXml' diberi nilai dua kali berturut-turut. Mungkin ini salah. Periksa baris: 164, 161. OpenXmlElement.cs 164 



internal string RawOuterXml
{
    get => _rawOuterXml;

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

        _rawOuterXml = value;
    }
}


Jenis string dapat memiliki 2 jenis nilai: null dan nilai teks. Jelas lebih aman menggunakan makna tekstual, tetapi kedua pendekatan itu valid. Dalam proyek ini , tidak dapat diterima untuk menggunakan nilai null dan ditulis ulang menjadi string.Empty ... setidaknya itulah maksudnya . Namun karena kesalahan dalam RawOuterXml , Anda masih dapat menulis null , lalu merujuk ke bidang ini, menerima NullReferenceException .



V3022 Expression 'namespaceUri! = Null' selalu benar. 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)  // <=
        {
            ....
        }
        ....
    }
    ....
}


Cuplikan ini menggunakan pendekatan yang sama, pembuat kode tidak membuat kesalahan besar apa pun, tetapi masih berbau seperti pemfaktoran ulang yang gagal. Kemungkinan besar, satu pemeriksaan dapat dihapus di sini, yang akan mengurangi lebar kode, dan, oleh karena itu, meningkatkan keterbacaannya.



Tentang kekompakan kode



gambar2.png


V3009 Aneh bahwa metode ini selalu mengembalikan satu dan nilai yang sama dari '".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";
    }
}


Saya tidak tahu apakah ada kesalahan ketik di sini, atau apakah penulis kode menulis kode yang "bagus" menurut pendapatnya. Saya yakin tidak ada gunanya mengembalikan begitu banyak nilai dengan tipe yang sama dari suatu fungsi, dan kodenya bisa sangat dikurangi.



Ini bukan satu-satunya tempat seperti itu. Berikut ini beberapa lagi peringatan ini:



  • V3009 Aneh bahwa metode ini selalu mengembalikan satu dan nilai yang sama dari '".xml"'. CustomPropertyPartTypeInfo.cs 25
  • V3009 Aneh bahwa metode ini selalu mengembalikan satu dan nilai yang sama dari '".bin"'. EmbeddedControlPersistenceBinaryDataPartTypeInfo.cs 22


Menarik untuk mendengar mengapa menulis seperti itu.



V3139 Dua atau lebih cabang kasus melakukan tindakan yang sama. OpenXmlPartReader.cs 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;
        ....
    }
    ....
}


Ada lebih sedikit pertanyaan tentang kode ini. Kemungkinan besar, kasus yang identik dapat digabungkan dan kodenya akan menjadi lebih pendek dan lebih jelas.



Beberapa tempat seperti itu:



  • V3139 Dua atau lebih cabang kasus melakukan tindakan yang sama. OpenXmlMiscNode.cs 312
  • V3139 Dua atau lebih cabang kasus melakukan tindakan yang sama. CustomPropertyPartTypeInfo.cs 30
  • V3139 Dua atau lebih cabang kasus melakukan tindakan yang sama. CustomXmlPartTypeInfo.cs 15
  • V3139 Dua atau lebih cabang kasus melakukan tindakan yang sama. OpenXmlElement.cs 1803


Itu Selalu benar / salah



Sekarang saatnya memberikan beberapa contoh kode yang menentukan pilihan gambar judul saya.



Peringatan 1



V3022 Ekspresi 'Complete ()' selalu salah. ParticleCollection.cs 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;
}


Properti IsComplete digunakan 2 kali, dan mudah dipahami dari kode bahwa nilainya tidak akan berubah. Jadi, di akhir fungsi, Anda cukup mengembalikan nilai kedua dari operator terner - true .



Peringatan 2 Ekspresi



V3022 '_elementStack.Count> 0' selalu benar. OpenXmlDomReader.cs 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;
    }
    ....
}


Jelas, jika tidak ada 0 elemen di _elementStack , maka elemen tersebut akan lebih banyak. Kode dapat dipersingkat setidaknya 8 baris.



Peringatan 3 Ekspresi



V3022 'rootElement == null' selalu salah. OpenXmlPartReader.cs 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(....);
  }
  ....
}


Fungsi CreateElement tidak bisa mengembalikan nol . Jika perusahaan membuat aturan untuk menulis metode untuk membuat node xml yang mengembalikan objek yang valid atau mengeluarkan pengecualian, maka pengguna fungsi tersebut tidak dapat menyalahgunakan pemeriksaan tambahan.



Peringatan 4



V3022 Ekspresi 'nameProvider' selalu tidak null. Operator '?.' berlebihan. 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();
}


Operator is memiliki pola berikut: 



expr is type varname


Jika expr bernilai true , maka objek varname valid dan tidak perlu dibandingkan dengan null lagi , seperti yang dilakukan dalam cuplikan kode ini.



Peringatan 5



Ekstensi Ekspresi V3022 == ".xlsx" || extension == ".xlsm" 'selalu salah. 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;
        }
        ....
    }
    ....
}


Kode yang menarik ternyata. Penulis pertama menyingkirkan semua dokumen dengan ekstensi berikut bukan .pptx , .pptm ,. potx dan. potm , lalu memutuskan untuk memeriksa bahwa tidak ada .xlsx dan .xlsm di antara keduanya . Fungsi PresentationDocument jelas merupakan korban refactoring.



Peringatan 7 Ekspresi



V3022 'OpenSettings.MarkupCompatibilityProcessSettings == null' selalu salah. 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;
        }
    }
}


Properti MarkupCompatibilityProcessSettings tidak pernah mengembalikan nol . Jika ternyata di getter bahwa bidang kelas adalah null , maka objek tersebut ditimpa dengan yang baru. Juga, perhatikan bahwa ini bukan panggilan rekursif dari satu properti, tetapi properti dengan nama yang sama dari kelas yang berbeda. Mungkin beberapa kebingungan telah menyebabkan penulisan cek yang tidak perlu.



Peringatan lainnya



Peringatan 1



V3080 Kemungkinan dereferensi nol. Pertimbangkan untuk memeriksa 'PreviousSibling'. OpenXmlCompositeElement.cs 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;    // <=
    ....
}


Dan berikut adalah contoh di mana verifikasi tambahan saja tidak cukup. Metode PreviousSibling bisa mengembalikan null , dan hasil dari fungsi ini langsung digunakan tanpa pemeriksaan.



2 tempat yang lebih berbahaya:



  • V3080 Kemungkinan dereferensi nol. Pertimbangkan untuk memeriksa 'prevNode'. OpenXmlCompositeElement.cs 489
  • V3080 Kemungkinan dereferensi nol. Pertimbangkan untuk memeriksa 'prevNode'. OpenXmlCompositeElement.cs 497


Peringatan 2



V3093 Operator '&' mengevaluasi kedua operan. Mungkin operator '&&' arus pendek harus digunakan sebagai gantinya. 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;
            }
        }
    }
    ....
}


Beberapa orang suka menggunakan operator '&' dalam ekspresi logis yang seharusnya tidak mereka gunakan. Dalam kasus operator ini, operan kedua dievaluasi terlebih dahulu, terlepas dari hasil yang pertama. Ini bukan kesalahan yang sangat serius di sini, tetapi kode yang ceroboh setelah pemfaktoran ulang dapat menyebabkan potensi pengecualian NullReferenceException .



Peringatan 3



V3097 Kemungkinan pengecualian: tipe yang ditandai dengan [Serializable] berisi anggota non-serializable yang tidak ditandai oleh [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; } // <=
}


Serialisasi kelas OpenXmlPackageValidationEventArgs mungkin gagal karena salah satu properti lupa untuk ditandai sebagai non-serializable. Atau perlu untuk mengubah tipe pengembalian properti ini menjadi serializable, jika tidak pengecualian dapat terjadi saat runtime.



Kesimpulan



Kami di perusahaan menyukai proyek dan teknologi Microsoft. Di bagian tempat kami mencantumkan proyek Sumber Terbuka yang diuji dengan PVS-Studio, kami bahkan mengalokasikan bagian terpisah untuk Microsoft. Sudah ada 21 proyek yang terkumpul, di mana 26 artikel telah ditulis. Ini tanggal 27.



Saya yakin Anda bertanya-tanya apakah ada Microsoft di antara pelanggan kami. Jawabannya iya! Tapi jangan lupa bahwa ini adalah perusahaan besar yang memimpin pembangunan di seluruh dunia. Pasti ada divisi yang sudah menggunakan PVS-Studio dalam proyeknya, tapi ada lebih banyak lagi yang tidak menggunakannya! Dan pengalaman kami dengan proyek open source menunjukkan bahwa mereka jelas kekurangan alat yang baik untuk menemukan bug;).





Lebih banyak berita dari berita bagi mereka yang tertarik untuk menganalisis kode di C ++, C # dan Java: kami baru-baru ini menambahkan dukungan untuk standar OWASP dan secara aktif meningkatkan cakupannya.





Jika Anda ingin berbagi artikel ini dengan audiens berbahasa Inggris, silakan gunakan tautan terjemahan: Svyatoslav Razmyslov. Menganalisis Kualitas Kode dari Open XML SDK Microsoft .


More articles:


All Articles